Repository navigation
test(vscode): wait for file watchers to arm before mutating E2E fixtures - #134
Merged
Merged
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Summary
Workspace discover suite > Extension should discover workspaces and projectsis flaky on Linux in rstack-ecosystem-ci. Adding a workspace folder, or changingconfigFileGlobPattern, makes detection reinstall its file watchers. The suite renames watched configs about 150 ms later.vscode.FileSystemWatcherhas no ready signal, and on Linux the watcher can take several hundred ms to start reporting events. Renames that land before then produce no events, detection never re-runs, and the assertion times out after 20 s.The suite now checks that the watcher reports events before it changes any fixture files:
await updateWorkspaceFolders(+workspace-2) // or config.update('configFileGlobPattern', …) await waitFor(tree shows workspace-2) +await waitForWatcherArmed(<a watched config>) + loop: rewrite file with identical content + until detection's watcher event count moves (≤500 ms per try, 15 s total) await fs.rename(rstest.config.ts → foo.config.ts)DetectionServicecounts watcher events (monotonic, E2E only), exposed asRstackExtensionExports.getDetectionWatcherEventCount(), next tosetDependencyPollIntervalForTest.workspace.test.tsthat mutate files right after a watcher reinstall. No other E2E suite does this.Evidence
Diagnostic run in rstack-ecosystem-ci with watcher logging (same commit, one red and two green). The only difference is the time between the watcher reinstall and the rename:
main(7f3ccbf) in rstest-ecosystem-ci from-commit runs: 6 failed / 1 passed, each failure being[e2e-fail] Workspace discover suite Extension should discover workspaces and projects: AssertionError [ERR_ASSERTION].rstest-ecosystem-ci-selected(suiterstack-editor): 5/5 passed, 0[e2e-fail].These runs used 8174c04. The final commit only stops resetting the counter when watchers are reinstalled, and the probe already compares against its own baseline.
pnpm lintpasses;pnpm test:unit420/420; rstest E2E slice passes on macOS and in a linux/arm64 container.Merge Danger
Door: two-way
Test-only behavior, plus one counter increment in the watcher event handler.
Blast Radius: E2E
RstackExtensionExportsgains one E2E-only method. Users see no change.