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

ILLink: Check for all expected warnings, then fail if any are unmatched #102286

Merged
merged 7 commits into from
May 23, 2024

Conversation

jtschuster
Copy link
Member

In Linker tests, we fail at the first ExpectedWarning that isn't matched, and print all the messages that haven't been matched yet, even though they may match another ExpectedWarning that just hasn't been processed yet.

Instead, we should keep a list of all expected warnings that were missing report them at once along with all unmatched warnings. This makes it much easier to debug when a test is failing.

@jtschuster jtschuster requested a review from sbomer May 15, 2024 21:25
@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 May 15, 2024
@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label May 15, 2024
@jtschuster jtschuster marked this pull request as ready for review May 16, 2024 17:28
@jtschuster jtschuster added area-Tools-ILLink .NET linker development as well as trimming analyzers and 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 May 16, 2024
@dotnet-policy-service dotnet-policy-service bot added linkable-framework Issues associated with delivering a linker friendly framework labels May 16, 2024
Copy link
Contributor

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

@@ -14,9 +16,9 @@ public TrimmingTestLogger ()
MessageContainers = new List<MessageContainer> ();
}

public List<MessageContainer> GetLoggedMessages ()
public ImmutableArray<MessageContainer> GetLoggedMessages ()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make sure we don't modify the message list outside of the logger, we should return an immutable array.

Copy link
Member

@sbomer sbomer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you! This will come in handy.

@jtschuster jtschuster merged commit 80414e9 into dotnet:main May 23, 2024
76 checks passed
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this pull request May 30, 2024
…ed (dotnet#102286)

In Linker tests, we fail at the first ExpectedWarning that isn't matched, and print all the messages that haven't been matched yet, even though they may match another ExpectedWarning that just hasn't been processed yet.

Instead, we should keep a list of all expected warnings that were missing report them at once along with all unmatched warnings. This makes it much easier to debug when a test is failing.
@github-actions github-actions bot locked and limited conversation to collaborators Jun 23, 2024
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 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