From ca6d90b6029f459d2049b46d15149bc2d6f2b787 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Mon, 6 Jul 2026 14:46:40 -0500 Subject: [PATCH] test(settings): cancel viewModelScope before db.close in SignaturesScreenTest to fix a Room teardown race (flaky on API-37 CI) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SignaturesScreenTest built a real SignaturesViewModel by hand but tore down with a bare `db.close()` that never cancelled viewModelScope. The ViewModel's `signatures` StateFlow is a Room InvalidationTracker Flow kept alive by stateIn(WhileSubscribed(5_000)), so the collector could stay live up to 5s after the UI detached — a re-query then landed on the just-closed in-memory DB and threw SQLITE_MISUSE ("connection is closed"). Timing-dependent, hence the intermittent API-37 CI failure in tappingRadioOnNonDefault_makesItTheDefault. Fix: hold the ViewModel in an androidx.lifecycle.ViewModelStore and, in @After, call store.clear() (→ ViewModel.onCleared() → cancels viewModelScope) BEFORE db.close(), so the collector is gone before the DB closes. Behaviour and assertions are unchanged; the fix removes the race by construction. Audited the androidTest tree for the same hazard and fixed two siblings the same way: - AccountSettingsScreenTest: had the same live-Room-Flow-vs-close race, previously worked around by never closing the in-memory DB at all; now clears the ViewModel then closes the DB. - ComposeScreenTest: ComposeViewModel's init launches a viewModelScope coroutine that reads the real accountSettings/signature Room repos; clear the store before db.close() to avoid the same in-flight-read-vs-close race. Verified locally: connectedDebugAndroidTest green for all three classes (12/12) on a cold-booted emulator, plus the JVM fast gate (assembleDebug, testDebugUnitTest, jacocoTestCoverageVerification, compileDebugAndroidTestKotlin, lintDebug, ktlintCheck, detekt). Co-Authored-By: Claude Opus 4.8 --- .../libremail/ui/compose/ComposeScreenTest.kt | 12 +++++++++ .../ui/settings/AccountSettingsScreenTest.kt | 27 +++++++++++++++---- .../ui/settings/SignaturesScreenTest.kt | 16 ++++++++++- 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt index 13e83f4..2180de9 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt @@ -15,6 +15,7 @@ import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performTextInput import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.ViewModelStore import androidx.room.Room import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry @@ -60,10 +61,20 @@ class ComposeScreenTest { private var db: AccountDatabase? = null + // Holds the real ComposeViewModel built by hand in setContent() below, so closeDb() can clear() + // it (triggering ViewModel.onCleared()) before closing the DB. + private val viewModelStore = ViewModelStore() + private fun string(resId: Int) = composeTestRule.activity.getString(resId) @After fun closeDb() { + // Clear the store (→ ViewModel.onCleared() → cancels viewModelScope) BEFORE closing the DB. + // ComposeViewModel's init block launches a viewModelScope coroutine that reads the real + // accountSettings/signature Room repositories (applySignature()); without this, that read can + // still be in flight when the DB closes, racing a SQLITE_MISUSE ("connection is closed") — + // the same class of teardown race fixed in SignaturesScreenTest/AccountSettingsScreenTest. + viewModelStore.clear() db?.close() } @@ -92,6 +103,7 @@ class ComposeScreenTest { signatureRepository = SignatureRepository(database.signatureDao()), settingsRepository = SettingsRepository(context), ) + viewModelStore.put("compose", viewModel) composeTestRule.setContent { LibreMailTheme(darkTheme = false, dynamicColor = false) { ComposeScreen(onBack = onBack, viewModel = viewModel) diff --git a/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt index 1bbd7b3..53b61c5 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt @@ -7,11 +7,13 @@ import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.ViewModelStore import androidx.room.Room import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.work.WorkManager import kotlinx.coroutines.runBlocking +import org.junit.After import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -54,13 +56,27 @@ class AccountSettingsScreenTest { private var manageSignaturesClicked = false + private lateinit var db: AccountDatabase + + // Holds the real AccountSettingsViewModel built by hand in setContent() below, so tearDown() can + // clear() it (triggering ViewModel.onCleared()) before closing the DB. + private val viewModelStore = ViewModelStore() + + @After + fun tearDown() { + // Clear the store (→ ViewModel.onCleared() → cancels viewModelScope) BEFORE closing the DB. + // The ViewModel's `settings`/`signatureCount`/`defaultSignatureName`/`account` Room + // InvalidationTracker Flows are kept alive by stateIn(WhileSubscribed(5_000)): without this, + // a collector can still be live up to 5s after the UI detaches, so a re-query lands on the + // just-closed in-memory DB and throws SQLITE_MISUSE ("connection is closed") — an intermittent + // teardown race, not a real bug. (Previously worked around by never closing the DB at all.) + viewModelStore.clear() + db.close() + } + private fun setContent(): AccountSettingsRepository { val context = ApplicationProvider.getApplicationContext() - // Intentionally not closed in an @After: the ViewModel's `settings` Room Flow (kept alive by - // stateIn/WhileSubscribed) keeps querying after the test body, so closing the in-memory DB out - // from under it races and crashes ("connection pool has been closed"). The DB is reclaimed with - // the test process. - val db = Room.inMemoryDatabaseBuilder(context, AccountDatabase::class.java).build() + db = Room.inMemoryDatabaseBuilder(context, AccountDatabase::class.java).build() val repository = AccountSettingsRepository(db.accountSettingsDao()) runBlocking { db.accountDao().upsert(account.toEntity()) // FK parent for the account_settings row @@ -74,6 +90,7 @@ class AccountSettingsScreenTest { syncScheduler = SyncScheduler(Provider { WorkManager.getInstance(context) }), settingsRepository = SettingsRepository(context), ) + viewModelStore.put("account-settings", viewModel) composeTestRule.setContent { LibreMailTheme(darkTheme = false, dynamicColor = false) { AccountSettingsScreen( diff --git a/app/src/androidTest/kotlin/org/libremail/ui/settings/SignaturesScreenTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/settings/SignaturesScreenTest.kt index 0a28ae2..a75cc8c 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/settings/SignaturesScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/settings/SignaturesScreenTest.kt @@ -14,6 +14,7 @@ import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.ViewModelStore import androidx.room.Room import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry @@ -50,6 +51,10 @@ class SignaturesScreenTest { private lateinit var db: AccountDatabase private lateinit var repository: SignatureRepository + // Holds the real SignaturesViewModel built by hand in setContent() below, so tearDown() can + // clear() it (triggering ViewModel.onCleared()) before closing the DB. + private val viewModelStore = ViewModelStore() + @Before fun setUp() { db = Room.inMemoryDatabaseBuilder(context, AccountDatabase::class.java).build() @@ -70,7 +75,15 @@ class SignaturesScreenTest { } @After - fun tearDown() = db.close() + fun tearDown() { + // Clear the store (→ ViewModel.onCleared() → cancels viewModelScope) BEFORE closing the DB. + // SignaturesViewModel.signatures is a Room InvalidationTracker Flow kept alive by + // stateIn(WhileSubscribed(5_000)): without this, the collector can still be live up to 5s + // after the UI detaches, so a re-query lands on the just-closed in-memory DB and throws + // SQLITE_MISUSE ("connection is closed") — an intermittent teardown race, not a real bug. + viewModelStore.clear() + db.close() + } private fun string(resId: Int) = composeTestRule.activity.getString(resId) @@ -81,6 +94,7 @@ class SignaturesScreenTest { SavedStateHandle(mapOf(Routes.SIGNATURES_ARG_ACCOUNT to accountId)), repository, ) + viewModelStore.put("signatures", viewModel) composeTestRule.setContent { LibreMailTheme(darkTheme = false, dynamicColor = false) { SignaturesScreen(onBack = {}, onEdit = {}, onAdd = {}, viewModel = viewModel) -- 2.47.3