Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wasm] Enable System.Xml.Linq.Tests.XTypeDescriptionProviderTests class #39709

Closed
MaximLipnin opened this issue Jul 21, 2020 · 14 comments · Fixed by #71821
Closed

[wasm] Enable System.Xml.Linq.Tests.XTypeDescriptionProviderTests class #39709

MaximLipnin opened this issue Jul 21, 2020 · 14 comments · Fixed by #71821
Assignees
Labels
arch-wasm WebAssembly architecture area-System.ComponentModel linkable-framework Issues associated with delivering a linker friendly framework
Milestone

Comments

@MaximLipnin
Copy link
Contributor

While enabling System.ComponentModel.TypeConverter.Tests on Browser WASM, I faced the failures in System.Xml.Linq.Tests.XTypeDescriptionProviderTests test class.

There are 7 test cases which mostly test the property-related logic for XElement and XAttribute classes (dotnet/corefx#33082).
Those classes are annotated with System.ComponentModel.TypeDescriptionProviderAttribute so that TypeDescriptor.GetProperties() can provide with some additional properties

public override PropertyDescriptorCollection GetProperties(Attribute[] attributes)
{
PropertyDescriptorCollection properties = new PropertyDescriptorCollection(null);
if (attributes == null)
{
if (typeof(T) == typeof(XElement))
{
properties.Add(new XElementAttributePropertyDescriptor());
properties.Add(new XElementDescendantsPropertyDescriptor());
properties.Add(new XElementElementPropertyDescriptor());
properties.Add(new XElementElementsPropertyDescriptor());
properties.Add(new XElementValuePropertyDescriptor());
properties.Add(new XElementXmlPropertyDescriptor());
}
else if (typeof(T) == typeof(XAttribute))
{
properties.Add(new XAttributeValuePropertyDescriptor());
}
}
foreach (PropertyDescriptor property in base.GetProperties(attributes))
{
properties.Add(property);
}
return properties;
}
}
.

The mentioned tests get a collection of property descriptors by calling something like below

var xel = new XElement("someElement");
var props = TypeDescriptor.GetProperties(xel);

Actually props contains only a collection of some general properties and doesn't include the additional ones.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Xml untriaged New issue has not been triaged by the area owner labels Jul 21, 2020
@ghost
Copy link

ghost commented Jul 21, 2020

Tagging subscribers to this area: @buyaa-n, @krwq
Notify danmosemsft if you want to be subscribed.

@ghost
Copy link

ghost commented Aug 4, 2020

Tagging subscribers to this area: @safern
See info in area-owners.md if you want to be subscribed.

@marek-safar marek-safar removed area-System.Xml untriaged New issue has not been triaged by the area owner labels Aug 4, 2020
@marek-safar marek-safar added this to the 5.0.0 milestone Aug 4, 2020
@ericstj ericstj added the linkable-framework Issues associated with delivering a linker friendly framework label Aug 31, 2020
@ericstj
Copy link
Member

ericstj commented Aug 31, 2020

cc @eerhardt looks like a linker correctness issue.

@eerhardt
Copy link
Member

eerhardt commented Sep 1, 2020

We annotated TypeDescriptionProviderAttribute correctly to preserve the TypeDescriptionProvider types in .NET 5 RC1. However, it appears this usage exposed an ILLinker issue - it doesn't seem to handle generic types correctly. I've logged dotnet/linker#1469 for this.

We won't be able to make progress on this issue in dotnet/runtime since this is an ILLinker issue. Adding blocked.

@eerhardt eerhardt added the blocked Issue/PR is blocked on something - see comments label Sep 1, 2020
@marek-safar marek-safar modified the milestones: 5.0.0, 6.0.0 Sep 3, 2020
@eerhardt
Copy link
Member

The underlying linker bug has been fixed. Removing "blocked". When we get a new linker in dotnet/runtime, we should be able to enable these tests now.

@eerhardt eerhardt removed the blocked Issue/PR is blocked on something - see comments label Sep 16, 2020
@MaximLipnin
Copy link
Contributor Author

#42201 should bring related linker changes

@MaximLipnin
Copy link
Contributor Author

