Skip to content

test(session): run the load-failure and watcher-grace paths instead of grepping them - #565

Merged
PathGao merged 1 commit into
masterfrom
test/session-behavior
Aug 10, 2026
Merged

test(session): run the load-failure and watcher-grace paths instead of grepping them#565
PathGao merged 1 commit into
masterfrom
test/session-behavior

Conversation

@PathGao

@PathGao PathGao commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

What this is

Second slice of the source-shape → behaviour conversion (after #564). Test files only, no source changes.

  • documentLoadFailure.test.ts — rewritten to drive a failing read through a real session.
  • documentWatcherSession.test.ts — deleted; the half worth keeping moves into externalChangeReload.test.ts, which already tests this subsystem for real.

Mechanism

Both files described documentSession.svelte.ts instead of running it, and the descriptions were weaker than they looked.

Load failure. The whole assertion was:

assert.doesNotMatch(session, /tabManager\.closeTab/);
assert.match(session, /options\.onError\('Error loading file', error\);/);

The first passes for any version of the file that does not contain those characters — including one that closes the tab through closeTabs, or through the tab list, or after a rename. The second pins a string literal. Neither looks at the thing that actually matters when a read fails while another process is replacing the file: the buffer the reader could still rescue, and the recent-file entry that is their way back to it.

It now opens a tab, makes every invoke throw, calls loadMarkdown, and checks the tab is the same tab, the text is unchanged, no recent entry was deleted, and exactly one error arrived with its cause intact rather than flattened to a string.

Watcher grace. The deleted file matched three lines inside shouldReloadExternalChange. Its subject — our own writes suppress the reload — was already covered as a real behaviour test one file over (our own writes still suppress the reload, since #—). So it was duplicating a stronger test with a weaker one.

What nothing covered was the other half: that the suppression expires. That is the half with teeth. A guard that never lifts turns Live Mode off for that file for the rest of the session, and reports nothing. The new test drives it with selfWriteGraceMs: 0 — the window is already over when the event arrives, which is what a real event a second later looks like — and asserts the second consultation agrees, so the expired entry is dropped rather than re-read.

Scope

  • makeSession gains an overrides argument in both files. The other eleven session tests each build their own session the same way; a shared harness for two more call sites would be a bigger change than the one being made, so none is introduced.
  • The source-shape assertions at the bottom of externalChangeReload.test.ts stay. They cover wiring inside MarkdownViewer.svelte, which this runner cannot import, and the file already says so under its own heading.
  • No source file changes. The two edits below were made to check the tests fail, then reverted.

Tests

Both directions were run, not reasoned about:

change to documentSession.svelte.ts result
catch block closes the tab and deletes the recent entry (the pre-fix behaviour) both load-failure tests red
shouldReloadExternalChange returns a bare false (suppression never expires) expiry test red, other 10 green

Verification

npm run check   # 673 files, 0 errors
npm test        # 943 pass, 0 fail

Not verified: the tests drive documentSession with stubbed invoke, so what Rust actually returns on a vanished file is still taken on trust — as it is in the eleven existing session tests.

🤖 Generated with Claude Code

…f grepping them

Both files described `documentSession.svelte.ts` rather than exercising
it, and both described it weakly.

`documentLoadFailure` asserted `assert.doesNotMatch(session,
/tabManager\.closeTab/)`. That passes for any file not containing those
characters — including one that closes the tab by another route — and it
says nothing about the buffer, which is the thing worth keeping when a
read fails mid-replace. It now opens a tab, makes every read throw, and
checks that the same tab, the same text and the recent-file entry are all
still there, and that the error reaches the caller with its cause intact.

`documentWatcherSession` matched three lines inside
`shouldReloadExternalChange`. Its subject — self-writes suppress the
reload — was already covered for real one file over, in
`externalChangeReload`. What was NOT covered is the half that has teeth:
that the suppression expires. A suppression that never lifts turns Live
Mode off for that file for the rest of the session and says nothing. So
the file is deleted and the expiry is added next to the test it belongs
with, driven by `selfWriteGraceMs: 0` rather than by sleeping.

Both go red the right way: closing the tab on a failed read fails the
first, and `shouldReloadExternalChange` returning a bare false fails the
second. Verified in both directions.

`makeSession` grows an overrides argument in each file, which is how the
grace period and the loading-state spy get in. The other eleven session
tests each build their own session the same way; no shared harness is
introduced for two more call sites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@PathGao
PathGao merged commit 43901c2 into master Aug 10, 2026
4 checks passed
@PathGao
PathGao deleted the test/session-behavior branch August 10, 2026 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant