Fix assertions mode when using coroutines in another thread #3604
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LeoColman
approved these changes
Jul 30, 2023
charleskorn
reviewed
Aug 6, 2023
| override fun inc() = context.set(context.get() + 1) | ||
| override fun get(): Int = values.get() | ||
| override fun reset() = values.set(0) | ||
| override fun inc() = values.set(values.get() + 1) |
There was a problem hiding this comment.
Is this safe if multiple coroutines are running for the same test simultaneously?
If all we need to track is "an assertion did / did not occur", perhaps an atomic boolean would be easier to reason about?
| // this is invoked after coroutine has suspended on current thread | ||
| override fun restoreThreadContext(context: CoroutineContext, oldState: Int) { | ||
| // need to put the current thread-local value into our backing map before the coroutine is switched out | ||
| testNameCounters[testName] = ThreadLocalAssertionCounter.values.get() |
There was a problem hiding this comment.
What happens if a test launches multiple coroutines and makes assertions in both?
What happens if multiple coroutines launch at the same time but the last one to finish doesn't make any assertions? Won't we end up with a count of 0 assertions for the test?
Merged
|
Closing in favour of #3623 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bit of coroutine trickery here @charleskorn
It swaps in the thread locals used by the assertion counters as coroutines are launched, but with test name affinity.
Fixes #3022