-
Notifications
You must be signed in to change notification settings - Fork 4
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
Put TraceLoggerFactoryTests into non-parallel collection #38
Put TraceLoggerFactoryTests into non-parallel collection #38
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #38 +/- ##
=======================================
Coverage 77.64% 77.64%
=======================================
Files 58 58
Lines 1897 1897
Branches 121 121
=======================================
Hits 1473 1473
Misses 401 401
Partials 23 23
|
We should probably do the same thing for AppComposerTests which might be the other culprit, here. The page you linked says there must be another test class involved to produce a race condition. And AppcomposerTests also uses a TraceLoggerFactory (via sut). |
Afais the proper way to do this is a tad more complicated: see here xunit/xunit#1999 (comment) |
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.
See my other comments.
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 we could have put them in one NotTheadSafeCollection, but two collections should also be OK
Description
There is an unstable test https://github.com/ZEISS/czicompress/actions/runs/6969814480/job/18966499474?pr=33 that is presumably caused by running our tests in parallel.
This change will move the TraceLoggerFactoryTests into their own collection with Parallelization disabled. This ought to run the other parallel-capable tests first and then run the TraceLoggerFactory tests sequentially. (see https://xunit.net/docs/running-tests-in-parallel for details)
Type of change
How Has This Been Tested?
This has been tested locally and I will be keeping an eye on the actions for this PR as well to see if the issue in question is indeed resolved.
Checklist: