985: Add Config for Render on State Change Only

square · Apr 25, 2023 · e65278a · e65278a
1 parent 49e1c1a
commit e65278a
Show file tree

Hide file tree

Showing 6 changed files with 184 additions and 5 deletions.
diff --git a/.github/workflows/kotlin.yml b/.github/workflows/kotlin.yml
@@ -188,6 +188,34 @@ jobs :
         with :
           report_paths : '**/build/test-results/test/TEST-*.xml'
 
+  jvm-stateChange-runtime-test :
+    name : Render On State Change Only Runtime JVM Tests
+    runs-on : ubuntu-latest
+    timeout-minutes : 20
+    steps :
+      - uses : actions/checkout@v3
+      - uses : gradle/wrapper-validation-action@v1
+      - name : set up JDK 11
+        uses : actions/setup-java@v3
+        with :
+          distribution : 'zulu'
+          java-version : 11
+
+      ## Actual task
+      - uses : gradle/gradle-build-action@v2
+        name : Check with Gradle
+        with :
+          arguments : |
+            jvmTest --continue -Pworkflow.runtime=stateChange
+          cache-read-only : false
+
+      # Report as Github Pull Request Check.
+      - name : Publish Test Report
+        uses : mikepenz/action-junit-report@v3
+        if : always() # always run even if the previous step fails
+        with :
+          report_paths : '**/build/test-results/test/TEST-*.xml'
+
   ios-tests :
     name : iOS Tests
     runs-on : macos-latest
@@ -280,7 +308,6 @@ jobs :
           profile : Galaxy Nexus
           api-level : ${{ matrix.api-level }}
           arch : x86_64
-          # Skip the benchmarks as this is running on emulators
           script : ./gradlew :benchmarks:performance-poetry:complex-poetry:connectedCheck --continue
 
       - name : Upload results
