From d09bf20809ddbf81996f99ed769d290cf1a98f22 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 25 Aug 2021 15:23:56 -0400 Subject: [PATCH 1/2] 262: Deprecate Worker.createSideEffect in favour of RenderContext.runningSideEffect Replace with runningSideEffect where possible or with a Worker instead of Worker. --- .../squareup/workflow1/BaseRenderContext.kt | 2 +- .../java/com/squareup/workflow1/Worker.kt | 15 +- .../workflow1/RenderWorkflowInTest.kt | 12 +- .../workflow1/internal/WorkflowRunnerTest.kt | 8 +- .../com/squareup/workflow1/rx2/RxWorkers.kt | 3 + .../WorkerCompositionIntegrationTest.kt | 28 +- .../java/com/squareup/workflow1/WorkerTest.kt | 4 + .../workflow1/testing/RealRenderTesterTest.kt | 407 +++++++++--------- .../workflow1/testing/WorkerTesterTest.kt | 2 +- 9 files changed, 256 insertions(+), 225 deletions(-) diff --git a/workflow-core/src/main/java/com/squareup/workflow1/BaseRenderContext.kt b/workflow-core/src/main/java/com/squareup/workflow1/BaseRenderContext.kt index 779fc5b820..a7ea8c2194 100644 --- a/workflow-core/src/main/java/com/squareup/workflow1/BaseRenderContext.kt +++ b/workflow-core/src/main/java/com/squareup/workflow1/BaseRenderContext.kt @@ -246,7 +246,7 @@ public fun * Ensures a [Worker] that never emits anything is running. Since [worker] can't emit anything, * it can't trigger any [WorkflowAction]s. * - * A simple way to create workers that don't output anything is using [Worker.createSideEffect]. + * If your [Worker] does not output anything, then simply use [runningSideEffect]. * * @param key An optional string key that is used to distinguish between identical [Worker]s. */ diff --git a/workflow-core/src/main/java/com/squareup/workflow1/Worker.kt b/workflow-core/src/main/java/com/squareup/workflow1/Worker.kt index 4affad2456..15075163d5 100644 --- a/workflow-core/src/main/java/com/squareup/workflow1/Worker.kt +++ b/workflow-core/src/main/java/com/squareup/workflow1/Worker.kt @@ -177,7 +177,7 @@ public interface Worker { * Shorthand for `flow { block() }.asWorker()`. * * Note: If your worker just needs to perform side effects and doesn't need to emit anything, - * use [createSideEffect] instead (since `Nothing` can't be used as a reified type parameter). + * do not use a [Worker] but instead all [BaseRenderContext::runningSideEffect] */ @OptIn(ExperimentalTypeInference::class) public inline fun create( @@ -193,14 +193,25 @@ public interface Worker { * fun logOnEntered(message: String) = Worker.createSideEffect() { * println("Entered state: $message") * } + * ``` * * Note that all workers created with this method are equivalent from the point of view of * their [Worker.doesSameWorkAs] methods. A workflow that needs multiple simultaneous * side effects can either bundle them all together into a single `createSideEffect` * call, or can use the `key` parameter to [BaseRenderContext.runningWorker] to prevent * conflicts. - * ``` + * + * Deprecated: This convenience extension is deprecated as redundant. + * [BaseRenderContext.runningSideEffect] can be used instead with a suspend function + * and a key to uniquely identify the side effect in the runtime. */ + @Deprecated( + message = "Worker not needed, simply call RenderContext.runningSideEffect " + + "with a suspend fun.", + ReplaceWith( + expression = "runningSideEffect(key, block)" + ) + ) public fun createSideEffect( block: suspend () -> Unit ): Worker = TypedWorker(TYPE_OF_NOTHING, flow { block() }) diff --git a/workflow-runtime/src/test/java/com/squareup/workflow1/RenderWorkflowInTest.kt b/workflow-runtime/src/test/java/com/squareup/workflow1/RenderWorkflowInTest.kt index 3e0d1370a6..03347581e7 100644 --- a/workflow-runtime/src/test/java/com/squareup/workflow1/RenderWorkflowInTest.kt +++ b/workflow-runtime/src/test/java/com/squareup/workflow1/RenderWorkflowInTest.kt @@ -364,11 +364,11 @@ class RenderWorkflowInTest { @Test fun `cancelling scope cancels runtime`() { var cancellationException: Throwable? = null val workflow = Workflow.stateless { - runningWorker(Worker.createSideEffect { + runningSideEffect(key = "test1") { suspendCancellableCoroutine { continuation -> continuation.invokeOnCancellation { cause -> cancellationException = cause } } - }) + } } renderWorkflowIn(workflow, expectedSuccessScope, MutableStateFlow(Unit)) {} assertNull(cancellationException) @@ -383,11 +383,11 @@ class RenderWorkflowInTest { @Test fun `failing scope cancels runtime`() { var cancellationException: Throwable? = null val workflow = Workflow.stateless { - runningWorker(Worker.createSideEffect { + runningSideEffect(key = "failing") { suspendCancellableCoroutine { continuation -> continuation.invokeOnCancellation { cause -> cancellationException = cause } } - }) + } } renderWorkflowIn(workflow, expectedSuccessScope, MutableStateFlow(Unit)) {} assertNull(cancellationException) @@ -418,13 +418,13 @@ class RenderWorkflowInTest { @Test fun `error from renderings collector cancels runtime`() { var cancellationException: Throwable? = null val workflow = Workflow.stateless { - runningWorker(Worker.createSideEffect { + runningSideEffect(key = "test") { suspendCancellableCoroutine { continuation -> continuation.invokeOnCancellation { cause -> cancellationException = cause } } - }) + } } val renderings = renderWorkflowIn(workflow, allowedToFailScope, MutableStateFlow(Unit)) {} diff --git a/workflow-runtime/src/test/java/com/squareup/workflow1/internal/WorkflowRunnerTest.kt b/workflow-runtime/src/test/java/com/squareup/workflow1/internal/WorkflowRunnerTest.kt index e096b12d4c..4a958716ac 100644 --- a/workflow-runtime/src/test/java/com/squareup/workflow1/internal/WorkflowRunnerTest.kt +++ b/workflow-runtime/src/test/java/com/squareup/workflow1/internal/WorkflowRunnerTest.kt @@ -171,11 +171,11 @@ internal class WorkflowRunnerTest { @Test fun `cancelRuntime() cancels runtime`() { var cancellationException: Throwable? = null val workflow = Workflow.stateless { - runningWorker(Worker.createSideEffect { + runningSideEffect(key = "test side effect") { suspendCancellableCoroutine { continuation -> continuation.invokeOnCancellation { cause -> cancellationException = cause } } - }) + } } val runner = WorkflowRunner(workflow, MutableStateFlow(Unit)) runner.nextRendering() @@ -209,11 +209,11 @@ internal class WorkflowRunnerTest { @Test fun `cancelling scope cancels runtime`() { var cancellationException: Throwable? = null val workflow = Workflow.stateless { - runningWorker(Worker.createSideEffect { + runningSideEffect(key = "test") { suspendCancellableCoroutine { continuation -> continuation.invokeOnCancellation { cause -> cancellationException = cause } } - }) + } } val runner = WorkflowRunner(workflow, MutableStateFlow(Unit)) runner.nextRendering() diff --git a/workflow-rx2/src/main/java/com/squareup/workflow1/rx2/RxWorkers.kt b/workflow-rx2/src/main/java/com/squareup/workflow1/rx2/RxWorkers.kt index a02d573c7c..d945aba0c5 100644 --- a/workflow-rx2/src/main/java/com/squareup/workflow1/rx2/RxWorkers.kt +++ b/workflow-rx2/src/main/java/com/squareup/workflow1/rx2/RxWorkers.kt @@ -78,5 +78,8 @@ public inline fun Single.asWorker(): Worker = * * The key is required for this operator because there is no type information available to * distinguish workers. + * + * TODO: https://github.com/square/workflow-kotlin/issues/526 once this is removed. */ +@Suppress("DEPRECATION") public fun Completable.asWorker(): Worker = Worker.createSideEffect { await() } diff --git a/workflow-testing/src/test/java/com/squareup/workflow1/WorkerCompositionIntegrationTest.kt b/workflow-testing/src/test/java/com/squareup/workflow1/WorkerCompositionIntegrationTest.kt index 06d4985bfa..4c1fcdece8 100644 --- a/workflow-testing/src/test/java/com/squareup/workflow1/WorkerCompositionIntegrationTest.kt +++ b/workflow-testing/src/test/java/com/squareup/workflow1/WorkerCompositionIntegrationTest.kt @@ -140,7 +140,9 @@ internal class WorkerCompositionIntegrationTest { @Test fun `runningWorker gets error`() { val workflow = Workflow.stateless { - runningWorker(Worker.createSideEffect { throw ExpectedException() }) + runningWorker(Worker.from { throw ExpectedException() }) { + action { } + } } assertFailsWith { @@ -156,8 +158,8 @@ internal class WorkerCompositionIntegrationTest { val channel = Channel() val workflow = Workflow.stateless { runningWorker( - channel.consumeAsFlow() - .asWorker() + channel.consumeAsFlow() + .asWorker() ) { fail("Expected handler to not be invoked.") } } @@ -180,12 +182,12 @@ internal class WorkerCompositionIntegrationTest { } val workflow = Workflow.stateful Unit>( - initialState = 0, - render = { state -> - runningWorker(triggerOutput) { action { setOutput(state) } } + initialState = 0, + render = { state -> + runningWorker(triggerOutput) { action { setOutput(state) } } - return@stateful { actionSink.send(incrementState) } - } + return@stateful { actionSink.send(incrementState) } + } ) workflow.launchForTestingFromStartWith { @@ -193,13 +195,13 @@ internal class WorkerCompositionIntegrationTest { assertEquals(0, awaitNextOutput()) awaitNextRendering() - .invoke() + .invoke() triggerOutput.send(Unit) assertEquals(1, awaitNextOutput()) awaitNextRendering() - .invoke() + .invoke() triggerOutput.send(Unit) assertEquals(2, awaitNextOutput()) @@ -223,9 +225,11 @@ internal class WorkerCompositionIntegrationTest { @Test fun `runningWorker doesn't throw when worker finishes`() { // No-op worker, completes immediately. - val worker = Worker.createSideEffect {} + val worker = Worker.from { } val workflow = Workflow.stateless { - runningWorker(worker) + runningWorker(worker) { + action { } + } } workflow.launchForTestingFromStartWith { diff --git a/workflow-testing/src/test/java/com/squareup/workflow1/WorkerTest.kt b/workflow-testing/src/test/java/com/squareup/workflow1/WorkerTest.kt index 95393c7eb2..356828c7ea 100644 --- a/workflow-testing/src/test/java/com/squareup/workflow1/WorkerTest.kt +++ b/workflow-testing/src/test/java/com/squareup/workflow1/WorkerTest.kt @@ -44,6 +44,7 @@ class WorkerTest { } } + @Suppress("DEPRECATION") @Test fun `createSideEffect returns equivalent workers`() { val worker1 = Worker.createSideEffect {} val worker2 = Worker.createSideEffect {} @@ -52,6 +53,7 @@ class WorkerTest { assertTrue(worker1.doesSameWorkAs(worker2)) } + @Suppress("DEPRECATION") @Test fun `createSideEffect runs`() { var ran = false val worker = Worker.createSideEffect { @@ -63,6 +65,7 @@ class WorkerTest { } } + @Suppress("DEPRECATION") @Test fun `createSideEffect finishes`() { val worker = Worker.createSideEffect {} @@ -71,6 +74,7 @@ class WorkerTest { } } + @Suppress("DEPRECATION") @Test fun `createSideEffect propagates exceptions`() { val worker = Worker.createSideEffect { throw ExpectedException() } diff --git a/workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt b/workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt index 94197df96a..5bd04b92ad 100644 --- a/workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt +++ b/workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt @@ -14,6 +14,7 @@ import com.squareup.workflow1.WorkflowAction import com.squareup.workflow1.WorkflowAction.Companion.noAction import com.squareup.workflow1.WorkflowIdentifier import com.squareup.workflow1.WorkflowOutput +import com.squareup.workflow1.action import com.squareup.workflow1.asWorker import com.squareup.workflow1.contraMap import com.squareup.workflow1.identifier @@ -52,16 +53,16 @@ class RealRenderTesterTest { renderChild(child2) { noAction() } } val tester = workflow.testRender(Unit) - .expectWorkflow(child1.identifier, rendering = Unit, output = WorkflowOutput(Unit)) - .expectWorkflow(child2.identifier, rendering = Unit, output = WorkflowOutput(Unit)) + .expectWorkflow(child1.identifier, rendering = Unit, output = WorkflowOutput(Unit)) + .expectWorkflow(child2.identifier, rendering = Unit, output = WorkflowOutput(Unit)) val failure = assertFailsWith { tester.render() } assertEquals( - "Expected only one output to be expected: child ${child2.identifier} " + - "expected to emit kotlin.Unit but WorkflowAction.noAction() was already processed.", - failure.message + "Expected only one output to be expected: child ${child2.identifier} " + + "expected to emit kotlin.Unit but WorkflowAction.noAction() was already processed.", + failure.message ) } @@ -73,18 +74,18 @@ class RealRenderTesterTest { runningWorker(worker) { noAction() } } val tester = workflow.testRender(Unit) - .expectWorkflow(child.identifier, rendering = Unit, output = WorkflowOutput(Unit)) - .expectWorker(typeOf>(), output = WorkflowOutput(Unit)) + .expectWorkflow(child.identifier, rendering = Unit, output = WorkflowOutput(Unit)) + .expectWorker(typeOf>(), output = WorkflowOutput(Unit)) val failure = assertFailsWith { tester.render() } assertEquals( - "Expected only one output to be expected: " + - "child worker ${typeOf>()} expected to emit " + - "kotlin.Unit but WorkflowAction.noAction() was already processed.", - failure.message + "Expected only one output to be expected: " + + "child worker ${typeOf>()} expected to emit " + + "kotlin.Unit but WorkflowAction.noAction() was already processed.", + failure.message ) } @@ -92,10 +93,10 @@ class RealRenderTesterTest { // Don't need an implementation, the test should fail before even calling render. val workflow = Workflow.stateless {} val tester = workflow.testRender(Unit) - .expectWorkflow( - OutputWhateverChild::class, rendering = Unit, - output = WorkflowOutput(Unit) - ) + .expectWorkflow( + OutputWhateverChild::class, rendering = Unit, + output = WorkflowOutput(Unit) + ) // Doesn't throw. tester.expectWorkflow(workflow::class, rendering = Unit) @@ -106,17 +107,17 @@ class RealRenderTesterTest { runningSideEffect("the key") {} } val tester = workflow.testRender(Unit) - .expectSideEffect(key = "the key") - .expectSideEffect(description = "duplicate match") { it == "the key" } + .expectSideEffect(key = "the key") + .expectSideEffect(description = "duplicate match") { it == "the key" } val error = assertFailsWith { tester.render() } assertEquals( - "Multiple expectations matched side effect with key \"the key\":\n" + - " side effect with key \"the key\"\n" + - " duplicate match", - error.message + "Multiple expectations matched side effect with key \"the key\":\n" + + " side effect with key \"the key\"\n" + + " duplicate match", + error.message ) } @@ -126,24 +127,24 @@ class RealRenderTesterTest { } workflow.testRender(Unit) - .expectSideEffect("the key") - .render {} + .expectSideEffect("the key") + .render {} } @Test fun `expectSideEffect doesn't match key`() { val workflow = Workflow.stateless {} val tester = workflow.testRender(Unit) - .expectSideEffect("the key") + .expectSideEffect("the key") val error = assertFailsWith { tester.render {} } assertEquals( - """ + """ Expected 1 more workflows, workers, or side effects to be run: side effect with key "the key" """.trimIndent(), - error.message + error.message ) } @@ -154,26 +155,26 @@ class RealRenderTesterTest { } val workflow = Workflow.stateful>( - initialState = Unit, - render = { actionSink.contraMap { it } } + initialState = Unit, + render = { actionSink.contraMap { it } } ) val action1 = TestAction("1") val action2 = TestAction("2") workflow.testRender(Unit) - .render { sink -> - sink.send(action1) - - val error = assertFailsWith { - sink.send(action2) - } - assertEquals( - "Tried to send action to sink after another action was already processed:\n" + - " processed action=$action1\n" + - " attempted action=$action2", - error.message - ) + .render { sink -> + sink.send(action1) + + val error = assertFailsWith { + sink.send(action2) } + assertEquals( + "Tried to send action to sink after another action was already processed:\n" + + " processed action=$action1\n" + + " attempted action=$action2", + error.message + ) + } } @Test fun `sending to sink throws when child output expected`() { @@ -183,27 +184,27 @@ class RealRenderTesterTest { } val workflow = Workflow.stateful>( - initialState = Unit, - render = { - // Need to satisfy the expectation. - runningWorker(Worker.finished()) { noAction() } - return@stateful actionSink.contraMap { it } - } + initialState = Unit, + render = { + // Need to satisfy the expectation. + runningWorker(Worker.finished()) { noAction() } + return@stateful actionSink.contraMap { it } + } ) workflow.testRender(Unit) - .expectWorker(typeOf>(), output = WorkflowOutput(Unit)) - .render { sink -> - val error = assertFailsWith { - sink.send(TestAction()) - } - assertEquals( - "Tried to send action to sink after another action was already processed:\n" + - " processed action=WorkflowAction.noAction()\n" + - " attempted action=TestAction", - error.message - ) + .expectWorker(typeOf>(), output = WorkflowOutput(Unit)) + .render { sink -> + val error = assertFailsWith { + sink.send(TestAction()) } + assertEquals( + "Tried to send action to sink after another action was already processed:\n" + + " processed action=WorkflowAction.noAction()\n" + + " attempted action=TestAction", + error.message + ) + } } @Test fun `failures from render block escape`() { @@ -229,8 +230,8 @@ class RealRenderTesterTest { tester.render() } assertEquals( - "Tried to render unexpected child ${child.identifier}", - error.message + "Tried to render unexpected child ${child.identifier}", + error.message ) } @@ -241,11 +242,11 @@ class RealRenderTesterTest { } workflow.testRender(Unit) - .expectWorkflow(exactMatch = false, description = "expect1") { Matched("one") } - .expectWorkflow(exactMatch = false, description = "expect2") { Matched("two") } - .render { rendering -> - assertEquals("one", rendering) - } + .expectWorkflow(exactMatch = false, description = "expect1") { Matched("one") } + .expectWorkflow(exactMatch = false, description = "expect2") { Matched("two") } + .render { rendering -> + assertEquals("one", rendering) + } } @Test fun `matching inexact workflow matches are allowed with matching exact match`() { @@ -255,20 +256,20 @@ class RealRenderTesterTest { } workflow.testRender(Unit) - .expectWorkflow(exactMatch = false, description = "expect1") { Matched("one") } - .expectWorkflow(exactMatch = true, description = "expect2") { Matched("two") } - .render { rendering -> - assertEquals("two", rendering) - } + .expectWorkflow(exactMatch = false, description = "expect1") { Matched("one") } + .expectWorkflow(exactMatch = true, description = "expect2") { Matched("two") } + .render { rendering -> + assertEquals("two", rendering) + } } @Test fun `unmatching inexact workflow matches are allowed`() { val workflow = Workflow.stateless {} workflow.testRender(Unit) - .expectWorkflow(exactMatch = false, description = "expect1") { Matched(Unit) } - .expectWorkflow(exactMatch = false, description = "expect2") { Matched(Unit) } - .render() + .expectWorkflow(exactMatch = false, description = "expect1") { Matched(Unit) } + .expectWorkflow(exactMatch = false, description = "expect2") { Matched(Unit) } + .render() } @Test fun `multiple matching inexact side effect matches are allowed`() { @@ -277,9 +278,9 @@ class RealRenderTesterTest { } workflow.testRender(Unit) - .expectSideEffect(exactMatch = false, description = "expect1") { true } - .expectSideEffect(exactMatch = false, description = "expect2") { true } - .render() + .expectSideEffect(exactMatch = false, description = "expect1") { true } + .expectSideEffect(exactMatch = false, description = "expect2") { true } + .render() } @Test fun `matching inexact side effect matches are allowed with matching exact match`() { @@ -288,17 +289,17 @@ class RealRenderTesterTest { } workflow.testRender(Unit) - .expectSideEffect(exactMatch = false, description = "expect1") { true } - .expectSideEffect(exactMatch = true, description = "expect2") { true } - .render() + .expectSideEffect(exactMatch = false, description = "expect1") { true } + .expectSideEffect(exactMatch = true, description = "expect2") { true } + .render() } @Test fun `non-matching inexact side effect is ignored`() { val workflow = Workflow.stateless {} workflow.testRender(Unit) - .expectSideEffect(exactMatch = false, description = "expect1") { false } - .render() + .expectSideEffect(exactMatch = false, description = "expect1") { false } + .render() } @Test fun `distinct side effect expectations match individual side effects`() { @@ -308,18 +309,18 @@ class RealRenderTesterTest { } workflow.testRender(Unit) - .expectSideEffect(exactMatch = true, description = "expect2") { it == "effect2" } - .expectSideEffect(exactMatch = true, description = "expect1") { it == "effect1" } - .render() + .expectSideEffect(exactMatch = true, description = "expect2") { it == "effect2" } + .expectSideEffect(exactMatch = true, description = "expect1") { it == "effect1" } + .render() } @Test fun `non-matched inexact side effect matches are allowed`() { val workflow = Workflow.stateless {} workflow.testRender(Unit) - .expectSideEffect(exactMatch = false, description = "expect1") { true } - .expectSideEffect(exactMatch = false, description = "expect2") { true } - .render() + .expectSideEffect(exactMatch = false, description = "expect1") { true } + .expectSideEffect(exactMatch = false, description = "expect2") { true } + .render() } @Test fun `runningSideEffect throws when no expectations match`() { @@ -340,7 +341,7 @@ class RealRenderTesterTest { runningSideEffect("unexpected") {} } val tester = workflow.testRender(Unit) - .expectSideEffect("expected") + .expectSideEffect("expected") val error = assertFailsWith { tester.render() @@ -353,17 +354,17 @@ class RealRenderTesterTest { runningSideEffect("effect") {} } val tester = workflow.testRender(Unit) - .expectSideEffect("effect") - .expectSideEffect(description = "custom", exactMatch = true) { key -> "effect" in key } + .expectSideEffect("effect") + .expectSideEffect(description = "custom", exactMatch = true) { key -> "effect" in key } val error = assertFailsWith { tester.render() } assertEquals( - "Multiple expectations matched side effect with key \"effect\":\n" + - " side effect with key \"effect\"\n" + - " custom", - error.message + "Multiple expectations matched side effect with key \"effect\":\n" + + " side effect with key \"effect\"\n" + + " custom", + error.message ) } @@ -374,7 +375,7 @@ class RealRenderTesterTest { } val tester = workflow.testRender(Unit) - .expectSideEffect("key") + .expectSideEffect("key") val error = assertFailsWith { tester.render() @@ -394,8 +395,8 @@ class RealRenderTesterTest { tester.render() } assertEquals( - "Tried to render unexpected child ${child.identifier}", - error.message + "Tried to render unexpected child ${child.identifier}", + error.message ) } @@ -406,14 +407,14 @@ class RealRenderTesterTest { renderChild(child) } val tester = workflow.testRender(Unit) - .expectWorkflow(OutputNothingChild::class, rendering = Unit) + .expectWorkflow(OutputNothingChild::class, rendering = Unit) val error = assertFailsWith { tester.render() } assertEquals( - "Tried to render unexpected child ${child.identifier}", - error.message + "Tried to render unexpected child ${child.identifier}", + error.message ) } @@ -424,14 +425,14 @@ class RealRenderTesterTest { renderChild(child) } val tester = workflow.testRender(Unit) - .expectWorkflow(OutputNothingChild::class, rendering = Unit) + .expectWorkflow(OutputNothingChild::class, rendering = Unit) val error = assertFailsWith { tester.render() } assertEquals( - "Tried to render unexpected child ${child.identifier}", - error.message + "Tried to render unexpected child ${child.identifier}", + error.message ) } @@ -446,8 +447,8 @@ class RealRenderTesterTest { tester.render() } assertEquals( - "Tried to render unexpected child ${child.identifier} with key \"key\"", - error.message + "Tried to render unexpected child ${child.identifier} with key \"key\"", + error.message ) } @@ -457,14 +458,14 @@ class RealRenderTesterTest { renderChild(child, key = "key") } val tester = workflow.testRender(Unit) - .expectWorkflow(OutputNothingChild::class, rendering = Unit) + .expectWorkflow(OutputNothingChild::class, rendering = Unit) val error = assertFailsWith { tester.render() } assertEquals( - "Tried to render unexpected child ${child.identifier} with key \"key\"", - error.message + "Tried to render unexpected child ${child.identifier} with key \"key\"", + error.message ) } @@ -475,14 +476,14 @@ class RealRenderTesterTest { renderChild(child, key = "key") } val tester = workflow.testRender(Unit) - .expectWorkflow(OutputNothingChild::class, rendering = Unit, key = "wrong key") + .expectWorkflow(OutputNothingChild::class, rendering = Unit, key = "wrong key") val error = assertFailsWith { tester.render() } assertEquals( - "Tried to render unexpected child ${child.identifier} with key \"key\"", - error.message + "Tried to render unexpected child ${child.identifier} with key \"key\"", + error.message ) } @@ -500,19 +501,19 @@ class RealRenderTesterTest { renderChild(Child()) } val tester = workflow.testRender(Unit) - .expectWorkflow(OutputNothingChild::class, rendering = Unit) - .expectWorkflow(Child::class, rendering = Unit) + .expectWorkflow(OutputNothingChild::class, rendering = Unit) + .expectWorkflow(Child::class, rendering = Unit) val error = assertFailsWith { tester.render() } assertEquals( - """ + """ Multiple expectations matched child ${Child::class.workflowIdentifier}: workflow identifier=${OutputNothingChild::class.workflowIdentifier}, key=, rendering=kotlin.Unit, output=null workflow identifier=${Child::class.workflowIdentifier}, key=, rendering=kotlin.Unit, output=null """.trimIndent(), - error.message + error.message ) } @@ -524,7 +525,7 @@ class RealRenderTesterTest { } val tester = workflow.testRender(Unit) - .expectWorkflow(child.identifier, Unit) + .expectWorkflow(child.identifier, Unit) val error = assertFailsWith { tester.render() @@ -552,6 +553,7 @@ class RealRenderTesterTest { override fun run(): Flow = emptyFlow() override fun toString(): String = "TestWorker" } + val worker = MySpecialWorker() val workflow = Workflow.stateless { @@ -561,8 +563,10 @@ class RealRenderTesterTest { val error = assertFailsWith { tester.render() } - assertEquals("Tried to render unexpected child worker ${typeOf()}", - error.message) + assertEquals( + "Tried to render unexpected child worker ${typeOf()}", + error.message + ) } @Test fun `render throws when worker expectation doesn't match`() { @@ -576,14 +580,14 @@ class RealRenderTesterTest { runningWorker(worker) { fail() } } val tester = workflow.testRender(Unit) - .expectWorker(typeOf>()) + .expectWorker(typeOf>()) val error = assertFailsWith { tester.render() } assertTrue( - error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") + error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") ) } @@ -613,13 +617,13 @@ class RealRenderTesterTest { runningWorker(worker, key = "key") } val tester = workflow.testRender(Unit) - .expectWorker(typeOf>()) + .expectWorker(typeOf>()) val error = assertFailsWith { tester.render() } assertTrue( - error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") + error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") ) } @@ -634,16 +638,16 @@ class RealRenderTesterTest { runningWorker(worker, key = "key") } val tester = workflow.testRender(Unit) - .expectWorker( - typeOf>(), - key = "wrong key" - ) + .expectWorker( + typeOf>(), + key = "wrong key" + ) val error = assertFailsWith { tester.render() } assertTrue( - error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") + error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") ) } @@ -659,19 +663,19 @@ class RealRenderTesterTest { runningWorker(worker) } val tester = workflow.testRender(Unit) - .expectWorker(worker) - .expectWorker(worker, description = "duplicate expectation") + .expectWorker(worker) + .expectWorker(worker, description = "duplicate expectation") val error = assertFailsWith { tester.render() } assertEquals( - """ + """ Multiple expectations matched child worker ${typeOf()}: worker TestWorker duplicate expectation """.trimIndent(), - error.message + error.message ) } @@ -684,9 +688,9 @@ class RealRenderTesterTest { runningWorker(stringWorker) { noAction() } } workflow.testRender(Unit) - .expectWorker(lifecycleWorker) - .expectWorker(stringWorker) - .render() + .expectWorker(lifecycleWorker) + .expectWorker(stringWorker) + .render() // No exception, no bug. } @@ -708,10 +712,14 @@ class RealRenderTesterTest { } @Test fun `runningWorker throws on duplicate call`() { - val worker = Worker.createSideEffect {} + val worker = Worker.from { } val workflow = Workflow.stateless { - runningWorker(worker) - runningWorker(worker) + runningWorker(worker) { + action { } + } + runningWorker(worker) { + action { } + } } val tester = workflow.testRender(Unit) @@ -719,9 +727,9 @@ class RealRenderTesterTest { tester.render() } assertEquals( - "Expected keys to be unique for worker " + - "com.squareup.workflow1.Worker: key=\"\"", - error.message + "Expected keys to be unique for worker " + + "com.squareup.workflow1.Worker: key=\"\"", + error.message ) } @@ -730,13 +738,13 @@ class RealRenderTesterTest { // Do nothing. } val tester = workflow.testRender(Unit) - .expectWorkflow(OutputNothingChild::class, rendering = Unit) + .expectWorkflow(OutputNothingChild::class, rendering = Unit) val error = assertFailsWith { tester.render() } assertTrue( - error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") + error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") ) } @@ -745,13 +753,13 @@ class RealRenderTesterTest { // Do nothing. } val tester = workflow.testRender(Unit) - .expectSideEffect("expectation", exactMatch = true) { true } + .expectSideEffect("expectation", exactMatch = true) { true } val error = assertFailsWith { tester.render() } assertTrue( - error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") + error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") ) } @@ -760,13 +768,13 @@ class RealRenderTesterTest { // Do nothing. } val tester = workflow.testRender(Unit) - .expectWorker(typeOf>()) + .expectWorker(typeOf>()) val error = assertFailsWith { tester.render() } assertTrue( - error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") + error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be run:") ) } @@ -783,7 +791,7 @@ class RealRenderTesterTest { renderChild(child) } val tester = workflow.testRender(Unit) - .expectWorkflow(OutputNothingChild::class, rendering = Unit) + .expectWorkflow(OutputNothingChild::class, rendering = Unit) tester.render() } @@ -795,7 +803,7 @@ class RealRenderTesterTest { } class TestImpostor(val proxy: Workflow<*, *, *>) : Workflow, - ImpostorWorkflow { + ImpostorWorkflow { override val realIdentifier: WorkflowIdentifier get() = proxy.identifier override fun asStatefulWorkflow(): StatefulWorkflow = throw NotImplementedError() @@ -805,11 +813,11 @@ class RealRenderTesterTest { renderChild(TestImpostor(TestWorkflow())) } workflow.testRender(Unit) - .expectWorkflow( - TestImpostor(TestWorkflow()).identifier, - Unit - ) - .render {} + .expectWorkflow( + TestImpostor(TestWorkflow()).identifier, + Unit + ) + .render {} } @Test @@ -825,7 +833,7 @@ class RealRenderTesterTest { } class TestImpostor(val proxy: Workflow<*, *, *>) : Workflow, - ImpostorWorkflow { + ImpostorWorkflow { override val realIdentifier: WorkflowIdentifier get() = proxy.identifier override fun asStatefulWorkflow(): StatefulWorkflow = throw NotImplementedError() @@ -838,13 +846,13 @@ class RealRenderTesterTest { val actualId = TestImpostor(TestWorkflowActual()).identifier val tester = workflow.testRender(Unit) - .expectWorkflow(expectedId, Unit) + .expectWorkflow(expectedId, Unit) val error = assertFailsWith { tester.render {} } assertEquals( - "Tried to render unexpected child $actualId", error.message + "Tried to render unexpected child $actualId", error.message ) } @@ -856,14 +864,14 @@ class RealRenderTesterTest { } class TestImpostorActual(val proxy: Workflow<*, *, *>) : Workflow, - ImpostorWorkflow { + ImpostorWorkflow { override val realIdentifier: WorkflowIdentifier get() = proxy.identifier override fun asStatefulWorkflow(): StatefulWorkflow = throw NotImplementedError() } class TestImpostorExpected(val proxy: Workflow<*, *, *>) : Workflow, - ImpostorWorkflow { + ImpostorWorkflow { override val realIdentifier: WorkflowIdentifier get() = proxy.identifier override fun asStatefulWorkflow(): StatefulWorkflow = throw NotImplementedError() @@ -875,8 +883,8 @@ class RealRenderTesterTest { val expectedId = TestImpostorExpected(TestWorkflow()).identifier workflow.testRender(Unit) - .expectWorkflow(expectedId, Unit) - .render {} + .expectWorkflow(expectedId, Unit) + .render {} } @Test fun `assertProps failure fails test`() { @@ -885,11 +893,11 @@ class RealRenderTesterTest { renderChild(child, "wrong props") } val tester = workflow.testRender(Unit) - .expectWorkflow( - workflowType = child::class, - rendering = Unit, - assertProps = { props -> throw AssertionError("bad props: $props") } - ) + .expectWorkflow( + workflowType = child::class, + rendering = Unit, + assertProps = { props -> throw AssertionError("bad props: $props") } + ) val error = assertFailsWith { tester.render() @@ -907,7 +915,7 @@ class RealRenderTesterTest { actionSink.contraMap { it } } val testResult = workflow.testRender(Unit) - .render { it.send(TestAction("noop")) } + .render { it.send(TestAction("noop")) } val error = assertFailsWith { testResult.verifyAction { throw AssertionError("action failed") } @@ -921,12 +929,12 @@ class RealRenderTesterTest { renderChild(child) { TestAction(it) } } val testResult = workflow.testRender(Unit) - .expectWorkflow( - workflowType = child::class, - rendering = Unit, - output = WorkflowOutput("output") - ) - .render() + .expectWorkflow( + workflowType = child::class, + rendering = Unit, + output = WorkflowOutput("output") + ) + .render() testResult.verifyAction { assertTrue(it is TestAction) @@ -940,8 +948,8 @@ class RealRenderTesterTest { runningWorker(worker) { TestAction(it) } } val testResult = workflow.testRender(Unit) - .expectWorker(worker, output = WorkflowOutput("output")) - .render() + .expectWorker(worker, output = WorkflowOutput("output")) + .render() testResult.verifyAction { assertTrue(it is TestAction) @@ -954,9 +962,9 @@ class RealRenderTesterTest { actionSink.contraMap { it } } val testResult = workflow.testRender(Unit) - .render { sink -> - sink.send(TestAction("event")) - } + .render { sink -> + sink.send(TestAction("event")) + } testResult.verifyAction { assertTrue(it is TestAction) @@ -969,9 +977,9 @@ class RealRenderTesterTest { actionSink.contraMap { it } } val testResult = workflow.testRender(Unit) - .render { - // Don't send to sink! - } + .render { + // Don't send to sink! + } testResult.verifyAction { assertEquals(noAction(), it) } testResult.verifyActionResult { newState, output -> @@ -985,9 +993,9 @@ class RealRenderTesterTest { actionSink.contraMap { it } } val testResult = workflow.testRender(Unit) - .render { - // Don't send to sink! - } + .render { + // Don't send to sink! + } testResult.verifyActionResult { newState, output -> assertSame(Unit, newState) @@ -1004,13 +1012,13 @@ class RealRenderTesterTest { } val workflow = Workflow.stateful>( - initialState = { "initial" }, - render = { _, _ -> actionSink.contraMap { it } } + initialState = { "initial" }, + render = { _, _ -> actionSink.contraMap { it } } ) val testResult = workflow.testRender(Unit) - .render { sink -> - sink.send(TestAction()) - } + .render { sink -> + sink.send(TestAction()) + } testResult.verifyActionResult { state, output -> assertEquals("new state", state) @@ -1023,7 +1031,7 @@ class RealRenderTesterTest { val workflow = Workflow.stateless { renderCount++ } workflow.testRender(Unit) - .render() + .render() assertEquals(2, renderCount) } @@ -1042,8 +1050,8 @@ class RealRenderTesterTest { @Test fun `createRenderChildInvocation() for Workflow-stateful{}`() { val workflow = Workflow.stateful( - initialState = "", - render = {} + initialState = "", + render = {} ) val invocation = createRenderChildInvocation(workflow, "props", "key") @@ -1167,10 +1175,10 @@ class RealRenderTesterTest { renderChild(ChildWorkflow()) } workflow.testRender(Unit) - .expectWorkflow(ChildWorkflow::class, rendering = 42) - .render { rendering -> - assertEquals(42, rendering) - } + .expectWorkflow(ChildWorkflow::class, rendering = 42) + .render { rendering -> + assertEquals(42, rendering) + } } @Test fun `worker ran after workflow matches workflow expectation`() { @@ -1184,13 +1192,13 @@ class RealRenderTesterTest { val childWorker = Worker.finished() val workflow = Workflow.stateless { renderChild(ChildWorkflow()) - .also { runningWorker(childWorker) { fail() } } + .also { runningWorker(childWorker) { fail() } } } workflow.testRender(Unit) - .expectWorkflow(ChildWorkflow::class, rendering = 42) - .render { rendering -> - assertEquals(42, rendering) - } + .expectWorkflow(ChildWorkflow::class, rendering = 42) + .render { rendering -> + assertEquals(42, rendering) + } } @Test fun `realTypeMatchesExpectation() matches exact type`() { @@ -1288,6 +1296,7 @@ class RealRenderTesterTest { val actual = mock().identifier assertTrue(actual.realTypeMatchesExpectation(expected)) } + @Test fun `realTypeMatchesExpectation() doesn't match mockito mock of unexpected interface`() { val expected = TestWorkflowInterface::class.workflowIdentifier val actual = mock>().identifier diff --git a/workflow-testing/src/test/java/com/squareup/workflow1/testing/WorkerTesterTest.kt b/workflow-testing/src/test/java/com/squareup/workflow1/testing/WorkerTesterTest.kt index 296342bd13..b500a8bfff 100644 --- a/workflow-testing/src/test/java/com/squareup/workflow1/testing/WorkerTesterTest.kt +++ b/workflow-testing/src/test/java/com/squareup/workflow1/testing/WorkerTesterTest.kt @@ -35,7 +35,7 @@ class WorkerTesterTest { } @Test fun `assertFinished fails when worker hasn't finished and hasn't emitted`() { - val worker = Worker.createSideEffect { suspendCancellableCoroutine {} } + val worker = Worker.from { suspendCancellableCoroutine {} } val error = assertFailsWith { worker.test { assertFinished() From 7d541ad233fa09e34499ef289b9ce2fd65c2c6c0 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 25 Aug 2021 17:36:52 -0400 Subject: [PATCH 2/2] Update workflow-core/src/main/java/com/squareup/workflow1/Worker.kt Co-authored-by: Ray Ryan --- workflow-core/src/main/java/com/squareup/workflow1/Worker.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow-core/src/main/java/com/squareup/workflow1/Worker.kt b/workflow-core/src/main/java/com/squareup/workflow1/Worker.kt index 15075163d5..c34346815b 100644 --- a/workflow-core/src/main/java/com/squareup/workflow1/Worker.kt +++ b/workflow-core/src/main/java/com/squareup/workflow1/Worker.kt @@ -177,7 +177,7 @@ public interface Worker { * Shorthand for `flow { block() }.asWorker()`. * * Note: If your worker just needs to perform side effects and doesn't need to emit anything, - * do not use a [Worker] but instead all [BaseRenderContext::runningSideEffect] + * do not use a [Worker] but instead call [BaseRenderContext::runningSideEffect] */ @OptIn(ExperimentalTypeInference::class) public inline fun create(