-
Notifications
You must be signed in to change notification settings - Fork 262
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
Fix flaky WhenSpecificDataSourceIsIgnoredViaIgnoreMessageProperty
#4553
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4553 +/- ##
==========================================
+ Coverage 65.31% 65.34% +0.02%
==========================================
Files 591 591
Lines 35570 35570
==========================================
+ Hits 23233 23242 +9
+ Misses 12337 12328 -9
Flags with carried forward coverage won't be shown. Click here to find out more. |
All green. @Evangelink I'll do another run to be more sure. And we will monitor the runs after merge and look if the test failed again. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
if (!lockTaken) | ||
{ | ||
// Can this happen? :/ | ||
throw new InvalidOperationException(); |
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.
Rarely but I think yes
cc @MarcoRossignoli
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.
I am fine with throwing the Unreachable
here.
// SystemConsole.ConsoleOut is set only once in static ctor. | ||
// So we are sure we will be doing Monitor.Exit on the same instance. |
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.
I think I need to doc this more, and mention that it's to avoid interleaving with user's calls to Console.WriteLine
, which will internally lock on Console.Out.
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.
I'll merge this as-is for now to unblock main CI that is currently blocked so we unblock DotNet sdk
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.
I already extended the comment so this comment wasn't valid anymore actually :)
@Evangelink It's passing again. The only failure in this run is unrelated to terminal ( |
Pushed a commit for more explanation, and triggering a third run :) |
So far this fixes part of the flakiness #4544. It's still flaky though.