test(node): give the compass lifecycle waits a CI-sized budget

[jamesarich]

Why

NodeDetailCompassLifecycleTest is the last known-red test on main after the CMP revert (#6664). It is not order-dependent, not CMP-version-dependent, and does not reproduce locally — it passed 24/24 here. The answer was only visible in CI artifacts.

androidx.compose.ui.test.ComposeTimeoutException: Condition still not satisfied after 1000 ms
  at NodeDetailCompassLifecycleTest.kt:140     ← waitUntil { headingProvider.stops == 2 }

waitUntil defaults to 1000 ms. On CI that is a rendering-speed assertion, not a liveness bound:

where	wall time	evidence from <system-out>
local (macOS)	~2.3 s	every waitUntil 0–55 ms
CI, passing	11.9 s / 12.8 s	waitForIdle has not finished after 5 seconds / 6 seconds
CI, failing	17.1 / 18.2 / 30.7 s	timeout at 1000 ms, line 140

waitForIdle has no cap — it warns and keeps waiting. waitUntil does. So the same underlying slowness is harmless in one and fatal in the other, which is exactly the coin-flip we've been seeing. Sampled across four CI runs: 3 failed attempts out of 7.

The condition is satisfiable. Only the budget was wrong.

Correction to an earlier revision of this description: I first wrote that failing attempts take ~35–50 s, derived by dividing suite totals by attempt count. Per-attempt data refutes that. Failing attempts are 17.1 / 18.2 / 30.7 s (mean ~22 s) and passing ones 18.3 / 14.6 / 12.8 / 11.9 s (mean ~14 s) — failures skew slower but overlap: in run 31657204256 the passing attempt (18.3 s) was slower than the failing one (17.1 s). So "failures are simply the slowest runs" is not supported; what matters is whether that one wait crossed 1000 ms, not the total.

Could this be masking a real race rather than fixing a timeout?

A fair challenge, and worth stating the bound explicitly. Every sampled failure is ComposeTimeoutException at line 140 — no assertion failure has ever been observed, including the starts == 2 / stops == 2 assertions that a genuine lifecycle race would be expected to trip. But a race in which cleanup never runs would also surface as this timeout, so that alone does not separate the two.

What bounds the risk: raising a timeout cannot hide a race that never completes — the test still fails, 30 s later instead of 1 s later. It can only absorb a race that completes late, and "completes late" is what "slow" means. So the failure mode this change could conceal is the one it is meant to tolerate.

The discriminator is this PR's own CI. If the compass test still times out at 30 s, the race hypothesis is right and this change should be rejected.

🐛 Changes

Raise the four lifecycle waits to a 30 s liveness bound. A genuine regression still fails — just later. 5 s would not have been enough: 6 s waits occur on runs that pass, which is why the earlier 1 s → 5 s attempt was directionally right but numerically short.

Each wait also gets a conditionDescription, so a future timeout names which of the four conditions was not met rather than reporting only a duration — the missing detail that made the original failures expensive to read.

Test-only; no production code touched.

Testing Performed

:feature:node:spotlessCheck ✅ · detekt ✅ · 3 fresh runs of the test, 1 testcase / 0 <failure> elements each, XML timestamps confirmed fresh (Develocity retries twice with failOnPassedAfterRetry=false, so build result alone is not evidence).

Local green proves only "no regression" — it passed locally before this change too. The real evidence is the CI artifact analysis above. At a ~43% base rate, one green CI run would not demonstrate a fix either; several are needed to be meaningful.

Hypotheses tested and falsified, so nobody re-runs them

• Cold compose-resources load — cold 2.52 s vs warm 2.38 s average. No effect.
• UnconfinedTestDispatcher with no scheduler advancement — swapping in StandardTestDispatcher, which queues everything, still passed 3/3. Something does drain it.
• CMP 1.12.0-rc01 — fails on 1.11.1 and rc01 alike; unrelated to chore(deps): bump the Compose stack to CMP 1.12.0-rc01 in lockstep #6662/revert(deps): back out CMP 1.12.0-rc01 until its test regressions are fixed #6664.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Improved compass lifecycle test reliability in slower environments.
  • Added clearer timeout handling and documentation while preserving existing validation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2a3c5615-1b09-4556-a7b2-d969a6c0544c

📥 Commits

Reviewing files that changed from the base of the PR and between d8361cc and 301aa41.

📒 Files selected for processing (1)

• feature/node/src/jvmTest/kotlin/org/meshtastic/feature/node/detail/NodeDetailCompassLifecycleTest.kt

---

📝 Walkthrough

Walkthrough

The compass lifecycle test now uses labeled waits with a shared 30-second timeout. The test continues to verify compass start, stop, restart, and dismissal cleanup counts.

Changes

Compass lifecycle test

Layer / File(s)	Summary
Lifecycle wait timing feature/node/src/jvmTest/kotlin/org/meshtastic/feature/node/detail/NodeDetailCompassLifecycleTest.kt	The test adds a documented 30-second timeout constant. Compass lifecycle waits now include descriptive labels and use this timeout for lifecycle and cleanup assertions.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Mergeability Score: ⚪ Minimal · up to 301aa

This PR only adjusts test lifecycle wait budgets and timeout diagnostics; it does not change product behavior. No actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

• meshtastic/Meshtastic-Android#6620: Updates lifecycle timing in the compass test.

Suggested labels: bugfix

Suggested reviewers: simulationstation

🚥 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 diff only changes four waitUntil calls and adds a timeout constant/documentation; it introduces no nullable, presence, guard, or physical-measurement field changes.
Tests Prove The Path, Not The End State	✅ Passed	The diff only changes waitUntil budgets and descriptions; the existing test asserts RecordingHeadingProvider start/stop call counters, not stored values, collection size, or emission order.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the CI-focused increase to compass lifecycle wait timeouts.

---

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.}