Merge pull request #1333 from square/ray/forget-it

Revert "Update expectRemember to provide result"

Author: steve-the-edwards

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 <init> (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 <init> (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;

workflow-testing/src/main/java/com/squareup/workflow1/testing/RealRenderTester.kt

@@ -94,7 +94,7 @@ internal class RealRenderTester<PropsT, StateT, OutputT, RenderingT>(
     }
 
     data class ExpectedRemember(
-      val matcher: (RememberInvocation) -> RememberMatch,
+      val matcher: (RememberInvocation) -> Boolean,
       val exactMatch: Boolean,
       val description: String,
     ) : Expectation<Nothing>() {
@@ -106,6 +106,7 @@ internal class RealRenderTester<PropsT, StateT, OutputT, RenderingT>(
 
   private var explicitWorkerExpectationsRequired: Boolean = false
   private var explicitSideEffectExpectationsRequired: Boolean = false
+  private var explicitRememberExpectationsRequired: Boolean = false
   private val stateAndOutput: Pair<StateT, WorkflowOutput<OutputT>?> by lazy {
     val action = processedAction ?: noAction()
     val (state, actionApplied) = action.applyTo(props, state)
@@ -152,7 +153,7 @@ internal class RealRenderTester<PropsT, StateT, OutputT, RenderingT>(
   override fun expectRemember(
     description: String,
     exactMatch: Boolean,
-    matcher: (RememberInvocation) -> RememberMatch
+    matcher: (RememberInvocation) -> Boolean
   ): RenderTester<PropsT, StateT, OutputT, RenderingT> = apply {
     expectations += ExpectedRemember(matcher, exactMatch, description)
   }
@@ -170,6 +171,11 @@ internal class RealRenderTester<PropsT, StateT, OutputT, RenderingT>(
       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<PropsT, StateT, OutputT, RenderingT>(
     val description = "remember with key \"$key\""
 
     val matches = expectations.filterIsInstance<ExpectedRemember>()
-      .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<PropsT, StateT, OutputT, RenderingT>(
     explicitSideEffectExpectationsRequired = true
   }
 
+  override fun requireExplicitRememberExpectations():
+    RenderTester<PropsT, StateT, OutputT, RenderingT> = this.apply {
+    explicitRememberExpectationsRequired = true
+  }
+
   override fun send(value: WorkflowAction<PropsT, StateT, OutputT>) {
     if (!frozen) {
       throw UnsupportedOperationException(

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<PropsT, StateT, OutputT, RenderingT> {
    * 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<PropsT, StateT, OutputT, RenderingT>
 
   /**
@@ -320,6 +318,9 @@ public abstract class RenderTester<PropsT, StateT, OutputT, RenderingT> {
   public abstract fun requireExplicitSideEffectExpectations():
     RenderTester<PropsT, StateT, OutputT, RenderingT>
 
+  public abstract fun requireExplicitRememberExpectations():
+    RenderTester<PropsT, StateT, OutputT, RenderingT>
+
   /**
    * Describes a call to
    * [RenderContext.renderChild][com.squareup.workflow1.BaseRenderContext.renderChild].
@@ -357,22 +358,6 @@ public abstract class RenderTester<PropsT, StateT, OutputT, RenderingT> {
     public val inputs: List<Any?>,
   )
 
-  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 <PropsT, StateT, OutputT, RenderingT>
  * @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 <PropsT, StateT, OutputT, RenderingT>
   RenderTester<PropsT, StateT, OutputT, RenderingT>.expectRemember(
   key: String,
   resultType: KType,
-  result: Any?,
   vararg inputs: Any?,
   description: String = "",
   assertInputs: (inputs: List<Any?>) -> Unit = {},
@@ -826,9 +807,9 @@ public fun <PropsT, StateT, OutputT, RenderingT>
       key == invocation.key
     ) {
       assertInputs(invocation.inputs)
-      RememberMatch.Matched(result)
+      true
     } else {
-      RememberMatch.NotMatched
+      false
     }
   }
 }

workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt

@@ -157,40 +157,31 @@ internal class RealRenderTesterTest {
     )
   }
 
-  @Test fun `remember throws when not expected`() {
+  @Test fun `remember runs and returns calculations`() {
     val workflow = Workflow.stateless<Unit, Nothing, String> {
       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<AssertionError> {
-      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`() {
     val workflow = Workflow.stateless<Unit, Nothing, String> {
       remember("the key", "the", "inputs") { "theOutput" }
     }
     val tester = workflow.testRender(Unit)
-      .expectRemember("the key", typeOf<String>(), result = "theOutput", "the", "inputs")
-      .expectRemember(
-        "the key",
-        typeOf<String>(),
-        result = "theOutput",
-        "the",
-        "inputs",
-        description = "duplicate match"
-      )
+      .expectRemember("the key", typeOf<String>(), "the", "inputs")
+      .expectRemember("the key", typeOf<String>(), "the", "inputs", description = "duplicate match")
 
     val error = assertFailsWith<AssertionError> {
       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<Int>(), result = 36)
-      .expectRemember(
-        "the key",
-        typeOf<String>(),
-        result = "string with string inputs",
-        "the",
-        "inputs"
-      )
-      .expectRemember("the key", typeOf<String>(), result = "string with number inputs", 1, 2, 3)
+      .expectRemember("the key", typeOf<Int>())
+      .expectRemember("the key", typeOf<String>(), "the", "inputs")
+      .expectRemember("the key", typeOf<String>(), 1, 2, 3)
       .render()
   }
 
-  @Test fun `expectRemember uses the result provided`() {
-    val workflow = Workflow.stateless<Unit, Nothing, String> {
-      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<Int>(), result = 36)
-      .expectRemember(
-        "the key",
-        typeOf<String>(),
-        result = "string with string inputs",
-        "the",
-        "inputs"
-      )
-      .expectRemember("the key", typeOf<String>(), 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<Unit, Nothing, Unit> {}
     val tester = workflow.testRender(Unit)
-      .expectRemember("the key", typeOf<String>(), result = "test", "the", "inputs")
+      .expectRemember("the key", typeOf<String>(), "the", "inputs")
 
     val error = assertFailsWith<AssertionError> {
       tester.render {}
@@ -266,6 +225,7 @@ internal class RealRenderTesterTest {
       remember("the key", "the", "inputs") { "theOutput" }
     }
     val tester = workflow.testRender(Unit)
+      .requireExplicitRememberExpectations()
 
     val error = assertFailsWith<AssertionError> {
       tester.render {}
@@ -281,7 +241,6 @@ internal class RealRenderTesterTest {
       .expectRemember(
         key = "the key",
         resultType = typeOf<String>(),
-        result = "theOutput",
         "the",
         "inputs",
         assertInputs = { inputs ->