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

Jump to bottom

Add a scope for launching background work in tests #3348

Merged
merged 13 commits into from Jul 12, 2022
Merged

Add a scope for launching background work in tests #3348

merged 13 commits into from Jul 12, 2022

Conversation

dkhalanskyjb
Copy link
Collaborator

Fixes #3287

@dkhalanskyjb dkhalanskyjb marked this pull request as ready for review June 27, 2022 13:32
@dkhalanskyjb
Copy link
Collaborator Author

Reported the compiler bug here: https://youtrack.jetbrains.com/issue/KT-52985/Native-a-function-with-type-T-returned-a-kotlinUnit-instead-of-null

@dkhalanskyjb
Add some tests, fix a bug
6fa024c 
The bug was that the test would time out if every non-background
coroutine was waiting for something from the background ones.
@dkhalanskyjb
backgroundWorkScope -> backgroundScope
cefc6b9
@dkhalanskyjb
Improve the backgroundScope documentation
bffa53a
@dkhalanskyjb
Catch more of the errors
1865848
qwwdfsad
qwwdfsad reviewed Jul 8, 2022
View reviewed changes
kotlinx-coroutines-test/common/src/TestBuilders.kt
completed = true
}
scheduler.onDispatchEvent {
// we received knowledge that `scheduler` observed a dispatch event, so we reset the timeout
Copy link
Member

Choose a reason for hiding this comment

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

also worth noting that it doesn't receive events from background jobs

qwwdfsad
qwwdfsad reviewed Jul 8, 2022
View reviewed changes
kotlinx-coroutines-test/common/src/TestScope.kt Outdated
// Only call this in a `synchronized(lock)` block.
@Suppress("INVISIBLE_MEMBER")
private fun collectUncaughtExceptions(): List<Throwable> {
val exceptions = LinkedHashSet(uncaughtExceptions)
Copy link
Member

Choose a reason for hiding this comment

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

In JobSupport we use an identity hash set to workaround throwables that throw exceptions from their hashCode/equals.
I don't think it's worth fixing here, but it's definitely worth to file an issue here and I'll escalate it to MPP stdlib

qwwdfsad
qwwdfsad reviewed Jul 11, 2022
View reviewed changes
Copy link
Member

@qwwdfsad qwwdfsad left a comment

Choose a reason for hiding this comment

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

In general looks good

kotlinx-coroutines-test/common/src/TestBuilders.kt Outdated Show resolved Hide resolved
kotlinx-coroutines-test/common/src/internal/ReportingSupervisorJob.kt Outdated Show resolved Hide resolved
@dkhalanskyjb dkhalanskyjb merged commit 143bdfa into develop Jul 12, 2022
@dkhalanskyjb dkhalanskyjb deleted the TestScope.backgroundWorkScope branch July 12, 2022 12:16
pablobaxter pushed a commit to pablobaxter/kotlinx.coroutines that referenced this pull request Sep 14, 2022
@dkhalanskyjb @pablobaxter
Add a scope for launching background work in tests (Kotlin#3348)
4f3f98f 
Fixes Kotlin#3287
pablobaxter added a commit to pablobaxter/kotlinx.coroutines that referenced this pull request Sep 14, 2022
@pablobaxter
Merge remote-tracking branch 'origin/baxter/upstream-flow-timeout' in…
1723b60 
…to baxter/upstream-flow-timeout

* origin/baxter/upstream-flow-timeout: (328 commits)
  Commit API dump
  Cleanup API, update knit
  Fix typo in runTest method docs (Kotlin#3417)
  Update coroutines-and-channels.md (Kotlin#3410)
  chore: update the website's release step (Kotlin#3397)
  ktl-695 chore: support Dokka HTML customization (Kotlin#3388)
  update: KT-50122 adding kotlinx.dependencies
  Improve bump-version.sh (Kotlin#3365)
  Fix documentation for `DEBUG_PROPERTY_VALUE_OFF` (Kotlin#3389)
  feat: moving coroutines hands-on to docs (Kotlin#3369)
  Version 1.6.4
  Improve CoroutineDispatcher documentation (Kotlin#3359)
  Update binary compatibility validator to 0.11.0 (Kotlin#3362)
  Add a scope for launching background work in tests (Kotlin#3348)
  Fix debug module publication with shadow plugin (Kotlin#3357)
  Comply with Subscriber rule 2.7 in the `await*` impl (Kotlin#3360)
  Update readme (Kotlin#3343)
  Reduce reachable references of disposed invokeOnTimeout handle (Kotlin#3353)
  breakleg; knit validation fix
  Additional comment in CoroutineScheduler
  ...

# Conflicts:
#	README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whyoleg whyoleg whyoleg left review comments

@qwwdfsad qwwdfsad qwwdfsad approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dkhalanskyjb @qwwdfsad @whyoleg