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

Improve test failure messages #89967

Merged

Conversation

mrvoorhe
Copy link
Contributor

@mrvoorhe mrvoorhe commented Aug 3, 2023

I've noticed that there are more coding error scenarios where a test will fail and the failure that is hit is the failed to resolve "test.exe" error. I don't fully have my head around which exceptions are being caught and logged but it seems like a lot more than there used to be.

Suffice it to say, when the only output from a test failure is about failing to resolve test.exe, that isn't particularly helpful when what actually happened is you had a mistake in your code and an exception was thrown.

With the linker now eating more exceptions and returning a non-zero exit code rather than throwing and crashing I think it's worth having the test framework fail with the messages from the linker if the exit code is non-zero.

For the tests that expect a non zero exit code, I added ExpectNonZeroExitCodeAttribute.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 3, 2023
@ghost ghost added linkable-framework Issues associated with delivering a linker friendly framework community-contribution Indicates that the PR has been added by a community member labels Aug 3, 2023
@ghost
Copy link

ghost commented Aug 3, 2023

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar
See info in area-owners.md if you want to be subscribed.

Issue Details

I've noticed that there are more coding error scenarios where a test will fail and the failure that is hit is the failed to resolve "test.exe" error. I don't fully have my head around which exceptions are being caught and logged but it seems like a lot more than there used to be.

Suffice it to say, when the only output from a test failure is about failing to resolve test.exe, that isn't particularly helpful when what actually happened is you had a mistake in your code and an exception was thrown.

With the linker now eating more exceptions and returning a non-zero exit code rather than throwing and crashing I think it's worth having the test framework fail with the messages from the linker if the exit code is non-zero.

For the tests that expect a non zero exit code, I added ExpectNonZeroExitCodeAttribute. Currently there is only 1 test that needs this.

Author: mrvoorhe
Assignees: -
Labels:

linkable-framework, needs-area-label

Milestone: -

@marek-safar marek-safar added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Aug 4, 2023
@ghost
Copy link

ghost commented Aug 4, 2023

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

I've noticed that there are more coding error scenarios where a test will fail and the failure that is hit is the failed to resolve "test.exe" error. I don't fully have my head around which exceptions are being caught and logged but it seems like a lot more than there used to be.

Suffice it to say, when the only output from a test failure is about failing to resolve test.exe, that isn't particularly helpful when what actually happened is you had a mistake in your code and an exception was thrown.

With the linker now eating more exceptions and returning a non-zero exit code rather than throwing and crashing I think it's worth having the test framework fail with the messages from the linker if the exit code is non-zero.

For the tests that expect a non zero exit code, I added ExpectNonZeroExitCodeAttribute. Currently there is only 1 test that needs this.

Author: mrvoorhe
Assignees: -
Labels:

linkable-framework, community-contribution, area-Tools-ILLink, needs-area-label

Milestone: -

@marek-safar marek-safar removed linkable-framework Issues associated with delivering a linker friendly framework needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 4, 2023
@ghost ghost added the linkable-framework Issues associated with delivering a linker friendly framework label Aug 4, 2023
@ghost
Copy link

ghost commented Aug 4, 2023

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar
See info in area-owners.md if you want to be subscribed.

Issue Details

I've noticed that there are more coding error scenarios where a test will fail and the failure that is hit is the failed to resolve "test.exe" error. I don't fully have my head around which exceptions are being caught and logged but it seems like a lot more than there used to be.

Suffice it to say, when the only output from a test failure is about failing to resolve test.exe, that isn't particularly helpful when what actually happened is you had a mistake in your code and an exception was thrown.

With the linker now eating more exceptions and returning a non-zero exit code rather than throwing and crashing I think it's worth having the test framework fail with the messages from the linker if the exit code is non-zero.

For the tests that expect a non zero exit code, I added ExpectNonZeroExitCodeAttribute. Currently there is only 1 test that needs this.

Author: mrvoorhe
Assignees: -
Labels:

linkable-framework, community-contribution, area-Tools-ILLink

Milestone: -

@mrvoorhe mrvoorhe force-pushed the linker-improve-failure-message branch from 0c25bd7 to 20eb69c Compare August 4, 2023 19:05
I've noticed that there are more coding error scenarios where a test will fail and the failure that is hit is the failed to resolve "test.exe" error.  I don't fully have my head around which exceptions are being caught and logged but it seems like a lot more than there used to be.

Suffice it to say, when the only output from a test failure is about failing to resolve test.exe, that isn't particularly helpful when what actually happened is you had a mistake in your code and an exception was thrown.

With the linker now eating more exceptions and returning a non-zero exit code rather than throwing and crashing I think it's worth having the test framework fail with the messages from the linker if the exit code is non-zero.

For the tests that expect a non zero exit code, I added `ExpectNonZeroExitCodeAttribute`.  Currently there is only 1 test that needs this.
@mrvoorhe mrvoorhe force-pushed the linker-improve-failure-message branch from 20eb69c to 90d4b6f Compare August 4, 2023 19:25
@marek-safar marek-safar merged commit 4415472 into dotnet:main Aug 7, 2023
60 checks passed
@ghost ghost locked as resolved and limited conversation to collaborators Sep 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers community-contribution Indicates that the PR has been added by a community member linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants