From 1b3ef912c334518f0970f2b4f73c6ebc6cbb986e Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Sun, 20 Sep 2026 20:08:44 -0400 Subject: [PATCH] test(chat): tear the coordinator down however a test ends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A coordinator test that failed its assertion hung the run instead of reporting the failure. Logging in starts a heartbeat that is a `while (true)` of delays on the coordinator's own scope rather than the test's `backgroundScope`, and `runTest` drains the scheduler once the body returns. These tests called `coordinator.teardown()` as a bare last statement, so an assertion that threw skipped it and left that loop advancing virtual time with nothing to stop it. Wrap each body in a `tornDown` helper that tears down in a `finally`. Breaking one assertion in ViewerStateStreamTest shows the difference: unwrapped it was still spinning when killed at 240s with nothing reported, wrapped it fails in 14s naming all three affected tests. GroupChatRoutingTest and ChatHistoryHydrationTest already did this and document why; this brings the remaining four files onto the same footing. ChatCoordinatorTeardownTest is left alone — it calls teardown() as its subject and never logs in, so no heartbeat is running. --- .../chat/ChatCoordinatorEagerBalanceTest.kt | 106 ++-- .../shared/chat/ChatCoordinatorEventsTest.kt | 538 +++++++++--------- .../shared/chat/ReceivedCounterTest.kt | 102 ++-- .../shared/chat/ViewerStateStreamTest.kt | 93 +-- 4 files changed, 466 insertions(+), 373 deletions(-) diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt index 03b15f6a8f..86978115cd 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt @@ -187,67 +187,89 @@ class ChatCoordinatorEagerBalanceTest { @Test fun `incoming cash message triggers tokenCoordinator add`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - val amount = Fiat(fiat = 5.0, currencyCode = CurrencyCode.CAD) - chatUpdatesChannel.send(chatUpdate(cashMessage(senderId = otherId, amount = amount))) - advanceTimeBy(1_000.milliseconds) - runCurrent() - - coVerify(exactly = 1) { tokenCoordinator.add(mint, amount) } - coordinator.teardown() + tornDown { + triggerCollection() + val amount = Fiat(fiat = 5.0, currencyCode = CurrencyCode.CAD) + chatUpdatesChannel.send(chatUpdate(cashMessage(senderId = otherId, amount = amount))) + advanceTimeBy(1_000.milliseconds) + runCurrent() + + coVerify(exactly = 1) { tokenCoordinator.add(mint, amount) } + } } @Test fun `self-sent cash message does not trigger tokenCoordinator add`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - chatUpdatesChannel.send(chatUpdate(cashMessage(senderId = selfId))) - advanceTimeBy(1_000.milliseconds) - runCurrent() + tornDown { + triggerCollection() + chatUpdatesChannel.send(chatUpdate(cashMessage(senderId = selfId))) + advanceTimeBy(1_000.milliseconds) + runCurrent() - coVerify(exactly = 0) { tokenCoordinator.add(any(), any()) } - coordinator.teardown() + coVerify(exactly = 0) { tokenCoordinator.add(any(), any()) } + } } @Test fun `text message does not trigger tokenCoordinator add`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - chatUpdatesChannel.send(chatUpdate(textMessage(senderId = otherId))) - advanceTimeBy(1_000.milliseconds) - runCurrent() + tornDown { + triggerCollection() + chatUpdatesChannel.send(chatUpdate(textMessage(senderId = otherId))) + advanceTimeBy(1_000.milliseconds) + runCurrent() - coVerify(exactly = 0) { tokenCoordinator.add(any(), any()) } - coordinator.teardown() + coVerify(exactly = 0) { tokenCoordinator.add(any(), any()) } + } } @Test fun `multiple cash messages in one update each trigger add`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - val mintB = Mint("BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBbbbbbbbbbbb") - val amount1 = Fiat(fiat = 5.0, currencyCode = CurrencyCode.USD) - val amount2 = Fiat(fiat = 10.0, currencyCode = CurrencyCode.USD) - val msg1 = cashMessage(senderId = otherId, amount = amount1, mint = mint) - val msg2 = cashMessage(senderId = otherId, amount = amount2, mint = mintB).copy(messageId = 3L) - - chatUpdatesChannel.send(chatUpdate(msg1, msg2)) - advanceTimeBy(1_000.milliseconds) - runCurrent() - - coVerify(exactly = 1) { tokenCoordinator.add(mint, amount1) } - coVerify(exactly = 1) { tokenCoordinator.add(mintB, amount2) } - coordinator.teardown() + tornDown { + triggerCollection() + val mintB = Mint("BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBbbbbbbbbbbb") + val amount1 = Fiat(fiat = 5.0, currencyCode = CurrencyCode.USD) + val amount2 = Fiat(fiat = 10.0, currencyCode = CurrencyCode.USD) + val msg1 = cashMessage(senderId = otherId, amount = amount1, mint = mint) + val msg2 = cashMessage(senderId = otherId, amount = amount2, mint = mintB).copy(messageId = 3L) + + chatUpdatesChannel.send(chatUpdate(msg1, msg2)) + advanceTimeBy(1_000.milliseconds) + runCurrent() + + coVerify(exactly = 1) { tokenCoordinator.add(mint, amount1) } + coVerify(exactly = 1) { tokenCoordinator.add(mintB, amount2) } + } } @Test fun `mixed self and incoming messages only triggers add for incoming`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - val incoming = cashMessage(senderId = otherId) - val outgoing = cashMessage(senderId = selfId).copy(messageId = 3L) + tornDown { + triggerCollection() + val incoming = cashMessage(senderId = otherId) + val outgoing = cashMessage(senderId = selfId).copy(messageId = 3L) - chatUpdatesChannel.send(chatUpdate(incoming, outgoing)) - advanceTimeBy(1_000.milliseconds) - runCurrent() + chatUpdatesChannel.send(chatUpdate(incoming, outgoing)) + advanceTimeBy(1_000.milliseconds) + runCurrent() + + coVerify(exactly = 1) { tokenCoordinator.add(any(), any()) } + } + } - coVerify(exactly = 1) { tokenCoordinator.add(any(), any()) } - coordinator.teardown() + /** + * Runs [block], then tears the coordinator down however it ends. + * + * The `finally` is the point. Logging in starts a heartbeat that is a `while (true)` of delays + * on the coordinator's own scope rather than the test's `backgroundScope`, and `runTest` drains + * the scheduler once the body returns. A failing assertion that skipped the teardown would + * leave that loop advancing virtual time with nothing to stop it, hanging the run instead of + * reporting the failure. + */ + private suspend fun tornDown(block: suspend () -> Unit) { + try { + block() + } finally { + coordinator.teardown() + } } } diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt index 0a3eba41ef..6033adb1a0 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt @@ -168,31 +168,32 @@ class ChatCoordinatorEventsTest { @Test fun `multiple events are flattened and deduped by messageId`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - - val msg1 = textMessage(id = 1, eventSequence = 1) - val msg1Edited = textMessage(id = 1, eventSequence = 2) // same messageId, higher sequence - val msg2 = textMessage(id = 2, eventSequence = 3) - - val update = ChatUpdate( - chatId = chatId, - events = listOf( - chatEvent(1, msg1), - chatEvent(2, msg1Edited), - chatEvent(3, msg2), - ), - ) - chatUpdatesChannel.send(update) - advanceTimeBy(1_000.milliseconds) - runCurrent() - - // Should have 2 unique messages (deduped by messageId, taking first by sorted eventSequence) - coVerify { - messageDataSource.upsert(chatId, match { messages -> - messages.size == 2 - }) + tornDown { + triggerCollection() + + val msg1 = textMessage(id = 1, eventSequence = 1) + val msg1Edited = textMessage(id = 1, eventSequence = 2) // same messageId, higher sequence + val msg2 = textMessage(id = 2, eventSequence = 3) + + val update = ChatUpdate( + chatId = chatId, + events = listOf( + chatEvent(1, msg1), + chatEvent(2, msg1Edited), + chatEvent(3, msg2), + ), + ) + chatUpdatesChannel.send(update) + advanceTimeBy(1_000.milliseconds) + runCurrent() + + // Should have 2 unique messages (deduped by messageId, taking first by sorted eventSequence) + coVerify { + messageDataSource.upsert(chatId, match { messages -> + messages.size == 2 + }) + } } - coordinator.teardown() } // endregion @@ -201,82 +202,85 @@ class ChatCoordinatorEventsTest { @Test fun `contiguous events advance sequence cursor`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - coEvery { metadataDataSource.getLatestEventSequence(chatId) } returns 0L - - val update = ChatUpdate( - chatId = chatId, - events = listOf( - chatEvent(1, textMessage(id = 1, eventSequence = 1)), - chatEvent(2, textMessage(id = 2, eventSequence = 2)), - ), - ) - chatUpdatesChannel.send(update) - advanceTimeBy(1_000.milliseconds) - runCurrent() + tornDown { + triggerCollection() + coEvery { metadataDataSource.getLatestEventSequence(chatId) } returns 0L + + val update = ChatUpdate( + chatId = chatId, + events = listOf( + chatEvent(1, textMessage(id = 1, eventSequence = 1)), + chatEvent(2, textMessage(id = 2, eventSequence = 2)), + ), + ) + chatUpdatesChannel.send(update) + advanceTimeBy(1_000.milliseconds) + runCurrent() - coVerify { metadataDataSource.updateLatestEventSequence(chatId, 2L) } - coordinator.teardown() + coVerify { metadataDataSource.updateLatestEventSequence(chatId, 2L) } + } } @Test fun `gap in events does not advance cursor past gap`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - coEvery { metadataDataSource.getLatestEventSequence(chatId) } returns 0L - - // Send seq 1, then seq 3 (gap at 2) - val update1 = ChatUpdate( - chatId = chatId, - events = listOf(chatEvent(1, textMessage(id = 1, eventSequence = 1))), - ) - chatUpdatesChannel.send(update1) - advanceTimeBy(500.milliseconds) - runCurrent() - - val update2 = ChatUpdate( - chatId = chatId, - events = listOf(chatEvent(3, textMessage(id = 3, eventSequence = 3))), - ) - chatUpdatesChannel.send(update2) - advanceTimeBy(500.milliseconds) - runCurrent() - - // Cursor should advance to 1 (contiguous), not 3 - coVerify { metadataDataSource.updateLatestEventSequence(chatId, 1L) } - coVerify(exactly = 0) { metadataDataSource.updateLatestEventSequence(chatId, 3L) } - coordinator.teardown() + tornDown { + triggerCollection() + coEvery { metadataDataSource.getLatestEventSequence(chatId) } returns 0L + + // Send seq 1, then seq 3 (gap at 2) + val update1 = ChatUpdate( + chatId = chatId, + events = listOf(chatEvent(1, textMessage(id = 1, eventSequence = 1))), + ) + chatUpdatesChannel.send(update1) + advanceTimeBy(500.milliseconds) + runCurrent() + + val update2 = ChatUpdate( + chatId = chatId, + events = listOf(chatEvent(3, textMessage(id = 3, eventSequence = 3))), + ) + chatUpdatesChannel.send(update2) + advanceTimeBy(500.milliseconds) + runCurrent() + + // Cursor should advance to 1 (contiguous), not 3 + coVerify { metadataDataSource.updateLatestEventSequence(chatId, 1L) } + coVerify(exactly = 0) { metadataDataSource.updateLatestEventSequence(chatId, 3L) } + } } @Test fun `late event fills gap and advances cursor`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - coEvery { metadataDataSource.getLatestEventSequence(chatId) } returns 0L - - // Send 1, then 3 (gap), then 2 (fills gap) - chatUpdatesChannel.send(ChatUpdate( - chatId = chatId, - events = listOf(chatEvent(1, textMessage(id = 1, eventSequence = 1))), - )) - advanceTimeBy(100.milliseconds) - runCurrent() - - chatUpdatesChannel.send(ChatUpdate( - chatId = chatId, - events = listOf(chatEvent(3, textMessage(id = 3, eventSequence = 3))), - )) - advanceTimeBy(100.milliseconds) - runCurrent() - - chatUpdatesChannel.send(ChatUpdate( - chatId = chatId, - events = listOf(chatEvent(2, textMessage(id = 2, eventSequence = 2))), - )) - advanceTimeBy(100.milliseconds) - runCurrent() - - // After filling the gap, cursor should advance to 3 - coVerify { metadataDataSource.updateLatestEventSequence(chatId, 3L) } - coordinator.teardown() + tornDown { + triggerCollection() + coEvery { metadataDataSource.getLatestEventSequence(chatId) } returns 0L + + // Send 1, then 3 (gap), then 2 (fills gap) + chatUpdatesChannel.send(ChatUpdate( + chatId = chatId, + events = listOf(chatEvent(1, textMessage(id = 1, eventSequence = 1))), + )) + advanceTimeBy(100.milliseconds) + runCurrent() + + chatUpdatesChannel.send(ChatUpdate( + chatId = chatId, + events = listOf(chatEvent(3, textMessage(id = 3, eventSequence = 3))), + )) + advanceTimeBy(100.milliseconds) + runCurrent() + + chatUpdatesChannel.send(ChatUpdate( + chatId = chatId, + events = listOf(chatEvent(2, textMessage(id = 2, eventSequence = 2))), + )) + advanceTimeBy(100.milliseconds) + runCurrent() + + // After filling the gap, cursor should advance to 3 + coVerify { metadataDataSource.updateLatestEventSequence(chatId, 3L) } + } } // endregion @@ -285,165 +289,169 @@ class ChatCoordinatorEventsTest { @Test fun `reaction update is applied to in-memory overlay`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - - val update = ChatUpdate( - chatId = chatId, - reactionUpdates = listOf( - ReactionUpdate( - messageId = 1L, - emoji = Emoji("\uD83D\uDE00"), - actor = otherId, - action = ReactionUpdate.Action.ADDED, - count = 1, - sequence = 1, - reactedAt = Instant.fromEpochSeconds(1000), + tornDown { + triggerCollection() + + val update = ChatUpdate( + chatId = chatId, + reactionUpdates = listOf( + ReactionUpdate( + messageId = 1L, + emoji = Emoji("\uD83D\uDE00"), + actor = otherId, + action = ReactionUpdate.Action.ADDED, + count = 1, + sequence = 1, + reactedAt = Instant.fromEpochSeconds(1000), + ), ), - ), - ) - chatUpdatesChannel.send(update) - advanceTimeBy(1_000.milliseconds) - runCurrent() - - val state = coordinator.state.value - val overlay = state.reactionOverlays[chatId] - assertNotNull(overlay) - val summary = overlay[1L] - assertNotNull(summary) - assertEquals(1, summary.reactions.size) - assertEquals("\uD83D\uDE00", summary.reactions[0].emoji.value) - assertEquals(1L, summary.reactions[0].count) - coordinator.teardown() + ) + chatUpdatesChannel.send(update) + advanceTimeBy(1_000.milliseconds) + runCurrent() + + val state = coordinator.state.value + val overlay = state.reactionOverlays[chatId] + assertNotNull(overlay) + val summary = overlay[1L] + assertNotNull(summary) + assertEquals(1, summary.reactions.size) + assertEquals("\uD83D\uDE00", summary.reactions[0].emoji.value) + assertEquals(1L, summary.reactions[0].count) + } } @Test fun `reaction LWW guard rejects stale updates`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - - // First update: count=3, sequence=5 - chatUpdatesChannel.send(ChatUpdate( - chatId = chatId, - reactionUpdates = listOf( - ReactionUpdate( - messageId = 1L, - emoji = Emoji("\uD83D\uDC4D"), - actor = otherId, - action = ReactionUpdate.Action.ADDED, - count = 3, - sequence = 5, - reactedAt = Instant.fromEpochSeconds(1000), + tornDown { + triggerCollection() + + // First update: count=3, sequence=5 + chatUpdatesChannel.send(ChatUpdate( + chatId = chatId, + reactionUpdates = listOf( + ReactionUpdate( + messageId = 1L, + emoji = Emoji("\uD83D\uDC4D"), + actor = otherId, + action = ReactionUpdate.Action.ADDED, + count = 3, + sequence = 5, + reactedAt = Instant.fromEpochSeconds(1000), + ), ), - ), - )) - advanceTimeBy(500.milliseconds) - runCurrent() - - // Stale update: count=1, sequence=2 (older) - chatUpdatesChannel.send(ChatUpdate( - chatId = chatId, - reactionUpdates = listOf( - ReactionUpdate( - messageId = 1L, - emoji = Emoji("\uD83D\uDC4D"), - actor = otherId, - action = ReactionUpdate.Action.REMOVED, - count = 1, - sequence = 2, // older than 5 - reactedAt = Instant.fromEpochSeconds(500), + )) + advanceTimeBy(500.milliseconds) + runCurrent() + + // Stale update: count=1, sequence=2 (older) + chatUpdatesChannel.send(ChatUpdate( + chatId = chatId, + reactionUpdates = listOf( + ReactionUpdate( + messageId = 1L, + emoji = Emoji("\uD83D\uDC4D"), + actor = otherId, + action = ReactionUpdate.Action.REMOVED, + count = 1, + sequence = 2, // older than 5 + reactedAt = Instant.fromEpochSeconds(500), + ), ), - ), - )) - advanceTimeBy(500.milliseconds) - runCurrent() - - val reactions = coordinator.state.value.reactionOverlays[chatId]?.get(1L)?.reactions - assertNotNull(reactions) - assertEquals(1, reactions.size) - assertEquals(3L, reactions[0].count) // stayed at 3, stale update rejected - assertEquals(5L, reactions[0].sequence) - coordinator.teardown() + )) + advanceTimeBy(500.milliseconds) + runCurrent() + + val reactions = coordinator.state.value.reactionOverlays[chatId]?.get(1L)?.reactions + assertNotNull(reactions) + assertEquals(1, reactions.size) + assertEquals(3L, reactions[0].count) // stayed at 3, stale update rejected + assertEquals(5L, reactions[0].sequence) + } } @Test fun `reaction with count zero is pruned`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - - // Add reaction - chatUpdatesChannel.send(ChatUpdate( - chatId = chatId, - reactionUpdates = listOf( - ReactionUpdate( - messageId = 1L, - emoji = Emoji("\uD83D\uDE00"), - actor = otherId, - action = ReactionUpdate.Action.ADDED, - count = 1, - sequence = 1, - reactedAt = Instant.fromEpochSeconds(1000), + tornDown { + triggerCollection() + + // Add reaction + chatUpdatesChannel.send(ChatUpdate( + chatId = chatId, + reactionUpdates = listOf( + ReactionUpdate( + messageId = 1L, + emoji = Emoji("\uD83D\uDE00"), + actor = otherId, + action = ReactionUpdate.Action.ADDED, + count = 1, + sequence = 1, + reactedAt = Instant.fromEpochSeconds(1000), + ), ), - ), - )) - advanceTimeBy(500.milliseconds) - runCurrent() - - // Remove reaction (count=0) - chatUpdatesChannel.send(ChatUpdate( - chatId = chatId, - reactionUpdates = listOf( - ReactionUpdate( - messageId = 1L, - emoji = Emoji("\uD83D\uDE00"), - actor = otherId, - action = ReactionUpdate.Action.REMOVED, - count = 0, - sequence = 2, - reactedAt = Instant.fromEpochSeconds(2000), + )) + advanceTimeBy(500.milliseconds) + runCurrent() + + // Remove reaction (count=0) + chatUpdatesChannel.send(ChatUpdate( + chatId = chatId, + reactionUpdates = listOf( + ReactionUpdate( + messageId = 1L, + emoji = Emoji("\uD83D\uDE00"), + actor = otherId, + action = ReactionUpdate.Action.REMOVED, + count = 0, + sequence = 2, + reactedAt = Instant.fromEpochSeconds(2000), + ), ), - ), - )) - advanceTimeBy(500.milliseconds) - runCurrent() - - val reactions = coordinator.state.value.reactionOverlays[chatId]?.get(1L)?.reactions - assertNotNull(reactions) - assertTrue(reactions.isEmpty()) - coordinator.teardown() + )) + advanceTimeBy(500.milliseconds) + runCurrent() + + val reactions = coordinator.state.value.reactionOverlays[chatId]?.get(1L)?.reactions + assertNotNull(reactions) + assertTrue(reactions.isEmpty()) + } } @Test fun `multiple emoji reactions on same message`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - - chatUpdatesChannel.send(ChatUpdate( - chatId = chatId, - reactionUpdates = listOf( - ReactionUpdate( - messageId = 1L, - emoji = Emoji("\uD83D\uDE00"), - actor = otherId, - action = ReactionUpdate.Action.ADDED, - count = 2, - sequence = 1, - reactedAt = Instant.fromEpochSeconds(1000), + tornDown { + triggerCollection() + + chatUpdatesChannel.send(ChatUpdate( + chatId = chatId, + reactionUpdates = listOf( + ReactionUpdate( + messageId = 1L, + emoji = Emoji("\uD83D\uDE00"), + actor = otherId, + action = ReactionUpdate.Action.ADDED, + count = 2, + sequence = 1, + reactedAt = Instant.fromEpochSeconds(1000), + ), + ReactionUpdate( + messageId = 1L, + emoji = Emoji("\uD83D\uDC4D"), + actor = otherId, + action = ReactionUpdate.Action.ADDED, + count = 5, + sequence = 2, + reactedAt = Instant.fromEpochSeconds(1000), + ), ), - ReactionUpdate( - messageId = 1L, - emoji = Emoji("\uD83D\uDC4D"), - actor = otherId, - action = ReactionUpdate.Action.ADDED, - count = 5, - sequence = 2, - reactedAt = Instant.fromEpochSeconds(1000), - ), - ), - )) - advanceTimeBy(1_000.milliseconds) - runCurrent() - - val reactions = coordinator.state.value.reactionOverlays[chatId]?.get(1L)?.reactions - assertNotNull(reactions) - assertEquals(2, reactions.size) - coordinator.teardown() + )) + advanceTimeBy(1_000.milliseconds) + runCurrent() + + val reactions = coordinator.state.value.reactionOverlays[chatId]?.get(1L)?.reactions + assertNotNull(reactions) + assertEquals(2, reactions.size) + } } // endregion @@ -452,32 +460,50 @@ class ChatCoordinatorEventsTest { @Test fun `re-login after reset resumes chat sync on a fresh scope`() = runTest(testDispatchers.dispatcher) { - // First session wires the delegate collectors onto the coordinator scope. - coordinator.onUserLoggedIn(mockk(relaxed = true)) - runCurrent() - - // Logout cancels the coordinator's supervisor job (and thus its scope). - coordinator.teardown() - runCurrent() - - // Re-login in the same process. Before the fix, the scope stayed cancelled, - // so every launch in onUserLoggedIn was a silent no-op and incoming chat - // updates were dropped until a process restart rebuilt the singleton. - coordinator.onUserLoggedIn(mockk(relaxed = true)) - runCurrent() - - val msg = textMessage(id = 7, eventSequence = 1) - chatUpdatesChannel.send(ChatUpdate(chatId = chatId, events = listOf(chatEvent(1, msg)))) - advanceTimeBy(1_000.milliseconds) - runCurrent() - - coVerify { - messageDataSource.upsert(chatId, match { messages -> - messages.size == 1 && messages[0].messageId == 7L - }) + tornDown { + // First session wires the delegate collectors onto the coordinator scope. + coordinator.onUserLoggedIn(mockk(relaxed = true)) + runCurrent() + + // Logout cancels the coordinator's supervisor job (and thus its scope). + coordinator.teardown() + runCurrent() + + // Re-login in the same process. Before the fix, the scope stayed cancelled, + // so every launch in onUserLoggedIn was a silent no-op and incoming chat + // updates were dropped until a process restart rebuilt the singleton. + coordinator.onUserLoggedIn(mockk(relaxed = true)) + runCurrent() + + val msg = textMessage(id = 7, eventSequence = 1) + chatUpdatesChannel.send(ChatUpdate(chatId = chatId, events = listOf(chatEvent(1, msg)))) + advanceTimeBy(1_000.milliseconds) + runCurrent() + + coVerify { + messageDataSource.upsert(chatId, match { messages -> + messages.size == 1 && messages[0].messageId == 7L + }) + } } - coordinator.teardown() } // endregion + + /** + * Runs [block], then tears the coordinator down however it ends. + * + * The `finally` is the point. Logging in starts a heartbeat that is a `while (true)` of delays + * on the coordinator's own scope rather than the test's `backgroundScope`, and `runTest` drains + * the scheduler once the body returns. A failing assertion that skipped the teardown would + * leave that loop advancing virtual time with nothing to stop it, hanging the run instead of + * reporting the failure. + */ + private suspend fun tornDown(block: suspend () -> Unit) { + try { + block() + } finally { + coordinator.teardown() + } + } } diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt index 1b98b96316..f89a2c6ba0 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt @@ -213,75 +213,99 @@ class ReceivedCounterTest { @Test fun `inbound tip increments tips and messages`() = runTest(testDispatchers.dispatcher) { - coordinator.onUserLoggedIn(mockk(relaxed = true)) - deliver(tipMessage(messageId = 1L, senderId = otherId)) + tornDown { + coordinator.onUserLoggedIn(mockk(relaxed = true)) + deliver(tipMessage(messageId = 1L, senderId = otherId)) - coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, 1.0) } - coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Messages, 1.0) } - coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.TipsValue, 5.0) } - coordinator.teardown() + coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, 1.0) } + coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Messages, 1.0) } + coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.TipsValue, 5.0) } + } } @Test fun `inbound text increments messages only`() = runTest(testDispatchers.dispatcher) { - coordinator.onUserLoggedIn(mockk(relaxed = true)) - deliver(textMessage(messageId = 1L, senderId = otherId)) + tornDown { + coordinator.onUserLoggedIn(mockk(relaxed = true)) + deliver(textMessage(messageId = 1L, senderId = otherId)) - coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Messages, 1.0) } - coVerify(exactly = 0) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, any()) } - coordinator.teardown() + coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Messages, 1.0) } + coVerify(exactly = 0) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, any()) } + } } @Test fun `self-sent messages are not counted`() = runTest(testDispatchers.dispatcher) { - coordinator.onUserLoggedIn(mockk(relaxed = true)) - deliver(tipMessage(messageId = 1L, senderId = selfId)) + tornDown { + coordinator.onUserLoggedIn(mockk(relaxed = true)) + deliver(tipMessage(messageId = 1L, senderId = selfId)) - coVerify(exactly = 0) { analytics.incrementReceivedCounter(any(), any()) } - coordinator.teardown() + coVerify(exactly = 0) { analytics.incrementReceivedCounter(any(), any()) } + } } @Test fun `redelivered messages are counted exactly once`() = runTest(testDispatchers.dispatcher) { - coordinator.onUserLoggedIn(mockk(relaxed = true)) - val msg = tipMessage(messageId = 7L, senderId = otherId) + tornDown { + coordinator.onUserLoggedIn(mockk(relaxed = true)) + val msg = tipMessage(messageId = 7L, senderId = otherId) - deliver(msg) - deliver(msg) // gap fill / reconnect replays the same message + deliver(msg) + deliver(msg) // gap fill / reconnect replays the same message - coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, 1.0) } - coordinator.teardown() + coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, 1.0) } + } } @Test fun `messages at or below the watermark are skipped`() = runTest(testDispatchers.dispatcher) { - countedThrough = 10L - coordinator.onUserLoggedIn(mockk(relaxed = true)) - deliver(textMessage(messageId = 10L, senderId = otherId)) + tornDown { + countedThrough = 10L + coordinator.onUserLoggedIn(mockk(relaxed = true)) + deliver(textMessage(messageId = 10L, senderId = otherId)) - coVerify(exactly = 0) { analytics.incrementReceivedCounter(any(), any()) } - coordinator.teardown() + coVerify(exactly = 0) { analytics.incrementReceivedCounter(any(), any()) } + } } @Test fun `tip value is normalised to USD`() = runTest(testDispatchers.dispatcher) { - coordinator.onUserLoggedIn(mockk(relaxed = true)) - val cad = Fiat(fiat = 10.0, currencyCode = CurrencyCode.CAD) - deliver(tipMessage(messageId = 1L, senderId = otherId, amount = cad)) + tornDown { + coordinator.onUserLoggedIn(mockk(relaxed = true)) + val cad = Fiat(fiat = 10.0, currencyCode = CurrencyCode.CAD) + deliver(tipMessage(messageId = 1L, senderId = otherId, amount = cad)) - coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.TipsValue, 5.0) } - coordinator.teardown() + coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.TipsValue, 5.0) } + } } @Test fun `missing rate still counts the tip but not its value`() = runTest(testDispatchers.dispatcher) { - every { exchange.rateToUsd(CurrencyCode.EUR) } returns null - coordinator.onUserLoggedIn(mockk(relaxed = true)) - val eur = Fiat(fiat = 10.0, currencyCode = CurrencyCode.EUR) - deliver(tipMessage(messageId = 1L, senderId = otherId, amount = eur)) - - coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, 1.0) } - coVerify(exactly = 0) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.TipsValue, any()) } - coordinator.teardown() + tornDown { + every { exchange.rateToUsd(CurrencyCode.EUR) } returns null + coordinator.onUserLoggedIn(mockk(relaxed = true)) + val eur = Fiat(fiat = 10.0, currencyCode = CurrencyCode.EUR) + deliver(tipMessage(messageId = 1L, senderId = otherId, amount = eur)) + + coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, 1.0) } + coVerify(exactly = 0) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.TipsValue, any()) } + } + } + + /** + * Runs [block], then tears the coordinator down however it ends. + * + * The `finally` is the point. Logging in starts a heartbeat that is a `while (true)` of delays + * on the coordinator's own scope rather than the test's `backgroundScope`, and `runTest` drains + * the scheduler once the body returns. A failing assertion that skipped the teardown would + * leave that loop advancing virtual time with nothing to stop it, hanging the run instead of + * reporting the failure. + */ + private suspend fun tornDown(block: suspend () -> Unit) { + try { + block() + } finally { + coordinator.teardown() + } } } diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ViewerStateStreamTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ViewerStateStreamTest.kt index 8822e8c4d9..9e622c34d5 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ViewerStateStreamTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ViewerStateStreamTest.kt @@ -149,31 +149,33 @@ class ViewerStateStreamTest { @Test fun `a timed mute from the stream is stored`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - val viewerState = ViewerState( - mute = MuteState.Until(Instant.fromEpochSeconds(2_000)), - version = 4, - ) + tornDown { + triggerCollection() + val viewerState = ViewerState( + mute = MuteState.Until(Instant.fromEpochSeconds(2_000)), + version = 4, + ) - chatUpdatesChannel.send(viewerStateUpdate(viewerState)) - advanceTimeBy(1_000.milliseconds) - runCurrent() + chatUpdatesChannel.send(viewerStateUpdate(viewerState)) + advanceTimeBy(1_000.milliseconds) + runCurrent() - coVerify(exactly = 1) { metadataDataSource.updateViewerState(chatId, viewerState) } - coordinator.teardown() + coVerify(exactly = 1) { metadataDataSource.updateViewerState(chatId, viewerState) } + } } @Test fun `an indefinite mute from the stream is stored`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - val viewerState = ViewerState(mute = MuteState.Forever, version = 7) + tornDown { + triggerCollection() + val viewerState = ViewerState(mute = MuteState.Forever, version = 7) - chatUpdatesChannel.send(viewerStateUpdate(viewerState)) - advanceTimeBy(1_000.milliseconds) - runCurrent() + chatUpdatesChannel.send(viewerStateUpdate(viewerState)) + advanceTimeBy(1_000.milliseconds) + runCurrent() - coVerify(exactly = 1) { metadataDataSource.updateViewerState(chatId, viewerState) } - coordinator.teardown() + coVerify(exactly = 1) { metadataDataSource.updateViewerState(chatId, viewerState) } + } } /** @@ -183,34 +185,53 @@ class ViewerStateStreamTest { @Test fun `an unmute from the stream is stored at its own version`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - val viewerState = ViewerState(mute = null, version = 8) + tornDown { + triggerCollection() + val viewerState = ViewerState(mute = null, version = 8) - chatUpdatesChannel.send(viewerStateUpdate(viewerState)) - advanceTimeBy(1_000.milliseconds) - runCurrent() + chatUpdatesChannel.send(viewerStateUpdate(viewerState)) + advanceTimeBy(1_000.milliseconds) + runCurrent() - coVerify(exactly = 1) { metadataDataSource.updateViewerState(chatId, viewerState) } - coordinator.teardown() + coVerify(exactly = 1) { metadataDataSource.updateViewerState(chatId, viewerState) } + } } @Test fun `an update carrying no viewer state leaves the stored one alone`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - - chatUpdatesChannel.send( - ChatUpdate( - chatId = chatId, - metadataUpdates = listOf( - MetadataUpdate.LastActivityChanged(Instant.fromEpochSeconds(2_000)), + tornDown { + triggerCollection() + + chatUpdatesChannel.send( + ChatUpdate( + chatId = chatId, + metadataUpdates = listOf( + MetadataUpdate.LastActivityChanged(Instant.fromEpochSeconds(2_000)), + ), ), - ), - ) - advanceTimeBy(1_000.milliseconds) - runCurrent() + ) + advanceTimeBy(1_000.milliseconds) + runCurrent() + + coVerify(exactly = 0) { metadataDataSource.updateViewerState(any(), any()) } + } + } - coVerify(exactly = 0) { metadataDataSource.updateViewerState(any(), any()) } + /** + * Runs [block], then tears the coordinator down however it ends. + * + * The `finally` is the point. Logging in starts a heartbeat that is a `while (true)` of delays + * on the coordinator's own scope rather than the test's `backgroundScope`, and `runTest` drains + * the scheduler once the body returns. A failing assertion that skipped the teardown would + * leave that loop advancing virtual time with nothing to stop it, hanging the run instead of + * reporting the failure. + */ + private suspend fun tornDown(block: suspend () -> Unit) { + try { + block() + } finally { coordinator.teardown() } + } }