From 4c597833102bad9fc2830f81d8cd28ba0bff5de3 Mon Sep 17 00:00:00 2001 From: Laimonas Turauskas Date: Tue, 10 Sep 2024 13:19:26 -0400 Subject: [PATCH] Increase formula coverage. --- formula-android/build.gradle.kts | 1 - .../android/FragmentLifecycleCallback.kt | 4 - .../formula/android/FragmentDataClassTest.kt | 1 - .../formula/android/internal/UtilsTest.kt | 31 +++++ .../com/instacart/formula/DeferredAction.kt | 4 +- .../formula/internal/ActionManager.kt | 117 ++++++++---------- .../instacart/formula/FormulaPluginTest.kt | 9 ++ .../instacart/formula/FormulaRuntimeTest.kt | 117 ++++++++++++++++++ .../formula/plugin/DispatcherTest.kt | 18 +++ .../formula/plugin/PluginTestExtensions.kt | 14 +++ .../subjects/IncrementingDispatcher.kt | 2 + 11 files changed, 246 insertions(+), 72 deletions(-) create mode 100644 formula-android/src/test/java/com/instacart/formula/android/internal/UtilsTest.kt create mode 100644 formula/src/test/java/com/instacart/formula/plugin/DispatcherTest.kt create mode 100644 formula/src/test/java/com/instacart/formula/plugin/PluginTestExtensions.kt diff --git a/formula-android/build.gradle.kts b/formula-android/build.gradle.kts index bffcb1c6..dddf15a7 100644 --- a/formula-android/build.gradle.kts +++ b/formula-android/build.gradle.kts @@ -31,7 +31,6 @@ dependencies { api(libs.rxandroid) api(libs.rxrelay) - testImplementation(libs.androidx.test.rules) testImplementation(libs.androidx.test.runner) testImplementation(libs.androidx.test.junit) diff --git a/formula-android/src/main/java/com/instacart/formula/android/FragmentLifecycleCallback.kt b/formula-android/src/main/java/com/instacart/formula/android/FragmentLifecycleCallback.kt index 81b49daa..cbc6cd92 100644 --- a/formula-android/src/main/java/com/instacart/formula/android/FragmentLifecycleCallback.kt +++ b/formula-android/src/main/java/com/instacart/formula/android/FragmentLifecycleCallback.kt @@ -10,10 +10,6 @@ import android.view.View * [androidx.fragment.app.Fragment.onDestroy] or [androidx.fragment.app.Fragment.onDetach] */ interface FragmentLifecycleCallback { - companion object { - internal val NO_OP = object : FragmentLifecycleCallback {} - } - /** * See [androidx.fragment.app.Fragment.onViewCreated] */ diff --git a/formula-android/src/test/java/com/instacart/formula/android/FragmentDataClassTest.kt b/formula-android/src/test/java/com/instacart/formula/android/FragmentDataClassTest.kt index ae0a6812..c1890d96 100644 --- a/formula-android/src/test/java/com/instacart/formula/android/FragmentDataClassTest.kt +++ b/formula-android/src/test/java/com/instacart/formula/android/FragmentDataClassTest.kt @@ -1,6 +1,5 @@ package com.instacart.formula.android -import com.google.common.truth.Truth import com.google.common.truth.Truth.assertThat import com.instacart.formula.android.events.ActivityResult import com.instacart.formula.android.events.FragmentLifecycleEvent diff --git a/formula-android/src/test/java/com/instacart/formula/android/internal/UtilsTest.kt b/formula-android/src/test/java/com/instacart/formula/android/internal/UtilsTest.kt new file mode 100644 index 00000000..f71535d4 --- /dev/null +++ b/formula-android/src/test/java/com/instacart/formula/android/internal/UtilsTest.kt @@ -0,0 +1,31 @@ +package com.instacart.formula.android.internal + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth +import io.reactivex.rxjava3.core.Observable +import io.reactivex.rxjava3.schedulers.Schedulers +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class UtilsTest { + + @Test + fun `assertMainThread does nothing on main thread`() { + Truth.assertThat(Utils.isMainThread()).isTrue() + Utils.assertMainThread() + } + + @Test + fun `assertMainThread throws exception when on bg thread`() { + val observer = Observable.fromCallable { + Truth.assertThat(Utils.isMainThread()).isFalse() + runCatching { Utils.assertMainThread() } + }.subscribeOn(Schedulers.newThread()).test() + + observer.awaitCount(1) + Truth.assertThat(observer.values().first().exceptionOrNull()).hasMessageThat().contains( + "should be called on main thread:" + ) + } +} \ No newline at end of file diff --git a/formula/src/main/java/com/instacart/formula/DeferredAction.kt b/formula/src/main/java/com/instacart/formula/DeferredAction.kt index 15b59c8b..5ec8454e 100644 --- a/formula/src/main/java/com/instacart/formula/DeferredAction.kt +++ b/formula/src/main/java/com/instacart/formula/DeferredAction.kt @@ -11,11 +11,11 @@ class DeferredAction( ) { private var cancelable: Cancelable? = null - internal var listener: (Event) -> Unit = initial + internal var listener: ((Event) -> Unit)? = initial internal fun start() { cancelable = action.start() { message -> - listener.invoke(message) + listener?.invoke(message) } } diff --git a/formula/src/main/java/com/instacart/formula/internal/ActionManager.kt b/formula/src/main/java/com/instacart/formula/internal/ActionManager.kt index c9598d6d..5f25c4fe 100644 --- a/formula/src/main/java/com/instacart/formula/internal/ActionManager.kt +++ b/formula/src/main/java/com/instacart/formula/internal/ActionManager.kt @@ -1,6 +1,7 @@ package com.instacart.formula.internal import com.instacart.formula.DeferredAction +import com.instacart.formula.Evaluation import com.instacart.formula.plugin.Inspector import kotlin.reflect.KClass @@ -12,18 +13,21 @@ internal class ActionManager( private val loggingType: KClass<*>, private val inspector: Inspector?, ) { - companion object { - val NO_OP: (Any?) -> Unit = {} - } - + /** + * Currently running actions + */ private var running: LinkedHashSet>? = null - private var actions: Set>? = null - private var startListInvalidated: Boolean = false - private var scheduledToStart: MutableList>? = null + /** + * Action list provided by [Evaluation.actions] + */ + private var actions: Set> = emptySet() - private var removeListInvalidated: Boolean = false - private var scheduledForRemoval: MutableList>? = null + private var recomputeCheckToStartList: Boolean = false + private var checkToStartActionList: MutableList>? = null + + private var recomputeCheckToRemoveList: Boolean = false + private var checkToRemoveActionList: MutableList>? = null /** * After evaluation, we might have a new list of actions that we need @@ -33,28 +37,26 @@ internal class ActionManager( fun prepareForPostEvaluation(new: Set>) { actions = new - startListInvalidated = true - removeListInvalidated = true + recomputeCheckToStartList = true + recomputeCheckToRemoveList = true } /** * Returns true if there was a transition while terminating streams. */ fun terminateOld(evaluationId: Long): Boolean { - prepareStoppedActionList() + recomputeCheckToRemoveActionListIfNeeded() - if (scheduledForRemoval.isNullOrEmpty()) { - return false - } + val runningActionList = running ?: return false + val scheduled = checkToRemoveActionList?.takeIf { it.isNotEmpty() } ?: return false - val actions = actions ?: emptyList() - val iterator = scheduledForRemoval?.iterator() - while (iterator?.hasNext() == true) { + val iterator = scheduled.iterator() + while (iterator.hasNext()) { val action = iterator.next() iterator.remove() if (!actions.contains(action)) { - running?.remove(action) + runningActionList.remove(action) finishAction(action) if (manager.isTerminated()) { @@ -70,22 +72,20 @@ internal class ActionManager( } fun startNew(evaluationId: Long): Boolean { - prepareNewActionList() + recomputeCheckToStartActionListIfNeeded() - val scheduled = scheduledToStart ?: return false - if (scheduled.isEmpty()) { - return false - } + val scheduled = checkToStartActionList?.takeIf { it.isNotEmpty() } ?: return false val iterator = scheduled.iterator() while (iterator.hasNext()) { val action = iterator.next() iterator.remove() - if (!isRunning(action)) { + val runningActions = getOrInitRunningActions() + if (!runningActions.contains(action)) { inspector?.onActionStarted(loggingType, action) - getOrInitRunningActions().add(action) + runningActions.add(action) action.start() if (manager.isTerminated()) { @@ -109,55 +109,44 @@ internal class ActionManager( } } - private fun prepareNewActionList() { - if (!startListInvalidated) { - return - } - - startListInvalidated = false - scheduledToStart?.clear() - - val actionList = actions ?: emptyList() - if (!actionList.isEmpty()) { - if (scheduledToStart == null) { - scheduledToStart = mutableListOf() - } - scheduledToStart?.addAll(actionList) - for (action in actionList) { - if (!isRunning(action)) { - val list = scheduledToStart ?: mutableListOf>().apply { - scheduledToStart = this - } - list.add(action) - } + private fun recomputeCheckToStartActionListIfNeeded() { + if (recomputeCheckToStartList) { + recomputeCheckToStartList = false + + checkToStartActionList?.clear() + val list = checkToStartActionList + if (actions.isEmpty()) { + list?.clear() + } else if (list != null) { + list.clear() + list.addAll(actions) + } else { + checkToStartActionList = ArrayList(actions) } } } - private fun prepareStoppedActionList() { - if (!removeListInvalidated) { - return - } - removeListInvalidated = false - - scheduledForRemoval?.clear() - if (!running.isNullOrEmpty()) { - if (scheduledForRemoval == null) { - scheduledForRemoval = mutableListOf() + private fun recomputeCheckToRemoveActionListIfNeeded() { + if (recomputeCheckToRemoveList) { + recomputeCheckToRemoveList = false + + val list = checkToRemoveActionList + val runningList = running?.takeIf { it.isNotEmpty() } + if (runningList == null) { + list?.clear() + } else if (list != null) { + list.clear() + list.addAll(runningList) + } else { + checkToRemoveActionList = ArrayList(runningList) } - - scheduledForRemoval?.addAll(running ?: emptyList()) } } - private fun isRunning(update: DeferredAction<*>): Boolean { - return running?.contains(update) ?: false - } - private fun finishAction(action: DeferredAction<*>) { inspector?.onActionFinished(loggingType, action) action.tearDown() - action.listener = NO_OP + action.listener = null } private fun getOrInitRunningActions(): LinkedHashSet> { diff --git a/formula/src/test/java/com/instacart/formula/FormulaPluginTest.kt b/formula/src/test/java/com/instacart/formula/FormulaPluginTest.kt index 247be6e1..3af2fbc1 100644 --- a/formula/src/test/java/com/instacart/formula/FormulaPluginTest.kt +++ b/formula/src/test/java/com/instacart/formula/FormulaPluginTest.kt @@ -75,4 +75,13 @@ class FormulaPluginTest { FormulaPlugins.setPlugin(TestPlugin(mainDispatcher = myDispatcher)) assertThat(FormulaPlugins.mainThreadDispatcher()).isEqualTo(myDispatcher) } + + @Test fun `default plugin implementation does nothing on duplicate child key`() { + val plugin = object : Plugin {} + plugin.onDuplicateChildKey( + parentType = Any::class.java, + childFormulaType = Any::class.java, + key = Unit, + ) + } } \ No newline at end of file diff --git a/formula/src/test/java/com/instacart/formula/FormulaRuntimeTest.kt b/formula/src/test/java/com/instacart/formula/FormulaRuntimeTest.kt index dcc6a70e..61e095fd 100644 --- a/formula/src/test/java/com/instacart/formula/FormulaRuntimeTest.kt +++ b/formula/src/test/java/com/instacart/formula/FormulaRuntimeTest.kt @@ -1352,6 +1352,123 @@ class FormulaRuntimeTest(val runtime: TestableRuntime, val name: String) { } } + // TODO: I'm not sure if this is the right behaviro + @Test + fun `action termination events are ignored`() { + val formula = object : Formula() { + override fun initialState(input: Boolean): Int = 0 + + override fun Snapshot.evaluate(): Evaluation { + return Evaluation( + output = state, + actions = context.actions { + if (input) { + val action = object : Action { + override fun start(send: (Unit) -> Unit): Cancelable { + return Cancelable { + send(Unit) + } + } + + override fun key(): Any? = null + } + + action.onEvent { + transition(state + 1) + } + } + } + ) + } + } + + val observer = runtime.test(formula) + observer.input(true) + observer.input(false) + observer.input(true) + observer.input(false) + observer.output { assertThat(this).isEqualTo(0) } + } + + @Test + fun `action triggers another transition on termination`() { + val newRelay = runtime.newRelay() + val formula = object : Formula() { + override fun initialState(input: Boolean): Int = 0 + + override fun Snapshot.evaluate(): Evaluation { + return Evaluation( + output = state, + actions = context.actions { + newRelay.action().onEvent { + transition(state + 1) + } + + if (input) { + val action = object : Action { + override fun start(send: (Unit) -> Unit): Cancelable { + return Cancelable { + newRelay.triggerEvent() + } + } + + override fun key(): Any? = null + } + + action.onEvent { + none() + } + } + } + ) + } + } + + val observer = runtime.test(formula) + observer.input(true) + observer.input(false) + observer.input(true) + observer.input(false) + observer.output { assertThat(this).isEqualTo(2) } + } + + @Test + fun `action triggers formula termination during termination`() { + var terminate = {} + val formula = object : Formula() { + override fun initialState(input: Boolean): Int = 0 + + override fun Snapshot.evaluate(): Evaluation { + return Evaluation( + output = state, + actions = context.actions { + if (input) { + val action = object : Action { + override fun start(send: (Unit) -> Unit): Cancelable { + return Cancelable { + terminate() + } + } + + override fun key(): Any? = null + } + + action.onEvent { + none() + } + } + } + ) + } + } + + val observer = runtime.test(formula) + terminate = { observer.dispose() } + observer.input(true) + observer.input(false) + observer.output { assertThat(this).isEqualTo(0) } + } + @Test fun `using from observable with input`() { val onItem = TestEventCallback() diff --git a/formula/src/test/java/com/instacart/formula/plugin/DispatcherTest.kt b/formula/src/test/java/com/instacart/formula/plugin/DispatcherTest.kt new file mode 100644 index 00000000..4e8b5092 --- /dev/null +++ b/formula/src/test/java/com/instacart/formula/plugin/DispatcherTest.kt @@ -0,0 +1,18 @@ +package com.instacart.formula.plugin + +import com.google.common.truth.Truth +import com.instacart.formula.subjects.TestDispatcherPlugin +import org.junit.Test + +class DispatcherTest { + @Test fun `main dispatcher delegates to plugin`() { + val plugin = TestDispatcherPlugin() + withPlugin(plugin) { + Dispatcher.Main.dispatch { } + plugin.mainDispatcher.assertCalled(1) + + Dispatcher.Main.isDispatchNeeded() + Truth.assertThat(plugin.mainDispatcher.isDispatchNeededCalled.get()).isEqualTo(1) + } + } +} \ No newline at end of file diff --git a/formula/src/test/java/com/instacart/formula/plugin/PluginTestExtensions.kt b/formula/src/test/java/com/instacart/formula/plugin/PluginTestExtensions.kt new file mode 100644 index 00000000..5a1901fd --- /dev/null +++ b/formula/src/test/java/com/instacart/formula/plugin/PluginTestExtensions.kt @@ -0,0 +1,14 @@ +package com.instacart.formula.plugin + +import com.instacart.formula.FormulaPlugins + + +fun withPlugin(plugin: Plugin?, continuation: () -> Unit) { + FormulaPlugins.setPlugin(plugin) + + try { + continuation() + } finally { + FormulaPlugins.setPlugin(null) + } +} \ No newline at end of file diff --git a/formula/src/test/java/com/instacart/formula/subjects/IncrementingDispatcher.kt b/formula/src/test/java/com/instacart/formula/subjects/IncrementingDispatcher.kt index 316b1078..4a044e36 100644 --- a/formula/src/test/java/com/instacart/formula/subjects/IncrementingDispatcher.kt +++ b/formula/src/test/java/com/instacart/formula/subjects/IncrementingDispatcher.kt @@ -6,6 +6,7 @@ import java.util.concurrent.atomic.AtomicInteger class IncrementingDispatcher : Dispatcher { val count = AtomicInteger(0) + val isDispatchNeededCalled = AtomicInteger(0) override fun dispatch(executable: () -> Unit) { count.incrementAndGet() @@ -13,6 +14,7 @@ class IncrementingDispatcher : Dispatcher { } override fun isDispatchNeeded(): Boolean { + isDispatchNeededCalled.getAndIncrement() return true }