-
Notifications
You must be signed in to change notification settings - Fork 5k
Add missing type name parsing test coverage #98562
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
Add missing type name parsing test coverage #98562
Conversation
Tagging subscribers to this area: @dotnet/area-system-reflection Issue DetailsWhile working on the new Type Name Parser API (#97566) I've discovered some testing gaps. Namely:
|
The tests are failing on Mono. Mono does not use the unified parser for all code paths yet - #45033 . I think you can disable the failing tests on Mono against this issue. |
While you are on it, could you please also add a coverage for byref-of-byref and pointer-of-byref (e.g. here runtime/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs Line 525 in 6d4fc1a
|
e67dd46
to
843023a
Compare
843023a
to
8d9b537
Compare
@jkotas I've addressed your feedback, PTAL. Thanks! |
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
yield return new object[] { "System.Void[]", expectedException, true }; | ||
yield return new object[] { "System.TypedReference[]", expectedException, true }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkotas these two test cases are falling on Mono. Is my understanding correct that with #94835 they should be working fine?
yield return new object[] { "System.Void[]", expectedException, true }; | |
yield return new object[] { "System.TypedReference[]", expectedException, true }; | |
if (!PlatformDetection.IsMonoRuntime) | |
{ | |
yield return new object[] { "System.Void[]", expectedException, true }; | |
yield return new object[] { "System.TypedReference[]", expectedException, true }; | |
} |
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs
Show resolved
Hide resolved
…/Type/TypeTests.cs
While working on the new Type Name Parser API (#97566) I've discovered some testing gaps.
Namely:
runtime/src/libraries/Common/src/System/Reflection/TypeNameParser.cs
Lines 326 to 331 in 7185df8
runtime/src/libraries/Common/src/System/Reflection/TypeNameParser.cs
Lines 200 to 207 in 7185df8
runtime/src/libraries/Common/src/System/Reflection/TypeNameParser.cs
Lines 439 to 450 in 7185df8
runtime/src/libraries/Common/src/System/Reflection/TypeNameParser.cs
Lines 628 to 631 in 7185df8
cc @GrabYourPitchforks @jeffhandley