Increase formula coverage.

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)

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]
      */

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

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:"
+        )
+    }
+}

formula/src/main/java/com/instacart/formula/DeferredAction.kt

@@ -11,11 +11,11 @@ class DeferredAction<Event>(
 ) {
     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)
         }
     }
 

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<DeferredAction<*>>? = null
-    private var actions: Set<DeferredAction<*>>? = null
 
-    private var startListInvalidated: Boolean = false
-    private var scheduledToStart: MutableList<DeferredAction<*>>? = null
+    /**
+     * Action list provided by [Evaluation.actions]
+     */
+    private var actions: Set<DeferredAction<*>> = emptySet()
 
-    private var removeListInvalidated: Boolean = false
-    private var scheduledForRemoval: MutableList<DeferredAction<*>>? = null
+    private var recomputeCheckToStartList: Boolean = false
+    private var checkToStartActionList: MutableList<DeferredAction<*>>? = null
+
+    private var recomputeCheckToRemoveList: Boolean = false
+    private var checkToRemoveActionList: MutableList<DeferredAction<*>>? = 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<DeferredAction<*>>) {
         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<DeferredAction<*>>().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<DeferredAction<*>> {

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,
+        )
+    }
 }

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 behavior
+    @Test
+    fun `action termination events are ignored`() {
+        val formula = object : Formula<Boolean, Int, Int>() {
+            override fun initialState(input: Boolean): Int = 0
+
+            override fun Snapshot<Boolean, Int>.evaluate(): Evaluation<Int> {
+                return Evaluation(
+                    output = state,
+                    actions = context.actions {
+                        if (input) {
+                            val action = object : Action<Unit> {
+                                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<Boolean, Int, Int>() {
+            override fun initialState(input: Boolean): Int = 0
+
+            override fun Snapshot<Boolean, Int>.evaluate(): Evaluation<Int> {
+                return Evaluation(
+                    output = state,
+                    actions = context.actions {
+                        newRelay.action().onEvent {
+                            transition(state + 1)
+                        }
+
+                        if (input) {
+                            val action = object : Action<Unit> {
+                                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<Boolean, Int, Int>() {
+            override fun initialState(input: Boolean): Int = 0
+
+            override fun Snapshot<Boolean, Int>.evaluate(): Evaluation<Int> {
+                return Evaluation(
+                    output = state,
+                    actions = context.actions {
+                        if (input) {
+                            val action = object : Action<Unit> {
+                                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<FromObservableWithInputFormula.Item>()