test: keep ViewModel coroutines inside the test that started them

[jamesarich]

Why

RadioConfigViewModelTest and ConnectionsViewModelTest were red for two days and cost several sessions a lot of misattribution. The root cause was the CMP 1.12.0-rc01 bump (#6662), reverted by #6664 — so both classes are green on main today and this PR does not fix a live failure.

It is prevention. CMP 1.12 will be re-landed (#6666 is the groundwork; the Renovate block is marked liftable), and one of these two defects is latent right now and will come straight back with it.

CMP 1.12.0-rc01 changed compose-resources' AsyncCache:

// 1.11.1 — load runs in the CALLER's context
coroutineScope { async(start = LAZY) { load() } }
// 1.12.0-rc01 — load runs on a library-owned scope == Dispatchers.Default
private val cacheScope = CoroutineScope(SupervisorJob())
cached = SharedRequest(cacheScope.async { load() }); ... request.deferred.await()

A cold getStringSuspend now completes on a real thread, which breaks two test assumptions:

1. advanceUntilIdle() cannot drain it, so anything downstream of a string lookup lands after the assertions.
2. A viewModelScope still awaiting one at test end resumes on Dispatchers.Main after Dispatchers.resetMain() → Module with the Main dispatcher is missing, reported as UncaughtExceptionsBeforeTest against whichever test starts next.

The second one is the important part: viewModelScope is not a child of runTest, so nothing cancels it when the test body returns. That was true on 1.11.1 too — the bump only made it reachable. Leaving it unfixed means the re-land reintroduces two red classes.

🧹 Changes

RadioConfigViewModelTest — track every ViewModel the class builds and cancel its scope in tearDown before resetMain(). Cancelling the awaiting continuation disposes the ResumeAwaitOnCompletion handler, so a deferred completing later on a background thread no longer touches Main. Also cancels the previously uncancelled NodeRestartTracker scope, and folds the one hand-inlined ViewModel construction into createViewModel so it cannot escape registration.

ConnectionsViewModelTest — pre-load the three firmware-notification strings before the test dispatcher is installed, so the awaits are already complete and the notification path stays inside virtual time. The warm-up is wrapped in safeCatchingAll: it runs on the desktop JVM test classpath where compose-resources can raise ExceptionInInitializerError from skiko's lazy static init, and warming is an optimisation rather than an assertion — if it cannot load, the suite must behave exactly as it did before it existed.

Test-only; no production code touched.

Testing Performed

Counted <failure> elements in the JUnit XML rather than trusting the Gradle result — this build retries twice with failOnPassedAfterRetry=false, so a green build can hide a failed first attempt.

what	CMP	result
before, RadioConfigViewModelTest	1.12.0-rc01	fails (UncaughtExceptionsBeforeTest / missing Main dispatcher)
before, ConnectionsViewModelTest	1.12.0-rc01	fails 5/5, incl. as a single method in isolation
after, RadioConfigViewModelTest	1.12.0-rc01	3/3 runs, 58 tests, 0 failures
after, ConnectionsViewModelTest	1.12.0-rc01	5/5 runs, 10 tests, 0 failures
after, both	1.11.1 (this base)	3/3 each; both modules fully green, 372 testcases, 0 failures
after review fix, ConnectionsViewModelTest	1.11.1	3/3 runs, 10 tests, 0 failures

The rc01 rows are the load-bearing evidence — on this base these tests pass without the change, so CI green here does not demonstrate the fix.

Full baseline on this base: spotlessApply spotlessCheck ✅ · detekt ✅ · assembleDebug ✅ · :core:ui + :feature:settings tests ✅.

Notes for reviewers

• ConnectionsViewModelTest was not order-dependent, despite being described that way — it failed 100% as a single method in isolation.
• A cheap flake tell that came out of this: RadioConfigViewModelTest has 58 @Test methods but reported 59 and 60 <testcase> entries while flaking. Retry attempts appear as extra entries, so a testcase count above the source count is a retry signal needing no failure text.
• NodeDetailCompassLifecycleTest is unrelated to this PR — CMP-version-independent and still flaky on main; being handled separately.

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes improve asynchronous test setup and cleanup. Connections tests preload firmware notification resources. Radio configuration tests track and cancel ViewModel and tracker coroutine scopes during teardown.

Changes

Test stability updates

Layer / File(s)	Summary
Firmware notification resource preload core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/viewmodel/ConnectionsViewModelTest.kt	The test setup preloads firmware notification strings before configuring the main dispatcher.
Radio configuration ViewModel cleanup feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModelTest.kt	Tests track created ViewModels, cancel their scopes and the shared tracker scope during teardown, and reuse the factory for the destination-node test.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: 🔵 Low · up to 83dd1

The change keeps ViewModel-related test behavior inside the originating test, but the new firmware-resource warmup could abort test setup on desktop JVM initialization failures and prevent those tests from running; merge is reasonable with explicit owner awareness or a follow-up to guard the warmup.

Possibly related PRs

• meshtastic/Meshtastic-Android#6666: Both changes preload asynchronous Compose resources in tests.

Suggested labels: bugfix

Suggested reviewers: jeremiah-k

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Sibling Call Sites And Presence Semantics	✅ Passed	The PR changes only two test files for coroutine cleanup and resource preloading; it changes no nullable field, zero-guard, or presence semantics, and no NodeItem call site.
Tests Prove The Path, Not The End State	✅ Passed	The diff adds no production code, test methods, or assertions; it only warms resources, cleans scopes, and centralizes construction. Existing notification tests assert dispatch and preference side...
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main test-only change: preventing ViewModel coroutines from outliving the tests that create them.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/viewmodel/ConnectionsViewModelTest.kt`:
- Around line 296-305: Update warmFirmwareNotificationStrings to wrap all three
getString calls in a single safeCatchingAll block, preserving
CancellationException propagation while preventing ExceptionInInitializerError
during initialization from aborting setup.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 794334be-6469-47ea-8b37-1365eee35944

📥 Commits

Reviewing files that changed from the base of the PR and between d8361cc and 83dd1bc.

📒 Files selected for processing (2)

• core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/viewmodel/ConnectionsViewModelTest.kt
• feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModelTest.kt