-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Return all error messages instead of throwing on the first error in linker tests #97605
Conversation
Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas Issue DetailsDebugging the linker would be easier if we could see all the missing members in the tests, but the tests currently fail at the first missing/extra member. Instead, we should return a sequence of error messages to make it easier to see a pattern in the missing members and make debugging easier.
|
Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar Issue DetailsDebugging the linker would be easier if we could see all the missing members in the tests, but the tests currently fail at the first missing/extra member. Instead, we should return a sequence of error messages to make it easier to see a pattern in the missing members and make debugging easier.
|
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.
Overall looks good, but we should try to keep the structure of the nativeaot AssemblyChecker similar. Would you mind making the same kind of change there too?
src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs
Outdated
Show resolved
Hide resolved
src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs
Outdated
Show resolved
Hide resolved
src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs
Outdated
Show resolved
Hide resolved
Made the same changes for NativeAOT. A few places still assert for invalid test parameters or failing to resolve a reference, but all the |
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.
LGTM once it builds and tests pass. Thanks!
src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyChecker.cs
Outdated
Show resolved
Hide resolved
Failures are known. |
Debugging the linker tests would be easier if we could see all the missing/extra members/warnings, but the tests currently throw an exception at the first unexpected thing. Instead, we could return a sequence of error messages to make it easier to see a pattern in the missing members and make debugging easier.