test: de-flake updater legacy-quarantine test - #332
Conversation
…uarantine side-effect) waitForGateway polls isPortOpen in a loop and the test sets the deadline/interval to 1ms, so the fixed false/false/true mock sequence could be fully consumed by the first poll — reporting gateway recovery BEFORE quarantineLegacyOpenclawState ran, so no /bin/bash call existed to assert on (~1-in-3 failure). Drive the probe off whether the quarantine bash step has actually run instead.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe gateway recovery test now waits for the legacy-state quarantine to invoke ChangesGateway recovery test
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
🦀 ClawReviewPoked my eyestalks out for this one. Quick tour: This PR de-flakes an intermittently-failing Vitest test in the updater suite. The old mock drove At a glance
Good to know
— ClawReview 🦀, scuttling off. General info only — see CodeRabbit for the detailed review. Conventions: docs. |
Problem
updater.test.ts > "quarantines known legacy gateway blockers and completes when the gateway recovers"fails intermittently (~1 in 3 runs) with:Root cause
waitForGateway()pollsisPortOpenin awhile (Date.now() < deadline)loop, and these tests setGATEWAY_HEALTH_WAIT_MS/GATEWAY_RECOVERY_WAIT_MS/GATEWAY_WAIT_INTERVAL_MSall to1. The test drove recovery with a fixed call sequence:Because the poll deadline is ~1ms, the number of
isPortOpencalls perwaitForGatewayinvocation is timing-dependent. When the first poll loop happens to iterate more than once, it consumes thetruebeforequarantineLegacyOpenclawState()runs — so the gateway "recovers" early, the/bin/bashquarantine step never executes, and there is no call for the assertion to find.Fix
Tie the probe to the actual quarantine side-effect instead of a fixed call count:
Now the gateway reports "offline" until the quarantine bash step has run, guaranteeing quarantine executes before completion regardless of poll timing.
Validation
Ran the updater suite 10× consecutively on-device (Jetson) — 26/26 passed every time (previously ~1-in-3 failed).
Summary by CodeRabbit