From 48374e261ff5a893a0f74e022acbb9b11ba31966 Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Fri, 30 May 2025 16:04:28 -0400 Subject: [PATCH] Revert "Update expectRemember to provide result" - This change forced us to provide an `expectRemember` call for every `remember` call - Under `STABLE_EVENT_HANDLERS` that means every `eventHandler` call, clearly impractical - So we could change it to allow providing an option of returning alternative value from the remember call, but default to run the real calculation; but we have found no use cases for that - So were just going back to the original implementation This reverts commit fa75ef8e54a1c2c472787089fb23cb8c2844e7ef. --- workflow-testing/api/workflow-testing.api | 17 +---- .../workflow1/testing/RealRenderTester.kt | 52 +++++++------- .../workflow1/testing/RenderTester.kt | 35 +++------- .../workflow1/testing/RealRenderTesterTest.kt | 69 ++++--------------- 4 files changed, 52 insertions(+), 121 deletions(-) diff --git a/workflow-testing/api/workflow-testing.api b/workflow-testing/api/workflow-testing.api index b1810c4360..fc8db0ba34 100644 --- a/workflow-testing/api/workflow-testing.api +++ b/workflow-testing/api/workflow-testing.api @@ -34,6 +34,7 @@ public abstract class com/squareup/workflow1/testing/RenderTester { public static synthetic fun expectSideEffect$default (Lcom/squareup/workflow1/testing/RenderTester;Ljava/lang/String;ZLkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/testing/RenderTester; public abstract fun render (Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/testing/RenderTestResult; public static synthetic fun render$default (Lcom/squareup/workflow1/testing/RenderTester;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/testing/RenderTestResult; + public abstract fun requireExplicitRememberExpectations ()Lcom/squareup/workflow1/testing/RenderTester; public abstract fun requireExplicitSideEffectExpectations ()Lcom/squareup/workflow1/testing/RenderTester; public abstract fun requireExplicitWorkerExpectations ()Lcom/squareup/workflow1/testing/RenderTester; } @@ -62,18 +63,6 @@ public final class com/squareup/workflow1/testing/RenderTester$RememberInvocatio public final fun getResultType ()Lkotlin/reflect/KType; } -public abstract class com/squareup/workflow1/testing/RenderTester$RememberMatch { -} - -public final class com/squareup/workflow1/testing/RenderTester$RememberMatch$Matched : com/squareup/workflow1/testing/RenderTester$RememberMatch { - public fun (Ljava/lang/Object;)V - public final fun getResult ()Ljava/lang/Object; -} - -public final class com/squareup/workflow1/testing/RenderTester$RememberMatch$NotMatched : com/squareup/workflow1/testing/RenderTester$RememberMatch { - public static final field INSTANCE Lcom/squareup/workflow1/testing/RenderTester$RememberMatch$NotMatched; -} - public final class com/squareup/workflow1/testing/RenderTester$RenderChildInvocation { public fun (Lcom/squareup/workflow1/Workflow;Ljava/lang/Object;Lkotlin/reflect/KTypeProjection;Lkotlin/reflect/KTypeProjection;Ljava/lang/String;)V public final fun getOutputType ()Lkotlin/reflect/KTypeProjection; @@ -86,8 +75,8 @@ public final class com/squareup/workflow1/testing/RenderTester$RenderChildInvoca public final class com/squareup/workflow1/testing/RenderTesterKt { public static final fun expectCovariantWorkflow (Lcom/squareup/workflow1/testing/RenderTester;Lkotlin/reflect/KClass;Lkotlin/reflect/KType;ILkotlin/reflect/KType;ILjava/lang/Object;Lcom/squareup/workflow1/WorkflowOutput;Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/testing/RenderTester; public static synthetic fun expectCovariantWorkflow$default (Lcom/squareup/workflow1/testing/RenderTester;Lkotlin/reflect/KClass;Lkotlin/reflect/KType;ILkotlin/reflect/KType;ILjava/lang/Object;Lcom/squareup/workflow1/WorkflowOutput;Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/testing/RenderTester; - public static final fun expectRemember (Lcom/squareup/workflow1/testing/RenderTester;Ljava/lang/String;Lkotlin/reflect/KType;Ljava/lang/Object;[Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/testing/RenderTester; - public static synthetic fun expectRemember$default (Lcom/squareup/workflow1/testing/RenderTester;Ljava/lang/String;Lkotlin/reflect/KType;Ljava/lang/Object;[Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/testing/RenderTester; + public static final fun expectRemember (Lcom/squareup/workflow1/testing/RenderTester;Ljava/lang/String;Lkotlin/reflect/KType;[Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/testing/RenderTester; + public static synthetic fun expectRemember$default (Lcom/squareup/workflow1/testing/RenderTester;Ljava/lang/String;Lkotlin/reflect/KType;[Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/testing/RenderTester; public static final fun expectSideEffect (Lcom/squareup/workflow1/testing/RenderTester;Ljava/lang/String;)Lcom/squareup/workflow1/testing/RenderTester; public static final fun expectWorkflow (Lcom/squareup/workflow1/testing/RenderTester;Lcom/squareup/workflow1/WorkflowIdentifier;Ljava/lang/Object;Lcom/squareup/workflow1/WorkflowOutput;Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/testing/RenderTester; public static final fun expectWorkflow (Lcom/squareup/workflow1/testing/RenderTester;Lcom/squareup/workflow1/WorkflowIdentifier;Ljava/lang/Object;Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/testing/RenderTester; diff --git a/workflow-testing/src/main/java/com/squareup/workflow1/testing/RealRenderTester.kt b/workflow-testing/src/main/java/com/squareup/workflow1/testing/RealRenderTester.kt index 31ff9f38cf..b408f0b579 100644 --- a/workflow-testing/src/main/java/com/squareup/workflow1/testing/RealRenderTester.kt +++ b/workflow-testing/src/main/java/com/squareup/workflow1/testing/RealRenderTester.kt @@ -94,7 +94,7 @@ internal class RealRenderTester( } data class ExpectedRemember( - val matcher: (RememberInvocation) -> RememberMatch, + val matcher: (RememberInvocation) -> Boolean, val exactMatch: Boolean, val description: String, ) : Expectation() { @@ -106,6 +106,7 @@ internal class RealRenderTester( private var explicitWorkerExpectationsRequired: Boolean = false private var explicitSideEffectExpectationsRequired: Boolean = false + private var explicitRememberExpectationsRequired: Boolean = false private val stateAndOutput: Pair?> by lazy { val action = processedAction ?: noAction() val (state, actionApplied) = action.applyTo(props, state) @@ -152,7 +153,7 @@ internal class RealRenderTester( override fun expectRemember( description: String, exactMatch: Boolean, - matcher: (RememberInvocation) -> RememberMatch + matcher: (RememberInvocation) -> Boolean ): RenderTester = apply { expectations += ExpectedRemember(matcher, exactMatch, description) } @@ -170,6 +171,11 @@ internal class RealRenderTester( expectSideEffect(description = "unexpected side effect", exactMatch = false) { true } } + if (!explicitRememberExpectationsRequired) { + // Allow unexpected remember calls. + expectRemember(description = "unexpected remembered value", exactMatch = false) { true } + } + frozen = false // Clone the expectations to run a "dry" render pass. val noopContext = deepCloneForRender() @@ -311,36 +317,27 @@ internal class RealRenderTester( val description = "remember with key \"$key\"" val matches = expectations.filterIsInstance() - .mapNotNull { - val matchResult = it.matcher(invocation) - if (matchResult is RememberMatch.Matched) Pair(it, matchResult) else null - } + .mapNotNull { if (it.matcher(invocation)) it else null } if (matches.isEmpty()) { throw AssertionError("Unexpected $description") } - val exactMatches = matches.filter { it.first.exactMatch } - val (_, match) = when { - exactMatches.size == 1 -> { - exactMatches.single() - .also { (expected, _) -> - expectations -= expected - consumedExpectations += expected - } - } + val exactMatches = matches.filter { it.exactMatch } + if (exactMatches.size > 1) { + throw AssertionError( + "Multiple expectations matched $description: \n" + + exactMatches.joinToString(separator = "\n") { " ${it.describe()}" } + ) + } - exactMatches.size > 1 -> { - throw AssertionError( - "Multiple expectations matched $description:\n" + - exactMatches.joinToString(separator = "\n") { " ${it.first.describe()}" } - ) + // Inexact matches are not consumable. + exactMatches.singleOrNull() + ?.let { expected -> + expectations -= expected + consumedExpectations += expected } - // Inexact matches are not consumable. - else -> matches.first() - } - @Suppress("UNCHECKED_CAST") - return match.result as ResultT + return calculation() } override fun requireExplicitWorkerExpectations(): @@ -353,6 +350,11 @@ internal class RealRenderTester( explicitSideEffectExpectationsRequired = true } + override fun requireExplicitRememberExpectations(): + RenderTester = this.apply { + explicitRememberExpectationsRequired = true + } + override fun send(value: WorkflowAction) { if (!frozen) { throw UnsupportedOperationException( diff --git a/workflow-testing/src/main/java/com/squareup/workflow1/testing/RenderTester.kt b/workflow-testing/src/main/java/com/squareup/workflow1/testing/RenderTester.kt index f96719e4bc..88062084ba 100644 --- a/workflow-testing/src/main/java/com/squareup/workflow1/testing/RenderTester.kt +++ b/workflow-testing/src/main/java/com/squareup/workflow1/testing/RenderTester.kt @@ -15,7 +15,6 @@ import com.squareup.workflow1.config.JvmTestRuntimeConfigTools import com.squareup.workflow1.identifier import com.squareup.workflow1.testing.RenderTester.ChildWorkflowMatch import com.squareup.workflow1.testing.RenderTester.Companion -import com.squareup.workflow1.testing.RenderTester.RememberMatch import com.squareup.workflow1.workflowIdentifier import kotlinx.coroutines.CoroutineScope import kotlin.reflect.KClass @@ -285,14 +284,13 @@ public abstract class RenderTester { * which case the first match will be used), and the expectation may match multiple side effects. * * @param matcher A function that is passed the parameters from - * [RenderContext.remember][com.squareup.workflow1.BaseRenderContext.remember] and determines if - * they match what the workflow specified. If they do match, this includes the result that should - * be provided to the workflow. + * [RenderContext.remember][com.squareup.workflow1.BaseRenderContext.remember] and return + * true if such a call expected. */ public abstract fun expectRemember( description: String, exactMatch: Boolean = true, - matcher: (RememberInvocation) -> RememberMatch + matcher: (RememberInvocation) -> Boolean ): RenderTester /** @@ -320,6 +318,9 @@ public abstract class RenderTester { public abstract fun requireExplicitSideEffectExpectations(): RenderTester + public abstract fun requireExplicitRememberExpectations(): + RenderTester + /** * Describes a call to * [RenderContext.renderChild][com.squareup.workflow1.BaseRenderContext.renderChild]. @@ -357,22 +358,6 @@ public abstract class RenderTester { public val inputs: List, ) - public sealed class RememberMatch { - /** - * Indicates that the remember specifications did not match what was used by the Workflow. - */ - public object NotMatched : RememberMatch() - - /** - * Indicates that the remember specifications were matched. - * - * @param result the result to return from the remember call. - */ - public class Matched( - public val result: Any?, - ) : RememberMatch() - } - public sealed class ChildWorkflowMatch { /** * Indicates that the child workflow did not match the predicate and must match a different @@ -789,9 +774,6 @@ public fun * @param resultType The type of the value returned by the `calculation` function passed * to [remember][com.squareup.workflow1.BaseRenderContext.remember]. * - * @param result The result to be provided from - * [remember][com.squareup.workflow1.BaseRenderContext.remember] if the invocation is matched. - * * @param inputs The `inputs` values passed to * [remember][com.squareup.workflow1.BaseRenderContext.remember], if any * @@ -805,7 +787,6 @@ public fun RenderTester.expectRemember( key: String, resultType: KType, - result: Any?, vararg inputs: Any?, description: String = "", assertInputs: (inputs: List) -> Unit = {}, @@ -826,9 +807,9 @@ public fun key == invocation.key ) { assertInputs(invocation.inputs) - RememberMatch.Matched(result) + true } else { - RememberMatch.NotMatched + false } } } 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 0946a72902..1dae9c2cd6 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 @@ -157,18 +157,16 @@ internal class RealRenderTesterTest { ) } - @Test fun `remember throws when not expected`() { + @Test fun `remember runs and returns calculations`() { val workflow = Workflow.stateless { val numOutput = remember("the key") { 36 } - "$numOutput" + val stringInputs = remember("the key", "the", "inputs") { "string with string inputs" } + val noInputs = remember("the key", 1, 2, 3) { "string with number inputs" } + "$numOutput-$stringInputs-$noInputs" } - val failure = assertFailsWith { - workflow.testRender(Unit).render() + workflow.testRender(Unit).render { + assertEquals("36-string with string inputs-string with number inputs", it) } - assertEquals( - "Unexpected remember with key \"the key\"", - failure.message - ) } @Test fun `expectRemember throws when already expecting remember with same key`() { @@ -176,21 +174,14 @@ internal class RealRenderTesterTest { remember("the key", "the", "inputs") { "theOutput" } } val tester = workflow.testRender(Unit) - .expectRemember("the key", typeOf(), result = "theOutput", "the", "inputs") - .expectRemember( - "the key", - typeOf(), - result = "theOutput", - "the", - "inputs", - description = "duplicate match" - ) + .expectRemember("the key", typeOf(), "the", "inputs") + .expectRemember("the key", typeOf(), "the", "inputs", description = "duplicate match") val error = assertFailsWith { tester.render() } assertEquals( - "Multiple expectations matched remember with key \"the key\":\n" + + "Multiple expectations matched remember with key \"the key\": \n" + " remember key=the key, inputs=[the, inputs], resultType=kotlin.String\n" + " duplicate match", error.message @@ -206,48 +197,16 @@ internal class RealRenderTesterTest { } workflow.testRender(Unit) - .expectRemember("the key", typeOf(), result = 36) - .expectRemember( - "the key", - typeOf(), - result = "string with string inputs", - "the", - "inputs" - ) - .expectRemember("the key", typeOf(), result = "string with number inputs", 1, 2, 3) + .expectRemember("the key", typeOf()) + .expectRemember("the key", typeOf(), "the", "inputs") + .expectRemember("the key", typeOf(), 1, 2, 3) .render() } - @Test fun `expectRemember uses the result provided`() { - val workflow = Workflow.stateless { - val numOutput = remember("the key") { 42 } - val stringInputs = remember("the key", "the", "inputs") { "a different string" } - val noInputs = remember("the key", 1, 2, 3) { "yet another string not used." } - "$numOutput-$stringInputs-$noInputs" - } - - workflow.testRender(Unit) - .expectRemember("the key", typeOf(), result = 36) - .expectRemember( - "the key", - typeOf(), - result = "string with string inputs", - "the", - "inputs" - ) - .expectRemember("the key", typeOf(), result = "string with number inputs", 1, 2, 3) - .render { - assertEquals( - "36-string with string inputs-string with number inputs", - it - ) - } - } - @Test fun `expectRemember doesn't match key`() { val workflow = Workflow.stateless {} val tester = workflow.testRender(Unit) - .expectRemember("the key", typeOf(), result = "test", "the", "inputs") + .expectRemember("the key", typeOf(), "the", "inputs") val error = assertFailsWith { tester.render {} @@ -266,6 +225,7 @@ internal class RealRenderTesterTest { remember("the key", "the", "inputs") { "theOutput" } } val tester = workflow.testRender(Unit) + .requireExplicitRememberExpectations() val error = assertFailsWith { tester.render {} @@ -281,7 +241,6 @@ internal class RealRenderTesterTest { .expectRemember( key = "the key", resultType = typeOf(), - result = "theOutput", "the", "inputs", assertInputs = { inputs ->