Model the fake voice recorder on a cancellable recording job (#7466)

* Model the fake voice recorder on a cancellable recording job

FakeVoiceRecorder pushed a whole recording synchronously from a single
suspending call, which made it both weaker and stricter than
DefaultVoiceRecorder.

Every level emission carried the same elapsed time and the emissions were
coalesced by Molecule, so no intermediate recording state was observable and
the level-limit test had to skip half the emissions to compensate. Each level
now advances the test time source and suspends for its share of the recording
duration, so the states arrive one at a time and that magic number is gone.

Call verification moved to the constructor lambdas the rest of the repo uses,
in place of hand-rolled counters and assertCalls. The counters discarded the
argument, so nothing pinned that cancelling a recording stops it with
cancelled = true; that is now asserted. Truth is no longer needed by the
fixture module.

A second startRecord() no longer raises an error where production only warns
and returns, and stopping or deleting terminates an in-flight emission loop
the way cancelling the real recording job does, so a stale recording state can
no longer arrive after the recording has finished.

* Keep each recording's level loop to its own recording

The level loop checked a single isRecording flag, which cannot tell "this
recording was cancelled" from "a later recording has started". After a cancel
followed by a new start, the first loop resumed, saw the flag set again, and
advanced the shared test time source, so the new recording reported the elapsed
time of both. Each recording now carries an id and its loop stops as soon as
that id is no longer the active one.

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/voicemessages/composer/DefaultVoiceMessageComposerPresenterTest.kt

@@ -47,6 +47,7 @@ import io.element.android.libraries.voicerecorder.api.VoiceRecorder
 import io.element.android.libraries.voicerecorder.test.FakeVoiceRecorder
 import io.element.android.services.analytics.test.FakeAnalyticsService
 import io.element.android.tests.testutils.WarmUpRule
+import io.element.android.tests.testutils.consumeItemsUntilTimeout
 import io.element.android.tests.testutils.lambda.any
 import io.element.android.tests.testutils.lambda.lambdaRecorder
 import io.element.android.tests.testutils.lambda.value
@@ -60,6 +61,7 @@ import org.junit.Rule
 import org.junit.Test
 import java.io.File
 import kotlin.time.Duration
+import kotlin.time.Duration.Companion.milliseconds
 import kotlin.time.Duration.Companion.seconds
 
 @Suppress("LargeClass")
@@ -67,8 +69,14 @@ class DefaultVoiceMessageComposerPresenterTest {
     @get:Rule
     val warmUpRule = WarmUpRule()
 
+    private val startRecordResult = lambdaRecorder<Unit> { }
+    private val stopRecordResult = lambdaRecorder<Boolean, Unit> { }
+    private val deleteRecordingResult = lambdaRecorder<Unit> { }
     private val voiceRecorder = FakeVoiceRecorder(
-        recordingDuration = RECORDING_DURATION
+        recordingDuration = RECORDING_DURATION,
+        startRecordResult = startRecordResult,
+        stopRecordResult = stopRecordResult,
+        deleteRecordingResult = deleteRecordingResult,
     )
     private val analyticsService = FakeAnalyticsService()
     private val sendVoiceMessageResult =
@@ -97,7 +105,9 @@ class DefaultVoiceMessageComposerPresenterTest {
 
     companion object {
         private val RECORDING_DURATION = 1.seconds
+        private val FIRST_LEVEL_DURATION = 500.milliseconds
         private val RECORDING_STATE = VoiceMessageState.Recording(RECORDING_DURATION, listOf(0.1f, 0.2f).toImmutableList())
+        private val FIRST_RECORDING_STATE = VoiceMessageState.Recording(FIRST_LEVEL_DURATION, listOf(0.1f).toImmutableList())
     }
 
     @Test
@@ -106,7 +116,7 @@ class DefaultVoiceMessageComposerPresenterTest {
         presenter.test {
             val initialState = awaitItem()
             assertThat(initialState.voiceMessageState).isEqualTo(VoiceMessageState.Idle)
-            voiceRecorder.assertCalls(started = 0)
+            startRecordResult.assertions().isNeverCalled()
 
             testPauseAndDestroy(initialState)
         }
@@ -118,9 +128,10 @@ class DefaultVoiceMessageComposerPresenterTest {
         presenter.test {
             awaitItem().eventSink(VoiceMessageComposerEvent.RecorderEvent(VoiceMessageRecorderEvent.Start))
 
+            assertThat(awaitItem().voiceMessageState).isEqualTo(FIRST_RECORDING_STATE)
             val finalState = awaitItem()
             assertThat(finalState.voiceMessageState).isEqualTo(RECORDING_STATE)
-            voiceRecorder.assertCalls(started = 1)
+            startRecordResult.assertions().isCalledOnce()
 
             testPauseAndDestroy(finalState)
         }
@@ -133,14 +144,16 @@ class DefaultVoiceMessageComposerPresenterTest {
         val voiceRecorder = FakeVoiceRecorder(
             levels = levels,
             recordingDuration = RECORDING_DURATION,
+            startRecordResult = { },
+            stopRecordResult = { },
+            deleteRecordingResult = { },
         )
         val presenter = createDefaultVoiceMessageComposerPresenter(
             voiceRecorder = voiceRecorder,
         )
         presenter.test {
             awaitItem().eventSink(VoiceMessageComposerEvent.RecorderEvent(VoiceMessageRecorderEvent.Start))
-            skipItems(numberOfLevels / 2 - 1)
-            val finalState = awaitItem()
+            val finalState = consumeItemsUntilTimeout().last()
             assertThat(finalState.voiceMessageState).isInstanceOf(VoiceMessageState.Recording::class.java)
             val recordingState = finalState.voiceMessageState as VoiceMessageState.Recording
             // The number of levels should be limited to 128 items
@@ -221,7 +234,8 @@ class DefaultVoiceMessageComposerPresenterTest {
 
             val finalState = awaitItem()
             assertThat(finalState.voiceMessageState).isEqualTo(aPreviewState())
-            voiceRecorder.assertCalls(started = 1, stopped = 1)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
 
             cancelAndIgnoreRemainingEvents()
         }
@@ -235,7 +249,9 @@ class DefaultVoiceMessageComposerPresenterTest {
             awaitItem().eventSink(VoiceMessageComposerEvent.RecorderEvent(VoiceMessageRecorderEvent.Cancel))
             val finalState = awaitItem()
             assertThat(finalState.voiceMessageState).isEqualTo(VoiceMessageState.Idle)
-            voiceRecorder.assertCalls(started = 1, stopped = 1, deleted = 1)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(true))
+            deleteRecordingResult.assertions().isCalledOnce()
             testPauseAndDestroy(finalState)
         }
     }
@@ -249,7 +265,9 @@ class DefaultVoiceMessageComposerPresenterTest {
 
             val finalState = awaitItem()
             assertThat(finalState.voiceMessageState).isEqualTo(aPreviewState())
-            voiceRecorder.assertCalls(started = 1, stopped = 1, deleted = 0)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
+            deleteRecordingResult.assertions().isNeverCalled()
 
             testPauseAndDestroy(finalState)
         }
@@ -265,8 +283,10 @@ class DefaultVoiceMessageComposerPresenterTest {
             }
 
             // Nothing should happen
-            assertThat(finalState.voiceMessageState).isEqualTo(RECORDING_STATE)
-            voiceRecorder.assertCalls(started = 1, stopped = 0, deleted = 0)
+            assertThat(finalState.voiceMessageState).isEqualTo(FIRST_RECORDING_STATE)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isNeverCalled()
+            deleteRecordingResult.assertions().isNeverCalled()
 
             testPauseAndDestroy(finalState)
         }
@@ -282,7 +302,9 @@ class DefaultVoiceMessageComposerPresenterTest {
             val finalState = awaitItem().also {
                 assertThat(it.voiceMessageState).isEqualTo(aPlayingState())
             }
-            voiceRecorder.assertCalls(started = 1, stopped = 1, deleted = 0)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
+            deleteRecordingResult.assertions().isNeverCalled()
 
             testPauseAndDestroy(finalState)
         }
@@ -299,7 +321,9 @@ class DefaultVoiceMessageComposerPresenterTest {
             val finalState = awaitItem().also {
                 assertThat(it.voiceMessageState).isEqualTo(aPausedState())
             }
-            voiceRecorder.assertCalls(started = 1, stopped = 1, deleted = 0)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
+            deleteRecordingResult.assertions().isNeverCalled()
 
             testPauseAndDestroy(finalState)
         }
@@ -340,7 +364,9 @@ class DefaultVoiceMessageComposerPresenterTest {
 
             val finalState = awaitItem()
             assertThat(finalState.voiceMessageState).isEqualTo(VoiceMessageState.Idle)
-            voiceRecorder.assertCalls(started = 1, stopped = 1, deleted = 1)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
+            deleteRecordingResult.assertions().isCalledOnce()
 
             testPauseAndDestroy(finalState)
         }
@@ -360,7 +386,9 @@ class DefaultVoiceMessageComposerPresenterTest {
 
             val finalState = awaitItem()
             assertThat(finalState.voiceMessageState).isEqualTo(VoiceMessageState.Idle)
-            voiceRecorder.assertCalls(started = 1, stopped = 1, deleted = 1)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
+            deleteRecordingResult.assertions().isCalledOnce()
 
             testPauseAndDestroy(finalState)
         }
@@ -377,7 +405,9 @@ class DefaultVoiceMessageComposerPresenterTest {
             val finalState = awaitItem()
             assertThat(finalState.voiceMessageState).isEqualTo(VoiceMessageState.Idle)
             sendVoiceMessageResult.assertions().isCalledOnce()
-            voiceRecorder.assertCalls(started = 1, stopped = 1, deleted = 1)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
+            deleteRecordingResult.assertions().isCalledOnce()
 
             testPauseAndDestroy(finalState)
         }
@@ -455,7 +485,9 @@ class DefaultVoiceMessageComposerPresenterTest {
             val finalState = awaitItem()
             assertThat(finalState.voiceMessageState).isEqualTo(VoiceMessageState.Idle)
             sendVoiceMessageResult.assertions().isCalledOnce()
-            voiceRecorder.assertCalls(started = 1, stopped = 1, deleted = 1)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
+            deleteRecordingResult.assertions().isCalledOnce()
 
             testPauseAndDestroy(finalState)
         }
@@ -476,7 +508,9 @@ class DefaultVoiceMessageComposerPresenterTest {
             val finalState = awaitItem()
             assertThat(finalState.voiceMessageState).isEqualTo(VoiceMessageState.Idle)
             sendVoiceMessageResult.assertions().isCalledOnce()
-            voiceRecorder.assertCalls(started = 1, stopped = 1, deleted = 1)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
+            deleteRecordingResult.assertions().isCalledOnce()
 
             testPauseAndDestroy(finalState)
         }
@@ -499,7 +533,9 @@ class DefaultVoiceMessageComposerPresenterTest {
             assertThat(finalState.voiceMessageState).isEqualTo(aPreviewState(isSending = true))
             sendVoiceMessageResult.assertions().isNeverCalled()
             assertThat(analyticsService.trackedErrors).isEmpty()
-            voiceRecorder.assertCalls(started = 1, stopped = 1, deleted = 0)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
+            deleteRecordingResult.assertions().isNeverCalled()
 
             testPauseAndDestroy(finalState)
         }
@@ -527,7 +563,9 @@ class DefaultVoiceMessageComposerPresenterTest {
             val finalState = awaitItem()
             assertThat(finalState.voiceMessageState).isEqualTo(VoiceMessageState.Idle)
             sendVoiceMessageResult.assertions().isCalledOnce()
-            voiceRecorder.assertCalls(started = 1, stopped = 1, deleted = 1)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
+            deleteRecordingResult.assertions().isCalledOnce()
 
             testPauseAndDestroy(finalState)
         }
@@ -577,7 +615,7 @@ class DefaultVoiceMessageComposerPresenterTest {
             assertThat(initialState.voiceMessageState).isEqualTo(VoiceMessageState.Idle)
             sendVoiceMessageResult.assertions().isNeverCalled()
             assertThat(analyticsService.trackedErrors).hasSize(1)
-            voiceRecorder.assertCalls(started = 0)
+            startRecordResult.assertions().isNeverCalled()
 
             testPauseAndDestroy(initialState)
         }
@@ -586,8 +624,14 @@ class DefaultVoiceMessageComposerPresenterTest {
     @Test
     fun `present - record error - security exceptions are tracked`() = runTest {
         val exception = SecurityException("")
-        voiceRecorder.givenThrowsSecurityException(exception)
-        val presenter = createDefaultVoiceMessageComposerPresenter()
+        val startRecordResult = lambdaRecorder<Unit> { throw exception }
+        val voiceRecorder = FakeVoiceRecorder(
+            recordingDuration = RECORDING_DURATION,
+            startRecordResult = startRecordResult,
+        )
+        val presenter = createDefaultVoiceMessageComposerPresenter(
+            voiceRecorder = voiceRecorder,
+        )
         presenter.test {
             val initialState = awaitItem()
             initialState.eventSink(VoiceMessageComposerEvent.RecorderEvent(VoiceMessageRecorderEvent.Start))
@@ -596,7 +640,7 @@ class DefaultVoiceMessageComposerPresenterTest {
             assertThat(analyticsService.trackedErrors).containsExactly(
                 VoiceMessageException.PermissionMissing(message = "Expected permission to record but none", cause = exception)
             )
-            voiceRecorder.assertCalls(started = 1)
+            startRecordResult.assertions().isCalledOnce()
 
             testPauseAndDestroy(initialState)
         }
@@ -616,7 +660,8 @@ class DefaultVoiceMessageComposerPresenterTest {
             assertThat(awaitItem().voiceMessageState).isEqualTo(VoiceMessageState.Idle)
 
             initialState.eventSink(VoiceMessageComposerEvent.RecorderEvent(VoiceMessageRecorderEvent.Stop))
-            voiceRecorder.assertCalls(stopped = 1)
+            startRecordResult.assertions().isNeverCalled()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
 
             permissionsPresenter.setPermissionGranted()
 
@@ -625,7 +670,8 @@ class DefaultVoiceMessageComposerPresenterTest {
 
             val finalState = expectMostRecentItem()
             assertThat(finalState.voiceMessageState).isEqualTo(RECORDING_STATE)
-            voiceRecorder.assertCalls(stopped = 1, started = 1)
+            startRecordResult.assertions().isCalledOnce()
+            stopRecordResult.assertions().isCalledOnce().with(value(false))
 
             testPauseAndDestroy(finalState)
         }
@@ -659,7 +705,7 @@ class DefaultVoiceMessageComposerPresenterTest {
 
             val finalState = expectMostRecentItem()
             assertThat(finalState.voiceMessageState).isEqualTo(RECORDING_STATE)
-            voiceRecorder.assertCalls(started = 1)
+            startRecordResult.assertions().isCalledOnce()
 
             testPauseAndDestroy(finalState)
         }
@@ -696,7 +742,7 @@ class DefaultVoiceMessageComposerPresenterTest {
                 assertThat(it.voiceMessageState).isEqualTo(VoiceMessageState.Idle)
                 assertThat(it.showPermissionRationaleDialog).isTrue()
             }
-            voiceRecorder.assertCalls(started = 0)
+            startRecordResult.assertions().isNeverCalled()
 
             cancelAndIgnoreRemainingEvents()
             testPauseAndDestroy(finalState)

libraries/voicerecorder/test/build.gradle.kts

@@ -19,6 +19,5 @@ dependencies {
     implementation(projects.tests.testutils)
 
     implementation(libs.coroutines.test)
-    implementation(libs.test.truth)
     implementation(projects.libraries.core)
 }

libraries/voicerecorder/test/src/main/kotlin/io/element/android/libraries/voicerecorder/test/FakeVoiceRecorder.kt

@@ -8,13 +8,13 @@
 
 package io.element.android.libraries.voicerecorder.test
 
-import com.google.common.truth.Truth.assertThat
 import io.element.android.libraries.core.mimetype.MimeTypes
 import io.element.android.libraries.voicerecorder.api.VoiceRecorder
 import io.element.android.libraries.voicerecorder.api.VoiceRecorderState
+import io.element.android.tests.testutils.lambda.lambdaError
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
-import kotlinx.coroutines.yield
 import java.io.File
 import kotlin.time.Duration
 import kotlin.time.Duration.Companion.seconds
@@ -23,51 +23,47 @@ import kotlin.time.TestTimeSource
 class FakeVoiceRecorder(
     private val timeSource: TestTimeSource = TestTimeSource(),
     private val recordingDuration: Duration = 0.seconds,
-    private val levels: List<Float> = listOf(0.1f, 0.2f)
+    private val levels: List<Float> = listOf(0.1f, 0.2f),
+    val waveform: List<Float> = A_WAVEFORM,
+    private val startRecordResult: () -> Unit = { lambdaError() },
+    private val stopRecordResult: (Boolean) -> Unit = { lambdaError() },
+    private val deleteRecordingResult: () -> Unit = { lambdaError() },
 ) : VoiceRecorder {
     private val _state = MutableStateFlow<VoiceRecorderState>(VoiceRecorderState.Idle)
     override val state: StateFlow<VoiceRecorderState> = _state
 
-    private var curRecording: File? = null
+    private val levelInterval = if (levels.isEmpty()) Duration.ZERO else recordingDuration / levels.size
 
-    private var securityException: SecurityException? = null
+    private var currentRecording: File? = null
+    private var lastRecordingId = 0
+    private var activeRecordingId = 0
 
-    private var startedCount = 0
-    private var stoppedCount = 0
-    private var deletedCount = 0
-
-    var waveform: List<Float> = listOf(0f, 1f, 2f, 3f, 4f, 5f, 6f, 7f, 8f, 9f, 8f, 7f, 6f, 5f, 4f, 3f, 2f, 1f, 0f)
     override suspend fun startRecord() {
-        startedCount += 1
+        startRecordResult()
+        if (activeRecordingId != 0) return
+        val recordingId = ++lastRecordingId
+        activeRecordingId = recordingId
+        currentRecording = File("file.ogg")
         val startedAt = timeSource.markNow()
-        securityException?.let { throw it }
-
-        if (curRecording != null) {
-            error("Previous recording was not cleared")
-        }
-        curRecording = File("file.ogg")
-
-        timeSource += recordingDuration
         for (i in 1..levels.size) {
+            delay(levelInterval)
+            if (activeRecordingId != recordingId) return
+            timeSource += levelInterval
             _state.emit(VoiceRecorderState.Recording(startedAt.elapsedNow(), levels.take(i)))
-            yield()
         }
     }
 
-    override suspend fun stopRecord(
-        cancelled: Boolean
-    ) {
-        stoppedCount++
-
+    override suspend fun stopRecord(cancelled: Boolean) {
+        stopRecordResult(cancelled)
+        activeRecordingId = 0
         if (cancelled) {
             deleteRecording()
         }
-
         _state.emit(
-            when (curRecording) {
+            when (val file = currentRecording) {
                 null -> VoiceRecorderState.Idle
                 else -> VoiceRecorderState.Finished(
-                    file = curRecording!!,
+                    file = file,
                     mimeType = MimeTypes.Ogg,
                     duration = recordingDuration,
                     waveform = waveform,
@@ -77,25 +73,11 @@ class FakeVoiceRecorder(
     }
 
     override suspend fun deleteRecording() {
-        deletedCount++
-        curRecording = null
-
-        _state.emit(
-            VoiceRecorderState.Idle
-        )
-    }
-
-    fun assertCalls(
-        started: Int = 0,
-        stopped: Int = 0,
-        deleted: Int = 0,
-    ) {
-        assertThat(startedCount).isEqualTo(started)
-        assertThat(stoppedCount).isEqualTo(stopped)
-        assertThat(deletedCount).isEqualTo(deleted)
-    }
-
-    fun givenThrowsSecurityException(exception: SecurityException) {
-        this.securityException = exception
+        deleteRecordingResult()
+        activeRecordingId = 0
+        currentRecording = null
+        _state.emit(VoiceRecorderState.Idle)
     }
 }
+
+private val A_WAVEFORM = listOf(0f, 1f, 2f, 3f, 4f, 5f, 6f, 7f, 8f, 9f, 8f, 7f, 6f, 5f, 4f, 3f, 2f, 1f, 0f)