diff --git a/scripts/editorPdfExport.test.ts b/scripts/editorPdfExport.test.ts index c02f5c9..ce0dac4 100644 --- a/scripts/editorPdfExport.test.ts +++ b/scripts/editorPdfExport.test.ts @@ -375,6 +375,13 @@ test('the refresh actually lands before the print', () => { test('a failed refresh does not pass off a stale export as a fresh one', () => { const body_ = sliceBetween(viewer, 'async function syncPreviewForPrint', 'async function exportAsPdf'); - assert.match(body_, /catch \(error\)/); - assert.match(body_, /addToast\(/); + // The export goes ahead on a failed refresh — printing the stale DOM beats + // not printing — so what carries the claim in this test's name is entirely + // the *severity* of what the user is told. `/catch \(error\)/` plus + // `/addToast\(/` is satisfied by a catch that reports success, which is the + // inverse of the contract: swapping the warning for + // `addToast('Export refreshed', 'success')` left both regexes matching. + const failure = sliceFrom(body_, 'catch (error)'); + const severities = [...failure.matchAll(/addToast\([^;]*?,\s*'([a-z]+)'\)/g)].map((match) => match[1]); + assert.deepEqual(severities, ['warning'], 'the failed refresh must warn, not report a fresh export'); }); diff --git a/scripts/editorToolbar.test.ts b/scripts/editorToolbar.test.ts index 293abdf..4498a32 100644 --- a/scripts/editorToolbar.test.ts +++ b/scripts/editorToolbar.test.ts @@ -5,11 +5,60 @@ import { DEFAULT_EDITOR_TOOLBAR_ORDER, getEditorToolbarAdjacentMove, getEditorToolbarReorderMove, + getEditorToolbarTools, getVisibleEditorToolbarTools, normalizeEditorToolbarHidden, normalizeEditorToolbarOrder, } from '../src/lib/utils/editorToolbar.js'; +/** + * `DEFAULT_EDITOR_TOOLBAR_ORDER` is `EDITOR_TOOLBAR_TOOLS.map((tool) => tool.id)`, + * so an expected value written in terms of it says nothing about what is in the + * catalogue: deleting the Underline tool outright left every assertion in this + * file green. The two tests below are the ones that hold the catalogue still — + * the same job `titlebarToolbar.test.ts` does with its literal id lists — and + * the derived expectations above are then free to describe the *reordering*, + * which is what they are actually about. + */ +test('the default order is the whole tool catalogue, in the order the toolbar renders it', () => { + assert.deepEqual(DEFAULT_EDITOR_TOOLBAR_ORDER, [ + 'fmt-bold', + 'fmt-italic', + 'fmt-underline', + 'fmt-inline-code', + 'fmt-code-block', + 'fmt-quote', + 'fmt-heading-1', + 'fmt-heading-2', + 'fmt-heading-3', + 'fmt-bullet-list', + 'fmt-numbered-list', + 'fmt-checklist', + 'fmt-link', + 'insert-table-simple', + ]); +}); + +test('each tool carries the label, name and shortcut the toolbar renders', () => { + const byId = new Map(getEditorToolbarTools(null).map((tool) => [tool.id, tool])); + + assert.deepEqual( + getEditorToolbarTools(null) + .filter((tool) => tool.group === 'inline') + .map((tool) => tool.id), + ['fmt-bold', 'fmt-italic', 'fmt-underline', 'fmt-inline-code'], + ); + + // The three tools whose accelerator the editor also binds; a tool that + // loses its shortcut still renders, so nothing else would notice. + assert.equal(byId.get('fmt-bold')?.shortcut?.('Ctrl'), 'Ctrl+B'); + assert.equal(byId.get('fmt-italic')?.shortcut?.('Cmd'), 'Cmd+I'); + assert.equal(byId.get('fmt-underline')?.shortcut?.('Ctrl'), 'Ctrl+U'); + assert.equal(byId.get('fmt-underline')?.label, 'U'); + assert.equal(byId.get('fmt-underline')?.name, 'Underline'); + assert.equal(byId.get('insert-table-simple')?.shortcut?.('Cmd'), 'Cmd+K T'); +}); + test('normalizeEditorToolbarOrder drops unknown ids, deduplicates, and appends new defaults', () => { assert.deepEqual( normalizeEditorToolbarOrder([ @@ -38,7 +87,10 @@ test('getVisibleEditorToolbarTools applies saved order and hidden ids', () => { assert.equal(tools[0]?.id, 'fmt-italic'); assert.equal(tools.some((tool) => tool.id === 'fmt-bold'), false); - assert.equal(tools.length, DEFAULT_EDITOR_TOOLBAR_ORDER.length - 1); + // 14 tools in the catalogue, one hidden. Counted rather than derived from + // the default order: `DEFAULT_EDITOR_TOOLBAR_ORDER.length - 1` shrinks with + // the catalogue and stays true. + assert.equal(tools.length, 13); }); test('toolbar reorder helpers resolve drag and keyboard moves', () => { diff --git a/scripts/exportRichContent.test.ts b/scripts/exportRichContent.test.ts index c258bc8..cb0d1d2 100644 --- a/scripts/exportRichContent.test.ts +++ b/scripts/exportRichContent.test.ts @@ -295,7 +295,16 @@ test('a document with no math carries no KaTeX fonts at all', async () => { // the file. They now go, rather than travelling as a lie about what the // document needs. const cssOnly = KATEX_CSS; - const document = inlineKatexFontFaces(cssOnly, new Set(), new Map()); + // Every face's bytes are in hand, so the family filter is the only thing + // left that can drop them. Handing this an empty byte map — which is what + // it used to do — deletes every face through the "bytes could not be read" + // branch instead, and deleting the family filter outright stayed green. + const bytes = new Map( + findKatexFontFaces(cssOnly).map((face) => [face.woff2Url!, 'data:font/woff2;base64,d09GMg==']), + ); + assert.equal(bytes.size, 3, 'the fixture stylesheet declares three faces, all with woff2 bytes'); + + const document = inlineKatexFontFaces(cssOnly, new Set(), bytes); assert.doesNotMatch(document, /@font-face/); assert.match(document, /\.katex \{/); }); diff --git a/scripts/exportSanitize.test.ts b/scripts/exportSanitize.test.ts index 1a00c13..65e9a2d 100644 --- a/scripts/exportSanitize.test.ts +++ b/scripts/exportSanitize.test.ts @@ -53,7 +53,10 @@ test('the proof-of-concept payload cannot reach an exported file', () => { // relative path and is meant to stay allowed. So nothing about the URI // policy stops this one — two other things do, and both are asserted here, // because on the unfixed export path neither was in the way. - assert.match(POC, /onerror=/); + // + // (`assert.match(POC, /onerror=/)` used to sit here. POC is a template + // literal three lines above, so the assertion restated the fixture back to + // itself and no change to `src/` could reach it.) assert.equal(uriIsAllowed('x'), true); // 1. The renderer output carrying the payload goes through the shared diff --git a/scripts/i18nCoverage.test.ts b/scripts/i18nCoverage.test.ts index 2d691df..8620ce8 100644 --- a/scripts/i18nCoverage.test.ts +++ b/scripts/i18nCoverage.test.ts @@ -450,5 +450,10 @@ test('per-locale completeness (reported, never enforced)', () => { } console.log(` ${'total'.padEnd(6)} ${total} untranslated key-slots (falling back to English)\n`); - assert.ok(total >= 0); + // No assertion, deliberately. `total` is a sum of array lengths, so the + // `assert.ok(total >= 0)` that used to close this block was true for every + // possible dictionary — it read as a gate while enforcing nothing. The gates + // are the two tests above: every key the source asks for exists in English, + // and no language defines a key English does not have. Completeness per + // locale stays a worklist. }); diff --git a/scripts/liveModeWatchedPath.test.ts b/scripts/liveModeWatchedPath.test.ts index edd23e4..70ed443 100644 --- a/scripts/liveModeWatchedPath.test.ts +++ b/scripts/liveModeWatchedPath.test.ts @@ -2,7 +2,7 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; -import { sliceBetween } from './sourceTree.js'; +import { callbackBodies, filesMatching, readSourceFiles, sliceBetween } from './sourceTree.js'; const runtime = readFileSync('src-tauri/src/window_runtime.rs', 'utf8'); const viewer = readFileSync('src/lib/MarkdownViewer.svelte', 'utf8'); @@ -23,7 +23,21 @@ test('Live Mode routes a watcher notification to its watched path', () => { test('Live Mode follows the active file instead of retaining a previous tab watcher', () => { assert.match(viewer, /if \(liveMode && currentFile\) \{\n\t\t\tinvoke\('watch_file', \{ path: currentFile \}\)/); - assert.doesNotMatch(readFileSync('src/lib/sessions/documentSession.svelte.ts', 'utf8'), /isLiveMode\(\)\) invoke\('watch_file'/); + + // The defect #296 fixed was `documentSession` re-issuing `watch_file` on + // every load, so the watcher followed whichever document loaded last rather + // than the active one. It used to be pinned as + // `/isLiveMode\(\)\) invoke\('watch_file'/` — the exact one-line spelling + // the defect happened to be written in, against a file where neither token + // occurs any more. Rewriting the same call as an ordinary `if (…) { … }` + // evaded it. What has to hold is who may call it: the effect above, alone. + const watchers = filesMatching(readSourceFiles('src'), /invoke\(\s*'watch_file'/); + assert.deepEqual(watchers, ['src/lib/MarkdownViewer.svelte'], 'one owner for the file watcher'); + + const watchingEffects = callbackBodies(viewer, '$effect').filter((body) => /'watch_file'/.test(body)); + assert.equal(watchingEffects.length, 1, 'and one effect inside it'); + assert.match(watchingEffects[0], /invoke\('unwatch_file'\)/, 'the same effect gives the watcher up'); + const toggleLiveMode = sliceBetween(viewer, 'function toggleLiveMode', 'async function saveImageAs'); assert.doesNotMatch(toggleLiveMode, /loadMarkdown\(/); }); diff --git a/scripts/mermaidPrintTheme.test.ts b/scripts/mermaidPrintTheme.test.ts index e2e456e..b478daa 100644 --- a/scripts/mermaidPrintTheme.test.ts +++ b/scripts/mermaidPrintTheme.test.ts @@ -3,7 +3,6 @@ import { readFileSync } from 'node:fs'; import test from 'node:test'; import { - MERMAID_PRINT_THEME, findRestorableDiagrams, readDiagramSource, rememberDiagramSource, @@ -100,18 +99,23 @@ test('exporting re-renders with the print theme and then restores the screen ren assert.equal(first.innerHTML, 'light:sequenceDiagram'); assert.equal(second.innerHTML, 'light:flowchart TD'); - assert.deepEqual(mermaid.themes, [MERMAID_PRINT_THEME]); + // Spelled out rather than compared against the constant the implementation + // reads. `[MERMAID_PRINT_THEME]` is satisfied by whatever that constant + // happens to say, so retheming the whole print path dark passes it — which + // is the one thing this file exists to prevent. The literal is the claim: + // paper gets the light theme. + assert.deepEqual(mermaid.themes, ['neutral']); restore(); assert.equal(first.innerHTML, 'dark-1'); assert.equal(second.innerHTML, 'dark-2'); - assert.deepEqual(mermaid.themes, [MERMAID_PRINT_THEME, 'dark']); + assert.deepEqual(mermaid.themes, ['neutral', 'dark']); // Restoring twice must not re-run initialize or clobber a later render. first.innerHTML = 're-rendered later'; restore(); assert.equal(first.innerHTML, 're-rendered later'); - assert.deepEqual(mermaid.themes, [MERMAID_PRINT_THEME, 'dark']); + assert.deepEqual(mermaid.themes, ['neutral', 'dark']); }); test('a diagram that fails to re-render keeps its screen rendering', async () => { @@ -142,14 +146,36 @@ test('a diagram that fails to re-render keeps its screen rendering', async () => }); test('the helper is inert when there is nothing to re-render', async () => { - const restore = await renderDiagramsForPrint({ - root: null, + // Both early returns. `typeof restore === 'function'` used to stand in for + // "inert" here, after the handle had already been called — so it could not + // be the assertion that fires, and nothing observed what the handle did. + // What has to hold is that the export's `finally` can call it without + // re-theming Mermaid or touching the DOM. + + // No renderer: the diagram under the root must be left exactly as it is. + const untouched = diagram('flowchart TD\n A --> B', 'screen'); + const withoutRenderer = await renderDiagramsForPrint({ + root: new FakeRoot([untouched]) as unknown as ParentNode, mermaid: null, sanitizeSvg: (svg) => svg, screenTheme: 'dark', }); - restore(); - assert.equal(typeof restore, 'function'); + withoutRenderer(); + assert.equal(untouched.innerHTML, 'screen'); + + // A root with no restorable diagrams: Mermaid must not be reconfigured at + // all, in either direction. An export of a document without diagrams that + // re-initializes Mermaid twice is a global side effect for nothing. + const mermaid = makeMermaid((source) => `${source}`); + const withoutDiagrams = await renderDiagramsForPrint({ + root: new FakeRoot([]) as unknown as ParentNode, + mermaid, + sanitizeSvg: (svg) => svg, + screenTheme: 'dark', + }); + assert.deepEqual(mermaid.themes, [], 'nothing to re-render must not re-theme mermaid'); + withoutDiagrams(); + assert.deepEqual(mermaid.themes, [], 'the inert restore must not re-theme mermaid either'); }); test('the PDF export wraps the print render and always restores', () => { @@ -182,9 +208,21 @@ test('the PDF export wraps the print render and always restores', () => { ); const screenTheme = viewer.match(/\bscreenTheme:\s*([^,\n]+),/); assert.ok(screenTheme, 'the print render must be told the screen theme so it can restore it'); - assert.match( - viewer, - new RegExp(`\\bmermaidTheme:\\s*${screenTheme![1].replace(/[.*+?^${}()|[\]\\]/g, '\\$&')},`), - 'the on-screen render and the print restore must resolve the theme the same way', + + // Every render, not "some render". The component hands `mermaidTheme` to the + // shared renderer twice — once for the live preview, once for the HTML + // export — and asserting that the expression appears *somewhere* in the file + // is satisfied by either one of them. Diverting only the preview left this + // green, which is precisely the divergence that makes the print restore put + // back a theme the preview is no longer using. + const renderThemes = [...viewer.matchAll(/\bmermaidTheme:\s*([^,\n]+),/g)].map((match) => match[1]); + assert.ok( + renderThemes.length >= 2, + 'the preview render and the HTML export both hand the shared renderer a theme', + ); + assert.deepEqual( + renderThemes.filter((expression) => expression !== screenTheme![1]), + [], + `every diagram render must resolve the theme the way the print restore does (${screenTheme![1]})`, ); }); diff --git a/scripts/previewAnchorRestore.test.ts b/scripts/previewAnchorRestore.test.ts index f7cab73..cbfeedf 100644 --- a/scripts/previewAnchorRestore.test.ts +++ b/scripts/previewAnchorRestore.test.ts @@ -324,7 +324,19 @@ test('the viewer restores through the measured resolver', async () => { const { readFileSync } = await import('node:fs'); const viewer = readFileSync('src/lib/MarkdownViewer.svelte', 'utf8'); - assert.match(viewer, /import \{[\s\S]*findAnchorElement[\s\S]*\} from '\.\/utils\/previewAnchor\.js'/); + // Read out of the one import statement rather than matched across the whole + // file: `import \{[\s\S]*findAnchorElement[\s\S]*\} from '…previewAnchor.js'` + // starts at the first import in the file and runs past every brace in + // between, so forking the two resolvers into another module while leaving + // the rest of the import behind satisfied it. + const anchorImport = viewer.match(/import \{([^}]*)\} from '\.\/utils\/previewAnchor\.js'/); + assert.ok(anchorImport, 'the viewer must import from previewAnchor.js'); + const imported = anchorImport[1].split(',').map((name) => name.trim()).filter(Boolean); + assert.deepEqual( + ['findAnchorElement', 'getAnchorScrollTop'].filter((name) => !imported.includes(name)), + [], + 'the restore must resolve through the measured helpers, not a fork of them', + ); assert.match( viewer, /const match = findAnchorElement\(body, tab\.anchorLine\);[\s\S]*body\.scrollTop = getAnchorScrollTop\(/, diff --git a/scripts/renderProtocol.test.ts b/scripts/renderProtocol.test.ts index d3054fd..5af79c8 100644 --- a/scripts/renderProtocol.test.ts +++ b/scripts/renderProtocol.test.ts @@ -249,11 +249,10 @@ test('display math reaches KaTeX with its underscores intact', () => { expected, `${name}: the rendered text no longer matches the math source`, ); - assert.equal( - (math[0].getAttribute('data-math-source') || '').split('_').length - 1, - expected.split('_').length - 1, - `${name}: underscores were consumed on the way to KaTeX`, - ); + // The underscore count used to be compared here as well, between + // `data-math-source` and `expected` — two strings the assertion above + // has already established are equal. It could only pass when it ran, + // and it only ran when the assertion that dominates it had passed. } }); diff --git a/scripts/scrollSync.test.ts b/scripts/scrollSync.test.ts index 3e3abb6..3e50fab 100644 --- a/scripts/scrollSync.test.ts +++ b/scripts/scrollSync.test.ts @@ -48,9 +48,11 @@ test('the front-matter boundary in one pane lands on the boundary in the other', // scrollTop/scrollMax ratio would put the editor's boundary at // (200 / 1000) * 600 = 120px in the preview — twice past the end of the // preview's own front matter, i.e. already scrolled into the body. - const naiveGlobalRatio = (EDITOR.frontMatterEnd / EDITOR.scrollMax) * PREVIEW.scrollMax; - assert.equal(naiveGlobalRatio, 120); - + // + // That 120 used to be asserted. It is arithmetic over the two fixture + // constants above and never reaches `src/`, so it belongs in this comment + // and not in an assert; the two assertions below are the claim, and they + // are on numbers the mapping returned. assert.equal(across(EDITOR, PREVIEW, 200), 60, 'editor boundary must land on the preview boundary'); assert.equal(across(PREVIEW, EDITOR, 60), 200, 'preview boundary must land on the editor boundary'); }); diff --git a/scripts/settingsPersistence.test.ts b/scripts/settingsPersistence.test.ts index 6df0894..f4af846 100644 --- a/scripts/settingsPersistence.test.ts +++ b/scripts/settingsPersistence.test.ts @@ -191,8 +191,11 @@ test('persistence installs one effect per key plus the storage listener', () => storageListeners.length = 0; registeredEffects.length = 0; - const store = new SettingsStore(); - assert.ok(store instanceof SettingsStore); + // Constructing the store is what installs the effects; the counts below are + // the assertion. `assert.ok(store instanceof SettingsStore)` stood here and + // could not fail — a constructor that throws fails the line above it, and + // one that returns fails nothing. + new SettingsStore(); const entryCount = createSettingsPersistence().length; assert.equal(registeredEffects.length, entryCount + 1); diff --git a/scripts/windowClosePerTab.test.ts b/scripts/windowClosePerTab.test.ts index 2613ac9..2f350ca 100644 --- a/scripts/windowClosePerTab.test.ts +++ b/scripts/windowClosePerTab.test.ts @@ -21,8 +21,12 @@ function closeHandler(): string { test('the aggregate unsaved-files modal is gone from the close handler', () => { const handler = closeHandler(); assert.doesNotMatch(handler, /youHaveUnsavedFiles/); - // and the old "clear all dirty flags then close" discard path with it - assert.doesNotMatch(handler, /tabManager\.tabs\.forEach\(\(t\) => \(t\.isDirty = false\)\)/); + // and the old "clear all dirty flags then close" discard path with it. + // Pinned as the assignment rather than as the `forEach` one-liner it was + // written in: the same silent discard spelled `for (const t of + // tabManager.tabs) t.isDirty = false;` passed the old regex, and the walk + // below then found nothing to review. + assert.doesNotMatch(handler, /\.isDirty\s*=\s*false/); }); test('dirty tabs are reviewed one at a time through the existing canCloseTab flow', () => { diff --git a/src/lib/utils/mermaidPrint.ts b/src/lib/utils/mermaidPrint.ts index 2f7ca8c..b1304a8 100644 --- a/src/lib/utils/mermaidPrint.ts +++ b/src/lib/utils/mermaidPrint.ts @@ -20,7 +20,7 @@ const SOURCE_ATTR = 'data-mermaid-source'; -export const MERMAID_PRINT_THEME = 'neutral'; +const MERMAID_PRINT_THEME = 'neutral'; interface MermaidRenderer { initialize(config: { startOnLoad: boolean; theme: string }): void;