Drive RepositoriesFeature delays with an injected clock - #727
Merged
Conversation
The GitHub-integration recovery loop, the delayed PR refresh, and the toast auto-dismiss slept on a real `ContinuousClock()`, so a TestStore could not control them. Under parallel CI the 15s recovery interval could elapse inside a test's window and emit an extra `refreshGithubIntegrationAvailability` -> `githubIntegrationAvailabilityUpdated` pair, failing `worktreeInfoEventRepositoryPullRequestRefreshQueuesWhileAvailabilityUnknown` with "Must handle 2 received actions before sending an action". It passed locally and in isolation, so it read as noise, but it is a real non-deterministic-clock defect. Inject `@Dependency(\.continuousClock)` and sleep on it at all three sites; the affected tests provide a `TestClock`, so the delays never fire unless advanced. Claude-Session: https://claude.ai/code/session_01AQ4X2bx8DnU74wwThhV9D8
The first pass injected the clock at all three delay sites, which forced every
test embedding RepositoriesFeature that triggers the toast auto-dismiss or the
delayed PR refresh to provide a clock — AppFeature tests hit the unimplemented
test clock in CI ("Unimplemented: ContinuousClock.now").
Only the GitHub-integration recovery loop actually flakes (it re-emits an action
on a 15s interval that a TestStore cannot outrun). Revert the toast and delayed
PR refresh to their original real clock, keeping the injected clock solely for
the recovery loop. Only the two tests that reach the recovery path need a
TestClock now; nothing else, in any test file, is affected.
Claude-Session: https://claude.ai/code/session_01AQ4X2bx8DnU74wwThhV9D8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
RepositoriesFeatureTests/worktreeInfoEventRepositoryPullRequestRefreshQueuesWhileAvailabilityUnknownfails intermittently in CI with:
It passes locally and in isolation (10/10), so it read as noise — but it is a real
non-deterministic-clock defect, not flakiness.
Root cause
The GitHub-integration recovery loop sleeps on a real
ContinuousClock()and re-emitsrefreshGithubIntegrationAvailabilityeverygithubIntegrationRecoveryInterval(15s). ATestStorecannot control a real clock, so under the full parallel CI run a test's window between starting the
recovery loop (on
githubIntegrationAvailabilityUpdated(false)) and cancelling it (onsetGithubIntegrationEnabled(false)) can exceed 15s of wall-clock — the real clock then fires anextra
refreshGithubIntegrationAvailability→githubIntegrationAvailabilityUpdatedpair right asthe test sends its next action, exactly the two unhandled actions above.
Fix
Inject
@Dependency(\.continuousClock)and sleep on it in the recovery loop. The two tests thatreach the recovery path now provide a
TestClockthat is never advanced, so the loop suspends andnever fires during the test — deterministic regardless of CI load, aligning with the repo rule
"in unit tests, never use
Task.sleep; useTestClock."The one-shot delayed PR refresh (2s) and toast auto-dismiss (3s) keep their original real clock:
they are not the flake, and injecting them would force every test embedding
RepositoriesFeaturethat triggers a toast or delayed refresh (e.g.
AppFeature*Tests) to supply a clock. Scoping thechange to the recovery loop keeps the blast radius to exactly the two recovery-path tests.
Verification
make check,make test(zero failures),make build-app.RepositoriesFeatureTests: 251 passed;the three
AppFeature*toast tests that a broad injection had broken pass unchanged. Thepreviously-flaky test is now deterministic (the recovery effect cannot fire without a clock advance).
https://claude.ai/code/session_01AQ4X2bx8DnU74wwThhV9D8