test(lsp): de-flake broker cancel-refresh recovery on slow CI - #255
Merged
Conversation
`broker_cancels_partial_refresh_without_poisoning_warm_client` asserts that aborting a partial refresh does not poison the broker: a subsequent refresh must spin up a clean client. On a loaded macOS CI runner the recovery client's `python3` cold-start intermittently exceeds the 3s initialize floor, surfacing a transient "initialize timed out" — a slow start, not a poisoned broker — which failed the test on two consecutive unrelated PRs (#253, #254). Retry the recovery refresh a bounded number of times (5, 50ms apart), tolerating only the transient initialize-timeout error and still failing fast on any other error. The Ready/total_errors assertions are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
broker_cancels_partial_refresh_without_poisoning_warm_client(tests/hooks_lsp_suite/lsp_code_diagnostics_test.rs) is a macOS CI flake. It failed on two consecutive unrelated PRs (#253 perf/git-resolver, #254 refactor/dedup), both with:Root cause
The test aborts a partial refresh, then asserts the broker isn't poisoned by driving a recovery refresh that must spin up a clean client. That recovery spawns a fresh
python3fake-LSP process; on a loaded runner its cold-start intermittently exceeds the 3s initialize floor (MIN_INITIALIZE_RESPONSE_TIMEOUT). That's a slow start, not a poisoned broker — but the single-shot.expect()treats it as a hard failure. Neither PR touches LSP runtime code.Fix
Retry the recovery refresh a bounded number of times (5 attempts, 50ms apart), tolerating only the transient
initialize timed outerror and still panicking immediately on any other error. TheEngineState::Ready/total_errors == 1assertions are unchanged, so the property under test (recovery from an aborted partial refresh) is preserved. No production timeout semantics changed.Validation
Target test 15/15 green on repeat; full
hooks_lsp_suite116 passed; fmt clean.🤖 Generated with Claude Code