Skip to content

Commit

Permalink
985: Add Ability to track whether state changed; Add Config for Rende…
Browse files Browse the repository at this point in the history
…r on State Change Only
  • Loading branch information
steve-the-edwards committed May 16, 2023
1 parent 9b15bdb commit cafac7e
Show file tree
Hide file tree
Showing 27 changed files with 616 additions and 176 deletions.
77 changes: 76 additions & 1 deletion .github/workflows/kotlin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ android.useAndroidX=true
systemProp.org.gradle.internal.publish.checksums.insecure=true

GROUP=com.squareup.workflow1
VERSION_NAME=1.10.0-beta06-SNAPSHOT
VERSION_NAME=1.10.0-beta06-state2-SNAPSHOT

POM_DESCRIPTION=Square Workflow

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@ class TimeMachineWorkflowTest {
val delegateWorkflow = Workflow.stateful<String, Nothing, DelegateRendering>(
initialState = "initial",
render = { renderState ->
DelegateRendering(renderState, setState = eventHandler { s -> state = s })
DelegateRendering(
renderState,
setState = eventHandler { s ->
state = s
}
)
}
)
val clock = TestTimeSource()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -26,6 +29,7 @@ public class AndroidRuntimeConfigTools {
@WorkflowExperimentalRuntime
public fun getAppWorkflowRuntimeConfig(): RuntimeConfig {
return when (BuildConfig.WORKFLOW_RUNTIME) {
"stateChange" -> RenderOnStateChangeOnly
"conflate" -> ConflateStaleRenderings
else -> RenderPerAction
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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
}
Expand Down
17 changes: 16 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 @@ -217,6 +231,7 @@ public final class com/squareup/workflow1/WorkflowAction$Companion {

public final class com/squareup/workflow1/WorkflowAction$Updater {
public fun <init> (Lcom/squareup/workflow1/WorkflowAction;Ljava/lang/Object;Ljava/lang/Object;)V
public final fun forceRerender ()V
public final fun getProps ()Ljava/lang/Object;
public final fun getState ()Ljava/lang/Object;
public final fun setOutput (Ljava/lang/Object;)V
Expand Down Expand Up @@ -271,7 +286,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
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,25 @@ public abstract class WorkflowAction<in PropsT, StateT, out OutputT> {
public val props: @UnsafeVariance PropsT,
public var state: StateT
) {
internal var maybeOutput: WorkflowOutput<@UnsafeVariance OutputT>? = null
internal val startingState = state
internal var forcedRerender: Boolean = false
internal var outputOrNull: WorkflowOutput<@UnsafeVariance OutputT>? = null
private set

/**
* Sets the value the workflow will emit as output when this action is applied.
* If this method is not called, there will be no output.
*/
public fun setOutput(output: @UnsafeVariance OutputT) {
this.maybeOutput = WorkflowOutput(output)
this.outputOrNull = WorkflowOutput(output)
}

/**
* The action will cause the Workflow to re-render regardless of whether or not state has
* changed, for all [RuntimeConfig]s.
*/
public fun forceRerender() {
forcedRerender = true
}
}

Expand Down Expand Up @@ -105,24 +115,24 @@ public fun <PropsT, StateT, OutputT> action(
public fun <PropsT, StateT, OutputT> WorkflowAction<PropsT, StateT, OutputT>.applyTo(
props: PropsT,
state: StateT
): Pair<StateT, WorkflowOutput<OutputT>?> {
): Pair<StateT, ActionApplied<OutputT>> {
val updater = Updater(props, state)
updater.apply()
return Pair(updater.state, updater.maybeOutput)
return Pair(
updater.state,
ActionApplied(
output = updater.outputOrNull,
stateChanged = updater.forcedRerender || updater.state != updater.startingState,
)
)
}

/**
* Only [WorkflowOutput] needs the generic OutputT so we do not include it in the root
* interface here.
* Box around a potentially nullable [OutputT]
*/
public sealed interface ActionProcessingResult

public object PropsUpdated : ActionProcessingResult

public object ActionsExhausted : ActionProcessingResult

/** Wrapper around a potentially-nullable [OutputT] value. */
public class WorkflowOutput<out OutputT>(public val value: OutputT) : ActionProcessingResult {
public class WorkflowOutput<out OutputT>(
public val value: OutputT
) {
override fun toString(): String = "WorkflowOutput($value)"

override fun equals(other: Any?): Boolean = when {
Expand All @@ -133,3 +143,28 @@ public class WorkflowOutput<out OutputT>(public val value: OutputT) : ActionProc

override fun hashCode(): Int = value.hashCode()
}

/**
* An [ActionProcessingResult] is any possible outcome after the runtime does a loop of processing.
*
* Only [ActionApplied] needs the generic OutputT so we do not include it in the root
* interface here.
*/
public sealed interface ActionProcessingResult

public object PropsUpdated : ActionProcessingResult

public object ActionsExhausted : ActionProcessingResult

/**
* Result of applying an action.
*
* @param output: the potentially null [WorkflowOutput]. If null, then no output was set by the
* action. Otherwise it is a [WorkflowOutput] around the output value of type [OutputT],
* which could be null.
* @param stateChanged: whether or not the action changed the state.
*/
public data class ActionApplied<out OutputT>(
public val output: WorkflowOutput<OutputT>?,
public val stateChanged: Boolean = false,
) : ActionProcessingResult
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import kotlin.test.fail

@OptIn(
ExperimentalCoroutinesApi::class,
ExperimentalStdlibApi::class,
)
internal class SinkTest {

Expand All @@ -42,9 +41,9 @@ internal class SinkTest {
assertEquals(1, sink.actions.size)
sink.actions.removeFirst()
.let { action ->
val (newState, output) = action.applyTo("props", "state")
val (newState, result) = action.applyTo("props", "state")
assertEquals("props state 1", newState)
assertEquals("output: 1", output?.value)
assertEquals("output: 1", result.output!!.value)
}
assertTrue(sink.actions.isEmpty())

Expand All @@ -53,9 +52,9 @@ internal class SinkTest {
assertEquals(1, sink.actions.size)
sink.actions.removeFirst()
.let { action ->
val (newState, output) = action.applyTo("props", "state")
val (newState, result) = action.applyTo("props", "state")
assertEquals("props state 2", newState)
assertEquals("output: 2", output?.value)
assertEquals("output: 2", result.output!!.value)
}

collector.cancel()
Expand Down Expand Up @@ -95,16 +94,16 @@ internal class SinkTest {
assertEquals(3, counter.getAndIncrement())
}
.applyTo(Unit, Unit)
.let { (_, output) ->
.let { (_, result) ->
assertEquals(6, counter.getAndIncrement())
assertEquals("a", output?.value)
assertEquals("a", result.output!!.value)
}

sentActions.removeFirst()
.applyTo(Unit, Unit)
.let { (_, output) ->
.let { (_, result) ->
assertEquals(7, counter.getAndIncrement())
assertEquals("b", output?.value)
assertEquals("b", result.output!!.value)
}

collectJob.cancel()
Expand All @@ -125,10 +124,10 @@ internal class SinkTest {
advanceUntilIdle()

val enqueuedAction = sink.actions.removeFirst()
val (newState, output) = enqueuedAction.applyTo("props", "state")
val (newState, result) = enqueuedAction.applyTo("props", "state")
assertEquals(1, applications)
assertEquals("props state applied", newState)
assertEquals("output", output?.value)
assertEquals("output", result.output!!.value)
}
}

Expand Down Expand Up @@ -171,11 +170,11 @@ internal class SinkTest {
val enqueuedAction = sink.actions.removeFirst()
sendJob.cancel()
advanceUntilIdle()
val (newState, output) = enqueuedAction.applyTo("unused props", "state")
val (newState, result) = enqueuedAction.applyTo("unused props", "state")

assertFalse(applied)
assertEquals("state", newState)
assertNull(output)
assertNull(result.output)
}

private class RecordingSink : Sink<WorkflowAction<String, String, String>> {
Expand Down
Loading

0 comments on commit cafac7e

Please sign in to comment.