Tried to run System.ComponentModel.TypeConverter tests on the latest master, which includes changes from #42201, and the failures still exist.
From artifacts/bin/System.ComponentModel.TypeConverter.Tests/net5.0-release/browser-wasm/AppBundle/xharness-output/testResults.xml:

      <test name="System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementElementsPropertyDescriptor" type="System.Xml.Linq.Tests.XTypeDescriptionProviderTests" method="XElementElementsPropertyDescriptor" time="0.0057949" result="Fail">
        <failure exception-type="Xunit.Sdk.NotNullException">
          <message><![CDATA[Assert.NotNull() Failure]]></message>
          <stack-trace><![CDATA[   at System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementElementsPropertyDescriptor()
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)]]></stack-trace>
        </failure>
      </test>
      <test name="System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementAttributePropertyDescriptor" type="System.Xml.Linq.Tests.XTypeDescriptionProviderTests" method="XElementAttributePropertyDescriptor" time="0.000769" result="Fail">
        <failure exception-type="Xunit.Sdk.NotNullException">
          <message><![CDATA[Assert.NotNull() Failure]]></message>
          <stack-trace><![CDATA[   at System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementAttributePropertyDescriptor()
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)]]></stack-trace>
        </failure>
      </test>
      <test name="System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementValuePropertyDescriptor" type="System.Xml.Linq.Tests.XTypeDescriptionProviderTests" method="XElementValuePropertyDescriptor" time="0.0081339" result="Fail">
        <failure exception-type="Xunit.Sdk.TrueException">
          <message><![CDATA[Assert.True() Failure\nExpected: True\nActual:   False]]></message>
          <stack-trace><![CDATA[   at System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementValuePropertyDescriptor()
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)]]></stack-trace>
        </failure>
      </test>
      <test name="System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XAttributeValuePropertyDescriptor" type="System.Xml.Linq.Tests.XTypeDescriptionProviderTests" method="XAttributeValuePropertyDescriptor" time="0.001141" result="Fail">
        <failure exception-type="Xunit.Sdk.TrueException">
          <message><![CDATA[Assert.True() Failure\nExpected: True\nActual:   False]]></message>
          <stack-trace><![CDATA[   at System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XAttributeValuePropertyDescriptor()
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)]]></stack-trace>
        </failure>
      </test>
      <test name="System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementDescendantsPropertyDescriptor" type="System.Xml.Linq.Tests.XTypeDescriptionProviderTests" method="XElementDescendantsPropertyDescriptor" time="0.000545" result="Fail">
        <failure exception-type="Xunit.Sdk.NotNullException">
          <message><![CDATA[Assert.NotNull() Failure]]></message>
          <stack-trace><![CDATA[   at System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementDescendantsPropertyDescriptor()
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)]]></stack-trace>
        </failure>
      </test>
      <test name="System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementXmlPropertyDescriptor" type="System.Xml.Linq.Tests.XTypeDescriptionProviderTests" method="XElementXmlPropertyDescriptor" time="0.0004369" result="Fail">
        <failure exception-type="Xunit.Sdk.NotNullException">
          <message><![CDATA[Assert.NotNull() Failure]]></message>
          <stack-trace><![CDATA[   at System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementXmlPropertyDescriptor()
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)]]></stack-trace>
        </failure>
      </test>
      <test name="System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementElementPropertyDescriptor" type="System.Xml.Linq.Tests.XTypeDescriptionProviderTests" method="XElementElementPropertyDescriptor" time="0.0006809" result="Fail">
        <failure exception-type="System.NullReferenceException">
          <message><![CDATA[System.NullReferenceException : Object reference not set to an instance of an object.]]></message>
          <stack-trace><![CDATA[   at System.Xml.Linq.Tests.XTypeDescriptionProviderTests.XElementElementPropertyDescriptor()
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)]]></stack-trace>
        </failure>
      </test>

@MaximLipnin
Copy link
Contributor Author

E.g. XElementElementsPropertyDescriptor test expects a property descriptor named Elements which doesn't exist:

        [Fact]
        public void XElementElementsPropertyDescriptor()
        {
            var xel = new XElement("someElement");
            var props = TypeDescriptor.GetProperties(xel);

            var xelElsPD = props["Elements"];
            Assert.NotNull(xelElsPD);
           // ...

Below is what props has:

FirstAttribute
HasAttributes
HasElements
IsEmpty
LastAttribute
Name
NodeType
Value
FirstNode
LastNode
NextNode
PreviousNode
BaseUri
Document
Parent

@eerhardt
Copy link
Member

eerhardt commented Oct 5, 2020

I got the time to investigate this issue today, and it appears to be another linker issue. I opened dotnet/linker#1536 for this issue. Marking this as blocked again.

@eerhardt eerhardt added the blocked Issue/PR is blocked on something - see comments label Oct 5, 2020
@EgorBo EgorBo removed their assignment Feb 10, 2021
@eerhardt eerhardt self-assigned this May 11, 2021
@eerhardt
Copy link
Member

I retested this scenario with the latest builds and it is still blocked by dotnet/linker#1536.

Note:

            var xel = new XElement("someElement");
            var props = TypeDescriptor.GetProperties(xel);

The call to TypeDescriptor.GetProperties(object) is now marked as "RequiresUnreferencedCode" and using it in a trimmed app will emit a warning. This is because the Type of the object can't be statically analyzed, and thus all the properties of XElement are getting trimmed. Which means the default TypeDescriptor system won't find those properties. So as written, I don't expect this test to work in a trimmed app.

@maryamariyan maryamariyan modified the milestones: 6.0.0, 7.0.0 Jul 23, 2021
@maryamariyan maryamariyan modified the milestones: 7.0.0, 6.0.0 Jul 23, 2021
@eerhardt
Copy link
Member

eerhardt commented Aug 6, 2021

Moving to 7.0 as this scenario isn't necessary in order to ship 6.0. The underlying linker issue dotnet/linker#1536 needs to be fixed before this issue can be resolved.

@eerhardt eerhardt modified the milestones: 6.0.0, 7.0.0 Aug 6, 2021
@lewing
Copy link
Member

lewing commented Jun 10, 2022

still blocked on dotnet/linker#1536 but that has been raised

@ilonatommy ilonatommy removed the blocked Issue/PR is blocked on something - see comments label Jun 14, 2022
@ilonatommy
Copy link
Member

Unblocked, if @eerhardt you could see if we are able to close it soon, it would be great.

@lewing
Copy link
Member

lewing commented Jun 14, 2022

looks like #70614 should have the linker fix now

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 8, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 9, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.ComponentModel linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants