From 65bc4acbabf6c315160b61c8bf5f6947b7307b9c Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Mon, 20 Jun 2022 11:23:02 +0200 Subject: [PATCH] Fix flaky tests for voice recording feature (#6330) --- changelog.d/6329.misc | 1 + .../im/vector/app/core/utils/WaitUntil.kt | 69 +++++++++++++++++++ .../app/features/voice/VoiceRecorderLTests.kt | 8 +-- .../app/features/voice/VoiceRecorderQTests.kt | 51 +++++--------- .../features/voice/VoiceRecorderTestExt.kt | 36 ++++++++++ .../features/voice/AbstractVoiceRecorder.kt | 3 +- 6 files changed, 130 insertions(+), 38 deletions(-) create mode 100644 changelog.d/6329.misc create mode 100644 vector/src/androidTest/java/im/vector/app/core/utils/WaitUntil.kt create mode 100644 vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderTestExt.kt diff --git a/changelog.d/6329.misc b/changelog.d/6329.misc new file mode 100644 index 0000000000..dd87c11f6e --- /dev/null +++ b/changelog.d/6329.misc @@ -0,0 +1 @@ +Fix flaky test in voice recording feature. diff --git a/vector/src/androidTest/java/im/vector/app/core/utils/WaitUntil.kt b/vector/src/androidTest/java/im/vector/app/core/utils/WaitUntil.kt new file mode 100644 index 0000000000..16abada04c --- /dev/null +++ b/vector/src/androidTest/java/im/vector/app/core/utils/WaitUntil.kt @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.core.utils + +import kotlinx.coroutines.delay +import org.amshove.kluent.fail +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.seconds + +/** + * Tries a [condition] several times until it returns true or a [timeout] is reached waiting for some [retryDelay] time between retries. + * On timeout it fails with an [errorMessage]. + */ +suspend fun waitUntilCondition( + errorMessage: String, + timeout: Duration = 1.seconds, + retryDelay: Duration = 50.milliseconds, + condition: () -> Boolean, +) { + val start = System.currentTimeMillis() + do { + if (condition()) return + delay(retryDelay.inWholeMilliseconds) + } while (System.currentTimeMillis() - start < timeout.inWholeMilliseconds) + fail(errorMessage) +} + +/** + * Tries a [block] several times until it runs with no errors or a [timeout] is reached waiting for some [retryDelay] time between retries. + * On timeout it fails with a custom [errorMessage] or a caught [AssertionError]. + */ +suspend fun waitUntil( + errorMessage: String? = null, + timeout: Duration = 1.seconds, + retryDelay: Duration = 50.milliseconds, + block: () -> Unit, +) { + var error: AssertionError? + val start = System.currentTimeMillis() + do { + try { + block() + return + } catch (e: AssertionError) { + error = e + } + delay(retryDelay.inWholeMilliseconds) + } while (System.currentTimeMillis() - start < timeout.inWholeMilliseconds) + if (errorMessage != null) { + fail(errorMessage) + } else { + throw error!! + } +} diff --git a/vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderLTests.kt b/vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderLTests.kt index c02c2cac80..1687ee4388 100644 --- a/vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderLTests.kt +++ b/vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderLTests.kt @@ -21,6 +21,7 @@ import androidx.test.platform.app.InstrumentationRegistry import androidx.test.rule.GrantPermissionRule import io.mockk.spyk import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.runBlocking import org.amshove.kluent.shouldBeNull import org.amshove.kluent.shouldExist import org.amshove.kluent.shouldNotBeNull @@ -42,8 +43,7 @@ class VoiceRecorderLTests { getVoiceMessageFile().shouldBeNull() startRecord("some_room_id") - - getVoiceMessageFile().shouldNotBeNullAndExist() + runBlocking { waitUntilRecordingFileExists() } stopRecord() } @@ -53,6 +53,7 @@ class VoiceRecorderLTests { getVoiceMessageFile().shouldBeNull() startRecord("some_room_id") + runBlocking { waitUntilRecordingFileExists() } stopRecord() getVoiceMessageFile().shouldNotBeNullAndExist() @@ -61,8 +62,7 @@ class VoiceRecorderLTests { @Test fun cancelRecordRemovesFile() = with(recorder) { startRecord("some_room_id") - val file = recorder.getVoiceMessageFile() - file.shouldNotBeNullAndExist() + val file = runBlocking { waitUntilRecordingFileExists() } cancelRecord() diff --git a/vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderQTests.kt b/vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderQTests.kt index 446d9e5b21..b9dc3f6d41 100644 --- a/vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderQTests.kt +++ b/vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderQTests.kt @@ -23,7 +23,6 @@ import androidx.test.platform.app.InstrumentationRegistry import androidx.test.rule.GrantPermissionRule import io.mockk.spyk import io.mockk.verify -import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import org.amshove.kluent.shouldBeNull import org.amshove.kluent.shouldExist @@ -43,50 +42,36 @@ class VoiceRecorderQTests { private val recorder = spyk(VoiceRecorderQ(context)) @Test - fun startRecordCreatesOggFile() = runBlocking { - with(recorder) { - getVoiceMessageFile().shouldBeNull() + fun startRecordCreatesOggFile() = with(recorder) { + getVoiceMessageFile().shouldBeNull() - startRecord("some_room_id") - waitForRecording() + startRecord("some_room_id") + runBlocking { waitUntilRecordingFileExists() } - getVoiceMessageFile().shouldNotBeNullAndExist() - - stopRecord() - } + stopRecord() } @Test - fun stopRecordKeepsFile() = runBlocking { - with(recorder) { - getVoiceMessageFile().shouldBeNull() + fun stopRecordKeepsFile() = with(recorder) { + getVoiceMessageFile().shouldBeNull() - startRecord("some_room_id") - waitForRecording() - stopRecord() + startRecord("some_room_id") + runBlocking { waitUntilRecordingFileExists() } + stopRecord() - getVoiceMessageFile().shouldNotBeNullAndExist() - } + getVoiceMessageFile().shouldNotBeNullAndExist() } @Test - fun cancelRecordRemovesFileAfterStopping() = runBlocking { - with(recorder) { - startRecord("some_room_id") - val file = recorder.getVoiceMessageFile() - file.shouldNotBeNullAndExist() + fun cancelRecordRemovesFileAfterStopping() = with(recorder) { + startRecord("some_room_id") + val file = runBlocking { waitUntilRecordingFileExists() } + cancelRecord() - waitForRecording() - cancelRecord() - - verify { stopRecord() } - getVoiceMessageFile().shouldBeNull() - file!!.shouldNotExist() - } + verify { stopRecord() } + getVoiceMessageFile().shouldBeNull() + file!!.shouldNotExist() } - - // Give MediaRecorder some time to actually start recording - private suspend fun waitForRecording() = delay(10) } private fun File?.shouldNotBeNullAndExist() { diff --git a/vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderTestExt.kt b/vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderTestExt.kt new file mode 100644 index 0000000000..4275ae89b3 --- /dev/null +++ b/vector/src/androidTest/java/im/vector/app/features/voice/VoiceRecorderTestExt.kt @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.voice + +import im.vector.app.core.utils.waitUntil +import org.amshove.kluent.shouldExist +import org.amshove.kluent.shouldNotBeNull +import java.io.File +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.seconds + +// Give voice recorders some time to start recording and create the audio file +suspend fun VoiceRecorder.waitUntilRecordingFileExists(timeout: Duration = 1.seconds, delay: Duration = 10.milliseconds): File? { + waitUntil(timeout = timeout, retryDelay = delay) { + getVoiceMessageFile().run { + shouldNotBeNull() + shouldExist() + } + } + return getVoiceMessageFile() +} diff --git a/vector/src/main/java/im/vector/app/features/voice/AbstractVoiceRecorder.kt b/vector/src/main/java/im/vector/app/features/voice/AbstractVoiceRecorder.kt index 91eb371f42..5e27aa5bb2 100644 --- a/vector/src/main/java/im/vector/app/features/voice/AbstractVoiceRecorder.kt +++ b/vector/src/main/java/im/vector/app/features/voice/AbstractVoiceRecorder.kt @@ -19,6 +19,7 @@ package im.vector.app.features.voice import android.content.Context import android.media.MediaRecorder import android.os.Build +import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.session.content.ContentAttachmentData import org.matrix.android.sdk.api.util.md5 import java.io.File @@ -80,7 +81,7 @@ abstract class AbstractVoiceRecorder( override fun stopRecord() { // Can throw when the record is less than 1 second. mediaRecorder?.let { - it.stop() + tryOrNull { it.stop() } it.reset() it.release() }