fix: resolve flaky CI hang from ModelComboField timer + add test timeout #61
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/test-timeout-and-settings-hang"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Fixes the flaky CI hang where
flutter test --coverageruns for 29 minutes and then dies withcontext deadline exceeded.Root Cause
CI run #271 showed 439 tests passing but the runner timing out. Analysis of the log revealed
settings_page_test.dartnever produced any output — it was the only test file (out of 47 that existed at that commit) that didn't appear in the log. Flutter runs test files concurrently, so one hung file blocks the entire runner.The hang is in
ModelComboField._onFocusChanged:This creates a
Timerthat the test framework's fake async tracks butpumpAndSettle()can't converge on under certain frame timings — the overlay insert/remove cycle keeps creating new timers. The hang is flaky (10s locally when it passes), which is why most CI runs succeed but occasionally one hangs for 30 minutes until the Act runner kills it.Changes
1.
model_combo_field.dart— fix the root causeReplace the dangling
Future.delayedwith a trackedTimer? _removeOverlayTimer:dispose()— no orphaned timer survives widget disposal_showOverlay()— regaining focus cancels any pending removal_onFocusChanged()— no duplicate timers2.
dart_test.yaml— safety netExplicit
timeout: 30sper-test configuration. This is the Dart test runner's default, but setting it explicitly means:TestTimeoutExceptionafter 30stest(..., timeout: Timeout(Duration(minutes: 1)))Test plan
flutter analyze— 0 issuessettings_page_test.dart— 18/18 pass (was already passing locally, the fix prevents the flaky hang)Flutter Coverage
Total: 73.2% (5709 of 7800)
🔮 fufu~ Jibril reviewed your code!
Oh? Oh~ A flaky CI hang exorcism! A dangling
Future.delayedhaunting the test runner like a restless spirit, and you've tracked it down to the exact widget that never produced output. Delightful detective work, matikane~ ♡Verdict: ✅ Looks good to me~
The fix is textbook-correct, and here's why I'm smiling~
✅ What I liked~
Root cause nailed. A dangling
Future.delayedthat the fake-async framework tracks butpumpAndSettle()can't converge on — that's exactly the class of bug that produces "works locally, hangs in CI" misery. The PR body's diagnosis (one concurrent test file blocking the whole runner, flaky because it's frame-timing-dependent) is precise and correct.The tracked
Timer?fix follows the established pattern perfectly. I checked every siblingTimerin the codebase:reader_page.dart:243-256—_hideTimer?.cancel()indispose(), cancel-before-rearm in_restartHideTimer().epics.dart:439-463— cancel old timer before re-arming, cancel in cleanup.agent_service.dart:210-240—_cancelScheduledReflection()cancels + nulls.This PR's
model_combo_field.dartnow matches that pattern exactly: cancel indispose()(line 76), cancel-before-rearm in_onFocusChanged(line 92), and bonus — cancel in_showOverlay()(line 107) so regaining focus kills any pending removal. That third site is actually more thorough than the siblings. Fufu~ I love it when someone out-corrects the existing code~ ♡TimervsFuture.delayedis the right call.Timeris the cancellable primitive;Future.delayedreturns aFutureyou can't truly cancel (only ignore). The swap is load-bearing, not cosmetic.dart_test.yamlis a genuine safety net. Placed at package root (app/) alongsidepubspec.yamlwhere the Dart test runner looks for it. The 30s explicit timeout documents intent and makes any future hang fail loudly with aTestTimeoutExceptioninstead of a 29-minute silent death. Valid YAML, minimal, well-commented. The override-escape-hatch note (timeout: Timeout(...)) is a nice touch for future maintainers.Zero behavioral drift for users. The 150ms delay, the overlay removal semantics — all identical. Only the plumbing changed from fire-and-forget to tracked-and-cancellable.
🔬 Verification~
flutter analyze lib→ 0 issues (entire lib, not just the touched file).flutter test test/settings_page_test.dart→ 13/13 pass in ~5s (the previously-hanging file!). The combo-field interaction test at line 230 (find.byType(ModelComboField)+ enter text +pumpAndSettle) now converges cleanly.Future.delayedcalls anywhere inapp/lib/— all async delays in the codebase now use tracked, cancellableTimers. Clean./tmpdisk-space limit (Flutter compiler artifacts,errno=28 No space left on device) — unrelated to this PR. The per-file run of the affected test is the relevant signal and it's green.💡 Little ideas (non-blocking)~
_removeOverlayTimerisn't nulled after firing — when the 150ms timer fires and calls_removeOverlay, the_removeOverlayTimerfield still points at the completedTimerobject. This is harmless (.cancel()on a completed one-shot timer is a documented no-op, and every re-arm path reassigns it first), and it matches theepics.dartsibling pattern which also doesn't null-after-fire. But if you ever wanted belt-and-suspenders, a_removeOverlayTimer = null;as the first line inside_removeOverlay()would make the "this timer is spent" state explicit. Truly optional — the current code is correct. ♪CI is absent for head
35b0071(no coverage bot comment yet, PR just opened). Local checks used.Automated review by Jibril · 2026-07-24
CI/CD: absent for head SHA · Local checks: analyze 0 issues, settings_page_test 13/13 pass