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.

Sign up for GitHub

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

Update Turbine to 0.13.0 #1035

Merged
merged 1 commit into from
Jun 20, 2023
Merged

Update Turbine to 0.13.0 #1035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ squareup-workflow = "1.0.0"

timber = "4.7.1"
truth = "1.1.3"
turbine = "0.12.1"
turbine = "0.13.0"
vanniktech-publish = "0.22.0"

[plugins]
Expand Down
6 changes: 3 additions & 3 deletions workflow-testing/dependencies/runtimeClasspath.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
:workflow-config:config-jvm
:workflow-core
:workflow-runtime
app.cash.turbine:turbine-jvm:0.12.1
app.cash.turbine:turbine:0.12.1
app.cash.turbine:turbine-jvm:0.13.0
app.cash.turbine:turbine:0.13.0
com.squareup.okio:okio-jvm:3.0.0
com.squareup.okio:okio:3.0.0
org.jetbrains.kotlin:kotlin-bom:1.8.10
org.jetbrains.kotlin:kotlin-reflect:1.8.10
org.jetbrains.kotlin:kotlin-stdlib-common:1.8.20
org.jetbrains.kotlin:kotlin-stdlib-common:1.8.21
org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.8.10
org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.10
org.jetbrains.kotlin:kotlin-stdlib:1.8.10
Expand Down
7 changes: 5 additions & 2 deletions workflow-testing/src/main/java/com/squareup/workflow1/testing/WorkerTester.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ public interface WorkerTester<T> {
public suspend fun nextOutput(): T

/**
* Throws an [AssertionError] if an output has been emitted since the last call to [nextOutput].
* Throws an [AssertionError] if an output, error, or completion has been emitted since the last
* call to [nextOutput].
*/
public fun assertNoOutput()

Expand Down Expand Up @@ -83,7 +84,9 @@ public fun <T> Worker<T>.test(
try {
expectNoEvents()
} catch (e: AssertionError) {
throw AssertionError("Expected no output to have been emitted.")
throw AssertionError(
"Expected no output, completion, or error to have been emitted."
)
}
}

Expand Down
24 changes: 21 additions & 3 deletions workflow-testing/src/test/java/com/squareup/workflow1/testing/WorkerTesterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,28 @@ import kotlin.test.assertFalse
@OptIn(ExperimentalCoroutinesApi::class)
class WorkerTesterTest {

@Test fun `assertNoOutput passes after worker finishes without emitting`() {
val worker = Worker.finished<Unit>()
@Test fun `assertNoOutput succeeds in live flow without output`() {
val worker = Worker.create<Unit> {
suspendCancellableCoroutine { }
}
worker.test {
assertNoOutput()
}
}

@Test fun `assertNoOutput fails after worker finishes without emitting`() {
Copy link
Contributor

Choose a reason for hiding this comment

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

if worker finishes without emitting anything, then assertNoOutput should succeed right? as the name suggest it should check for no output

Copy link
Contributor

Choose a reason for hiding this comment

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

@steve-the-edwards I'm confused by this too.

Copy link
Contributor

@rjrjr rjrjr Jun 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I'm not confused by the test method name, but by the semantic: why is it now an error for a worker to complete when we assertNoOutput()? This change broke some existing tests that were building workers via Observable.just(), for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you guys are right I should have made more noise about the change in this semantic. I just took the easy way out and inherited the change from expectNoEvents() when it started throwing on Completion after Turbine 0.3.0.

I've done a quick look and I'm not sure there is good way to reproduce the behavior we had before. @RotBolt could you create an issue to update this or rename the assertion to something like assertNoOutputOrCompletion()?

I think I am ok with having places where we use flowOf { 1 } or Observable.just(false) to start using assertFinished() instead of assertNoOutput(). But maybe that's a cop out?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's easy to replace Observable.just(false) with BehaviorRelay.createDefault(false). Is there a similar idiom for Flow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

flow { emit(false); suspendCancellableCoroutine {} }

so tidy right?

hmm. must be something better

val worker = Worker.finished<Unit>()
val error = assertFailsWith<AssertionError> {
worker.test {
assertNoOutput()
}
}
assertEquals(
expected = "Expected no output, completion, or error to have been emitted.",
actual = error.message
)
}

@Test fun `assertNotFinished fails after worker finished`() {
val worker = Worker.finished<Unit>()
val error = assertFailsWith<AssertionError> {
Expand All @@ -46,7 +61,10 @@ class WorkerTesterTest {
assertNoOutput()
}
}
assertEquals("Expected no output to have been emitted.", error.message)
assertEquals(
expected = "Expected no output, completion, or error to have been emitted.",
actual = error.message
)
}

@Test fun `assertFinished passes when worker finishes without emitting`() {
Expand Down