Repository navigation
The live-edit test waits for the listener, and named waits say what they saw when they time out - #84
Merged
ww-mw merged 2 commits intoOct 19, 2026
Conversation
The text-view edit test in liveEdit waited for the shared document to go dirty, which is true as soon as the edit applies, so it no longer waited for the provider's onDidChangeTextDocument listener and its comment said it did. A provider with the listener removed passed it. The test now builds a provider over a stand-in panel on the same document, paints it once, and after the text edit waits for the repaint that listener posts. Removing the listener now fails the test as a timeout naming that repaint. The comments say what the test waits for.
Three integration waits used to print the state they were watching when they gave up: the names the index held (forgetOnDiskChange), the message types the panel had been sent (twoTabBaseline's lastRows), and how many content changes and empty dirty-state events had arrived (jsonDirtyEventRepaint). Since those loops became named waits, a timeout names only the condition. waitFor now takes an optional detail(), read once at the timeout and added to the message in parentheses; a detail that throws still leaves the timeout naming what it waited for. The three sites pass what they printed before. A new unit test drives the harness's waitFor and checks the message, and that the three sites still pass their detail.
ww-mw
deleted the
live-edit-waits-for-the-listener-and-waits-say-what-they-saw
branch
October 19, 2026 08:45
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.
Two follow-ups to #80, which turned the integration suite's fixed sleeps into named waits. Both change tests only. Nothing in
src/changes.B-38: the live-edit test waits for the change listener, not the dirty flag
Ruling (group F, the flake fixes): "convert the known members to waits on a named predicate", one that polls "the actual condition ... failing with the predicate's name on timeout". The wait has to be on the thing the next assertion needs.
What was wrong. In
test-integration/suite/liveEdit.test.ts, the test 'a text-view edit dirties the shared document (single source of truth)' waited ondoc0.isDirty. That flag is true as soon as the edit applies. So the "extension remains active" check after it no longer waited for the provider'sonDidChangeTextDocumentlistener, even though the comment above the wait said it did.Root cause. #80 named the wait after the first condition it could see (the dirty flag), not after the listener's own visible effect.
Fix. The test now builds a real
SlddTextEditorProvider(imported fromsrc) over the harness'sfakePanel, on the samedoc0. It sendsready, waits for the first paint (setRows) and clears the posts. After the text edit it waits for "the change listener to repaint the table for the text edit": asetRowsorupdateEntryRowspost. Nothing else in the test makes the view repaint, so that post shows the listener ran. The existing dirty wait and asserts come after it.view.close()runs in afinally. The comments now say what the test waits for, and thefindEntrySpanLocalcomment no longer claims the bundle is free of jsonc-parser (it now carries it through the provider import, as the other suite files that import the provider do).Tests, fail-before. The mutant is a provider whose
changeSubnever subscribes, so the listener is dropped.AssertionError: timed out after 20000ms waiting for the change listener to repaint the table for the text edit. The other 7 tests in the file pass.src, the liveEdit suite file passed 10 runs out of 10 (8 passing each). The full integration suite passed 3 runs.Existing tests changed. Only this one test, which the row names. Its
isDirtywait stood in for the listener, and the ruling removes exactly that.B-39: a named wait's timeout says what it saw
Ruling (group F, the flake fixes): waits "failing with the predicate's name on timeout". Before #80, three of these sites also printed what they had seen (two loops, and in jsonDirtyEventRepaint a fixed sleep followed by asserts that print what they saw), and #80 dropped that for the two loops. The new test's header comment and the commit body say "loops" too; wording only.
What was wrong. On main, a timeout from these three waits says only
timed out after Xms waiting for <what>:forgetOnDiskChange: it used to print the names the index held;twoTabBaselinelastRows: it used to printsaw <message types>;jsonDirtyEventRepaint: it used to print the number of content changes and empty events it saw, and the kinds posted.Root cause. The harness
waitForhad no way to carry what a wait saw, so the conversion had to drop it.Fix.
test-integration/suite/tools/hostHarness.tswaitFor(what, done, ms = 20000, detail?):detail()is read once, at the timeout, and added to the message as(<detail>).detailthrows, the message becomes(and its detail failed: <msg>), so the timeout still names what it waited for.assert.oktoif (...) assert.fail(...), so the detail is built only once, at the timeout, and not on every poll. It is still anAssertionError, and with no detail the message is unchanged.The three sites pass what they printed before:
saw [types], the old text exactly;saw N content changes, N empties, and the posts [...]. Its default 20000 is now written out, becausedetailis the fourth argument.I left the other waits #80 added in these files alone. Their old loops printed nothing extra.
Tests, fail-before. The new file is
test/hostHarnessWaitFor.test.ts(vitest). It drives the real harnesswaitForand mocks onlyvscode, whichwaitFordoes not use. It has 8 tests:Before the change, 6 of the 8 failed for the right reason: the message had no detail, and the three sites passed 3 (or 2) arguments. The 2 that passed (no detail, detail not asked for) are trivially true today. After the change all 8 pass.
I also ran an end-to-end demo in real VS Code, not committed: I made each of the three waits impossible to meet, and each timeout printed what it saw, for example
... the index to follow the file (the index held ["GainNew"]).Existing tests changed. Only the three waits the row names. Each gained a detail argument and keeps its condition, name and cap.
Left out
Nothing from either row.
Gates (at head bf0d592)
npm run verify: exit 0. vitest: 184 files, 3547 tests passed. Leak checks OK.npm run test:integration: 275 passing, 1 pending, exit 0.Review (both lenses MERGE): under the drop-listener mutant the new liveEdit test fails with its named timeout while the old one passed; with the harness hunk reverted 3 behaviour tests fail; 4 harness and 4 call-site mutants are killed.
Implements the maintainer's rulings, as audited in group H (2026-10-19).