@@ -384,6 +411,54 @@ jobs :
           name : instrumentation-test-results-${{ matrix.api-level }}
           path : ./**/build/reports/androidTests/connected/**
 
+  stateChange-runtime-instrumentation-tests :
+    name : Render on State Change Only Instrumentation tests
+    runs-on : macos-latest
+    timeout-minutes : 45
+    strategy :
+      # Allow tests to continue on other devices if they fail on one device.
+      fail-fast : false
+      matrix :
+        api-level :
+          - 29
+      # Unclear that older versions actually honor command to disable animation.
+      # Newer versions are reputed to be too slow: https://github.com/ReactiveCircus/android-emulator-runner/issues/222
+    steps :
+      - uses : actions/checkout@v3
+      - name : set up JDK 11
+        uses : actions/setup-java@v3
+        with :
+          distribution : 'zulu'
+          java-version : 11
+
+      ## Build before running tests, using cache.
+      - uses : gradle/gradle-build-action@v2
+        name : Build instrumented tests
+        with :
+          # Unfortunately I don't think we can key this cache based on our project property so
+          # we clean and rebuild.
+          arguments : |
+            clean assembleDebugAndroidTest -Pworkflow.runtime=stateChange
+          cache-read-only : false
+
+      ## Actual task
+      - name : Instrumentation Tests
+        uses : reactivecircus/android-emulator-runner@v2
+        with :
+          # @ychescale9 suspects Galaxy Nexus is the fastest one
+          profile : Galaxy Nexus
+          api-level : ${{ matrix.api-level }}
+          arch : x86_64
+          # Skip the benchmarks as this is running on emulators
+          script : ./gradlew connectedCheck -x :benchmarks:dungeon-benchmark:connectedCheck -x :benchmarks:performance-poetry:complex-benchmark:connectedCheck -x :benchmarks:performance-poetry:complex-poetry:connectedCheck -Pworkflow.runtime=stateChange
+
+      - name : Upload results
+        if : ${{ always() }}
+        uses : actions/upload-artifact@v3
+        with :
+          name : instrumentation-test-results-${{ matrix.api-level }}
+          path : ./**/build/reports/androidTests/connected/**
+
   upload-to-mobiledev :
     name : mobile.dev | Build & Upload
     runs-on : ubuntu-latest

diff --git a/...g/config-android/src/main/java/com/squareup/workflow1/config/AndroidRuntimeConfigTools.kt b/...g/config-android/src/main/java/com/squareup/workflow1/config/AndroidRuntimeConfigTools.kt
@@ -2,6 +2,7 @@ package com.squareup.workflow1.config
 
 import com.squareup.workflow1.RuntimeConfig
 import com.squareup.workflow1.RuntimeConfig.ConflateStaleRenderings
+import com.squareup.workflow1.RuntimeConfig.RenderOnStateChangeOnly
 import com.squareup.workflow1.RuntimeConfig.RenderPerAction
 import com.squareup.workflow1.WorkflowExperimentalRuntime
 
@@ -18,6 +19,8 @@ public class AndroidRuntimeConfigTools {
      * this function, and then pass that to the call to [renderWorkflowIn] as the [RuntimeConfig].
      *
      * Current options are:
+     * "stateChange" : [RenderOnStateChangeOnly] Only re-render when the state of some WorkflowNode
+     *      has changed.
      * "conflate" : [ConflateStaleRenderings] Process all queued actions before passing rendering
      *      to the UI layer.
      * "baseline" : [RenderPerAction] Original Workflow Runtime. Note that this doesn't need to
@@ -26,6 +29,7 @@ public class AndroidRuntimeConfigTools {
     @WorkflowExperimentalRuntime
     public fun getAppWorkflowRuntimeConfig(): RuntimeConfig {
       return when (BuildConfig.WORKFLOW_RUNTIME) {
+        "stateChange" -> RenderOnStateChangeOnly
         "conflate" -> ConflateStaleRenderings
         else -> RenderPerAction
       }

diff --git a/...onfig/config-jvm/src/main/java/com/squareup/workflow1/config/JvmTestRuntimeConfigTools.kt b/...onfig/config-jvm/src/main/java/com/squareup/workflow1/config/JvmTestRuntimeConfigTools.kt
@@ -2,6 +2,7 @@ package com.squareup.workflow1.config
 
 import com.squareup.workflow1.RuntimeConfig
 import com.squareup.workflow1.RuntimeConfig.ConflateStaleRenderings
+import com.squareup.workflow1.RuntimeConfig.RenderOnStateChangeOnly
 import com.squareup.workflow1.RuntimeConfig.RenderPerAction
 import com.squareup.workflow1.WorkflowExperimentalRuntime
 
@@ -17,6 +18,8 @@ public class JvmTestRuntimeConfigTools {
      * [RuntimeConfig].
      *
      * Current options are:
+     * "stateChange" : [RenderOnStateChangeOnly] Only re-render when the state of some WorkflowNode
+     *      has changed.
      * "conflate" : [ConflateStaleRenderings] Process all queued actions before passing rendering
      *      to the UI layer.
      * "baseline" : [RenderPerAction] Original Workflow Runtime. Note that this doesn't need to
@@ -26,6 +29,7 @@ public class JvmTestRuntimeConfigTools {
     public fun getTestRuntimeConfig(): RuntimeConfig {
       val runtimeConfig = System.getProperty("workflow.runtime", "baseline")
       return when (runtimeConfig) {
+        "stateChange" -> RenderOnStateChangeOnly
         "conflate" -> ConflateStaleRenderings
         else -> RenderPerAction
       }

diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowAction.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowAction.kt
@@ -130,8 +130,8 @@ public class WorkflowOutput<out OutputT>(
 
   override fun equals(other: Any?): Boolean = when {
     this === other -> true
-    other !is ActionApplied<*> -> false
-    else -> value == other.output
+    other !is WorkflowOutput<*> -> false
+    else -> value == other.value
   }
 
   override fun hashCode(): Int = value.hashCode()

diff --git a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt
@@ -1,6 +1,7 @@
 package com.squareup.workflow1
 
 import com.squareup.workflow1.RuntimeConfig.ConflateStaleRenderings
+import com.squareup.workflow1.RuntimeConfig.RenderOnStateChangeOnly
 import com.squareup.workflow1.RuntimeConfig.RenderPerAction
 import com.squareup.workflow1.internal.ParameterizedTestRunner
 import kotlinx.coroutines.CancellationException
@@ -45,6 +46,7 @@ class RenderWorkflowInTest {
   private val runtimeOptions = arrayOf(
     RenderPerAction,
     ConflateStaleRenderings,
+    RenderOnStateChangeOnly
   ).asSequence()
 
   private val runtimeTestRunner = ParameterizedTestRunner<RuntimeConfig>()
@@ -174,8 +176,13 @@ class RenderWorkflowInTest {
   private val runtimeMatrix = arrayOf(
     Pair(RenderPerAction, RenderPerAction),
     Pair(RenderPerAction, ConflateStaleRenderings),
+    Pair(RenderPerAction, RenderOnStateChangeOnly),
     Pair(ConflateStaleRenderings, RenderPerAction),
     Pair(ConflateStaleRenderings, ConflateStaleRenderings),
+    Pair(ConflateStaleRenderings, RenderOnStateChangeOnly),
+    Pair(RenderOnStateChangeOnly, RenderPerAction),
+    Pair(RenderOnStateChangeOnly, ConflateStaleRenderings),
+    Pair(RenderOnStateChangeOnly, RenderOnStateChangeOnly),
   ).asSequence()
   private val runtimeMatrixTestRunner =
     ParameterizedTestRunner<Pair<RuntimeConfig, RuntimeConfig>>()
@@ -275,11 +282,22 @@ class RenderWorkflowInTest {
       scope.launch {
         renderings.collect { emitted += it }
       }
-      sink.send("unchanging state")
+
+      if (runtimeConfig is RenderOnStateChangeOnly) {
+        // we have to change state then or it won't render.
+        sink.send("changing state")
+      } else {
+        sink.send("unchanging state")
+      }
       testScope.advanceUntilIdle()
       testScope.runCurrent()
 
-      sink.send("unchanging state")
+      if (runtimeConfig is RenderOnStateChangeOnly) {
+        // we have to change state then or it won't render.
+        sink.send("changing state, again")
+      } else {
+        sink.send("unchanging state")
+      }
       testScope.advanceUntilIdle()
       testScope.runCurrent()
 
@@ -949,5 +967,81 @@ class RenderWorkflowInTest {
     }
   }
 
+  @Test fun for_render_on_state_change_only_we_do_not_render_if_state_not_changed() {
+    runtimeTestRunner.runParametrizedTest(
+      paramSource = arrayOf(RenderOnStateChangeOnly).asSequence(),
+      before = ::setup,
+    ) { runtimeConfig: RuntimeConfig ->
+      check(runtimeConfig is RenderOnStateChangeOnly)
+      lateinit var sink: Sink<String>
+
+      val workflow = Workflow.stateful<Unit, String, Nothing, String>(
+        initialState = { "unchanging state" },
+        render = { _, renderState ->
+          sink = actionSink.contraMap { action { state = it } }
+          renderState
+        }
+      )
+      val props = MutableStateFlow(Unit)
+      val renderings = renderWorkflowIn(
+        workflow = workflow,
+        scope = testScope,
+        props = props,
+        runtimeConfig = runtimeConfig
+      ) {}
+
+      val emitted = mutableListOf<RenderingAndSnapshot<String>>()
+      val scope = CoroutineScope(Unconfined)
+      scope.launch {
+        renderings.collect { emitted += it }
+      }
+
+      sink.send("unchanging state")
+      testScope.advanceUntilIdle()
+      testScope.runCurrent()
+      scope.cancel()
+
+      assertEquals(1, emitted.size)
+    }
+  }
+
+  @Test fun for_render_on_state_change_only_we_render_if_state_changed() {
+    runtimeTestRunner.runParametrizedTest(
+      paramSource = arrayOf(RenderOnStateChangeOnly).asSequence(),
+      before = ::setup,
+    ) { runtimeConfig: RuntimeConfig ->
+      check(runtimeConfig is RenderOnStateChangeOnly)
+      lateinit var sink: Sink<String>
+
+      val workflow = Workflow.stateful<Unit, String, Nothing, String>(
+        initialState = { "unchanging state" },
+        render = { _, renderState ->
+          sink = actionSink.contraMap { action { state = it } }
+          renderState
+        }
+      )
+      val props = MutableStateFlow(Unit)
+      val renderings = renderWorkflowIn(
+        workflow = workflow,
+        scope = testScope,
+        props = props,
+        runtimeConfig = runtimeConfig
+      ) {}
+
+      val emitted = mutableListOf<RenderingAndSnapshot<String>>()
+      val scope = CoroutineScope(Unconfined)
+      scope.launch {
+        renderings.collect { emitted += it }
+      }
+
+      sink.send("changing state")
+      testScope.advanceUntilIdle()
+      testScope.runCurrent()
+      scope.cancel()
+
+      assertEquals(2, emitted.size)
+    }
+  }
+
   private class ExpectedException : RuntimeException()
 }
diff --git a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowRunnerTest.kt b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowRunnerTest.kt
@@ -5,6 +5,7 @@ import com.squareup.workflow1.NoopWorkflowInterceptor
 import com.squareup.workflow1.RuntimeConfig
 import com.squareup.workflow1.RuntimeConfig.Companion
 import com.squareup.workflow1.RuntimeConfig.ConflateStaleRenderings
+import com.squareup.workflow1.RuntimeConfig.RenderOnStateChangeOnly
 import com.squareup.workflow1.RuntimeConfig.RenderPerAction
 import com.squareup.workflow1.Worker
 import com.squareup.workflow1.Workflow
@@ -33,6 +34,7 @@ internal class WorkflowRunnerTest {
   private val runtimeOptions = arrayOf(
     RenderPerAction,
     ConflateStaleRenderings,
+    RenderOnStateChangeOnly
   ).asSequence()
 
   private fun setup() {