Maintain: fix flaky tests in operationRepoTests#2318
Merged
jinliu9508 merged 1 commit intoidentity_verification_betafrom Jun 17, 2025
Merged
Maintain: fix flaky tests in operationRepoTests#2318jinliu9508 merged 1 commit intoidentity_verification_betafrom
jinliu9508 merged 1 commit intoidentity_verification_betafrom
Conversation
jinliu9508
force-pushed
the
fix-failed-test-operationrepotests
branch
2 times, most recently
from
e3c5504 to
56e9082
Compare
jinliu9508
changed the title
jinliu9508
changed the title
jkasten2
requested changes
Jun 17, 2025
Comment on lines
+7
to
+49
| import kotlinx.coroutines.suspendCancellableCoroutine | ||
| import kotlin.coroutines.resume | ||
|
|
||
| object OSPrimaryCoroutineScope { | ||
| // CoroutineScope tied to the main thread | ||
| private val mainScope = CoroutineScope(newSingleThreadContext(name = "OSPrimaryCoroutineScope")) | ||
| private val activeJobs = mutableSetOf<Job>() | ||
|
|
||
| /** | ||
| * Executes the given [block] on the OS primary coroutine scope. | ||
| */ | ||
| fun execute(block: suspend () -> Unit) { | ||
| mainScope.launch { | ||
| block() | ||
| val job = | ||
| mainScope.launch { | ||
| block() | ||
| } | ||
| activeJobs.add(job) | ||
| job.invokeOnCompletion { | ||
| activeJobs.remove(job) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Suspends until there are no active tasks running in the mainScope. | ||
| * This is useful for testing or ensuring all background work is complete. | ||
| */ | ||
| suspend fun waitForIdle() = | ||
| suspendCancellableCoroutine { continuation -> | ||
| if (activeJobs.isEmpty()) { | ||
| continuation.resume(Unit) | ||
| return@suspendCancellableCoroutine | ||
| } | ||
|
|
||
| val completionJob = | ||
| mainScope.launch { | ||
| activeJobs.forEach { it.join() } | ||
| continuation.resume(Unit) | ||
| } | ||
|
|
||
| continuation.invokeOnCancellation { | ||
| completionJob.cancel() | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
I don't think we need to track a list of active jobs. Instead in waitForIdle(), why not just add a job and wait for it to be completed?
I think something like this would work:
suspend fun waitForIdle() = mainScope.launch { }.join()The blank job will only finish once all jobs before it are done, so we don't need to also track all the jobs with our own set/list.
Contributor
Author
There was a problem hiding this comment.
Good idea! Was trying to resume the blocking until completion, but didn't realize that mainScope is all we need to look after. I made the change in the fixup commit.
jkasten2
approved these changes
Jun 17, 2025
jinliu9508
force-pushed
the
fix-failed-test-operationrepotests
branch
from
b65847a to
4580941
Compare
Contributor
Author
|
Squashed the fixup commit and merging... |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
One Line Summary
Fixed some flaky test units in operationRepoTests due to previous commits
Details
Motivation
Fix the flaky test units that have been blocking the runner to complete successfully
Other
This PR also added a waitForIdle() method for testing in OSPrimaryCoroutineScope to allow thread to wait for the completion of operation enqueue.
Testing
Unit testing
The test units are fixed and passed.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is