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.

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

985: render() again only if state has been changed by the action (or its parent's output handling) #992

Merged
merged 1 commit into from
Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
153 changes: 152 additions & 1 deletion .github/workflows/kotlin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,62 @@ jobs :
with :
report_paths : '**/build/test-results/test/TEST-*.xml'

jvm-stateChange-runtime-test :
Copy link
Contributor

Choose a reason for hiding this comment

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

Woot! @RBusarow should own reviewing this for correctness, though.

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 did do some cleanup after this - #1011

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=baseline-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'

jvm-conflate-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=conflate-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
Expand Down Expand Up @@ -280,7 +336,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
Expand Down Expand Up @@ -384,6 +439,102 @@ 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=baseline-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=baseline-stateChange

- name : Upload results
if : ${{ always() }}
uses : actions/upload-artifact@v3
with :
name : instrumentation-test-results-${{ matrix.api-level }}
path : ./**/build/reports/androidTests/connected/**

conflate-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=conflate-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=conflate-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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import com.squareup.benchmarks.performance.complex.poetry.instrumentation.Simula
import com.squareup.sample.container.SampleContainers
import com.squareup.sample.poetry.model.Poem
import com.squareup.workflow1.RuntimeConfig
import com.squareup.workflow1.RuntimeConfig.RenderPerAction
import com.squareup.workflow1.RuntimeConfigOptions.Companion.RENDER_PER_ACTION
import com.squareup.workflow1.WorkflowExperimentalRuntime
import com.squareup.workflow1.WorkflowInterceptor
import com.squareup.workflow1.ui.Screen
Expand Down Expand Up @@ -87,7 +87,7 @@ class PerformancePoetryActivity : AppCompatActivity() {
installedInterceptor = ActionHandlingTracingInterceptor()
}

val runtimeConfig = RenderPerAction
val runtimeConfig = RENDER_PER_ACTION

val component =
PerformancePoetryComponent(installedInterceptor, simulatedPerfConfig, runtimeConfig)
Expand Down
2 changes: 1 addition & 1 deletion workflow-config/config-android/api/config-android.api
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ public final class com/squareup/workflow1/config/AndroidRuntimeConfigTools {
}

public final class com/squareup/workflow1/config/AndroidRuntimeConfigTools$Companion {
public final fun getAppWorkflowRuntimeConfig ()Lcom/squareup/workflow1/RuntimeConfig;
public final fun getAppWorkflowRuntimeConfig ()Ljava/util/Set;
}

public final class com/squareup/workflow1/config/BuildConfig {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package com.squareup.workflow1.config

import com.squareup.workflow1.RuntimeConfig
import com.squareup.workflow1.RuntimeConfig.ConflateStaleRenderings
import com.squareup.workflow1.RuntimeConfig.RenderPerAction
import com.squareup.workflow1.RuntimeConfigOptions
import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS
import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES
import com.squareup.workflow1.WorkflowExperimentalRuntime

public class AndroidRuntimeConfigTools {
Expand All @@ -11,23 +12,34 @@ public class AndroidRuntimeConfigTools {
/**
* Helper for Configuration for the workflow runtime in an application.
* This allows one to specify a project property from the gradle build to choose a runtime.
* e.g. add "-Pworkflow.runtime=timeout" in your gradle build to build the timeout runtime into
* the application.
* e.g. add "-Pworkflow.runtime=conflate" in your gradle build to build the conflate runtime
* into the application.
*
* Note that this must be specified in the application built for any ui/integration tests. Call
* this function, and then pass that to the call to [renderWorkflowIn] as the [RuntimeConfig].
*
* Current options are:
* "conflate" : [ConflateStaleRenderings] Process all queued actions before passing rendering
* "conflate" : Process all queued actions before passing rendering
* to the UI layer.
* "baseline" : [RenderPerAction] Original Workflow Runtime. Note that this doesn't need to
* "baseline" : Original Workflow Runtime. Note that this doesn't need to
* be specified as it is the current default and is assumed by this utility.
*
* Then, these can be combined (via '-') with:
* "stateChange" : Only re-render when the state of some WorkflowNode has been changed by an
* action cascade.
*
* E.g., "baseline-stateChange" to turn on the stateChange option with the baseline runtime.
*
*/
@WorkflowExperimentalRuntime
public fun getAppWorkflowRuntimeConfig(): RuntimeConfig {
return when (BuildConfig.WORKFLOW_RUNTIME) {
"conflate" -> ConflateStaleRenderings
else -> RenderPerAction
"conflate" -> setOf(CONFLATE_STALE_RENDERINGS)
"conflate-stateChange" -> setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES)
"baseline-stateChange" -> setOf(RENDER_ONLY_WHEN_STATE_CHANGES)
"", "baseline" -> RuntimeConfigOptions.RENDER_PER_ACTION
else ->
throw IllegalArgumentException("Unrecognized config \"${BuildConfig.WORKFLOW_RUNTIME}\"")
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion workflow-config/config-jvm/api/config-jvm.api
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ public final class com/squareup/workflow1/config/JvmTestRuntimeConfigTools {
}

public final class com/squareup/workflow1/config/JvmTestRuntimeConfigTools$Companion {
public final fun getTestRuntimeConfig ()Lcom/squareup/workflow1/RuntimeConfig;
public final fun getTestRuntimeConfig ()Ljava/util/Set;
}

Original file line number Diff line number Diff line change
@@ -1,33 +1,46 @@
package com.squareup.workflow1.config

import com.squareup.workflow1.RuntimeConfig
import com.squareup.workflow1.RuntimeConfig.ConflateStaleRenderings
import com.squareup.workflow1.RuntimeConfig.RenderPerAction
import com.squareup.workflow1.RuntimeConfigOptions
import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS
import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES
import com.squareup.workflow1.WorkflowExperimentalRuntime

public class JvmTestRuntimeConfigTools {
public companion object {
/**
* Helper for Configuration for the Workflow Runtime while running tests on the JVM.
* Helper for Configuration for the workflow runtime in an application.
* This allows one to specify a project property from the gradle build to choose a runtime.
* e.g. add "-Pworkflow.runtime=timeout" in your gradle build to build the timeout runtime.
* e.g. add "-Pworkflow.runtime=conflate" in your gradle build to build the conflate runtime
* into the application.
*
* The [WorkflowTestRuntime] already calls this utility, but if starting your own runtime, then
* call this function and pass the result to the call to [renderWorkflowIn] as the
* [RuntimeConfig].
*
* Current options are:
* "conflate" : [ConflateStaleRenderings] Process all queued actions before passing rendering
* "conflate" : Process all queued actions before passing rendering
* to the UI layer.
* "baseline" : [RenderPerAction] Original Workflow Runtime. Note that this doesn't need to
* "baseline" : Original Workflow Runtime. Note that this doesn't need to
* be specified as it is the current default and is assumed by this utility.
*
* Then, these can be combined (via '-') with:
* "stateChange" : Only re-render when the state of some WorkflowNode has been changed by an
* action cascade.
*
* E.g., "baseline-stateChange" to turn on the stateChange option with the baseline runtime.
*
*/
@OptIn(WorkflowExperimentalRuntime::class)
public fun getTestRuntimeConfig(): RuntimeConfig {
val runtimeConfig = System.getProperty("workflow.runtime", "baseline")
return when (runtimeConfig) {
"conflate" -> ConflateStaleRenderings
else -> RenderPerAction
return when
(val runtimeConfig = System.getProperty("workflow.runtime", "baseline")) {
"conflate" -> setOf(CONFLATE_STALE_RENDERINGS)
"conflate-stateChange" -> setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES)
"baseline-stateChange" -> setOf(RENDER_ONLY_WHEN_STATE_CHANGES)
"", "baseline" -> RuntimeConfigOptions.RENDER_PER_ACTION
else ->
throw IllegalArgumentException("Unrecognized config \"$runtimeConfig\"")
}
}
}
Expand Down
16 changes: 15 additions & 1 deletion workflow-core/api/workflow-core.api
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
public final class com/squareup/workflow1/ActionApplied : com/squareup/workflow1/ActionProcessingResult {
public fun <init> (Lcom/squareup/workflow1/WorkflowOutput;Z)V
public synthetic fun <init> (Lcom/squareup/workflow1/WorkflowOutput;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun component1 ()Lcom/squareup/workflow1/WorkflowOutput;
public final fun component2 ()Z
public final fun copy (Lcom/squareup/workflow1/WorkflowOutput;Z)Lcom/squareup/workflow1/ActionApplied;
public static synthetic fun copy$default (Lcom/squareup/workflow1/ActionApplied;Lcom/squareup/workflow1/WorkflowOutput;ZILjava/lang/Object;)Lcom/squareup/workflow1/ActionApplied;
public fun equals (Ljava/lang/Object;)Z
public final fun getOutput ()Lcom/squareup/workflow1/WorkflowOutput;
public final fun getStateChanged ()Z
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
}

public abstract interface class com/squareup/workflow1/ActionProcessingResult {
}

Expand Down Expand Up @@ -271,7 +285,7 @@ public final class com/squareup/workflow1/WorkflowIdentifierType$Unsnapshottable
public fun toString ()Ljava/lang/String;
}

public final class com/squareup/workflow1/WorkflowOutput : com/squareup/workflow1/ActionProcessingResult {
public final class com/squareup/workflow1/WorkflowOutput {
public fun <init> (Ljava/lang/Object;)V
public fun equals (Ljava/lang/Object;)Z
public final fun getValue ()Ljava/lang/Object;
Expand Down
Loading