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

Test failure System.Reflection.Tests.ModuleTests.GetMethods #50831

Closed
VincentBu opened this issue Apr 7, 2021 · 8 comments · Fixed by #56859
Closed

Test failure System.Reflection.Tests.ModuleTests.GetMethods #50831

VincentBu opened this issue Apr 7, 2021 · 8 comments · Fixed by #56859
Assignees
Labels
arch-arm32 area-System.Reflection help wanted [up-for-grabs] Good issue for external contributors JitStress CLR JIT issues involving JIT internal stress modes os-linux Linux OS (any supported distro)
Milestone

Comments

@VincentBu
Copy link
Contributor

Run: runtime-coreclr libraries-jitstress 20210406.1

Failed test:

net6.0-Linux-Release-arm-CoreCLR_checked-jitstress2-(Ubuntu.1804.Arm32.Open)[email protected]/dotnet-buildtools/prereqs:ubuntu-18.04-helix-arm32v7-bfcd90a-20200121150440
 -System.Reflection.Tests.ModuleTests.GetMethods

Error message:

Showing first 10 differences\n
  Position 0: Expected: TestMethodFoo, Actual: TestMethodBar
  Position 2: Expected: TestMethodBar, Actual: TestMethodFoo
Total number of differences: 2 out of 3


Stack trace
   at System.AssertExtensions.SequenceEqual[T](ReadOnlySpan`1 expected, ReadOnlySpan`1 actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 473
   at System.AssertExtensions.SequenceEqual[T](Span`1 expected, Span`1 actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 478
   at System.AssertExtensions.SequenceEqual[T](T[] expected, T[] actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 480
   at System.Reflection.Tests.ModuleTests.GetMethods() in /_/src/libraries/System.Runtime/tests/System/Reflection/ModuleTests.cs:line 230
@VincentBu VincentBu added arch-arm32 area-System.Reflection os-linux Linux OS (any supported distro) JitStress CLR JIT issues involving JIT internal stress modes labels Apr 7, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 7, 2021
@BruceForstall
Copy link
Member

Looks like this test was added with #50076

@BruceForstall
Copy link
Member

@MichalStrehovsky @erafalovsky PTAL

@MichalStrehovsky
Copy link
Member

@erafalovsky would you be interested in fixing this? Maybe we can just sort the sequence before comparing. I forgot that the reflection stack doesn't guarantee ordering of members.

From the docs:

Remarks

The GetMethods method does not return methods in a particular order, such as alphabetical or declaration order. Your code must not depend on the order in which methods are returned, because that order varies.

@buyaa-n buyaa-n removed the untriaged New issue has not been triaged by the area owner label Apr 12, 2021
@buyaa-n buyaa-n added this to the 6.0.0 milestone Apr 12, 2021
@VincentBu
Copy link
Contributor Author

Failed again in runtime-coreclr libraries-jitstress 20210428.1

Failed test:

net6.0-Linux-Release-arm-CoreCLR_checked-no_tiered_compilation-(Ubuntu.1804.Arm32.Open)[email protected]/dotnet-buildtools/prereqs:ubuntu-18.04-helix-arm32v7-bfcd90a-20200121150440
 -System.Reflection.Tests.ModuleTests.GetMethods

Error message:

Showing first 10 differences
 Position 0: Expected: TestMethodFoo, Actual: TestMethodBar
 Position 2: Expected: TestMethodBar, Actual: TestMethodFoo
Total number of differences: 2 out of 3

Stack trace
   at System.AssertExtensions.SequenceEqual[T](ReadOnlySpan`1 expected, ReadOnlySpan`1 actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 473
   at System.AssertExtensions.SequenceEqual[T](Span`1 expected, Span`1 actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 478
   at System.AssertExtensions.SequenceEqual[T](T[] expected, T[] actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 480
   at System.Reflection.Tests.ModuleTests.GetMethods() in /_/src/libraries/System.Runtime/tests/System/Reflection/ModuleTests.cs:line 238

@VincentBu
Copy link
Contributor Author

Failed again in runtime-coreclr libraries-jitstress 20210512.1

Failed test:

net6.0-Linux-Release-arm64-CoreCLR_checked-jitstress2-(Ubuntu.1804.Arm64.Open)[email protected]/dotnet-buildtools/prereqs:ubuntu-18.04-helix-arm64v8-a45aeeb-20190620155855
 -System.Reflection.Tests.ModuleTests.GetMethods

Error message:

Showing first 10 differences
Position 0: Expected: TestMethodFoo, Actual: TestMethodBar
Position 2: Expected: TestMethodBar, Actual: TestMethodFoo
Total number of differences: 2 out of 3


Stack trace
   at System.AssertExtensions.SequenceEqual[T](ReadOnlySpan`1 expected, ReadOnlySpan`1 actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 473
   at System.AssertExtensions.SequenceEqual[T](Span`1 expected, Span`1 actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 478
   at System.AssertExtensions.SequenceEqual[T](T[] expected, T[] actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 480

@VincentBu
Copy link
Contributor Author

Failed again in runtime-coreclr libraries-jitstress2-jitstressregs 20210605.1

Failed test:

net6.0-windows-Release-x86-CoreCLR_checked-jitstress2_jitstressregs2-Windows.10.Amd64.Open

- System.Reflection.Tests.ModuleTests.GetMethods

Error message:

Showing first 10 differences
Position 0: Expected: TestMethodFoo, Actual: TestMethodBar
Position 2: Expected: TestMethodBar, Actual: TestMethodFoo
Total number of differences: 2 out of 3


Stack trace
   at System.AssertExtensions.SequenceEqual[T](ReadOnlySpan`1 expected, ReadOnlySpan`1 actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 473
   at System.AssertExtensions.SequenceEqual[T](Span`1 expected, Span`1 actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 491
   at System.AssertExtensions.SequenceEqual[T](T[] expected, T[] actual) in /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 493
   at System.Reflection.Tests.ModuleTests.GetMethods() in /_/src/libraries/System.Runtime/tests/System/Reflection/ModuleTests.cs:line 241

@BruceForstall
Copy link
Member

Another failure: net6.0-windows-Release-arm64-CoreCLR_checked-jitstress2_jitstressregs1:

https://dev.azure.com/dnceng/public/_build/results?buildId=1198817&view=ms.vss-test-web.build-test-results-tab&runId=35932930&resultId=180136&paneView=debug

    System.Reflection.Tests.ModuleTests.GetMethods [FAIL]
      Showing first 10 differences
        Position 0: Expected: TestMethodFoo, Actual: TestMethodBar
        Position 2: Expected: TestMethodBar, Actual: TestMethodFoo
      Total number of differences: 2 out of 3
      Stack Trace:
        /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs(473,0): at System.AssertExtensions.SequenceEqual[T](ReadOnlySpan`1 expected, ReadOnlySpan`1 actual)
        /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs(491,0): at System.AssertExtensions.SequenceEqual[T](Span`1 expected, Span`1 actual)
        /_/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs(493,0): at System.AssertExtensions.SequenceEqual[T](T[] expected, T[] actual)

BruceForstall added a commit to BruceForstall/runtime that referenced this issue Jun 22, 2021
@jkotas jkotas added the help wanted [up-for-grabs] Good issue for external contributors label Jun 22, 2021
@steveharter
Copy link
Member

The proposed fix is trivial: sort the returned methods and compare:

Maybe we can just sort the sequence before comparing

FWIW the tests in System.Text.Json also sort in a similar fashion - the properties for a JSON object produced by serializing a CLR object are typically sorted by property name due to nondeterminstic reflection ordering (i.e. the JSON is serialized in the order of the reflected properties).

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 4, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 5, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm32 area-System.Reflection help wanted [up-for-grabs] Good issue for external contributors JitStress CLR JIT issues involving JIT internal stress modes os-linux Linux OS (any supported distro)
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants