test: wait for the setup compile deterministically in workspace init - #552
Merged
Merged
Conversation
PR #545 made in-test file mutations wait on `lsp/check` via `awaitCheck`, but the per-suite workspace swap in `init` still used fixed `sleep`s (`processFileChange`: sleep 1s, awaitIdle, sleep 1s). `awaitIdle` (`flix.allJobsFinished`) resolves immediately whenever the queue is momentarily empty, which it is if the file-system watcher hasn't fired yet. Under CI load `init` could therefore return before the check triggered by copying the suite's files had compiled them. Suites that only query after `init` (e.g. RenameProvider, which does no in-test mutation) depend entirely on this wait, so the rename request hit a server whose index lacked the symbol and failed with "Error: No result." — the residual flakiness seen in CI. Make the setup path deterministic like the mutation path: - `settleAfterChange(baseline)` waits for `checkCount` to advance past a baseline captured before the change (proving the watcher fired and a check completed), then drains to idle repeatedly until the check count is stable across a reconcile-debounce window (the client schedules a 300ms-debounced reconciliation that can enqueue a follow-up check). - `init` baselines and settles separately for the clear and the copy, and waits for the removals to be observed before copying so VS Code reports the new files as creates rather than coalescing a delete-then-create into an unhandled change event. - The first suite (extension not yet active) has no watcher/baseline, so the copied files are picked up by the initial workspace scan on `ext.activate()`; `settleAfterChange(0)` then waits for that compile. Removes the now-unused `processFileChange`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
CI still shows intermittent VS Code extension test failures (example run):
Error: No result.means thevscode.executeDocumentRenameProviderrequest came back empty — the server's index had no symbol at that position because the workspace hadn't been compiled yet when the test ran.Root cause
#545 converted in-test file mutations to wait on
lsp/checkviaawaitCheck, but the per-suite workspace swap ininitstill used fixed sleeps (processFileChange:sleep(1000)→awaitIdle→sleep(1000)).awaitIdle(flix.allJobsFinished) resolves immediately whenever the compiler's queue is momentarily empty — which it is if the file-system watcher hasn't fired yet aftercopyDirContents. Under CI load,initcould return before the check triggered by copying the suite's files had finished compiling them.RenameProvider(suite #12) does no in-test mutation, so it never benefits fromawaitCheck— its correctness depends entirely oninitleaving the workspace compiled and idle. Hence the residual flakiness.Fix
Make the setup path deterministic, the same way the mutation path already is:
settleAfterChange(baseline)waits forcheckCountto advance past a baseline captured before the change (proving the watcher fired and a check completed), then repeatedly drains to idle until the check count is stable across a reconcile-debounce window (the client schedules a 300 ms-debounced reconciliation that can enqueue a follow-up check).initbaselines and settles separately for the clear and the copy, and waits for the removals to be observed before copying — so VS Code reports the new files as creates rather than coalescing a delete-then-create of the same path into a single (unhandled) change event.ext.activate(), andsettleAfterChange(0)waits for that compile.processFileChange.Net effect: the fixed ~4 s of per-suite sleeps are replaced by a deterministic wait anchored to observable compiler progress (typically faster, and no longer racy).
Testing
npm run buildandtsc --build ./testpass. The change is timing-only in the test harness; validated end-to-end on CI (local single runs can't reproduce load-dependent flakiness).