test(guards): re-anchor nine assertions that could not fail

[PathGao]

A survey instrumented all 82 test files (22,477 assertion invocations logged) and then applied 35 source mutations, one full suite run each. It found 33 assertion sites that cannot fail. Nine of them sit on real guards, and those nine are this PR. The rest are handled separately.

These tests are recent, and the series that is fixing them is the series that wrote them. 16 of the 18 affected test files did not exist at the last release. None of this is inherited.

The failure mode, and why #440 did not cover it

#440 fixed anchors that were missing — source.slice(source.indexOf(absent)) slices from -1, so the subject degenerated to the last character of the file and no doesNotMatch could fail against it. That is a hole in the helper, and sliceFrom / sliceBetween / offsetOf now assert their anchors exist.

This is the opposite end. Both anchors are present; they are too far apart. The slice runs past the function under test into a neighbour, and something in the neighbour satisfies the match. The helper cannot see it, because nothing is missing.

sliceBetween(viewer, 'async function handleDetach', 'async function carryActiveTabToNextWindow')

  async function handleDetach(tabId) {
      if (!(await documentSession.ensureFullContent(tabId))) {   ← subject
          ...
  async function moveTabToWindow(tabId, targetLabel, ...) {
      if (!(await documentSession.ensureFullContent(tabId))) {   ← still inside the slice

Delete the first one. assert.match(detach, /ensureFullContent/) passes on the second.

The six that reproduced

Every mutation below was applied to master before any fix, one full npm test per mutation. All six reproduced.

1. Both partial-buffer transfer guards — truncatedBufferGuard.test.ts

The file exists for one claim: a buffer holding only the first 50KB of a large file is never written back over that file. Two of its three source-level assertions did not hold it.

sliceBetween(viewer, 'canTransfer: (tabId)', 'transferPayload:') runs from MarkdownViewer.svelte:504 past canDetach at :508. Both predicates end in && !tab.isTruncated, so assert.match(canTransfer, /isTruncated/) is satisfied by the neighbour. Same shape for the detach slice above.

Both mutations are the transfer-a-truncated-buffer-then-auto-save-truncates-the-file path.

Fixed by functionSource(viewer, name) — see below — one call per predicate and per mover. The two neighbours that were doing the covering, canDetach and moveTabToWindow, were never asserted about at all; they are now, and the guard asserted on the movers is the whole four-line block rather than the bare identifier.

2. The exit review can be deleted outright — viewModeWithoutSaving.test.ts

sliceBetween(viewer, 'async function appExit()', 'async function toggleEdit') spans 969 lines. appExit is 16.

Moving the unsaved-tab confirmation verbatim into a helper 20 lines below that nobody calls, and reducing appExit to appWindow.close(), is green.

Fixed by the file's own pluck('appExit'), a comment- and string-aware brace counter that was already there for the transpiled harness. One assertion added: the answer has to be acted on, not merely asked for. Asking and then closing anyway loses the same buffer the dialog exists to protect, and /if \(\w+ !== 'discard'\) return;/ leaves the identifier unpinned because which local holds the answer is not the contract.

3. The {@html} sink list saw only bare identifiers — previewSanitize.test.ts

[...viewerSource.matchAll(/\{@html\s+([A-Za-z_$][\w$]*)\s*\}/g)]

{@html tooltip.html} — already live at MarkdownViewer.svelte:3543 — is invisible to that capture, so the list it compared was ['sanitizedHtml'] by construction. The assertion's stated purpose is that "adding a second injection point of the raw document is a failure instead of an unnoticed addition", and that is the one thing it could not do.

Fixed by capturing the whole expression and comparing against an allowlist. tooltip.html is on it with a reason rather than by accident: the footnote tooltip is a clone of a subtree of markdownBody, i.e. of the DOM the sanitized sink already injected — the same bytes read back out of the document, not a second policy. Two assertions pin that direction (found via markdownBody?.querySelector, taken from .innerHTML, never from rawContent), so a future tooltip.html fed from the raw buffer is red.

4. Closed windows are never deregistered — windowOrganization.test.ts

assert.match(runtime, /window_registry[\s\S]*?remove\(window\.label\(\)\)/);

window_registry is the struct field at window_runtime.rs:26. remove(window.label()) is the watcher cleanup at :453. Nothing required them to be the same statement, and the registry removal in the Destroyed arm at :484 is deletable without a test noticing — after which every closed window stays in list_viewer_windows as a move target that no longer exists.

Fixed by slicing the Destroyed arm and reading the removal inside it. The bare assert.match(runtime, /window_registry/) above it is dropped as subsumed.

5. The recent-files key and cap were compared with themselves — recentFilesMultiWindow.test.ts

RECENT_FILES_KEY and RECENT_FILES_LIMIT were exported from recentFiles.ts only for this test, and every assertion compared the module's value with itself.

Renaming the key is green — and a rename silently orphans every existing user's list, because the literal appears nowhere else to contradict it. Changing the cap from 9 to 3 is green, because the boundedness test seeded LIMIT + 5 and then asserted the result was LIMIT long.

Fixed by writing both values out in the test. Both are on-disk format: a localStorage key and the length of a list the user can see are things a reader should be able to look up, and things that must not change without someone deciding to. The exports go with them — this is the only source change in the PR, and it is the shape that started the survey: an export that exists to be asserted against itself is not a contract with anyone.

The boundedness claim was alive but only half-alive: the source caps the list twice, in promoteRecentFile and again in updateStoredRecentFiles, and an open goes through both — so removing either alone was green and only removing both was red. Tightened: one new test drives each layer on its own.

6. t() falls back to English — editorContextMenuI18n.test.ts

assert.notEqual(t(key, lang), key, `${key} resolves for ${lang}`);

t() returns the English string when a language lacks the key, so this can only fail when English lacks it — which the line above already proved. The 26-language loop restated line 65 twenty-six times.

Deleting the German menu.inlineCode entry is green. German renders the English label, which is the user-visible regression.

Fixed by reading translations[lang] directly, without the fallback. These twelve labels are deliberately held to a stricter bar than the dictionary at large — i18nCoverage.test.ts reports per-locale gaps rather than failing on them, because 19 locales are missing >100 keys and nobody should have to translate a new English string 25 times before landing it. That trade does not apply to a fixed list of a dozen context-menu entries that all 26 languages already define, where an eleventh action means adding a row to FORMATTING_ACTIONS by hand.

The three that no mutation can reach

Deleting is the right answer for two of them. The third turned out not to be in that class.

site	why no mutation applies	done
previewSanitize.test.ts:92	assert.match(POC_STYLE, /^<style>/) — POC_STYLE is a template literal declared 59 lines above, in this file. Both sides are the test.	Deleted. The payload was documentation pretending to be an assertion; it is a comment now, verbatim.
editorContextMenuI18n.test.ts:161	counts occurrences of a pattern containing uiLanguage, one line after assert.doesNotMatch(editor, /uiLanguage/). The line above must pass to reach it, and once it has the count is zero by construction.	Deleted. No state of the component makes it, and not the line above, the failure.
previewSanitize.test.ts:91	reported as the same class — MARKDOWN_SANITIZE_CONFIG.ALLOWED_URI_REGEXP against the constant it is assigned from one line earlier, measured as the same object reference.	Kept. It is reachable: rebuilding the config's regexp from the exported one's .source and .flags fails it. That is a copy of the URI pattern, which is precisely the regression this file exists for, so identity is the right comparison and the import stays. The identical assertion is duplicated at exportSanitize.test.ts:68.

functionSource

Sites 1 and 2 are the same defect: an anchor pair that names the neighbour rather than the subject. sourceTree.ts gained AST helpers in #440 for exactly this reason, so the fix is a third one rather than another hand-rolled anchor pair:

export function functionSource(text: string, name: string): string {
	const found = functionScopes(text).filter((scope) => scope.name === name);
	assert.equal(found.length, 1, `expected exactly one function named ${JSON.stringify(name)}, found ${found.length}`);
	return text.slice(found[0].start, found[0].end);
}

It reads a function bound to name in any of the spellings src/ uses — function f() {}, const f = () => {}, { f: () => {} }, class methods — and assert.equal on the count rather than "the first one" means a rename or a second definition fails here, loudly, instead of quietly changing what the caller is asserting about. Pinned in sourceTree.test.ts against string literals, so refactoring the app cannot make that test fail and changing the helper is the only thing that can.

Mutation table

Green measured against master before the fix; red measured after, on the rebased branch. Each mutation applied to src/ or src-tauri/, the suite run, the source restored.

#	mutation	before	after	failure message
1a	canTransfer: drop && !tab.isTruncated	GREEN 584	RED	canTransfer must refuse a tab whose buffer is still the preview slice
1b	handleDetach: delete the four-line ensureFullContent guard	GREEN 584	RED	handleDetach must complete the buffer before the payload is built, and stop when it cannot
1c	canDetach: drop && !tab.isTruncated (new coverage)	GREEN 584	RED	canDetach must refuse a tab whose buffer is still the preview slice
1d	moveTabToWindow: delete the four-line guard (new coverage)	GREEN 584	RED	moveTabToWindow must complete the buffer before the payload is built, and stop when it cannot
2a	appExit: exit review moved verbatim into an uncalled helper	GREEN 584	RED	The input did not match /tabManager\.tabs\.some\(\(t\) => t\.isDirty …
2b	appExit: ask, then close regardless of the answer (new coverage)	GREEN 584	RED	an answer other than 'discard' stops the exit
3a	add {@html unsafe.rawHtml} beside the sanitized sink	GREEN 584	RED	unexpected {@html} sink — what the preview injects is the shared sanitizer output
3b	footnote tooltip HTML taken from the raw buffer (new coverage)	GREEN 584	RED	and taken from the DOM the shared sanitizer already filtered
4	window_runtime.rs: drop the registry removal from the Destroyed arm	GREEN 584	RED	a destroyed window must be deregistered, or it is still offered as a move target
5a	RECENT_FILES_KEY: 'recent-files' → 'markpad-recent'	GREEN 584	RED (6 tests)	Expected values to be strictly deep-equal
5b	RECENT_FILES_LIMIT: 9 → 3	GREEN 584	RED (2 tests)	promoteRecentFile caps what it returns
5c	drop the cap in promoteRecentFile only	GREEN 584	RED	promoteRecentFile caps what it returns
5d	drop the cap in updateStoredRecentFiles only	GREEN 584	RED	and the stored list is capped even when the mutation is not
5e	drop both caps (already caught before this PR)	RED	RED	the list stays bounded no matter how the entries arrived
6	i18n.ts: delete the German menu.inlineCode entry	GREEN 584	RED	menu.inlineCode is translated for de; it currently falls back to the English label
A1	previewSanitize.test.ts:92	—	—	no mutation applies; the subject is a literal in the test file. Deleted.
A2	sanitize.ts: config rebuilds the URI regexp from .source + .flags	GREEN 584	RED	Values have same structure but are not reference-equal — the site was reachable, so it is kept
A3	editorContextMenuI18n.test.ts:161	—	—	no mutation applies; strictly subsumed by :160. Deleted.

One correction to the survey's own report: the mutation for site 2 as literally worded — delete the review block from appExit and close immediately — is red, caught by windowStateRestore.test.ts:107, which slices appExit with the tight '\n\t}' end anchor. The dead-assertion claim holds for the harder variant in the table, where the block stays inside the 969-line span.

What did not change

No src/ or src-tauri/ behaviour. The only source edit is export const → const on the two recent-files constants (case 5), which is the one exception the audit allows and only because the assertion that needed them is gone.

No existing assertion was weakened. Two were dropped as strictly subsumed by a stronger one alongside them (windowOrganization's bare /window_registry/, editorContextMenuI18n's uiLanguage count) and two were dropped as unfalsifiable. Everything else is the same claim on a tighter subject, plus six guards that had never been asserted about at all.

Verification

npm test 594/594, npm run check 0 errors 0 warnings, npm run build, cargo test 150/150 — all measured after rebasing onto 5be7c18 (#448), not before.

🤖 Generated with Claude Code