From 02bb191f97eaa7585b6c6709047fc04a52d7b869 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 11:13:50 -0400 Subject: [PATCH 01/14] Fix trigger selection for forwarded copies Amp-Thread-ID: https://ampcode.com/threads/T-019d4981-c9b8-74c2-b64f-b60f74471dd9 Co-authored-by: Amp --- .../main/java/papa/MainThreadTriggerStack.kt | 31 +++--- .../java/papa/MainThreadTriggerStackTest.kt | 94 ++++++++++++++++--- 2 files changed, 102 insertions(+), 23 deletions(-) diff --git a/papa/src/main/java/papa/MainThreadTriggerStack.kt b/papa/src/main/java/papa/MainThreadTriggerStack.kt index d8dfb06..b9af576 100644 --- a/papa/src/main/java/papa/MainThreadTriggerStack.kt +++ b/papa/src/main/java/papa/MainThreadTriggerStack.kt @@ -5,15 +5,27 @@ object MainThreadTriggerStack { val earliestInteractionTrigger: InteractionTrigger? get() { Handlers.checkOnMainThread() - return interactionTriggerStack.minByOrNull { it.triggerUptime } + return interactionTriggerStack.reduceOrNull { earliestTrigger, trigger -> + when { + trigger.triggerUptime < earliestTrigger.triggerUptime -> trigger + trigger.triggerUptime == earliestTrigger.triggerUptime && + trigger.name == earliestTrigger.name -> trigger + else -> earliestTrigger + } + } } val inputEventInteractionTriggers: List> get() { Handlers.checkOnMainThread() - return interactionTriggerStack.mapNotNull { - it.toInputEventTriggerOrNull() + val inputEventTriggersByKey = + linkedMapOf, InteractionTriggerWithPayload>() + interactionTriggerStack.forEach { trigger -> + trigger.toInputEventTriggerOrNull()?.let { + inputEventTriggersByKey[it.name to it.triggerUptime] = it + } } + return inputEventTriggersByKey.values.toList() } private val interactionTriggerStack = mutableListOf() @@ -31,8 +43,9 @@ object MainThreadTriggerStack { /** * Must be called from the main thread. - * Adds [trigger] to the [interactionTriggerStack], it will replace any existing trigger with - * the same [InteractionTrigger.name] and [InteractionTrigger.triggerUptime]. + * Adds [trigger] to the [interactionTriggerStack] for the duration of [block]. Equal-but- + * distinct triggers intentionally coexist on the stack so callers can forward a copied trigger + * without evicting the original trigger instance that is still responsible for later cleanup. * * @param endTraceAfterBlock Finish the interaction trace after [block] runs. * @param block The code to run, during whose call stack the trigger added will be available @@ -44,15 +57,11 @@ object MainThreadTriggerStack { block: () -> T ): T { Handlers.checkOnMainThread() - // First, remove based on object equality (which uses name/triggerUptime). This has the effect - // of replacing any existing same-named, same-timed triggers. - // After the block() we remove just this instance from the stack. - interactionTriggerStack.removeAll { it == trigger } - interactionTriggerStack.add(trigger) + pushTriggeredBy(trigger) try { return block() } finally { - interactionTriggerStack.removeAll { it === trigger } + popTriggeredBy(trigger) if (endTraceAfterBlock) { trigger.takeOverInteractionTrace()?.endTrace() } diff --git a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt index 5fa068a..5ba4494 100644 --- a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt +++ b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt @@ -1,14 +1,18 @@ package papa +import android.view.InputEvent +import android.view.MotionEvent import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotSame import org.junit.Assert.assertNull +import org.junit.Assert.assertSame import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner +import kotlin.time.Duration import kotlin.time.Duration.Companion.nanoseconds @RunWith(RobolectricTestRunner::class) @@ -23,6 +27,22 @@ class MainThreadTriggerStackTest { } } + private fun createInputEventPayload(): InputEventTrigger { + val motionEvent = MotionEvent.obtain(0L, 1L, MotionEvent.ACTION_UP, 0f, 0f, 0) + val constructor = InputEventTrigger::class.java.getDeclaredConstructor( + InputEvent::class.java, + Long::class.javaPrimitiveType, + Class.forName("kotlin.jvm.internal.DefaultConstructorMarker") + ) + constructor.isAccessible = true + return constructor.newInstance(motionEvent, durationRawValue(1000.nanoseconds), null) + } + + private fun durationRawValue(duration: Duration): Long { + val unboxMethod = Duration::class.java.getDeclaredMethod("unbox-impl") + return unboxMethod.invoke(duration) as Long + } + @Test fun `currentTriggers returns empty list initially`() { val triggers = MainThreadTriggerStack.currentTriggers @@ -107,21 +127,49 @@ class MainThreadTriggerStackTest { } @Test - fun `triggeredBy removes existing trigger with same properties`() { - val trigger1 = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") - val trigger2 = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") // Same properties - - MainThreadTriggerStack.triggeredBy(trigger1, endTraceAfterBlock = false) { - // First trigger should be in stack - assertEquals(1, MainThreadTriggerStack.currentTriggers.size) - assertEquals(trigger1, MainThreadTriggerStack.currentTriggers[0]) + fun `earliestInteractionTrigger prefers most recent trigger with same properties`() { + val originalTrigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") + val forwardedTrigger = + SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") // Same properties - MainThreadTriggerStack.triggeredBy(trigger2, endTraceAfterBlock = false) { - // Second trigger should replace first (only one trigger in stack) + MainThreadTriggerStack.pushTriggeredBy(originalTrigger) + try { + MainThreadTriggerStack.triggeredBy(forwardedTrigger, endTraceAfterBlock = false) { val triggers = MainThreadTriggerStack.currentTriggers - assertEquals(1, triggers.size) - assertEquals(trigger2, triggers[0]) + assertEquals(2, triggers.size) + assertSame(originalTrigger, triggers[0]) + assertSame(forwardedTrigger, triggers[1]) + assertSame(forwardedTrigger, MainThreadTriggerStack.earliestInteractionTrigger) } + + assertSame(originalTrigger, MainThreadTriggerStack.earliestInteractionTrigger) + + val triggersAfterBlock = MainThreadTriggerStack.currentTriggers + assertEquals(1, triggersAfterBlock.size) + assertSame(originalTrigger, triggersAfterBlock[0]) + } finally { + MainThreadTriggerStack.popTriggeredBy(originalTrigger) + } + } + + @Test + fun `triggeredBy leaves original trigger on stack after equal forwarded copy exits`() { + val originalTrigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") + val forwardedTrigger = + SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") // Same properties + + MainThreadTriggerStack.pushTriggeredBy(originalTrigger) + try { + MainThreadTriggerStack.triggeredBy(forwardedTrigger, endTraceAfterBlock = false) { + assertSame(forwardedTrigger, MainThreadTriggerStack.earliestInteractionTrigger) + } + + val triggersAfterBlock = MainThreadTriggerStack.currentTriggers + assertEquals(1, triggersAfterBlock.size) + assertSame(originalTrigger, triggersAfterBlock[0]) + assertSame(originalTrigger, MainThreadTriggerStack.earliestInteractionTrigger) + } finally { + MainThreadTriggerStack.popTriggeredBy(originalTrigger) } } @@ -297,6 +345,28 @@ class MainThreadTriggerStackTest { } } + @Test + fun `inputEventInteractionTriggers prefers most recent trigger with same properties`() { + val payload = createInputEventPayload() + val originalTrigger = InteractionTriggerWithPayload(1000.nanoseconds, "tap", null, payload) + val forwardedTrigger = InteractionTriggerWithPayload(1000.nanoseconds, "tap", null, payload) + + MainThreadTriggerStack.pushTriggeredBy(originalTrigger) + try { + MainThreadTriggerStack.triggeredBy(forwardedTrigger, endTraceAfterBlock = false) { + val inputTriggers = MainThreadTriggerStack.inputEventInteractionTriggers + assertEquals(1, inputTriggers.size) + assertSame(forwardedTrigger, inputTriggers.single()) + } + + val inputTriggersAfterBlock = MainThreadTriggerStack.inputEventInteractionTriggers + assertEquals(1, inputTriggersAfterBlock.size) + assertSame(originalTrigger, inputTriggersAfterBlock.single()) + } finally { + MainThreadTriggerStack.popTriggeredBy(originalTrigger) + } + } + @Test fun `currentTriggers returns copy of stack`() { val trigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") From bea250453b7000cc3b07db197e631dcdae536a91 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 12:16:39 -0400 Subject: [PATCH 02/14] Reduce trigger stack selection overhead Amp-Thread-ID: https://ampcode.com/threads/T-019d4981-c9b8-74c2-b64f-b60f74471dd9 Co-authored-by: Amp --- .../main/java/papa/MainThreadTriggerStack.kt | 86 ++++++++++++++++--- 1 file changed, 76 insertions(+), 10 deletions(-) diff --git a/papa/src/main/java/papa/MainThreadTriggerStack.kt b/papa/src/main/java/papa/MainThreadTriggerStack.kt index b9af576..9e1f759 100644 --- a/papa/src/main/java/papa/MainThreadTriggerStack.kt +++ b/papa/src/main/java/papa/MainThreadTriggerStack.kt @@ -1,31 +1,97 @@ package papa +import kotlin.time.Duration + object MainThreadTriggerStack { val earliestInteractionTrigger: InteractionTrigger? get() { Handlers.checkOnMainThread() - return interactionTriggerStack.reduceOrNull { earliestTrigger, trigger -> + var earliestTrigger: InteractionTrigger? = null + var hasOtherTriggerAtEarliestUptime = false + interactionTriggerStack.forEach { trigger -> when { - trigger.triggerUptime < earliestTrigger.triggerUptime -> trigger - trigger.triggerUptime == earliestTrigger.triggerUptime && - trigger.name == earliestTrigger.name -> trigger - else -> earliestTrigger + earliestTrigger == null -> earliestTrigger = trigger + trigger.triggerUptime < earliestTrigger.triggerUptime -> { + earliestTrigger = trigger + hasOtherTriggerAtEarliestUptime = false + } + trigger.triggerUptime == earliestTrigger.triggerUptime -> { + hasOtherTriggerAtEarliestUptime = true + } + } + } + + if (earliestTrigger == null || !hasOtherTriggerAtEarliestUptime) { + return earliestTrigger + } + + for (index in interactionTriggerStack.lastIndex downTo 0) { + val trigger = interactionTriggerStack[index] + if (trigger.triggerUptime == earliestTrigger.triggerUptime && + trigger.name == earliestTrigger.name + ) { + return trigger } } + + return earliestTrigger } val inputEventInteractionTriggers: List> get() { Handlers.checkOnMainThread() - val inputEventTriggersByKey = - linkedMapOf, InteractionTriggerWithPayload>() + var firstInputEventTrigger: InteractionTriggerWithPayload? = null + var inputEventTriggers: ArrayList>? = null + var inputEventTriggersByKey: + LinkedHashMap, InteractionTriggerWithPayload>? = null + interactionTriggerStack.forEach { trigger -> - trigger.toInputEventTriggerOrNull()?.let { - inputEventTriggersByKey[it.name to it.triggerUptime] = it + val inputEventTrigger = trigger.toInputEventTriggerOrNull() ?: return@forEach + when { + firstInputEventTrigger == null -> firstInputEventTrigger = inputEventTrigger + inputEventTriggersByKey != null -> { + inputEventTriggersByKey[inputEventTrigger.name to inputEventTrigger.triggerUptime] = + inputEventTrigger + } + inputEventTriggers == null -> { + val firstTrigger = requireNotNull(firstInputEventTrigger) + if (inputEventTrigger.name == firstTrigger.name && + inputEventTrigger.triggerUptime == firstTrigger.triggerUptime + ) { + val triggerKey = firstTrigger.name to firstTrigger.triggerUptime + inputEventTriggersByKey = linkedMapOf(triggerKey to firstTrigger) + inputEventTriggersByKey[triggerKey] = inputEventTrigger + } else { + inputEventTriggers = arrayListOf(firstTrigger, inputEventTrigger) + } + } + else -> { + val duplicateIndex = inputEventTriggers.indexOfFirst { + it.name == inputEventTrigger.name && it.triggerUptime == inputEventTrigger.triggerUptime + } + if (duplicateIndex == -1) { + inputEventTriggers.add(inputEventTrigger) + } else { + inputEventTriggersByKey = LinkedHashMap(inputEventTriggers.size + 1) + inputEventTriggers.forEach { existingTrigger -> + inputEventTriggersByKey[existingTrigger.name to existingTrigger.triggerUptime] = + existingTrigger + } + inputEventTriggersByKey[inputEventTrigger.name to inputEventTrigger.triggerUptime] = + inputEventTrigger + inputEventTriggers = null + } + } } } - return inputEventTriggersByKey.values.toList() + + return when { + inputEventTriggersByKey != null -> inputEventTriggersByKey.values.toList() + inputEventTriggers != null -> inputEventTriggers + firstInputEventTrigger != null -> listOf(firstInputEventTrigger) + else -> emptyList() + } } private val interactionTriggerStack = mutableListOf() From 0029a0521a72a5ad7e0dc165acf46ecf1f2f4a27 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 13:12:03 -0400 Subject: [PATCH 03/14] Skip MainThreadMessageSpyTest on API 23 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add API 23 to the existing class-level skip predicate (API 28 was already skipped for similar reasons). On API 23 the Choreographer FrameDisplayEventReceiver timing differs from newer APIs, causing inputEventDispatching_not_in_MainThread_Message to flake: the tap lands while a FrameDisplayEventReceiver message is still the current Looper message, so currentMessageAsString is non-null instead of the expected null. This is a pre-existing flaky test unrelated to the trigger-forwarding fix — neither the test file nor MainThreadMessageSpy.kt were changed by this PR. Amp-Thread-ID: https://ampcode.com/threads/T-019d49fc-3bb3-71df-b4a6-eab56e473ee8 Co-authored-by: Amp --- .../androidTest/java/papa/test/MainThreadMessageSpyTest.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt b/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt index 9a35564..60ebfab 100644 --- a/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt +++ b/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt @@ -31,7 +31,10 @@ class MainThreadMessageSpyTest { @get:Rule val skipTestRule = SkipTestIf { - VERSION.SDK_INT == 28 + // API 28: known message dispatching timing issue. + // API 23: Choreographer FrameDisplayEventReceiver timing makes + // inputEventDispatching_not_in_MainThread_Message flaky. + VERSION.SDK_INT == 28 || VERSION.SDK_INT == 23 } @Test fun current_message_set_to_runnable_to_string() { From d41adb403e2d71f20fdcee0f48cf0a5f1d4c971b Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 14:25:41 -0400 Subject: [PATCH 04/14] Simplify trigger stack selection with reduceOrNull and in-place dedup Replace the two-pass forward+reverse scan in earliestInteractionTrigger with a single reduceOrNull call that prefers the last-pushed trigger on equal name+uptime ties. Remove LinkedHashMap and Pair key allocations from inputEventInteractionTriggers. Duplicates are now handled by in-place replacement in the ArrayList via indexOfFirst, and the single-element case just reassigns firstInputEventTrigger. Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-019d49fc-3bb3-71df-b4a6-eab56e473ee8 --- .../main/java/papa/MainThreadTriggerStack.kt | 87 +++++++++---------- 1 file changed, 41 insertions(+), 46 deletions(-) diff --git a/papa/src/main/java/papa/MainThreadTriggerStack.kt b/papa/src/main/java/papa/MainThreadTriggerStack.kt index 9e1f759..85931ab 100644 --- a/papa/src/main/java/papa/MainThreadTriggerStack.kt +++ b/papa/src/main/java/papa/MainThreadTriggerStack.kt @@ -1,71 +1,74 @@ package papa -import kotlin.time.Duration - object MainThreadTriggerStack { + /** + * Returns the trigger with the earliest (minimum) [InteractionTrigger.triggerUptime], or `null` + * if the stack is empty. + * + * When forwarded trigger copies coexist on the stack (same name + triggerUptime), the most + * recently pushed copy is preferred so that the active forwarded trace wins while it is in + * scope. + * + * Uses [reduceOrNull] for a single O(n) pass with zero allocations. On a strictly smaller + * uptime the new trigger wins outright; on an equal name + uptime tie the later element wins + * (most recently pushed). This is optimal — finding a minimum in an unsorted collection + * requires examining every element at least once. + */ val earliestInteractionTrigger: InteractionTrigger? get() { Handlers.checkOnMainThread() - var earliestTrigger: InteractionTrigger? = null - var hasOtherTriggerAtEarliestUptime = false - interactionTriggerStack.forEach { trigger -> - when { - earliestTrigger == null -> earliestTrigger = trigger - trigger.triggerUptime < earliestTrigger.triggerUptime -> { - earliestTrigger = trigger - hasOtherTriggerAtEarliestUptime = false - } - trigger.triggerUptime == earliestTrigger.triggerUptime -> { - hasOtherTriggerAtEarliestUptime = true - } - } - } - - if (earliestTrigger == null || !hasOtherTriggerAtEarliestUptime) { - return earliestTrigger - } - - for (index in interactionTriggerStack.lastIndex downTo 0) { - val trigger = interactionTriggerStack[index] - if (trigger.triggerUptime == earliestTrigger.triggerUptime && - trigger.name == earliestTrigger.name + return interactionTriggerStack.reduceOrNull { acc, trigger -> + if (trigger.triggerUptime < acc.triggerUptime || + (trigger.triggerUptime == acc.triggerUptime && trigger.name == acc.name) ) { - return trigger + trigger + } else { + acc } } - - return earliestTrigger } + /** + * Returns the distinct input-event triggers currently on the stack, deduplicated by + * (name, triggerUptime). When forwarded copies with the same key coexist, the most recently + * pushed copy is kept so callers see the active forwarded trace. + * + * Performance: O(n) single pass over [interactionTriggerStack], with tiered allocation to + * avoid object creation in the common cases: + * - **0 input triggers**: returns [emptyList], no allocations. + * - **1 input trigger** (no duplicates): returns a single-element list, no intermediate + * collection. + * - **2+ distinct input triggers**: allocates an [ArrayList] and deduplicates in-place via + * [ArrayList.indexOfFirst]. This is faster than a [LinkedHashMap] for the small stack sizes + * seen in practice (typically 1–3 triggers) because it avoids hashing, Pair-key allocation, + * and Map.Entry overhead. + */ val inputEventInteractionTriggers: List> get() { Handlers.checkOnMainThread() var firstInputEventTrigger: InteractionTriggerWithPayload? = null var inputEventTriggers: ArrayList>? = null - var inputEventTriggersByKey: - LinkedHashMap, InteractionTriggerWithPayload>? = null interactionTriggerStack.forEach { trigger -> val inputEventTrigger = trigger.toInputEventTriggerOrNull() ?: return@forEach when { + // First input trigger found: track it without allocating a list. firstInputEventTrigger == null -> firstInputEventTrigger = inputEventTrigger - inputEventTriggersByKey != null -> { - inputEventTriggersByKey[inputEventTrigger.name to inputEventTrigger.triggerUptime] = - inputEventTrigger - } + // Second input trigger found, still in the single-element fast path. inputEventTriggers == null -> { val firstTrigger = requireNotNull(firstInputEventTrigger) if (inputEventTrigger.name == firstTrigger.name && inputEventTrigger.triggerUptime == firstTrigger.triggerUptime ) { - val triggerKey = firstTrigger.name to firstTrigger.triggerUptime - inputEventTriggersByKey = linkedMapOf(triggerKey to firstTrigger) - inputEventTriggersByKey[triggerKey] = inputEventTrigger + // Duplicate of first: replace in-place, stay in single-element path. + firstInputEventTrigger = inputEventTrigger } else { + // Distinct: promote to ArrayList. inputEventTriggers = arrayListOf(firstTrigger, inputEventTrigger) } } + // 2+ triggers already in the list: deduplicate by in-place replacement. else -> { val duplicateIndex = inputEventTriggers.indexOfFirst { it.name == inputEventTrigger.name && it.triggerUptime == inputEventTrigger.triggerUptime @@ -73,21 +76,13 @@ object MainThreadTriggerStack { if (duplicateIndex == -1) { inputEventTriggers.add(inputEventTrigger) } else { - inputEventTriggersByKey = LinkedHashMap(inputEventTriggers.size + 1) - inputEventTriggers.forEach { existingTrigger -> - inputEventTriggersByKey[existingTrigger.name to existingTrigger.triggerUptime] = - existingTrigger - } - inputEventTriggersByKey[inputEventTrigger.name to inputEventTrigger.triggerUptime] = - inputEventTrigger - inputEventTriggers = null + inputEventTriggers[duplicateIndex] = inputEventTrigger } } } } return when { - inputEventTriggersByKey != null -> inputEventTriggersByKey.values.toList() inputEventTriggers != null -> inputEventTriggers firstInputEventTrigger != null -> listOf(firstInputEventTrigger) else -> emptyList() From 9863a8b8e019dbadbd6c4e614a946d21fb2e86cf Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 14:59:26 -0400 Subject: [PATCH 05/14] Revert API 23 skip to test if flake reproduces Amp-Thread-ID: https://ampcode.com/threads/T-019d49fc-3bb3-71df-b4a6-eab56e473ee8 Co-authored-by: Amp --- .../androidTest/java/papa/test/MainThreadMessageSpyTest.kt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt b/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt index 60ebfab..9a35564 100644 --- a/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt +++ b/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt @@ -31,10 +31,7 @@ class MainThreadMessageSpyTest { @get:Rule val skipTestRule = SkipTestIf { - // API 28: known message dispatching timing issue. - // API 23: Choreographer FrameDisplayEventReceiver timing makes - // inputEventDispatching_not_in_MainThread_Message flaky. - VERSION.SDK_INT == 28 || VERSION.SDK_INT == 23 + VERSION.SDK_INT == 28 } @Test fun current_message_set_to_runnable_to_string() { From 93dfb5937289263a54574cdd16cbcd047377fca6 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 15:26:05 -0400 Subject: [PATCH 06/14] Remove trigger value equality Drop the unused InteractionTrigger equals overrides so trigger identity stays consistent with the stack semantics and we no longer advertise value-style equality without a matching hashCode. Update the trigger tests and stack assertions to check identity explicitly. Amp-Thread-ID: https://ampcode.com/threads/T-019d4a72-a08b-755a-a093-f77c3896b250 Co-authored-by: Amp --- papa/src/main/java/papa/InteractionTrigger.kt | 14 -------------- papa/src/test/java/papa/InteractionTriggerTest.kt | 14 +++++++------- .../test/java/papa/MainThreadTriggerStackTest.kt | 12 ++++++------ 3 files changed, 13 insertions(+), 27 deletions(-) diff --git a/papa/src/main/java/papa/InteractionTrigger.kt b/papa/src/main/java/papa/InteractionTrigger.kt index d3eede3..70e3223 100644 --- a/papa/src/main/java/papa/InteractionTrigger.kt +++ b/papa/src/main/java/papa/InteractionTrigger.kt @@ -8,8 +8,6 @@ sealed interface InteractionTrigger { val name: String fun takeOverInteractionTrace(): InteractionTrace? - override fun equals(other: Any?): Boolean - companion object { fun triggerNow( name: String @@ -37,12 +35,6 @@ class SimpleInteractionTrigger( } } - override fun equals(other: Any?): Boolean { - if (other == null) return false - if (other !is SimpleInteractionTrigger) return false - return other.name == name && other.triggerUptime == triggerUptime - } - override fun toString(): String { return "InteractionTrigger(name='$name', triggerUptime=$triggerUptime)" } @@ -57,10 +49,4 @@ class InteractionTriggerWithPayload( override fun toString(): String { return "InteractionTrigger(name='$name', triggerUptime=$triggerUptime, payload=$payload)" } - - override fun equals(other: Any?): Boolean { - if (other == null) return false - if (other !is InteractionTriggerWithPayload<*>) return false - return other.name == name && other.triggerUptime == triggerUptime - } } diff --git a/papa/src/test/java/papa/InteractionTriggerTest.kt b/papa/src/test/java/papa/InteractionTriggerTest.kt index a70370a..abd7ffc 100644 --- a/papa/src/test/java/papa/InteractionTriggerTest.kt +++ b/papa/src/test/java/papa/InteractionTriggerTest.kt @@ -92,7 +92,7 @@ class InteractionTriggerTest { } @Test - fun `SimpleInteractionTrigger equals compares name and triggerUptime`() { + fun `SimpleInteractionTrigger uses reference equality for distinct instances`() { val triggerUptime = 1000.nanoseconds val name = "test-trigger" @@ -100,8 +100,8 @@ class InteractionTriggerTest { val trigger2 = SimpleInteractionTrigger(triggerUptime, name) val trigger3 = SimpleInteractionTrigger(triggerUptime, name, FakeInteractionTrace()) - assertEquals(trigger1, trigger2) - assertEquals(trigger2, trigger3) + assertNotEquals(trigger1, trigger2) + assertNotEquals(trigger2, trigger3) } @Test @@ -174,7 +174,7 @@ class InteractionTriggerTest { } @Test - fun `InteractionTriggerWithPayload equals works correctly`() { + fun `InteractionTriggerWithPayload uses reference equality for distinct instances`() { val triggerUptime = 1000.nanoseconds val name = "test-trigger" val payload = "test-payload" @@ -185,9 +185,9 @@ class InteractionTriggerTest { InteractionTriggerWithPayload(triggerUptime, name, FakeInteractionTrace(), payload) val trigger4 = InteractionTriggerWithPayload(triggerUptime, name, null, "other-payload") - assertEquals(trigger1, trigger2) - assertEquals(trigger1, trigger3) // Should equal based on name and triggerUptime - assertEquals(trigger1, trigger4) // Should equal based on name and triggerUptime + assertNotEquals(trigger1, trigger2) + assertNotEquals(trigger1, trigger3) + assertNotEquals(trigger1, trigger4) } @Test diff --git a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt index 5ba4494..c3cd7de 100644 --- a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt +++ b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt @@ -68,7 +68,7 @@ class MainThreadTriggerStackTest { MainThreadTriggerStack.triggeredBy(trigger, endTraceAfterBlock = false) { val currentTriggers = MainThreadTriggerStack.currentTriggers assertEquals(1, currentTriggers.size) - assertEquals(trigger, currentTriggers[0]) + assertSame(trigger, currentTriggers[0]) } } @@ -235,7 +235,7 @@ class MainThreadTriggerStackTest { MainThreadTriggerStack.triggeredBy(trigger2, endTraceAfterBlock = false) { MainThreadTriggerStack.triggeredBy(trigger3, endTraceAfterBlock = false) { val earliest = MainThreadTriggerStack.earliestInteractionTrigger - assertEquals(trigger2, earliest) + assertSame(trigger2, earliest) } } } @@ -251,7 +251,7 @@ class MainThreadTriggerStackTest { val triggers = MainThreadTriggerStack.currentTriggers assertEquals(1, triggers.size) - assertEquals(trigger, triggers[0]) + assertSame(trigger, triggers[0]) // Clean up MainThreadTriggerStack.popTriggeredBy(trigger) @@ -390,14 +390,14 @@ class MainThreadTriggerStackTest { MainThreadTriggerStack.triggeredBy(outerTrigger, endTraceAfterBlock = false) { assertEquals(1, MainThreadTriggerStack.currentTriggers.size) - assertEquals(outerTrigger, MainThreadTriggerStack.earliestInteractionTrigger) + assertSame(outerTrigger, MainThreadTriggerStack.earliestInteractionTrigger) MainThreadTriggerStack.triggeredBy( innerTrigger, endTraceAfterBlock = false ) { assertEquals(2, MainThreadTriggerStack.currentTriggers.size) - assertEquals( + assertSame( outerTrigger, MainThreadTriggerStack.earliestInteractionTrigger ) // Still earliest @@ -409,7 +409,7 @@ class MainThreadTriggerStackTest { // Inner trigger should be removed assertEquals(1, MainThreadTriggerStack.currentTriggers.size) - assertEquals(outerTrigger, MainThreadTriggerStack.currentTriggers[0]) + assertSame(outerTrigger, MainThreadTriggerStack.currentTriggers[0]) } // All triggers should be removed From a0dd7d35dbbe4c6aa2162a8579f4adeed4fcb17a Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 15:33:28 -0400 Subject: [PATCH 07/14] Add test helper for InputEventTrigger Expose a narrow internal factory for unit tests that only need an InputEventTrigger payload without wiring window frame tracking. This replaces reflective construction in MainThreadTriggerStackTest so the test no longer depends on Kotlin Duration JVM internals or synthetic constructors. Amp-Thread-ID: https://ampcode.com/threads/T-019d4a72-a08b-755a-a093-f77c3896b250 Co-authored-by: Amp --- papa/src/main/java/papa/InputEventTrigger.kt | 7 +++++++ .../test/java/papa/MainThreadTriggerStackTest.kt | 15 +-------------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/papa/src/main/java/papa/InputEventTrigger.kt b/papa/src/main/java/papa/InputEventTrigger.kt index 5140d06..637a2e1 100644 --- a/papa/src/main/java/papa/InputEventTrigger.kt +++ b/papa/src/main/java/papa/InputEventTrigger.kt @@ -49,6 +49,13 @@ class InputEventTrigger private constructor( }) return trigger } + + internal fun createForTest( + inputEvent: InputEvent, + deliveryUptime: Duration + ): InputEventTrigger { + return InputEventTrigger(inputEvent, deliveryUptime) + } } } diff --git a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt index c3cd7de..f48ab43 100644 --- a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt +++ b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt @@ -1,6 +1,5 @@ package papa -import android.view.InputEvent import android.view.MotionEvent import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -12,7 +11,6 @@ import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner -import kotlin.time.Duration import kotlin.time.Duration.Companion.nanoseconds @RunWith(RobolectricTestRunner::class) @@ -29,18 +27,7 @@ class MainThreadTriggerStackTest { private fun createInputEventPayload(): InputEventTrigger { val motionEvent = MotionEvent.obtain(0L, 1L, MotionEvent.ACTION_UP, 0f, 0f, 0) - val constructor = InputEventTrigger::class.java.getDeclaredConstructor( - InputEvent::class.java, - Long::class.javaPrimitiveType, - Class.forName("kotlin.jvm.internal.DefaultConstructorMarker") - ) - constructor.isAccessible = true - return constructor.newInstance(motionEvent, durationRawValue(1000.nanoseconds), null) - } - - private fun durationRawValue(duration: Duration): Long { - val unboxMethod = Duration::class.java.getDeclaredMethod("unbox-impl") - return unboxMethod.invoke(duration) as Long + return InputEventTrigger.createForTest(motionEvent, 1000.nanoseconds) } @Test From 486eff0121438d87adb26f1fcf513c3cdaf09ca2 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 15:35:01 -0400 Subject: [PATCH 08/14] Document trigger stack test intent Add KDoc for the InputEventTrigger test helper and explain the equal-trigger stack assertions in MainThreadTriggerStackTest so the forwarding behavior under test is obvious when reading the file. Amp-Thread-ID: https://ampcode.com/threads/T-019d4a72-a08b-755a-a093-f77c3896b250 Co-authored-by: Amp --- papa/src/main/java/papa/InputEventTrigger.kt | 5 +++++ papa/src/test/java/papa/MainThreadTriggerStackTest.kt | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/papa/src/main/java/papa/InputEventTrigger.kt b/papa/src/main/java/papa/InputEventTrigger.kt index 637a2e1..dbcaddd 100644 --- a/papa/src/main/java/papa/InputEventTrigger.kt +++ b/papa/src/main/java/papa/InputEventTrigger.kt @@ -50,6 +50,11 @@ class InputEventTrigger private constructor( return trigger } + /** + * Test-only helper for callers that need an [InputEventTrigger] payload without also wiring + * frame-render tracking through a real [Window]. This keeps unit tests focused on stack and + * payload behavior instead of Kotlin/JVM constructor details or Android frame callbacks. + */ internal fun createForTest( inputEvent: InputEvent, deliveryUptime: Duration diff --git a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt index f48ab43..6e7961a 100644 --- a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt +++ b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt @@ -119,6 +119,8 @@ class MainThreadTriggerStackTest { val forwardedTrigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") // Same properties + // Forwarding creates a distinct trigger instance with the same logical key. While both are on + // the stack, readers should resolve to the newer forwarded copy so the active trace wins. MainThreadTriggerStack.pushTriggeredBy(originalTrigger) try { MainThreadTriggerStack.triggeredBy(forwardedTrigger, endTraceAfterBlock = false) { @@ -145,6 +147,8 @@ class MainThreadTriggerStackTest { val forwardedTrigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") // Same properties + // The forwarded copy should be scoped to the nested block only. Once it exits, the original + // trigger must still be present so later work can continue attributing to the original source. MainThreadTriggerStack.pushTriggeredBy(originalTrigger) try { MainThreadTriggerStack.triggeredBy(forwardedTrigger, endTraceAfterBlock = false) { From 8944cbcd529cabe66c5ccaf82d735da355e9cb87 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 16:04:56 -0400 Subject: [PATCH 09/14] Clarify input trigger projection KDoc Document that inputEventInteractionTriggers is a distinct effective-input projection rather than a raw stack dump, and explain why equal forwarded copies are collapsed to the active instance. Amp-Thread-ID: https://ampcode.com/threads/T-019d4a72-a08b-755a-a093-f77c3896b250 Co-authored-by: Amp --- papa/src/main/java/papa/MainThreadTriggerStack.kt | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/papa/src/main/java/papa/MainThreadTriggerStack.kt b/papa/src/main/java/papa/MainThreadTriggerStack.kt index 85931ab..fc6bc0b 100644 --- a/papa/src/main/java/papa/MainThreadTriggerStack.kt +++ b/papa/src/main/java/papa/MainThreadTriggerStack.kt @@ -30,9 +30,17 @@ object MainThreadTriggerStack { } /** - * Returns the distinct input-event triggers currently on the stack, deduplicated by - * (name, triggerUptime). When forwarded copies with the same key coexist, the most recently - * pushed copy is kept so callers see the active forwarded trace. + * Returns the distinct effective input-event triggers currently visible on the stack, + * deduplicated by (name, triggerUptime). + * + * This is intentionally not a raw stack dump. Forwarding can temporarily place multiple + * equal-but-distinct copies of the same logical input trigger on the stack at once so cleanup + * can remain instance-based. Callers of this property care about distinct user inputs, not every + * forwarded stack entry, so equal copies are collapsed back to a single logical trigger here. + * + * When forwarded copies with the same key coexist, the most recently pushed copy is kept so + * callers see the active forwarded trace while it is in scope. Once that forwarded scope exits, + * the original copy remains on the stack and becomes visible again. * * Performance: O(n) single pass over [interactionTriggerStack], with tiered allocation to * avoid object creation in the common cases: From ce9e14ebc766deb09a5c240a01bae178dac609d5 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 16:11:47 -0400 Subject: [PATCH 10/14] Tighten input trigger KDoc wording Clarify the deduped input-event trigger view without overstating a broader consumer contract. The KDoc now focuses on the actual behavior from this change: equal forwarded copies can coexist on the raw stack, while this property returns a single representative and prefers the active forwarded copy while it is in scope. Amp-Thread-ID: https://ampcode.com/threads/T-019d4a72-a08b-755a-a093-f77c3896b250 Co-authored-by: Amp --- .../main/java/papa/MainThreadTriggerStack.kt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/papa/src/main/java/papa/MainThreadTriggerStack.kt b/papa/src/main/java/papa/MainThreadTriggerStack.kt index fc6bc0b..c21f6d0 100644 --- a/papa/src/main/java/papa/MainThreadTriggerStack.kt +++ b/papa/src/main/java/papa/MainThreadTriggerStack.kt @@ -30,17 +30,17 @@ object MainThreadTriggerStack { } /** - * Returns the distinct effective input-event triggers currently visible on the stack, - * deduplicated by (name, triggerUptime). + * Returns the input-event triggers currently visible on the stack, deduplicated by + * (name, triggerUptime). * - * This is intentionally not a raw stack dump. Forwarding can temporarily place multiple - * equal-but-distinct copies of the same logical input trigger on the stack at once so cleanup - * can remain instance-based. Callers of this property care about distinct user inputs, not every - * forwarded stack entry, so equal copies are collapsed back to a single logical trigger here. + * This is intentionally not a raw stack dump. Forwarding can temporarily place equal-but- + * distinct copies of the same logical input trigger on the stack at once because stack cleanup + * remains instance-based. * - * When forwarded copies with the same key coexist, the most recently pushed copy is kept so - * callers see the active forwarded trace while it is in scope. Once that forwarded scope exits, - * the original copy remains on the stack and becomes visible again. + * This view collapses those equal copies back to a single representative trigger. When forwarded + * copies with the same key coexist, the most recently pushed copy is kept so the active + * forwarded trigger is the one readers observe while it is in scope. Once that forwarded scope + * exits, the original copy remains on the stack and becomes visible again. * * Performance: O(n) single pass over [interactionTriggerStack], with tiered allocation to * avoid object creation in the common cases: From 1f17f0de6da131269ed1ccf25932a331cb56bde3 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 16:14:06 -0400 Subject: [PATCH 11/14] Keep input triggers in stack order Stop deduplicating inputEventInteractionTriggers so it remains a filtered view of the current stack. Update the stack and instrumentation tests to stop assuming a single representative trigger and instead read the most recently visible input trigger when needed. Amp-Thread-ID: https://ampcode.com/threads/T-019d4a72-a08b-755a-a093-f77c3896b250 Co-authored-by: Amp --- .../papa/test/MainThreadMessageSpyTest.kt | 2 +- .../main/java/papa/MainThreadTriggerStack.kt | 47 ++++--------------- .../java/papa/MainThreadTriggerStackTest.kt | 7 +-- 3 files changed, 15 insertions(+), 41 deletions(-) diff --git a/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt b/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt index 9a35564..388cf5b 100644 --- a/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt +++ b/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt @@ -231,7 +231,7 @@ class MainThreadMessageSpyTest { // Update UI, triggering a frame. text = "Got ACTION_UP" val tapTrigger = MainThreadTriggerStack.inputEventInteractionTriggers - .single() + .last() .payload inputTriggerFrameRenderedUptimeOnClick = tapTrigger.renderedUptime tapTrigger.onInputEventFrameRendered { frameRenderedUptime -> diff --git a/papa/src/main/java/papa/MainThreadTriggerStack.kt b/papa/src/main/java/papa/MainThreadTriggerStack.kt index c21f6d0..d17371f 100644 --- a/papa/src/main/java/papa/MainThreadTriggerStack.kt +++ b/papa/src/main/java/papa/MainThreadTriggerStack.kt @@ -30,27 +30,17 @@ object MainThreadTriggerStack { } /** - * Returns the input-event triggers currently visible on the stack, deduplicated by - * (name, triggerUptime). + * Returns the input-event triggers currently visible on the stack in stack order. * - * This is intentionally not a raw stack dump. Forwarding can temporarily place equal-but- - * distinct copies of the same logical input trigger on the stack at once because stack cleanup - * remains instance-based. - * - * This view collapses those equal copies back to a single representative trigger. When forwarded - * copies with the same key coexist, the most recently pushed copy is kept so the active - * forwarded trigger is the one readers observe while it is in scope. Once that forwarded scope - * exits, the original copy remains on the stack and becomes visible again. + * This is a filtered view of [interactionTriggerStack], not a deduplicated projection. If + * forwarding temporarily places equal-but-distinct copies of the same logical input trigger on + * the stack, both copies are returned here in their current stack order. * * Performance: O(n) single pass over [interactionTriggerStack], with tiered allocation to * avoid object creation in the common cases: * - **0 input triggers**: returns [emptyList], no allocations. - * - **1 input trigger** (no duplicates): returns a single-element list, no intermediate - * collection. - * - **2+ distinct input triggers**: allocates an [ArrayList] and deduplicates in-place via - * [ArrayList.indexOfFirst]. This is faster than a [LinkedHashMap] for the small stack sizes - * seen in practice (typically 1–3 triggers) because it avoids hashing, Pair-key allocation, - * and Map.Entry overhead. + * - **1 input trigger**: returns a single-element list, no intermediate collection. + * - **2+ input triggers**: allocates an [ArrayList] and appends matches in a single pass. */ val inputEventInteractionTriggers: List> get() { @@ -63,30 +53,13 @@ object MainThreadTriggerStack { when { // First input trigger found: track it without allocating a list. firstInputEventTrigger == null -> firstInputEventTrigger = inputEventTrigger - // Second input trigger found, still in the single-element fast path. + // Second input trigger found, promote to ArrayList. inputEventTriggers == null -> { val firstTrigger = requireNotNull(firstInputEventTrigger) - if (inputEventTrigger.name == firstTrigger.name && - inputEventTrigger.triggerUptime == firstTrigger.triggerUptime - ) { - // Duplicate of first: replace in-place, stay in single-element path. - firstInputEventTrigger = inputEventTrigger - } else { - // Distinct: promote to ArrayList. - inputEventTriggers = arrayListOf(firstTrigger, inputEventTrigger) - } - } - // 2+ triggers already in the list: deduplicate by in-place replacement. - else -> { - val duplicateIndex = inputEventTriggers.indexOfFirst { - it.name == inputEventTrigger.name && it.triggerUptime == inputEventTrigger.triggerUptime - } - if (duplicateIndex == -1) { - inputEventTriggers.add(inputEventTrigger) - } else { - inputEventTriggers[duplicateIndex] = inputEventTrigger - } + inputEventTriggers = arrayListOf(firstTrigger, inputEventTrigger) } + // 2+ triggers already in the list: append in stack order. + else -> inputEventTriggers.add(inputEventTrigger) } } diff --git a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt index 6e7961a..dc24caf 100644 --- a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt +++ b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt @@ -337,7 +337,7 @@ class MainThreadTriggerStackTest { } @Test - fun `inputEventInteractionTriggers prefers most recent trigger with same properties`() { + fun `inputEventInteractionTriggers keeps equal forwarded copies in stack order`() { val payload = createInputEventPayload() val originalTrigger = InteractionTriggerWithPayload(1000.nanoseconds, "tap", null, payload) val forwardedTrigger = InteractionTriggerWithPayload(1000.nanoseconds, "tap", null, payload) @@ -346,8 +346,9 @@ class MainThreadTriggerStackTest { try { MainThreadTriggerStack.triggeredBy(forwardedTrigger, endTraceAfterBlock = false) { val inputTriggers = MainThreadTriggerStack.inputEventInteractionTriggers - assertEquals(1, inputTriggers.size) - assertSame(forwardedTrigger, inputTriggers.single()) + assertEquals(2, inputTriggers.size) + assertSame(originalTrigger, inputTriggers[0]) + assertSame(forwardedTrigger, inputTriggers[1]) } val inputTriggersAfterBlock = MainThreadTriggerStack.inputEventInteractionTriggers From 8dfa986e2633a97ba50b6a93ab33d4e9c283af4b Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 16:16:32 -0400 Subject: [PATCH 12/14] Simplify input trigger stack view Replace the custom fast-path implementation of inputEventInteractionTriggers with the direct filtered-stack form now that we intentionally preserve duplicate forwarded copies. The behavior stays the same while the code becomes much easier to read. Amp-Thread-ID: https://ampcode.com/threads/T-019d4a72-a08b-755a-a093-f77c3896b250 Co-authored-by: Amp --- .../main/java/papa/MainThreadTriggerStack.kt | 30 +------------------ 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/papa/src/main/java/papa/MainThreadTriggerStack.kt b/papa/src/main/java/papa/MainThreadTriggerStack.kt index d17371f..f67d6cb 100644 --- a/papa/src/main/java/papa/MainThreadTriggerStack.kt +++ b/papa/src/main/java/papa/MainThreadTriggerStack.kt @@ -35,39 +35,11 @@ object MainThreadTriggerStack { * This is a filtered view of [interactionTriggerStack], not a deduplicated projection. If * forwarding temporarily places equal-but-distinct copies of the same logical input trigger on * the stack, both copies are returned here in their current stack order. - * - * Performance: O(n) single pass over [interactionTriggerStack], with tiered allocation to - * avoid object creation in the common cases: - * - **0 input triggers**: returns [emptyList], no allocations. - * - **1 input trigger**: returns a single-element list, no intermediate collection. - * - **2+ input triggers**: allocates an [ArrayList] and appends matches in a single pass. */ val inputEventInteractionTriggers: List> get() { Handlers.checkOnMainThread() - var firstInputEventTrigger: InteractionTriggerWithPayload? = null - var inputEventTriggers: ArrayList>? = null - - interactionTriggerStack.forEach { trigger -> - val inputEventTrigger = trigger.toInputEventTriggerOrNull() ?: return@forEach - when { - // First input trigger found: track it without allocating a list. - firstInputEventTrigger == null -> firstInputEventTrigger = inputEventTrigger - // Second input trigger found, promote to ArrayList. - inputEventTriggers == null -> { - val firstTrigger = requireNotNull(firstInputEventTrigger) - inputEventTriggers = arrayListOf(firstTrigger, inputEventTrigger) - } - // 2+ triggers already in the list: append in stack order. - else -> inputEventTriggers.add(inputEventTrigger) - } - } - - return when { - inputEventTriggers != null -> inputEventTriggers - firstInputEventTrigger != null -> listOf(firstInputEventTrigger) - else -> emptyList() - } + return interactionTriggerStack.mapNotNull { it.toInputEventTriggerOrNull() } } private val interactionTriggerStack = mutableListOf() From 86a89fd12bd4c8cbd4ac3d015a532cd3adea559f Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Wed, 1 Apr 2026 16:50:56 -0400 Subject: [PATCH 13/14] Update Papa API dump Sync the generated public API after removing the InteractionTrigger equals declarations and implementations. Amp-Thread-ID: https://ampcode.com/threads/T-019d4a72-a08b-755a-a093-f77c3896b250 Co-authored-by: Amp --- papa/api/papa.api | 3 --- 1 file changed, 3 deletions(-) diff --git a/papa/api/papa.api b/papa/api/papa.api index 43154bf..c646fe4 100644 --- a/papa/api/papa.api +++ b/papa/api/papa.api @@ -310,7 +310,6 @@ public final class papa/InteractionTrace$Companion { public abstract interface class papa/InteractionTrigger { public static final field Companion Lpapa/InteractionTrigger$Companion; - public abstract fun equals (Ljava/lang/Object;)Z public abstract fun getName ()Ljava/lang/String; public abstract fun getTriggerUptime-UwyO8pc ()J public abstract fun takeOverInteractionTrace ()Lpapa/InteractionTrace; @@ -322,7 +321,6 @@ public final class papa/InteractionTrigger$Companion { public final class papa/InteractionTriggerWithPayload : papa/InteractionTrigger { public synthetic fun (JLjava/lang/String;Lpapa/InteractionTrace;Ljava/lang/Object;Lkotlin/jvm/internal/DefaultConstructorMarker;)V - public fun equals (Ljava/lang/Object;)Z public fun getName ()Ljava/lang/String; public final fun getPayload ()Ljava/lang/Object; public fun getTriggerUptime-UwyO8pc ()J @@ -584,7 +582,6 @@ public final class papa/SentEvent { public final class papa/SimpleInteractionTrigger : papa/InteractionTrigger { public synthetic fun (JLjava/lang/String;Lpapa/InteractionTrace;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public synthetic fun (JLjava/lang/String;Lpapa/InteractionTrace;Lkotlin/jvm/internal/DefaultConstructorMarker;)V - public fun equals (Ljava/lang/Object;)Z public fun getName ()Ljava/lang/String; public fun getTriggerUptime-UwyO8pc ()J public fun takeOverInteractionTrace ()Lpapa/InteractionTrace; From 4e02ed8ae8e2cb05ae80d5396d9906349998ef80 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Thu, 2 Apr 2026 09:37:00 -0400 Subject: [PATCH 14/14] Address trigger stack review feedback Amp-Thread-ID: https://ampcode.com/threads/T-019d4e52-35f6-717f-b19c-fa60909252bf Co-authored-by: Amp --- .../papa/test/MainThreadMessageSpyTest.kt | 2 +- .../main/java/papa/MainThreadTriggerStack.kt | 27 ++- .../test/java/papa/InteractionTriggerTest.kt | 97 +++++----- .../java/papa/MainThreadTriggerStackTest.kt | 180 +++++++----------- 4 files changed, 124 insertions(+), 182 deletions(-) diff --git a/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt b/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt index 388cf5b..9a35564 100644 --- a/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt +++ b/papa/src/androidTest/java/papa/test/MainThreadMessageSpyTest.kt @@ -231,7 +231,7 @@ class MainThreadMessageSpyTest { // Update UI, triggering a frame. text = "Got ACTION_UP" val tapTrigger = MainThreadTriggerStack.inputEventInteractionTriggers - .last() + .single() .payload inputTriggerFrameRenderedUptimeOnClick = tapTrigger.renderedUptime tapTrigger.onInputEventFrameRendered { frameRenderedUptime -> diff --git a/papa/src/main/java/papa/MainThreadTriggerStack.kt b/papa/src/main/java/papa/MainThreadTriggerStack.kt index f67d6cb..e14a0a0 100644 --- a/papa/src/main/java/papa/MainThreadTriggerStack.kt +++ b/papa/src/main/java/papa/MainThreadTriggerStack.kt @@ -6,22 +6,18 @@ object MainThreadTriggerStack { * Returns the trigger with the earliest (minimum) [InteractionTrigger.triggerUptime], or `null` * if the stack is empty. * - * When forwarded trigger copies coexist on the stack (same name + triggerUptime), the most - * recently pushed copy is preferred so that the active forwarded trace wins while it is in - * scope. + * When duplicate triggers with identical uptime coexist on the stack, the most recently pushed + * one is preferred. * - * Uses [reduceOrNull] for a single O(n) pass with zero allocations. On a strictly smaller - * uptime the new trigger wins outright; on an equal name + uptime tie the later element wins - * (most recently pushed). This is optimal — finding a minimum in an unsorted collection - * requires examining every element at least once. + * Uses [reduceOrNull] for a single O(n) pass with zero allocations. On an equal-uptime tie the + * later element wins. This is optimal - finding a minimum in an unsorted collection requires + * examining every element at least once. */ val earliestInteractionTrigger: InteractionTrigger? get() { Handlers.checkOnMainThread() return interactionTriggerStack.reduceOrNull { acc, trigger -> - if (trigger.triggerUptime < acc.triggerUptime || - (trigger.triggerUptime == acc.triggerUptime && trigger.name == acc.name) - ) { + if (trigger.triggerUptime <= acc.triggerUptime) { trigger } else { acc @@ -32,9 +28,8 @@ object MainThreadTriggerStack { /** * Returns the input-event triggers currently visible on the stack in stack order. * - * This is a filtered view of [interactionTriggerStack], not a deduplicated projection. If - * forwarding temporarily places equal-but-distinct copies of the same logical input trigger on - * the stack, both copies are returned here in their current stack order. + * This is a filtered view of [interactionTriggerStack]. Duplicate triggers are returned in + * their current stack order. */ val inputEventInteractionTriggers: List> get() { @@ -57,9 +52,9 @@ object MainThreadTriggerStack { /** * Must be called from the main thread. - * Adds [trigger] to the [interactionTriggerStack] for the duration of [block]. Equal-but- - * distinct triggers intentionally coexist on the stack so callers can forward a copied trigger - * without evicting the original trigger instance that is still responsible for later cleanup. + * Adds [trigger] to the [interactionTriggerStack] for the duration of [block]. Duplicate + * trigger instances intentionally coexist on the stack so nested scopes do not evict an earlier + * instance that is still responsible for later cleanup. * * @param endTraceAfterBlock Finish the interaction trace after [block] runs. * @param block The code to run, during whose call stack the trigger added will be available diff --git a/papa/src/test/java/papa/InteractionTriggerTest.kt b/papa/src/test/java/papa/InteractionTriggerTest.kt index abd7ffc..cadf28a 100644 --- a/papa/src/test/java/papa/InteractionTriggerTest.kt +++ b/papa/src/test/java/papa/InteractionTriggerTest.kt @@ -1,12 +1,6 @@ package papa -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertNotEquals -import org.junit.Assert.assertNotNull -import org.junit.Assert.assertNull -import org.junit.Assert.assertSame -import org.junit.Assert.assertTrue +import com.google.common.truth.Truth.assertThat import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -34,10 +28,10 @@ class InteractionTriggerTest { val afterTime = System.nanoTime() - assertEquals(name, trigger.name) - assertTrue(trigger.triggerUptime.inWholeNanoseconds >= beforeTime) - assertTrue(trigger.triggerUptime.inWholeNanoseconds <= afterTime) - assertTrue(trigger is SimpleInteractionTrigger) + assertThat(trigger.name).isEqualTo(name) + assertThat(trigger.triggerUptime.inWholeNanoseconds).isAtLeast(beforeTime) + assertThat(trigger.triggerUptime.inWholeNanoseconds).isAtMost(afterTime) + assertThat(trigger).isInstanceOf(SimpleInteractionTrigger::class.java) } @Test @@ -47,8 +41,8 @@ class InteractionTriggerTest { val trigger = SimpleInteractionTrigger(triggerUptime, name) - assertEquals(triggerUptime, trigger.triggerUptime) - assertEquals(name, trigger.name) + assertThat(trigger.triggerUptime).isEqualTo(triggerUptime) + assertThat(trigger.name).isEqualTo(name) } @Test @@ -59,8 +53,8 @@ class InteractionTriggerTest { val trigger = SimpleInteractionTrigger(triggerUptime, name, trace) - assertEquals(triggerUptime, trigger.triggerUptime) - assertEquals(name, trigger.name) + assertThat(trigger.triggerUptime).isEqualTo(triggerUptime) + assertThat(trigger.name).isEqualTo(name) } @Test @@ -71,13 +65,11 @@ class InteractionTriggerTest { val trigger = SimpleInteractionTrigger(triggerUptime, name, trace) - // First call should return the trace val returnedTrace = trigger.takeOverInteractionTrace() - assertSame(trace, returnedTrace) + assertThat(returnedTrace).isSameInstanceAs(trace) - // Second call should return null since trace was taken over val secondTrace = trigger.takeOverInteractionTrace() - assertNull(secondTrace) + assertThat(secondTrace).isNull() } @Test @@ -88,7 +80,7 @@ class InteractionTriggerTest { val trigger = SimpleInteractionTrigger(triggerUptime, name, null) val returnedTrace = trigger.takeOverInteractionTrace() - assertNull(returnedTrace) + assertThat(returnedTrace).isNull() } @Test @@ -100,8 +92,8 @@ class InteractionTriggerTest { val trigger2 = SimpleInteractionTrigger(triggerUptime, name) val trigger3 = SimpleInteractionTrigger(triggerUptime, name, FakeInteractionTrace()) - assertNotEquals(trigger1, trigger2) - assertNotEquals(trigger2, trigger3) + assertThat(trigger1).isNotEqualTo(trigger2) + assertThat(trigger2).isNotEqualTo(trigger3) } @Test @@ -113,7 +105,7 @@ class InteractionTriggerTest { val trigger1 = SimpleInteractionTrigger(time1, name) val trigger2 = SimpleInteractionTrigger(time2, name) - assertNotEquals(trigger1, trigger2) + assertThat(trigger1).isNotEqualTo(trigger2) } @Test @@ -125,7 +117,7 @@ class InteractionTriggerTest { val trigger1 = SimpleInteractionTrigger(time, name1) val trigger2 = SimpleInteractionTrigger(time, name2) - assertNotEquals(trigger1, trigger2) + assertThat(trigger1).isNotEqualTo(trigger2) } @Test @@ -136,9 +128,9 @@ class InteractionTriggerTest { val trigger = SimpleInteractionTrigger(triggerUptime, name) val toString = trigger.toString() - assertTrue(toString.contains(name)) - assertTrue(toString.contains(triggerUptime.toString())) - assertTrue(toString.contains("InteractionTrigger")) + assertThat(toString).contains(name) + assertThat(toString).contains(triggerUptime.toString()) + assertThat(toString).contains("InteractionTrigger") } @Test @@ -150,9 +142,9 @@ class InteractionTriggerTest { val trigger = InteractionTriggerWithPayload(triggerUptime, name, trace, payload) - assertEquals(triggerUptime, trigger.triggerUptime) - assertEquals(name, trigger.name) - assertEquals(payload, trigger.payload) + assertThat(trigger.triggerUptime).isEqualTo(triggerUptime) + assertThat(trigger.name).isEqualTo(name) + assertThat(trigger.payload).isEqualTo(payload) } @Test @@ -164,13 +156,11 @@ class InteractionTriggerTest { val trigger = InteractionTriggerWithPayload(triggerUptime, name, trace, payload) - // Should delegate to underlying SimpleInteractionTrigger val returnedTrace = trigger.takeOverInteractionTrace() - assertSame(trace, returnedTrace) + assertThat(returnedTrace).isSameInstanceAs(trace) - // Second call should return null val secondTrace = trigger.takeOverInteractionTrace() - assertNull(secondTrace) + assertThat(secondTrace).isNull() } @Test @@ -185,9 +175,9 @@ class InteractionTriggerTest { InteractionTriggerWithPayload(triggerUptime, name, FakeInteractionTrace(), payload) val trigger4 = InteractionTriggerWithPayload(triggerUptime, name, null, "other-payload") - assertNotEquals(trigger1, trigger2) - assertNotEquals(trigger1, trigger3) - assertNotEquals(trigger1, trigger4) + assertThat(trigger1).isNotEqualTo(trigger2) + assertThat(trigger1).isNotEqualTo(trigger3) + assertThat(trigger1).isNotEqualTo(trigger4) } @Test @@ -200,7 +190,7 @@ class InteractionTriggerTest { val trigger1 = InteractionTriggerWithPayload(time1, name, null, payload) val trigger2 = InteractionTriggerWithPayload(time2, name, null, payload) - assertNotEquals(trigger1, trigger2) + assertThat(trigger1).isNotEqualTo(trigger2) } @Test @@ -213,7 +203,7 @@ class InteractionTriggerTest { val trigger1 = InteractionTriggerWithPayload(time, name1, null, payload) val trigger2 = InteractionTriggerWithPayload(time, name2, null, payload) - assertNotEquals(trigger1, trigger2) + assertThat(trigger1).isNotEqualTo(trigger2) } @Test @@ -225,10 +215,10 @@ class InteractionTriggerTest { val trigger = InteractionTriggerWithPayload(triggerUptime, name, null, payload) val toString = trigger.toString() - assertTrue(toString.contains(name)) - assertTrue(toString.contains(triggerUptime.toString())) - assertTrue(toString.contains(payload)) - assertTrue(toString.contains("InteractionTrigger")) + assertThat(toString).contains(name) + assertThat(toString).contains(triggerUptime.toString()) + assertThat(toString).contains(payload) + assertThat(toString).contains("InteractionTrigger") } @Test @@ -245,9 +235,9 @@ class InteractionTriggerTest { val trigger = InteractionTriggerWithPayload(triggerUptime, name, null, payload) - assertEquals(payload, trigger.payload) - assertEquals(42, trigger.payload.id) - assertEquals("test description", trigger.payload.description) + assertThat(trigger.payload).isEqualTo(payload) + assertThat(trigger.payload.id).isEqualTo(42) + assertThat(trigger.payload.description).isEqualTo("test description") } @Test @@ -258,7 +248,7 @@ class InteractionTriggerTest { val simpleTrigger = SimpleInteractionTrigger(triggerUptime, name) val payloadTrigger = InteractionTriggerWithPayload(triggerUptime, name, null, "payload") - assertNotEquals(simpleTrigger, payloadTrigger) + assertThat(simpleTrigger).isNotEqualTo(payloadTrigger) } @Test @@ -268,9 +258,8 @@ class InteractionTriggerTest { val trigger = InteractionTrigger.triggerNow(name) as SimpleInteractionTrigger val trace = trigger.takeOverInteractionTrace() - assertNotNull(trace) - // Trace should be null after being taken over - assertNull(trigger.takeOverInteractionTrace()) + assertThat(trace).isNotNull() + assertThat(trigger.takeOverInteractionTrace()).isNull() } @Test @@ -281,16 +270,16 @@ class InteractionTriggerTest { val trigger = SimpleInteractionTrigger(millisDuration, name) - assertEquals(millisDuration, trigger.triggerUptime) - assertEquals(nanosDuration, trigger.triggerUptime) + assertThat(trigger.triggerUptime).isEqualTo(millisDuration) + assertThat(trigger.triggerUptime).isEqualTo(nanosDuration) } @Test fun `InteractionTrace endTrace can be called`() { val trace = FakeInteractionTrace() - assertFalse(trace.endTraceCalled) + assertThat(trace.endTraceCalled).isFalse() trace.endTrace() - assertTrue(trace.endTraceCalled) + assertThat(trace.endTraceCalled).isTrue() } } diff --git a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt index dc24caf..0d53a79 100644 --- a/papa/src/test/java/papa/MainThreadTriggerStackTest.kt +++ b/papa/src/test/java/papa/MainThreadTriggerStackTest.kt @@ -1,13 +1,8 @@ package papa import android.view.MotionEvent -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertNotSame -import org.junit.Assert.assertNull -import org.junit.Assert.assertSame +import com.google.common.truth.Truth.assertThat import org.junit.Assert.assertThrows -import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -33,19 +28,19 @@ class MainThreadTriggerStackTest { @Test fun `currentTriggers returns empty list initially`() { val triggers = MainThreadTriggerStack.currentTriggers - assertTrue(triggers.isEmpty()) + assertThat(triggers).isEmpty() } @Test fun `earliestInteractionTrigger returns null when stack is empty`() { val earliest = MainThreadTriggerStack.earliestInteractionTrigger - assertNull(earliest) + assertThat(earliest).isNull() } @Test fun `inputEventInteractionTriggers returns empty list when stack is empty`() { val triggers = MainThreadTriggerStack.inputEventInteractionTriggers - assertTrue(triggers.isEmpty()) + assertThat(triggers).isEmpty() } @Test @@ -54,8 +49,7 @@ class MainThreadTriggerStackTest { MainThreadTriggerStack.triggeredBy(trigger, endTraceAfterBlock = false) { val currentTriggers = MainThreadTriggerStack.currentTriggers - assertEquals(1, currentTriggers.size) - assertSame(trigger, currentTriggers[0]) + assertThat(currentTriggers).containsExactly(trigger) } } @@ -64,12 +58,10 @@ class MainThreadTriggerStackTest { val trigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") MainThreadTriggerStack.triggeredBy(trigger, endTraceAfterBlock = false) { - // Trigger should be present during execution - assertFalse(MainThreadTriggerStack.currentTriggers.isEmpty()) + assertThat(MainThreadTriggerStack.currentTriggers).isNotEmpty() } - // Trigger should be removed after execution - assertTrue(MainThreadTriggerStack.currentTriggers.isEmpty()) + assertThat(MainThreadTriggerStack.currentTriggers).isEmpty() } @Test @@ -81,7 +73,7 @@ class MainThreadTriggerStackTest { expectedResult } - assertEquals(expectedResult, result) + assertThat(result).isEqualTo(expectedResult) } @Test @@ -89,15 +81,13 @@ class MainThreadTriggerStackTest { val trace = FakeInteractionTrace() val trigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger", trace) - assertFalse(trace.endTraceCalled) + assertThat(trace.endTraceCalled).isFalse() MainThreadTriggerStack.triggeredBy(trigger, endTraceAfterBlock = true) { - // Trace should not be ended yet during block execution - assertFalse(trace.endTraceCalled) + assertThat(trace.endTraceCalled).isFalse() } - // Trace should be ended after block execution - assertTrue(trace.endTraceCalled) + assertThat(trace.endTraceCalled).isTrue() } @Test @@ -109,56 +99,55 @@ class MainThreadTriggerStackTest { // Do nothing } - // Trace should not be ended - assertFalse(trace.endTraceCalled) + assertThat(trace.endTraceCalled).isFalse() } @Test - fun `earliestInteractionTrigger prefers most recent trigger with same properties`() { + fun `earliestInteractionTrigger prefers most recent duplicate trigger with same uptime`() { val originalTrigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") - val forwardedTrigger = - SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") // Same properties + val duplicateTrigger = + SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") // Same uptime - // Forwarding creates a distinct trigger instance with the same logical key. While both are on - // the stack, readers should resolve to the newer forwarded copy so the active trace wins. MainThreadTriggerStack.pushTriggeredBy(originalTrigger) try { - MainThreadTriggerStack.triggeredBy(forwardedTrigger, endTraceAfterBlock = false) { + MainThreadTriggerStack.triggeredBy(duplicateTrigger, endTraceAfterBlock = false) { val triggers = MainThreadTriggerStack.currentTriggers - assertEquals(2, triggers.size) - assertSame(originalTrigger, triggers[0]) - assertSame(forwardedTrigger, triggers[1]) - assertSame(forwardedTrigger, MainThreadTriggerStack.earliestInteractionTrigger) + assertThat(triggers).containsExactly(originalTrigger, duplicateTrigger).inOrder() + assertThat(MainThreadTriggerStack.earliestInteractionTrigger).isSameInstanceAs( + duplicateTrigger + ) } - assertSame(originalTrigger, MainThreadTriggerStack.earliestInteractionTrigger) + assertThat(MainThreadTriggerStack.earliestInteractionTrigger).isSameInstanceAs( + originalTrigger + ) val triggersAfterBlock = MainThreadTriggerStack.currentTriggers - assertEquals(1, triggersAfterBlock.size) - assertSame(originalTrigger, triggersAfterBlock[0]) + assertThat(triggersAfterBlock).containsExactly(originalTrigger) } finally { MainThreadTriggerStack.popTriggeredBy(originalTrigger) } } @Test - fun `triggeredBy leaves original trigger on stack after equal forwarded copy exits`() { + fun `triggeredBy leaves original trigger on stack after duplicate exits`() { val originalTrigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") - val forwardedTrigger = - SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") // Same properties + val duplicateTrigger = + SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") // Same uptime - // The forwarded copy should be scoped to the nested block only. Once it exits, the original - // trigger must still be present so later work can continue attributing to the original source. MainThreadTriggerStack.pushTriggeredBy(originalTrigger) try { - MainThreadTriggerStack.triggeredBy(forwardedTrigger, endTraceAfterBlock = false) { - assertSame(forwardedTrigger, MainThreadTriggerStack.earliestInteractionTrigger) + MainThreadTriggerStack.triggeredBy(duplicateTrigger, endTraceAfterBlock = false) { + assertThat(MainThreadTriggerStack.earliestInteractionTrigger).isSameInstanceAs( + duplicateTrigger + ) } val triggersAfterBlock = MainThreadTriggerStack.currentTriggers - assertEquals(1, triggersAfterBlock.size) - assertSame(originalTrigger, triggersAfterBlock[0]) - assertSame(originalTrigger, MainThreadTriggerStack.earliestInteractionTrigger) + assertThat(triggersAfterBlock).containsExactly(originalTrigger) + assertThat(MainThreadTriggerStack.earliestInteractionTrigger).isSameInstanceAs( + originalTrigger + ) } finally { MainThreadTriggerStack.popTriggeredBy(originalTrigger) } @@ -171,12 +160,9 @@ class MainThreadTriggerStackTest { SimpleInteractionTrigger(2000.nanoseconds, "test-trigger-2") // Different properties MainThreadTriggerStack.triggeredBy(trigger1, endTraceAfterBlock = false) { - // Should not throw since triggers are different MainThreadTriggerStack.triggeredBy(trigger2, endTraceAfterBlock = false) { val triggers = MainThreadTriggerStack.currentTriggers - assertEquals(2, triggers.size) - assertTrue(triggers.contains(trigger1)) - assertTrue(triggers.contains(trigger2)) + assertThat(triggers).containsExactly(trigger1, trigger2).inOrder() } } } @@ -185,18 +171,14 @@ class MainThreadTriggerStackTest { fun `triggeredBy removes trigger even when exception is thrown`() { val trigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") - try { + assertThrows(RuntimeException::class.java) { MainThreadTriggerStack.triggeredBy(trigger, endTraceAfterBlock = false) { - // Verify trigger is in stack - assertEquals(1, MainThreadTriggerStack.currentTriggers.size) + assertThat(MainThreadTriggerStack.currentTriggers).containsExactly(trigger) throw RuntimeException("Test exception") } - } catch (_: RuntimeException) { - // Expected exception } - // Trigger should still be removed from stack despite exception - assertTrue(MainThreadTriggerStack.currentTriggers.isEmpty()) + assertThat(MainThreadTriggerStack.currentTriggers).isEmpty() } @Test @@ -204,16 +186,13 @@ class MainThreadTriggerStackTest { val trace = FakeInteractionTrace() val trigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger", trace) - try { + assertThrows(RuntimeException::class.java) { MainThreadTriggerStack.triggeredBy(trigger, endTraceAfterBlock = true) { throw RuntimeException("Test exception") } - } catch (_: RuntimeException) { - // Expected exception } - // Trace should still be ended despite exception - assertTrue(trace.endTraceCalled) + assertThat(trace.endTraceCalled).isTrue() } @Test @@ -225,8 +204,7 @@ class MainThreadTriggerStackTest { MainThreadTriggerStack.triggeredBy(trigger1, endTraceAfterBlock = false) { MainThreadTriggerStack.triggeredBy(trigger2, endTraceAfterBlock = false) { MainThreadTriggerStack.triggeredBy(trigger3, endTraceAfterBlock = false) { - val earliest = MainThreadTriggerStack.earliestInteractionTrigger - assertSame(trigger2, earliest) + assertThat(MainThreadTriggerStack.earliestInteractionTrigger).isSameInstanceAs(trigger2) } } } @@ -236,15 +214,13 @@ class MainThreadTriggerStackTest { fun `pushTriggeredBy adds trigger to stack`() { val trigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") - assertTrue(MainThreadTriggerStack.currentTriggers.isEmpty()) + assertThat(MainThreadTriggerStack.currentTriggers).isEmpty() MainThreadTriggerStack.pushTriggeredBy(trigger) val triggers = MainThreadTriggerStack.currentTriggers - assertEquals(1, triggers.size) - assertSame(trigger, triggers[0]) + assertThat(triggers).containsExactly(trigger) - // Clean up MainThreadTriggerStack.popTriggeredBy(trigger) } @@ -258,27 +234,24 @@ class MainThreadTriggerStackTest { MainThreadTriggerStack.pushTriggeredBy(trigger) // Same instance } - // Clean up MainThreadTriggerStack.popTriggeredBy(trigger) } @Test - fun `pushTriggeredBy allows different trigger instances with same properties`() { + fun `pushTriggeredBy allows different trigger instances with same uptime`() { val trigger1 = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") val trigger2 = SimpleInteractionTrigger( 1000.nanoseconds, "test-trigger" - ) // Same properties, different instance + ) // Same uptime, different instance MainThreadTriggerStack.pushTriggeredBy(trigger1) - // Should not throw since it's a different instance MainThreadTriggerStack.pushTriggeredBy(trigger2) val triggers = MainThreadTriggerStack.currentTriggers - assertEquals(2, triggers.size) + assertThat(triggers).containsExactly(trigger1, trigger2).inOrder() - // Clean up MainThreadTriggerStack.popTriggeredBy(trigger1) MainThreadTriggerStack.popTriggeredBy(trigger2) } @@ -288,10 +261,10 @@ class MainThreadTriggerStackTest { val trigger = SimpleInteractionTrigger(1000.nanoseconds, "test-trigger") MainThreadTriggerStack.pushTriggeredBy(trigger) - assertEquals(1, MainThreadTriggerStack.currentTriggers.size) + assertThat(MainThreadTriggerStack.currentTriggers).containsExactly(trigger) MainThreadTriggerStack.popTriggeredBy(trigger) - assertTrue(MainThreadTriggerStack.currentTriggers.isEmpty()) + assertThat(MainThreadTriggerStack.currentTriggers).isEmpty() } @Test @@ -300,15 +273,13 @@ class MainThreadTriggerStackTest { val otherTrigger = SimpleInteractionTrigger(2000.nanoseconds, "other-trigger") MainThreadTriggerStack.pushTriggeredBy(trigger) - assertEquals(1, MainThreadTriggerStack.currentTriggers.size) + assertThat(MainThreadTriggerStack.currentTriggers).containsExactly(trigger) - // Popping a different trigger should not affect the stack MainThreadTriggerStack.popTriggeredBy(otherTrigger) - assertEquals(1, MainThreadTriggerStack.currentTriggers.size) + assertThat(MainThreadTriggerStack.currentTriggers).containsExactly(trigger) - // Clean up MainThreadTriggerStack.popTriggeredBy(trigger) - assertTrue(MainThreadTriggerStack.currentTriggers.isEmpty()) + assertThat(MainThreadTriggerStack.currentTriggers).isEmpty() } @Test @@ -329,31 +300,26 @@ class MainThreadTriggerStackTest { inputEventTrigger, endTraceAfterBlock = false ) { - val inputTriggers = MainThreadTriggerStack.inputEventInteractionTriggers - // Both triggers should be filtered out since neither has InputEventTrigger payload - assertTrue(inputTriggers.isEmpty()) + assertThat(MainThreadTriggerStack.inputEventInteractionTriggers).isEmpty() } } } @Test - fun `inputEventInteractionTriggers keeps equal forwarded copies in stack order`() { + fun `inputEventInteractionTriggers keeps duplicate triggers in stack order`() { val payload = createInputEventPayload() val originalTrigger = InteractionTriggerWithPayload(1000.nanoseconds, "tap", null, payload) - val forwardedTrigger = InteractionTriggerWithPayload(1000.nanoseconds, "tap", null, payload) + val duplicateTrigger = InteractionTriggerWithPayload(1000.nanoseconds, "tap", null, payload) MainThreadTriggerStack.pushTriggeredBy(originalTrigger) try { - MainThreadTriggerStack.triggeredBy(forwardedTrigger, endTraceAfterBlock = false) { + MainThreadTriggerStack.triggeredBy(duplicateTrigger, endTraceAfterBlock = false) { val inputTriggers = MainThreadTriggerStack.inputEventInteractionTriggers - assertEquals(2, inputTriggers.size) - assertSame(originalTrigger, inputTriggers[0]) - assertSame(forwardedTrigger, inputTriggers[1]) + assertThat(inputTriggers).containsExactly(originalTrigger, duplicateTrigger).inOrder() } val inputTriggersAfterBlock = MainThreadTriggerStack.inputEventInteractionTriggers - assertEquals(1, inputTriggersAfterBlock.size) - assertSame(originalTrigger, inputTriggersAfterBlock.single()) + assertThat(inputTriggersAfterBlock).containsExactly(originalTrigger) } finally { MainThreadTriggerStack.popTriggeredBy(originalTrigger) } @@ -367,11 +333,8 @@ class MainThreadTriggerStackTest { val triggers1 = MainThreadTriggerStack.currentTriggers val triggers2 = MainThreadTriggerStack.currentTriggers - // Should return different instances (copies) - assertNotSame(triggers1, triggers2) - - // But with same content - assertEquals(triggers1, triggers2) + assertThat(triggers1).isNotSameInstanceAs(triggers2) + assertThat(triggers1).containsExactlyElementsIn(triggers2).inOrder() } } @@ -381,30 +344,25 @@ class MainThreadTriggerStackTest { val innerTrigger = SimpleInteractionTrigger(2000.nanoseconds, "inner") MainThreadTriggerStack.triggeredBy(outerTrigger, endTraceAfterBlock = false) { - assertEquals(1, MainThreadTriggerStack.currentTriggers.size) - assertSame(outerTrigger, MainThreadTriggerStack.earliestInteractionTrigger) + assertThat(MainThreadTriggerStack.currentTriggers).containsExactly(outerTrigger) + assertThat(MainThreadTriggerStack.earliestInteractionTrigger).isSameInstanceAs(outerTrigger) MainThreadTriggerStack.triggeredBy( innerTrigger, endTraceAfterBlock = false ) { - assertEquals(2, MainThreadTriggerStack.currentTriggers.size) - assertSame( + assertThat(MainThreadTriggerStack.currentTriggers).containsExactly( outerTrigger, + innerTrigger + ).inOrder() + assertThat( MainThreadTriggerStack.earliestInteractionTrigger - ) // Still earliest - - val triggers = MainThreadTriggerStack.currentTriggers - assertTrue(triggers.contains(outerTrigger)) - assertTrue(triggers.contains(innerTrigger)) + ).isSameInstanceAs(outerTrigger) } - // Inner trigger should be removed - assertEquals(1, MainThreadTriggerStack.currentTriggers.size) - assertSame(outerTrigger, MainThreadTriggerStack.currentTriggers[0]) + assertThat(MainThreadTriggerStack.currentTriggers).containsExactly(outerTrigger) } - // All triggers should be removed - assertTrue(MainThreadTriggerStack.currentTriggers.isEmpty()) + assertThat(MainThreadTriggerStack.currentTriggers).isEmpty() } }