From ca705a24624c03045ba7e2cf64f3832779546b15 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 21:53:34 +0200 Subject: [PATCH 01/14] test: type-check before the unit suite npm run test:unit ran Vitest only, which strips types, so a test that passes there could still fail typecheck in CI. It now runs the 1 s native typecheck first. npm run test keeps svelte-check, which already covers test files, and calls Vitest directly. --- AGENTS.md | 2 +- package.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 89f5280b..5e57f40a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,7 +40,7 @@ root symlinks for local Obsidian loading. - `npm run check:a11y`: run `lint:tsconfig`, then run `svelte-check --tsgo-experimental-api --fail-on-warnings` for source and test projects. - `npm run lint:tsconfig`: fail with TS18003 when `tsconfig.json` or `tsconfig.test.json` matches no files, using the native compiler without type checking. - `npm run test`: run Svelte checks and the application Vitest suite. -- `npm run test:unit`: run the application Vitest suite without the Svelte checks. +- `npm run test:unit`: run `typecheck`, then the application Vitest suite, without the Svelte checks. Bare `vitest` does not type-check. - `npm run test:tooling`: run the real-process Svelte checker failure tests and the build's syntax floor test, including rejection of tagged templates that disable Obsidian's eval cache. CI runs these separately on every PR. - `npm run build`: type-check and produce the production plugin bundle. - `npm run docs:build`: build the Starlight documentation and check its internal links. diff --git a/package.json b/package.json index 992d0a5e..6b545340 100644 --- a/package.json +++ b/package.json @@ -20,8 +20,8 @@ "lint": "oxlint --deny-warnings src tests/e2e tests/tooling scripts .claude/skills/verify && svelte-policy", "lint:tsconfig": "node node_modules/@typescript/native/bin/tsc --project tsconfig.json --noEmit --noCheck && node node_modules/@typescript/native/bin/tsc --project tsconfig.test.json --noEmit --noCheck", "format:check": "npm run format -- --check", - "test": "npm run check:a11y && npm run test:unit --", - "test:unit": "vitest --configLoader bundle", + "test": "npm run check:a11y && vitest --configLoader bundle", + "test:unit": "npm run typecheck && vitest --configLoader bundle", "test:e2e": "npm run build && node scripts/obsidian-e2e.mjs test", "obsidian:e2e": "node scripts/obsidian-e2e.mjs run", "provision:e2e-vault": "node scripts/obsidian-e2e.mjs provision", From 45a9bf026dbf4bfcdd9f4b44692171998d80ee96 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 22:06:57 +0200 Subject: [PATCH 02/14] test: run the e2e suite on a fresh vault every time test:e2e reused a warm instance and its vault, so fixtures, layout, and runtime errors from manual verification or an interrupted run became the suite's baseline and failed later tests. It now stops this worktree's instance, removes the vault, provisions a fresh one, and stops the instance afterwards. A fresh vault adds no measurable time. --- AGENTS.md | 27 ++++++++++----------------- CONTRIBUTING.md | 2 +- scripts/e2e-instance.mjs | 9 +++------ scripts/obsidian-e2e.mjs | 17 +++++++++-------- 4 files changed, 23 insertions(+), 32 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5e57f40a..744c67df 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -181,32 +181,28 @@ side by side. Two variables select another build: next to `PODNOTES_OBSIDIAN_APP`, so set it only when the client lives elsewhere. -Stop this worktree's instance before you switch builds. The runner refuses to -reuse an instance that a different build launched, and its error calls that a -mid-session update. Run the suite on 1.13.x with: +Run the suite on 1.13.x with: ```bash -npm run stop:e2e-obsidian PODNOTES_OBSIDIAN_APP=/opt/Obsidian/obsidian npm run test:e2e ``` -`test:e2e` stops the instance it launched, so the next command starts 1.14.4 -again. Any other command against a 1.13.x instance needs the same variable. +For other commands, stop this worktree's instance before you switch builds. +The runner refuses to reuse an instance that a different build launched, and +its error calls that a mid-session update. Every command against a 1.13.x +instance needs the same variable. ### E2E suite `npm run test:e2e` is the one command. Vitest arguments pass through (`npm run test:e2e -- -t "timestamp"`), every test is listed, and a filter that matches no test fails the run. -- On both platforms it builds, starts or reuses this worktree's isolated - instance, runs `tests/e2e` against it, and stops the instance afterwards only - if this run launched it. It never uses the shared `dev` vault. -- The suite fails before any test runs when the instance lacks these - resolver rules. Stop the instance, then run - `npm run start:e2e-obsidian`. +- On both platforms it builds, stops this worktree's isolated instance, + provisions a fresh vault, starts the instance, runs `tests/e2e` against it, + and stops it afterwards. State left by manual work or an interrupted run + never reaches the suite. It never uses the shared `dev` vault. - The suite fails when Obsidian has more than one window open, because a second - vault window keeps Obsidian from quitting. Close it, or stop the instance, - before you rerun. + vault window keeps Obsidian from quitting. - The harness snapshots the workspace, app settings in `app.json`, the core plugins with `core-plugins.json`, and PodNotes' `data.json` before each test, and restores them after it. A test can turn on a core plugin, such as the file @@ -228,9 +224,6 @@ matches no test fails the run. `tests/e2e/runtime.ts`, never with `JSON.stringify` alone. - `harness-lifecycle.test.ts` runs the suites in `tests/e2e/harness-fixtures` in their own Vitest processes and checks how each run ends. -- An interrupted run can leave test fixtures in the vault's `data.json`, the - 280 px sidebar, and the interrupted test's layout. Reset the vault with - `npm run stop:e2e-obsidian && rm -rf .obsidian-e2e-vaults`. ### Evidence (Linux) Screenshots and recordings come from the same instance. Write them under the diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8944ea32..2418bb8f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -49,7 +49,7 @@ Tests sit next to the code as `*.test.ts`. A test file that needs a DOM starts w Behavior such as playback, timestamp links, downloads, local files, and settings migrations depends on Obsidian itself. Check it in a real Obsidian before you open a pull request. -The end-to-end suite in `tests/e2e` runs against the isolated Obsidian. On Linux, `npm run test:e2e` starts it, or reuses the one already running. It stops the instance afterwards only if it started it. On macOS, start it first: +The end-to-end suite in `tests/e2e` runs against the isolated Obsidian. On Linux, `npm run test:e2e` stops it, starts it on a fresh vault, and stops it afterwards. On macOS, start it first: ``` eval "$(npm run --silent \ diff --git a/scripts/e2e-instance.mjs b/scripts/e2e-instance.mjs index 8191249b..15364fed 100644 --- a/scripts/e2e-instance.mjs +++ b/scripts/e2e-instance.mjs @@ -44,17 +44,14 @@ export function launchPodNotesInstance(target, deps = {}) { }); } -/** @param {() => void} [onLaunch] @returns {import("obsidian-e2e/runner").CliDependencies} */ -export function podNotesRunnerDependencies(onLaunch = () => {}) { +/** @returns {import("obsidian-e2e/runner").CliDependencies} */ +export function podNotesRunnerDependencies() { return { reapOrphanedInstances: async () => ({ reaped: [], scanned: 0 }), ensureObsidianInstance: (options, config, deps) => ensureObsidianInstance(options, config, { ...deps, - launchObsidianInstance: async (target, execDeps) => { - onLaunch(); - await launchPodNotesInstance(target, execDeps); - }, + launchObsidianInstance: launchPodNotesInstance, }), }; } diff --git a/scripts/obsidian-e2e.mjs b/scripts/obsidian-e2e.mjs index fb307832..cbdf6dad 100644 --- a/scripts/obsidian-e2e.mjs +++ b/scripts/obsidian-e2e.mjs @@ -1,5 +1,5 @@ -// The repo's e2e command. `test` runs tests/e2e against this worktree's isolated -// Obsidian instance, `screenshot` and `record` capture the PodNotes view, +// The repo's e2e command. `test` runs tests/e2e against a fresh instance on a +// freshly provisioned vault, `screenshot` and `record` capture the PodNotes view, // `capture` is upstream `obsidian-e2e capture` against the instance, and // provision, start, stop and run go to the obsidian-e2e instance runner. import { spawn } from "node:child_process"; @@ -67,7 +67,6 @@ async function defaultInstance() { async function runTests(args) { const instance = await defaultInstance(); const results = path.join(os.tmpdir(), `podnotes-e2e-${process.pid}.json`); - let launched = false; /** @type {import("node:child_process").ChildProcess | undefined} */ let child; /** @type {NodeJS.Signals | undefined} */ @@ -83,10 +82,12 @@ async function runTests(args) { }; for (const signal of FORWARDED_SIGNALS) process.on(signal, onSignal); try { - const deps = podNotesRunnerDependencies(() => { - launched = true; - }); - const started = await runObsidianE2ECli(["start"], deps); + // Manual work and interrupted runs leave fixtures, layout and errors in the + // vault that fail later tests. + const stopped = await runObsidianE2ECli(["stop"]); + if (stopped !== 0) return stopped; + await fsp.rm(instance.vaultPath, { recursive: true, force: true }); + const started = await runObsidianE2ECli(["start"], podNotesRunnerDependencies()); if (started !== 0) return started; if (interrupted) return 128 + os.constants.signals[interrupted]; child = spawn( @@ -130,7 +131,7 @@ async function runTests(args) { } finally { for (const signal of FORWARDED_SIGNALS) process.off(signal, onSignal); await fsp.rm(results, { force: true }); - if (launched) await runObsidianE2ECli(["stop"]); + await runObsidianE2ECli(["stop"]); } } From d9e393ade9e5aac1da97b150a54cf6b622213f4f Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 22:07:03 +0200 Subject: [PATCH 03/14] test: give each e2e test its own sandbox Tests in one file shared the file's sandbox, so two tests creating the same folder collided with "Folder already exists". The harness now creates a sandbox per test inside the file's sandbox, which teardown still removes. harness-isolation covers it with two tests that create the same folder. --- AGENTS.md | 1 + tests/e2e/harness-isolation.test.ts | 9 +++++++++ tests/e2e/harness.ts | 17 +++++++++++++---- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 744c67df..06f58ba9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -220,6 +220,7 @@ matches no test fails the run. because a quit can leave `app.json` empty and Obsidian then opens Settings in a popout. Workspace cleanup restores the original app settings in memory and the exact `app.json` bytes. +- Each test gets its own `sandbox` folder inside its file's sandbox. - Build code for Obsidian to evaluate with `jsLiteral` from `tests/e2e/runtime.ts`, never with `JSON.stringify` alone. - `harness-lifecycle.test.ts` runs the suites in `tests/e2e/harness-fixtures` diff --git a/tests/e2e/harness-isolation.test.ts b/tests/e2e/harness-isolation.test.ts index f1be8ef6..88246c3b 100644 --- a/tests/e2e/harness-isolation.test.ts +++ b/tests/e2e/harness-isolation.test.ts @@ -184,6 +184,15 @@ test("closes a leaked window and reports it during workspace rollback", async () } }); +describe("each test's sandbox", () => { + test.each(["first", "second"])("lets the %s test create the same folder", async () => { + const { obsidian, sandbox } = getContext(); + await obsidian.dev.evalJsonAsync( + `(async () => { await app.vault.createFolder(${jsLiteral(sandbox.path("Moved"))}); return true; })()`, + ); + }); +}); + const TRICKY = `say "hi" \\ to\n \u2028\u2029 end`; test("evaluates quoted values as the same values, and matches attributes holding them", async () => { diff --git a/tests/e2e/harness.ts b/tests/e2e/harness.ts index 7295c284..f24fe1cf 100644 --- a/tests/e2e/harness.ts +++ b/tests/e2e/harness.ts @@ -3,6 +3,7 @@ import { fileURLToPath } from "node:url"; import { type ObsidianClient, createObsidianClient, + createSandboxApi, resolveObsidianEnvOptions, type SandboxApi, } from "obsidian-e2e"; @@ -127,9 +128,17 @@ export function createPodNotesE2EHarness(testName: string) { async () => lock?.release(), ); }, 60_000); - const getContext = createSharedHarness(testName); - aroundEach(async (runTest) => { - const { obsidian, plugin } = getContext(); + const getFileContext = createSharedHarness(testName); + let testSandbox: SandboxApi | undefined; + const getContext = () => ({ ...getFileContext(), sandbox: testSandbox! }); + aroundEach(async (runTest, { task }) => { + const { obsidian, plugin, sandbox } = getFileContext(); + // The file's sandbox is removed at teardown, and each test's folder with it. + testSandbox = await createSandboxApi({ + obsidian, + sandboxRoot: sandbox.root, + testName: task.name, + }); await assertNoNewRuntimeErrors(obsidian, "before this test started"); await assertSingleWindow(obsidian); await assertSettingsOpenInMainWindow(obsidian); @@ -337,7 +346,7 @@ export type PlaybackState = { title: string | null; }; -// The seeded file outlives a test, so each harness sandbox writes it once. +// Each sandbox writes the seeded file once. const seededAudioPaths = new Set(); export async function seedAudio( From cdfe3a482941ec8c2b5531c2fc10ef92ca634a86 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 22:07:45 +0200 Subject: [PATCH 04/14] test: add npm run plant to prove a test catches a defect Planting a defect by hand went wrong in the same few ways: an edit that matched more than once, a plant that failed typecheck so no test ran, a filter that matched no test read as a catch, and a git checkout restore that dropped uncommitted work. The script replaces exactly one match, type-checks, passes only when the named test is in the run's FAIL summary, restores from a backup copy, and rebuilds main.js after an e2e run. --- AGENTS.md | 1 + package.json | 1 + scripts/plant.mjs | 95 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 scripts/plant.mjs diff --git a/AGENTS.md b/AGENTS.md index 06f58ba9..5e2dd134 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,6 +43,7 @@ root symlinks for local Obsidian loading. - `npm run test:unit`: run `typecheck`, then the application Vitest suite, without the Svelte checks. Bare `vitest` does not type-check. - `npm run test:tooling`: run the real-process Svelte checker failure tests and the build's syntax floor test, including rejection of tagged templates that disable Obsidian's eval cache. CI runs these separately on every PR. - `npm run build`: type-check and produce the production plugin bundle. +- `npm run plant -- [-- ]`: plant a defect and pass only when the named test fails on it. It requires exactly one match, type-checks the plant, restores the file from a backup, and rebuilds after a `test:e2e` command. The command defaults to the unit suite filtered to the test name. - `npm run docs:build`: build the Starlight documentation and check its internal links. `@typescript/native` aliases TypeScript 7. `typecheck` and `lint:tsconfig` call diff --git a/package.json b/package.json index 6b545340..7b82fa6a 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "size": "node scripts/size.mjs", "size:data": "node scripts/persistence-size.mjs", "count:writes": "node scripts/write-counts.mjs", + "plant": "node scripts/plant.mjs", "typecheck": "node node_modules/@typescript/native/bin/tsc --project tsconfig.json --noEmit && node node_modules/@typescript/native/bin/tsc --project tsconfig.test.json --noEmit", "lint": "oxlint --deny-warnings src tests/e2e tests/tooling scripts .claude/skills/verify && svelte-policy", "lint:tsconfig": "node node_modules/@typescript/native/bin/tsc --project tsconfig.json --noEmit --noCheck && node node_modules/@typescript/native/bin/tsc --project tsconfig.test.json --noEmit --noCheck", diff --git a/scripts/plant.mjs b/scripts/plant.mjs new file mode 100644 index 00000000..1cf03d0b --- /dev/null +++ b/scripts/plant.mjs @@ -0,0 +1,95 @@ +// Proves a test catches a defect: replaces the one occurrence of in +// , type-checks, runs the tests, and passes only when failed. +// The file is always restored from a backup copy, never from git. +// +// npm run plant -- [-- ] +// +// The command defaults to the unit suite filtered to . +import { spawn } from "node:child_process"; +import fsp from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const VITEST = fileURLToPath(new URL("../node_modules/.bin/vitest", import.meta.url)); + +/** @param {string} command @param {string[]} args @returns {Promise<{ code: number | null, output: string }>} */ +function run(command, args) { + return new Promise((resolve, reject) => { + const child = spawn(command, args, { stdio: ["ignore", "pipe", "pipe"] }); + let output = ""; + child.stdout.on("data", (chunk) => { + output += chunk; + process.stdout.write(chunk); + }); + child.stderr.on("data", (chunk) => { + output += chunk; + process.stderr.write(chunk); + }); + child.on("error", reject); + child.on("close", (code) => resolve({ code, output })); + }); +} + +async function main() { + const argv = process.argv.slice(2); + const split = argv.indexOf("--"); + const [file, search, replace, testName] = split === -1 ? argv : argv.slice(0, split); + if (!file || !search || replace === undefined || !testName) { + throw new Error( + "Usage: npm run plant -- [-- ]", + ); + } + const [command, ...args] = + split === -1 + ? [ + VITEST, + "run", + "--configLoader", + "bundle", + "-t", + testName.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), + ] + : argv.slice(split + 1); + + const original = await fsp.readFile(file, "utf8"); + const matches = original.split(search).length - 1; + if (matches !== 1) throw new Error(` matched ${matches} times in ${file}, not once.`); + + const backup = path.join( + await fsp.mkdtemp(path.join(os.tmpdir(), "podnotes-plant-")), + path.basename(file), + ); + await fsp.writeFile(backup, original); + process.stderr.write(`Backup of ${file}: ${backup}\n`); + try { + await fsp.writeFile( + file, + original.replace(search, () => replace), + ); + const check = file.endsWith(".svelte") ? "check:a11y" : "typecheck"; + if ((await run("npm", ["run", "--silent", check])).code !== 0) { + throw new Error("The plant does not type-check, so no test ran. Make it compile."); + } + const { code, output } = await run(command, args); + const failed = output + .split("\n") + .some((line) => /^\s*FAIL\s.* > /.test(line) && line.includes(testName)); + if (code === 0 || !failed) { + throw new Error(`Not caught: no failed test named "${testName}" in the run's summary.`); + } + process.stderr.write(`Caught: "${testName}" failed on the plant.\n`); + } finally { + await fsp.copyFile(backup, file); + process.stderr.write(`Restored ${file}.\n`); + if (command === "npm" && args.includes("test:e2e")) { + // test:e2e built root main.js from the planted source. + await run("npm", ["run", "--silent", "build"]); + } + } +} + +main().catch((error) => { + process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); + process.exitCode = 1; +}); From 51a3f1783bf1e31b810617d14f35c1ec1446965f Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 22:07:51 +0200 Subject: [PATCH 05/14] docs: add the plant and Obsidian-launch rules, trim stale AGENTS prose Keeps two rules that tooling cannot enforce: a new test must fail on planted defects, proved with npm run plant and listed in the PR body, and an Obsidian executable is never run by hand because it opens the host's real profile. Trims history from the svelte-check, run --reload, Settings popout, and workflow concurrency notes. --- AGENTS.md | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5e2dd134..e1089bb1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -53,13 +53,10 @@ compiler's API (`--tsgo-experimental-api`), but it still loads the `typescript` package, as do the TypeScript ESLint packages loaded by oxlint's Obsidian rules. Keep `typescript` on version 6. -svelte-check exits non-zero when its check throws, for example when the native -compiler cannot start, since 4.7.7 includes -[sveltejs/language-tools#3133](https://github.com/sveltejs/language-tools/pull/3133). -The native API path still passes when a tsconfig matches no files, which -`lint:tsconfig` catches. -`tests/tooling/svelteCheck.test.ts` runs the real `check:a11y` in temporary -workspaces and requires it to fail on each of these cases. +svelte-check's native API path passes when a tsconfig matches no files, which +`lint:tsconfig` catches. `tests/tooling/svelteCheck.test.ts` runs the real +`check:a11y` in temporary workspaces and requires it to fail on that case and +when the native compiler cannot start. After `npm run build`, `npm run size` enforces 330,000 raw bytes and 103,000 gzip bytes at level 9 for root `main.js` in the Test workflow's Build and sizes worker. `npm run size:data` reports a synthetic v2 stress library with 50 feeds, 2,000 history entries, and 200 saved episodes from frozen NPR metadata, including duplicated snapshots. Its 1.5 MB document goal is advisory. Pass an additive candidate JSON path to report optional-field overhead without changing existing data. In the isolated Obsidian instance, `npm run count:writes -- start`, `report`, and `stop` record writes to PodNotes' exact `data.json` path. Stop the recorder before stopping the instance, because recorder commands start a stopped instance; plugin reloads invalidate its counts. @@ -87,6 +84,13 @@ or `document` fails with that hint. Prefer unit tests for pure utility, parser, store, API, and component behavior. Use Testing Library for Svelte component behavior instead of asserting on implementation details. +A new or strengthened test counts only once it fails on a planted defect: the +revert of the fix, and each plausible wrong fix (the right check through the +wrong API, the right handler on the wrong scope). Prove each with +`npm run plant`, make the plant reproduce the old code exactly, imports and +timing included, and list the plants and the tests that caught them in the PR +body. + When a bug depends on real Obsidian runtime behavior, reproduce it in Obsidian before changing code and verify it there after the fix. Timestamp links, URI handling, playback restore, downloaded/local media, file writes, settings @@ -146,8 +150,8 @@ npm run build && npm run obsidian:e2e -- --reload eval code='app.plugins.plugins | Evidence | `npx obsidian-e2e capture launch` (a separate instance) | the capture commands below, against this instance | - `obsidian:e2e` brings the instance up when needed. A warm instance is used as - is: since obsidian-e2e 0.11 `run` does not reload the plugin, so pass - `--reload` after every rebuild or you test the old bundle. + is: `run` does not reload the plugin, so pass `--reload` after every rebuild + or you test the old bundle. - `npm run start:e2e-obsidian -- --print-env` prints only `export` lines (`OBSIDIAN_E2E_VAULT`, `_VAULT_PATH`, `_OBSIDIAN_HOME`, legacy `PODNOTES_E2E_*` aliases). On macOS, after @@ -162,8 +166,11 @@ npm run build && npm run obsidian:e2e -- --reload eval code='app.plugins.plugins ### Obsidian versions (Linux) The isolated instance runs Obsidian 1.14.4 by default. `manifest.json` `minAppVersion` is 1.13.0, so the suite must also pass on 1.13.x, which the -`.deb` installs at `/opt/Obsidian`. Install 1.14.4 once, without root, from the -official release asset, and check its SHA-256: +`.deb` installs at `/opt/Obsidian`. Never run an Obsidian executable yourself, +not even with `--version`: it opens the host's real profile. Launch it only +through the npm scripts, with `PODNOTES_OBSIDIAN_APP` below to pick a build. +Install 1.14.4 once, without root, from the official release asset, and check +its SHA-256: ```bash dir="$HOME/.local/share/obsidian-versions" @@ -217,10 +224,7 @@ matches no test fails the run. restores the original layout after its last test. - Settings must open in the main window. A test fails at once, naming the `settingsPopoutWindow` config, when Obsidian would open it in a popout. - The harness reapplies the runner's required app settings on every restart, - because a quit can leave `app.json` empty and Obsidian then opens Settings - in a popout. Workspace cleanup restores the original app settings in memory - and the exact `app.json` bytes. + The harness reapplies the runner's required app settings on every restart. - Each test gets its own `sandbox` folder inside its file's sandbox. - Build code for Obsidian to evaluate with `jsLiteral` from `tests/e2e/runtime.ts`, never with `JSON.stringify` alone. @@ -389,13 +393,10 @@ also matches the `Test` workflow by its top-level `name:`, so renaming the workflow stops release planning without failing any check. The `Test`, `Documentation`, `PR Title`, `Dependency Review`, and `Platforms` -workflows have no `concurrency` block, so GitHub does not cancel their runs. -Duplicate events on one PR head, such as a reopen or an edit right after a -push, start duplicate runs of the required checks, and every run must finish. A -cancelled run on the head turns the commit's check rollup to `FAILURE`. Do not -add a `concurrency` group to these workflows. By default a group cancels a -pending run when a newer run joins it, even with `cancel-in-progress: false`. -Runs for a superseded head also finish. They cost CI minutes but block nothing. +workflows have no `concurrency` block. Do not add one: a group cancels a +pending run when a newer run joins it, even with `cancel-in-progress: false`, +and a cancelled run on a PR head turns its check rollup to `FAILURE`. Duplicate +runs on one head, such as after a reopen, must all finish. Recovery is for the owner. Agents never dispatch release workflows. The release stage accepts only the refs below, re-derives and re-verifies the release commit From 9aa983838290648eb33c8419b3c7afe16124a077 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 22:55:01 +0200 Subject: [PATCH 06/14] test: refuse a second test:e2e run in the same worktree The fresh-vault reset stopped the instance and deleted the vault even while another run used them. test:e2e now takes a run lock first and fails at once when another run holds it. --- CONTRIBUTING.md | 6 +----- scripts/obsidian-e2e.mjs | 9 +++++++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2418bb8f..8fa669fc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -49,14 +49,10 @@ Tests sit next to the code as `*.test.ts`. A test file that needs a DOM starts w Behavior such as playback, timestamp links, downloads, local files, and settings migrations depends on Obsidian itself. Check it in a real Obsidian before you open a pull request. -The end-to-end suite in `tests/e2e` runs against the isolated Obsidian. On Linux, `npm run test:e2e` stops it, starts it on a fresh vault, and stops it afterwards. On macOS, start it first: +The end-to-end suite in `tests/e2e` runs against the isolated Obsidian on Linux and macOS. `npm run test:e2e` stops it, starts it on a fresh vault, and stops it afterwards: ``` -eval "$(npm run --silent \ - start:e2e-obsidian \ - -- --print-env)" npm run test:e2e -npm run stop:e2e-obsidian ``` The runner does not support Windows, so check your change by hand there. [AGENTS.md](AGENTS.md#isolated-worktree-instance-macos-and-linux) shows how to send commands to the isolated Obsidian and how to stop leftover instances. diff --git a/scripts/obsidian-e2e.mjs b/scripts/obsidian-e2e.mjs index cbdf6dad..cd508176 100644 --- a/scripts/obsidian-e2e.mjs +++ b/scripts/obsidian-e2e.mjs @@ -16,6 +16,7 @@ import { resolveProvisionOptions, runObsidianE2ECli, } from "obsidian-e2e/runner"; +import { acquireVaultRunLock } from "obsidian-e2e"; import { podNotesRunnerDependencies } from "./e2e-instance.mjs"; import { jsLiteral } from "./js-literal.mjs"; @@ -80,6 +81,13 @@ async function runTests(args) { if (child?.pid) process.kill(-child.pid, signal); } catch {} }; + // The reset below would destroy another run's instance and vault. + const lock = await acquireVaultRunLock({ + vaultName: instance.vaultName, + vaultPath: instance.vaultPath, + lockRoot: path.join(os.tmpdir(), "podnotes-e2e-runs"), + onBusy: "fail", + }); for (const signal of FORWARDED_SIGNALS) process.on(signal, onSignal); try { // Manual work and interrupted runs leave fixtures, layout and errors in the @@ -132,6 +140,7 @@ async function runTests(args) { for (const signal of FORWARDED_SIGNALS) process.off(signal, onSignal); await fsp.rm(results, { force: true }); await runObsidianE2ECli(["stop"]); + await lock.release(); } } From a92c5cb4e3f2871b0a2d80d4450279d0ddb15577 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 22:55:01 +0200 Subject: [PATCH 07/14] test: keep each e2e sandbox inside its test getContext().sandbox now throws outside a test instead of returning undefined or the previous test's folder. The isolation test also checks that both tests' folders share the file's sandbox as parent. --- tests/e2e/harness-isolation.test.ts | 7 +++++++ tests/e2e/harness.ts | 23 +++++++++++++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/tests/e2e/harness-isolation.test.ts b/tests/e2e/harness-isolation.test.ts index 88246c3b..dfea2ac4 100644 --- a/tests/e2e/harness-isolation.test.ts +++ b/tests/e2e/harness-isolation.test.ts @@ -185,12 +185,19 @@ test("closes a leaked window and reports it during workspace rollback", async () }); describe("each test's sandbox", () => { + const roots: string[] = []; test.each(["first", "second"])("lets the %s test create the same folder", async () => { const { obsidian, sandbox } = getContext(); + roots.push(sandbox.root); await obsidian.dev.evalJsonAsync( `(async () => { await app.vault.createFolder(${jsLiteral(sandbox.path("Moved"))}); return true; })()`, ); }); + test("sits inside the file's sandbox, which teardown removes", () => { + const [first, second] = roots.map((root) => path.posix.dirname(root)); + expect(first).toBe(second); + expect(first).toMatch(/harness-isolation/); + }); }); const TRICKY = `say "hi" \\ to\n \u2028\u2029 end`; diff --git a/tests/e2e/harness.ts b/tests/e2e/harness.ts index f24fe1cf..023dfa40 100644 --- a/tests/e2e/harness.ts +++ b/tests/e2e/harness.ts @@ -50,8 +50,9 @@ import { * Suite-scoped PodNotes E2E harness built on obsidian-e2e's shared * `createPluginHarness`: one vault lock + sandbox + reload per file, per-test * diagnostics reset and data restore, failure-artifact capture, and the dev - * vault symlink preflight. Returns the `(testName) => () => context` getter the - * test bodies already consume. + * vault symlink preflight. Each test gets its own sandbox inside the file's. + * Returns the `(testName) => () => context` getter the test bodies already + * consume. * * PodNotes flushes pending saves and detaches its player views before every * `restoreData()` via `beforeDataRestore`, preserving the original @@ -130,7 +131,17 @@ export function createPodNotesE2EHarness(testName: string) { }, 60_000); const getFileContext = createSharedHarness(testName); let testSandbox: SandboxApi | undefined; - const getContext = () => ({ ...getFileContext(), sandbox: testSandbox! }); + const getContext = () => { + const { obsidian, plugin } = getFileContext(); + return { + obsidian, + plugin, + get sandbox(): SandboxApi { + if (!testSandbox) throw new Error("The sandbox exists only inside a test."); + return testSandbox; + }, + }; + }; aroundEach(async (runTest, { task }) => { const { obsidian, plugin, sandbox } = getFileContext(); // The file's sandbox is removed at teardown, and each test's folder with it. @@ -148,7 +159,11 @@ export function createPodNotesE2EHarness(testName: string) { // The shared harness tracks writes through its client; this snapshots data.json // for tests that change settings through Obsidian instead. await plugin.data().patch(() => undefined); - await runTest(); + try { + await runTest(); + } finally { + testSandbox = undefined; + } }, async () => { if (!hasJourneySnapshot(obsidian)) return; From 333b3c5c27c91bc438f844f8fb9fc307d19de5ab Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 22:55:01 +0200 Subject: [PATCH 08/14] test: harden npm run plant against false catches and lost restores - Match the exact test name, so a sibling test's failure is no catch. - Run Vitest with NO_COLOR, so CI colors cannot hide the FAIL line. - Require the command to pass before the plant. - Restore on exit and on SIGINT, SIGTERM, and SIGHUP, from memory, keeping the file mode. - Accept several edits across files. - Fail when the post-e2e rebuild fails. tests/tooling/plant.test.ts runs the real script and catches each of these defects when planted back. --- AGENTS.md | 2 +- scripts/plant.mjs | 149 +++++++++++++++++++++----------- tests/tooling/plant.test.ts | 164 ++++++++++++++++++++++++++++++++++++ 3 files changed, 263 insertions(+), 52 deletions(-) create mode 100644 tests/tooling/plant.test.ts diff --git a/AGENTS.md b/AGENTS.md index e1089bb1..adf78f51 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,7 +43,7 @@ root symlinks for local Obsidian loading. - `npm run test:unit`: run `typecheck`, then the application Vitest suite, without the Svelte checks. Bare `vitest` does not type-check. - `npm run test:tooling`: run the real-process Svelte checker failure tests and the build's syntax floor test, including rejection of tagged templates that disable Obsidian's eval cache. CI runs these separately on every PR. - `npm run build`: type-check and produce the production plugin bundle. -- `npm run plant -- [-- ]`: plant a defect and pass only when the named test fails on it. It requires exactly one match, type-checks the plant, restores the file from a backup, and rebuilds after a `test:e2e` command. The command defaults to the unit suite filtered to the test name. +- `npm run plant -- [ ...] [-- ]`: plant a defect and pass only when the exactly named test fails on it. The command must pass first. Each search must match exactly once, the plant must type-check, the files are restored on exit or interrupt, and `main.js` is rebuilt after a `test:e2e` command. The command defaults to the unit suite filtered to the test name. - `npm run docs:build`: build the Starlight documentation and check its internal links. `@typescript/native` aliases TypeScript 7. `typecheck` and `lint:tsconfig` call diff --git a/scripts/plant.mjs b/scripts/plant.mjs index 1cf03d0b..453a9457 100644 --- a/scripts/plant.mjs +++ b/scripts/plant.mjs @@ -1,92 +1,139 @@ -// Proves a test catches a defect: replaces the one occurrence of in -// , type-checks, runs the tests, and passes only when failed. -// The file is always restored from a backup copy, never from git. +// Proves a test catches a defect. Each edit replaces the one occurrence of +// in . The command must pass first; with the edits in place it +// must type-check and fail . Files are always restored from memory. // -// npm run plant -- [-- ] +// npm run plant -- [ ...] [-- ] // // The command defaults to the unit suite filtered to . import { spawn } from "node:child_process"; -import fsp from "node:fs/promises"; +import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; -const VITEST = fileURLToPath(new URL("../node_modules/.bin/vitest", import.meta.url)); +const USAGE = + "Usage: npm run plant -- [ ...] [-- ]"; +const VITEST = fileURLToPath(new URL("../node_modules/vitest/vitest.mjs", import.meta.url)); -/** @param {string} command @param {string[]} args @returns {Promise<{ code: number | null, output: string }>} */ +/** @type {Map} */ +const originals = new Map(); +// Only a SIGKILL skips the restore, and then this copy is the way back. +const backups = fs.mkdtempSync(path.join(os.tmpdir(), "podnotes-plant-")); +/** @type {import("node:child_process").ChildProcess | undefined} */ +let child; + +function restore() { + for (const [file, original] of originals) fs.writeFileSync(file, original); + if (originals.size > 0) process.stderr.write(`Restored ${[...originals.keys()].join(", ")}.\n`); + originals.clear(); + fs.rmSync(backups, { recursive: true, force: true }); +} + +process.stdout.on("error", () => {}); +process.stderr.on("error", () => {}); +process.on("exit", restore); +for (const signal of /** @type {NodeJS.Signals[]} */ (["SIGINT", "SIGTERM", "SIGHUP"])) { + process.on(signal, () => { + child?.kill(signal); + restore(); + process.exit(128 + os.constants.signals[signal]); + }); +} + +/** + * @param {string} command @param {string[]} args + * @returns {Promise<{ code: number | null, output: string }>} + */ function run(command, args) { return new Promise((resolve, reject) => { - const child = spawn(command, args, { stdio: ["ignore", "pipe", "pipe"] }); + const spawned = spawn(command, args, { + stdio: ["ignore", "pipe", "pipe"], + env: { ...process.env, NO_COLOR: "1" }, + // npm is a .cmd shim on Windows, which only a shell can launch. + shell: process.platform === "win32" && command !== process.execPath, + }); + child = spawned; let output = ""; - child.stdout.on("data", (chunk) => { + spawned.stdout.on("data", (chunk) => { output += chunk; process.stdout.write(chunk); }); - child.stderr.on("data", (chunk) => { + spawned.stderr.on("data", (chunk) => { output += chunk; process.stderr.write(chunk); }); - child.on("error", reject); - child.on("close", (code) => resolve({ code, output })); + spawned.on("error", reject); + spawned.on("close", (code) => resolve({ code, output })); }); } +/** @param {string} output @param {string} testName */ +function failed(output, testName) { + return output + .split(/\r?\n/) + .some((line) => /^\s*FAIL\s/.test(line) && line.trimEnd().endsWith(` > ${testName}`)); +} + async function main() { const argv = process.argv.slice(2); const split = argv.indexOf("--"); - const [file, search, replace, testName] = split === -1 ? argv : argv.slice(0, split); - if (!file || !search || replace === undefined || !testName) { - throw new Error( - "Usage: npm run plant -- [-- ]", - ); - } + const [testName, ...edits] = split === -1 ? argv : argv.slice(0, split); + if (!testName || edits.length === 0 || edits.length % 3 !== 0) throw new Error(USAGE); const [command, ...args] = split === -1 ? [ + process.execPath, VITEST, "run", "--configLoader", "bundle", "-t", - testName.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), + `${testName.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}$`, ] : argv.slice(split + 1); - const original = await fsp.readFile(file, "utf8"); - const matches = original.split(search).length - 1; - if (matches !== 1) throw new Error(` matched ${matches} times in ${file}, not once.`); - - const backup = path.join( - await fsp.mkdtemp(path.join(os.tmpdir(), "podnotes-plant-")), - path.basename(file), - ); - await fsp.writeFile(backup, original); - process.stderr.write(`Backup of ${file}: ${backup}\n`); - try { - await fsp.writeFile( + /** @type {Map} */ + const planted = new Map(); + for (let index = 0; index < edits.length; index += 3) { + const [file, search, replace] = edits.slice(index, index + 3); + const source = planted.get(file) ?? fs.readFileSync(file, "utf8"); + const matches = source.split(search).length - 1; + if (matches !== 1) + throw new Error(` matched ${matches} times in ${file}, not once.`); + planted.set( file, - original.replace(search, () => replace), + source.replace(search, () => replace), + ); + } + + if ((await run(command, args)).code !== 0) { + throw new Error("The command fails without the plant, so a failure would prove nothing."); + } + process.stderr.write(`Backups: ${backups}\n`); + for (const [index, [file, source]] of [...planted].entries()) { + const original = fs.readFileSync(file, "utf8"); + fs.writeFileSync(path.join(backups, `${index}-${path.basename(file)}`), original); + originals.set(file, original); + fs.writeFileSync(file, source); + } + const check = [...planted.keys()].some((file) => file.endsWith(".svelte")) + ? "check:a11y" + : "typecheck"; + if ((await run("npm", ["run", "--silent", check])).code !== 0) { + throw new Error("The plant does not type-check, so no test ran. Make it compile."); + } + const { code, output } = await run(command, args); + restore(); + // test:e2e built root main.js from the planted source. + if (args.includes("test:e2e") && (await run("npm", ["run", "--silent", "build"])).code !== 0) { + throw new Error( + "Rebuilding main.js failed, so it still holds the plant. Run npm run build.", ); - const check = file.endsWith(".svelte") ? "check:a11y" : "typecheck"; - if ((await run("npm", ["run", "--silent", check])).code !== 0) { - throw new Error("The plant does not type-check, so no test ran. Make it compile."); - } - const { code, output } = await run(command, args); - const failed = output - .split("\n") - .some((line) => /^\s*FAIL\s.* > /.test(line) && line.includes(testName)); - if (code === 0 || !failed) { - throw new Error(`Not caught: no failed test named "${testName}" in the run's summary.`); - } - process.stderr.write(`Caught: "${testName}" failed on the plant.\n`); - } finally { - await fsp.copyFile(backup, file); - process.stderr.write(`Restored ${file}.\n`); - if (command === "npm" && args.includes("test:e2e")) { - // test:e2e built root main.js from the planted source. - await run("npm", ["run", "--silent", "build"]); - } } + if (code === 0 || !failed(output, testName)) { + throw new Error(`Not caught: "${testName}" is not a failed test in the run's summary.`); + } + process.stderr.write(`Caught: "${testName}" failed on the plant.\n`); } main().catch((error) => { diff --git a/tests/tooling/plant.test.ts b/tests/tooling/plant.test.ts new file mode 100644 index 00000000..186c62a7 --- /dev/null +++ b/tests/tooling/plant.test.ts @@ -0,0 +1,164 @@ +import { spawn, spawnSync } from "node:child_process"; +import { + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { describe, expect, it } from "vitest"; + +const root = process.cwd(); +const plant = join(root, "scripts/plant.mjs"); +const vitest = join(root, "node_modules/vitest/vitest.mjs"); +const source = 'export const extension = (url) => url.split(/[?#]/)[0].split(".").pop();\n'; +const files: Record = { + "package.json": JSON.stringify({ scripts: { typecheck: "node -e 0" } }), + "vitest.config.mjs": 'export default { test: { include: ["src/**/*.test.mjs"] } };\n', + "src/extension.mjs": source, + "src/extension.test.mjs": [ + 'import { expect, test } from "vitest";', + 'import { extension } from "./extension.mjs";', + 'test("reads the extension", () => expect(extension("a/b.mp3")).toBe("mp3"));', + 'test("reads the extension with params", () => expect(extension("b.mp3?x=1")).toBe("mp3"));', + "", + ].join("\n"), +}; +// CI and FORCE_COLOR turn Vitest's colors on, which the FAIL match must survive. +const env: NodeJS.ProcessEnv = { ...process.env, CI: "true", FORCE_COLOR: "1" }; +delete env.NO_COLOR; + +function withWorkspace(run: (workspace: string) => void | Promise) { + return async () => { + const workspace = realpathSync(mkdtempSync(join(tmpdir(), "podnotes-plant-test-"))); + try { + symlinkSync(join(root, "node_modules"), join(workspace, "node_modules"), "junction"); + for (const [name, text] of Object.entries(files)) { + mkdirSync(dirname(join(workspace, name)), { recursive: true }); + writeFileSync(join(workspace, name), text); + } + await run(workspace); + } finally { + rmSync(workspace, { recursive: true, force: true }); + } + }; +} + +function runPlant(workspace: string, args: string[]) { + const result = spawnSync(process.execPath, [plant, ...args], { + cwd: workspace, + encoding: "utf8", + env, + timeout: 60_000, + }); + expect(result.error).toBeUndefined(); + return { status: result.status, output: result.stdout + result.stderr }; +} + +const sourceOf = (workspace: string) => readFileSync(join(workspace, "src/extension.mjs"), "utf8"); + +describe("npm run plant", () => { + it( + "passes when the named test fails on the plant, and restores the file", + withWorkspace((workspace) => { + const { status, output } = runPlant(workspace, [ + "reads the extension", + "src/extension.mjs", + ".pop()", + ".shift()", + ]); + expect(status, output).toBe(0); + expect(output).toContain('Caught: "reads the extension"'); + expect(sourceOf(workspace)).toBe(source); + }), + 60_000, + ); + + it( + "fails when only a test whose name extends the named one fails", + withWorkspace((workspace) => { + const { status, output } = runPlant(workspace, [ + "reads the extension", + "src/extension.mjs", + "/[?#]/", + "/#/", + "--", + process.execPath, + vitest, + "run", + ]); + expect(status, output).toBe(1); + expect(output).toContain("Not caught"); + expect(sourceOf(workspace)).toBe(source); + }), + 60_000, + ); + + it( + "fails when the named test already fails without the plant", + withWorkspace((workspace) => { + writeFileSync(join(workspace, "src/extension.mjs"), source.replace("pop", "shift")); + const { status, output } = runPlant(workspace, [ + "reads the extension", + "src/extension.mjs", + ".shift()", + ".shift()", + ]); + expect(status, output).toBe(1); + expect(output).toContain("fails without the plant"); + }), + 60_000, + ); + + it( + "refuses a search that does not match exactly once", + withWorkspace((workspace) => { + const { status, output } = runPlant(workspace, [ + "reads the extension", + "src/extension.mjs", + "split", + "cut", + ]); + expect(status, output).toBe(1); + expect(output).toContain("matched 2 times"); + expect(sourceOf(workspace)).toBe(source); + }), + ); + + it.skipIf(process.platform === "win32")( + "restores the file when it is stopped mid-run", + withWorkspace(async (workspace) => { + // Passes at once on the baseline and hangs once the plant is in. + const hang = + 'require("fs").readFileSync("src/extension.mjs", "utf8").includes("shift") && setTimeout(() => {}, 30_000)'; + const child = spawn( + process.execPath, + [ + plant, + "reads the extension", + "src/extension.mjs", + ".pop()", + ".shift()", + "--", + process.execPath, + "-e", + hang, + ], + { cwd: workspace, env }, + ); + let output = ""; + child.stdout.on("data", (chunk) => (output += chunk)); + child.stderr.on("data", (chunk) => (output += chunk)); + const exited = new Promise((resolve) => child.on("close", resolve)); + await expect.poll(() => sourceOf(workspace), { timeout: 30_000 }).not.toBe(source); + child.kill("SIGTERM"); + expect(await exited, output).toBe(143); + expect(sourceOf(workspace)).toBe(source); + }), + 90_000, + ); +}); From d8f75c23cac1f2c95b8800324063fac8da18d35d Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 23:13:37 +0200 Subject: [PATCH 09/14] test: build inside the test:e2e run lock A second run rebuilt root main.js before it reached the lock, so the first run's later files loaded the second run's bundle. runTests now takes the lock, then builds, and a busy lock names the other run. --- package.json | 2 +- scripts/obsidian-e2e.mjs | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 7b82fa6a..295ca791 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "format:check": "npm run format -- --check", "test": "npm run check:a11y && vitest --configLoader bundle", "test:unit": "npm run typecheck && vitest --configLoader bundle", - "test:e2e": "npm run build && node scripts/obsidian-e2e.mjs test", + "test:e2e": "node scripts/obsidian-e2e.mjs test", "obsidian:e2e": "node scripts/obsidian-e2e.mjs run", "provision:e2e-vault": "node scripts/obsidian-e2e.mjs provision", "start:e2e-obsidian": "node scripts/obsidian-e2e.mjs start", diff --git a/scripts/obsidian-e2e.mjs b/scripts/obsidian-e2e.mjs index cd508176..eb9143ff 100644 --- a/scripts/obsidian-e2e.mjs +++ b/scripts/obsidian-e2e.mjs @@ -81,15 +81,21 @@ async function runTests(args) { if (child?.pid) process.kill(-child.pid, signal); } catch {} }; - // The reset below would destroy another run's instance and vault. + // The build and reset below would replace another run's bundle, instance and vault. const lock = await acquireVaultRunLock({ vaultName: instance.vaultName, vaultPath: instance.vaultPath, lockRoot: path.join(os.tmpdir(), "podnotes-e2e-runs"), onBusy: "fail", + }).catch((/** @type {Error} */ error) => { + throw new Error( + `Another npm run test:e2e is running in this worktree. Wait for it to finish. (${error.message})`, + ); }); for (const signal of FORWARDED_SIGNALS) process.on(signal, onSignal); try { + const [built] = await once(spawn("npm", ["run", "build"], { stdio: "inherit" }), "close"); + if (built !== 0) return built; // Manual work and interrupted runs leave fixtures, layout and errors in the // vault that fail later tests. const stopped = await runObsidianE2ECli(["stop"]); From 1d7628621ca61c01b0b9671cf337114719077e93 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 23:13:37 +0200 Subject: [PATCH 10/14] test: create each e2e sandbox just before its test runs A failing pre-test check left the sandbox set for later hooks. Each sandbox test now checks its own parent, so it runs alone. --- tests/e2e/harness-isolation.test.ts | 23 ++++++++++------------- tests/e2e/harness.ts | 12 ++++++------ 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/tests/e2e/harness-isolation.test.ts b/tests/e2e/harness-isolation.test.ts index dfea2ac4..1738e678 100644 --- a/tests/e2e/harness-isolation.test.ts +++ b/tests/e2e/harness-isolation.test.ts @@ -185,19 +185,16 @@ test("closes a leaked window and reports it during workspace rollback", async () }); describe("each test's sandbox", () => { - const roots: string[] = []; - test.each(["first", "second"])("lets the %s test create the same folder", async () => { - const { obsidian, sandbox } = getContext(); - roots.push(sandbox.root); - await obsidian.dev.evalJsonAsync( - `(async () => { await app.vault.createFolder(${jsLiteral(sandbox.path("Moved"))}); return true; })()`, - ); - }); - test("sits inside the file's sandbox, which teardown removes", () => { - const [first, second] = roots.map((root) => path.posix.dirname(root)); - expect(first).toBe(second); - expect(first).toMatch(/harness-isolation/); - }); + test.each(["first", "second"])( + "lets the %s test create the same folder inside the file's sandbox", + async () => { + const { obsidian, sandbox } = getContext(); + expect(path.posix.dirname(sandbox.root)).toMatch(/(^|\/)harness-isolation-[^/]+$/); + await obsidian.dev.evalJsonAsync( + `(async () => { await app.vault.createFolder(${jsLiteral(sandbox.path("Moved"))}); return true; })()`, + ); + }, + ); }); const TRICKY = `say "hi" \\ to\n \u2028\u2029 end`; diff --git a/tests/e2e/harness.ts b/tests/e2e/harness.ts index 023dfa40..5ba828c9 100644 --- a/tests/e2e/harness.ts +++ b/tests/e2e/harness.ts @@ -144,12 +144,6 @@ export function createPodNotesE2EHarness(testName: string) { }; aroundEach(async (runTest, { task }) => { const { obsidian, plugin, sandbox } = getFileContext(); - // The file's sandbox is removed at teardown, and each test's folder with it. - testSandbox = await createSandboxApi({ - obsidian, - sandboxRoot: sandbox.root, - testName: task.name, - }); await assertNoNewRuntimeErrors(obsidian, "before this test started"); await assertSingleWindow(obsidian); await assertSettingsOpenInMainWindow(obsidian); @@ -160,6 +154,12 @@ export function createPodNotesE2EHarness(testName: string) { // for tests that change settings through Obsidian instead. await plugin.data().patch(() => undefined); try { + // The file's sandbox is removed at teardown, and each test's folder with it. + testSandbox = await createSandboxApi({ + obsidian, + sandboxRoot: sandbox.root, + testName: task.name, + }); await runTest(); } finally { testSandbox = undefined; From c95e11c63d31400c5fc70be4a7f91c991d38b426 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 23:13:37 +0200 Subject: [PATCH 11/14] test: plant, then restore, then require a passing baseline - Key files by resolved path, so two spellings of one file plant and restore it once. - Restore only a file that still holds the plant, otherwise keep the backups and fail. - Run the baseline after the restore. A build in the command then replaces anything built from the plant. - Launch npm through npm_execpath, so no shell splits arguments. - Print the FAIL lines that caught the plant. --- AGENTS.md | 6 ++- scripts/plant.mjs | 103 ++++++++++++++++++++---------------- tests/tooling/plant.test.ts | 26 ++++++++- 3 files changed, 86 insertions(+), 49 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index adf78f51..7d4657c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,7 +43,7 @@ root symlinks for local Obsidian loading. - `npm run test:unit`: run `typecheck`, then the application Vitest suite, without the Svelte checks. Bare `vitest` does not type-check. - `npm run test:tooling`: run the real-process Svelte checker failure tests and the build's syntax floor test, including rejection of tagged templates that disable Obsidian's eval cache. CI runs these separately on every PR. - `npm run build`: type-check and produce the production plugin bundle. -- `npm run plant -- [ ...] [-- ]`: plant a defect and pass only when the exactly named test fails on it. The command must pass first. Each search must match exactly once, the plant must type-check, the files are restored on exit or interrupt, and `main.js` is rebuilt after a `test:e2e` command. The command defaults to the unit suite filtered to the test name. +- `npm run plant -- [ ...] [-- ]`: plant a defect and pass only when the exactly named test fails on it and the command passes again once restored. Each search must match exactly once, and the plant must type-check. The files are restored on exit or interrupt. The command defaults to the unit suite filtered to the test name. - `npm run docs:build`: build the Starlight documentation and check its internal links. `@typescript/native` aliases TypeScript 7. `typecheck` and `lint:tsconfig` call @@ -208,7 +208,9 @@ matches no test fails the run. - On both platforms it builds, stops this worktree's isolated instance, provisions a fresh vault, starts the instance, runs `tests/e2e` against it, and stops it afterwards. State left by manual work or an interrupted run - never reaches the suite. It never uses the shared `dev` vault. + never reaches the suite. It never uses the shared `dev` vault. A second run + in the same worktree fails at once instead of replacing the first's bundle + or vault. - The suite fails when Obsidian has more than one window open, because a second vault window keeps Obsidian from quitting. - The harness snapshots the workspace, app settings in `app.json`, the core diff --git a/scripts/plant.mjs b/scripts/plant.mjs index 453a9457..b11d1ca2 100644 --- a/scripts/plant.mjs +++ b/scripts/plant.mjs @@ -1,6 +1,6 @@ // Proves a test catches a defect. Each edit replaces the one occurrence of -// in . The command must pass first; with the edits in place it -// must type-check and fail . Files are always restored from memory. +// in . With the edits in place the code must type-check and the +// command must fail ; with them restored, the command must pass. // // npm run plant -- [ ...] [-- ] // @@ -14,19 +14,34 @@ import { fileURLToPath } from "node:url"; const USAGE = "Usage: npm run plant -- [ ...] [-- ]"; const VITEST = fileURLToPath(new URL("../node_modules/vitest/vitest.mjs", import.meta.url)); +// npm's own entry point, so no shell has to launch npm's Windows shim. +const NPM = process.env.npm_execpath ? [process.execPath, process.env.npm_execpath] : ["npm"]; -/** @type {Map} */ -const originals = new Map(); +/** @type {Map} */ +const files = new Map(); // Only a SIGKILL skips the restore, and then this copy is the way back. const backups = fs.mkdtempSync(path.join(os.tmpdir(), "podnotes-plant-")); /** @type {import("node:child_process").ChildProcess | undefined} */ let child; +/** Restores every planted file, unless one changed during the run. */ function restore() { - for (const [file, original] of originals) fs.writeFileSync(file, original); - if (originals.size > 0) process.stderr.write(`Restored ${[...originals.keys()].join(", ")}.\n`); - originals.clear(); + const changed = [...files.keys()].filter( + (file) => fs.readFileSync(file, "utf8") !== files.get(file)?.planted, + ); + if (changed.length > 0) { + files.clear(); + process.stderr.write( + `${changed.join(", ")} changed during the run, so nothing was restored. Originals: ${backups}\n`, + ); + process.exitCode = 1; + return false; + } + for (const [file, { original }] of files) fs.writeFileSync(file, original); + if (files.size > 0) process.stderr.write(`Restored ${[...files.keys()].join(", ")}.\n`); + files.clear(); fs.rmSync(backups, { recursive: true, force: true }); + return true; } process.stdout.on("error", () => {}); @@ -36,21 +51,22 @@ for (const signal of /** @type {NodeJS.Signals[]} */ (["SIGINT", "SIGTERM", "SIG process.on(signal, () => { child?.kill(signal); restore(); + process.stderr.write( + "Interrupted: a build in the run may hold the plant. Run npm run build.\n", + ); process.exit(128 + os.constants.signals[signal]); }); } /** - * @param {string} command @param {string[]} args + * @param {string[]} command * @returns {Promise<{ code: number | null, output: string }>} */ -function run(command, args) { +function run([command, ...args]) { return new Promise((resolve, reject) => { const spawned = spawn(command, args, { stdio: ["ignore", "pipe", "pipe"], env: { ...process.env, NO_COLOR: "1" }, - // npm is a .cmd shim on Windows, which only a shell can launch. - shell: process.platform === "win32" && command !== process.execPath, }); child = spawned; let output = ""; @@ -67,19 +83,13 @@ function run(command, args) { }); } -/** @param {string} output @param {string} testName */ -function failed(output, testName) { - return output - .split(/\r?\n/) - .some((line) => /^\s*FAIL\s/.test(line) && line.trimEnd().endsWith(` > ${testName}`)); -} - async function main() { const argv = process.argv.slice(2); const split = argv.indexOf("--"); const [testName, ...edits] = split === -1 ? argv : argv.slice(0, split); if (!testName || edits.length === 0 || edits.length % 3 !== 0) throw new Error(USAGE); - const [command, ...args] = + const [first, ...rest] = split === -1 ? [] : argv.slice(split + 1); + const command = split === -1 ? [ process.execPath, @@ -90,50 +100,53 @@ async function main() { "-t", `${testName.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}$`, ] - : argv.slice(split + 1); + : [...(first === "npm" ? NPM : [first]), ...rest]; /** @type {Map} */ - const planted = new Map(); + const originals = new Map(); + /** @type {Map} */ + const sources = new Map(); for (let index = 0; index < edits.length; index += 3) { - const [file, search, replace] = edits.slice(index, index + 3); - const source = planted.get(file) ?? fs.readFileSync(file, "utf8"); + const [name, search, replace] = edits.slice(index, index + 3); + const file = path.resolve(name); + if (!originals.has(file)) originals.set(file, fs.readFileSync(file, "utf8")); + const source = sources.get(file) ?? /** @type {string} */ (originals.get(file)); const matches = source.split(search).length - 1; if (matches !== 1) - throw new Error(` matched ${matches} times in ${file}, not once.`); - planted.set( + throw new Error(` matched ${matches} times in ${name}, not once.`); + sources.set( file, source.replace(search, () => replace), ); } - if ((await run(command, args)).code !== 0) { - throw new Error("The command fails without the plant, so a failure would prove nothing."); - } process.stderr.write(`Backups: ${backups}\n`); - for (const [index, [file, source]] of [...planted].entries()) { - const original = fs.readFileSync(file, "utf8"); - fs.writeFileSync(path.join(backups, `${index}-${path.basename(file)}`), original); - originals.set(file, original); - fs.writeFileSync(file, source); + for (const [file, planted] of sources) { + const original = /** @type {string} */ (originals.get(file)); + fs.writeFileSync(path.join(backups, `${files.size}-${path.basename(file)}`), original); + files.set(file, { original, planted }); + fs.writeFileSync(file, planted); } - const check = [...planted.keys()].some((file) => file.endsWith(".svelte")) + const check = [...sources.keys()].some((file) => file.endsWith(".svelte")) ? "check:a11y" : "typecheck"; - if ((await run("npm", ["run", "--silent", check])).code !== 0) { + if ((await run([...NPM, "run", "--silent", check])).code !== 0) { throw new Error("The plant does not type-check, so no test ran. Make it compile."); } - const { code, output } = await run(command, args); - restore(); - // test:e2e built root main.js from the planted source. - if (args.includes("test:e2e") && (await run("npm", ["run", "--silent", "build"])).code !== 0) { - throw new Error( - "Rebuilding main.js failed, so it still holds the plant. Run npm run build.", - ); - } - if (code === 0 || !failed(output, testName)) { + const { code, output } = await run(command); + if (!restore()) return; + const caught = output + .split(/\r?\n/) + .filter((line) => /^\s*FAIL\s/.test(line) && line.trimEnd().endsWith(` > ${testName}`)); + if (code === 0 || caught.length === 0) { throw new Error(`Not caught: "${testName}" is not a failed test in the run's summary.`); } - process.stderr.write(`Caught: "${testName}" failed on the plant.\n`); + // Restored, the command must pass, or the failure proves nothing. A build in it + // also replaces anything built from the plant. + if ((await run(command)).code !== 0) { + throw new Error("The command also fails without the plant, so its failure proves nothing."); + } + process.stderr.write(`Caught by:\n${caught.map((line) => line.trim()).join("\n")}\n`); } main().catch((error) => { diff --git a/tests/tooling/plant.test.ts b/tests/tooling/plant.test.ts index 186c62a7..698429fa 100644 --- a/tests/tooling/plant.test.ts +++ b/tests/tooling/plant.test.ts @@ -72,7 +72,9 @@ describe("npm run plant", () => { ".shift()", ]); expect(status, output).toBe(0); - expect(output).toContain('Caught: "reads the extension"'); + expect(output).toContain( + "Caught by:\nFAIL src/extension.test.mjs > reads the extension\n", + ); expect(sourceOf(workspace)).toBe(source); }), 60_000, @@ -129,10 +131,30 @@ describe("npm run plant", () => { }), ); + it( + "applies every edit to a file named two ways, and restores it", + withWorkspace((workspace) => { + // Only the two renames together keep the code working. + const { status, output } = runPlant(workspace, [ + "reads the extension", + "src/extension.mjs", + "(url)", + "(link)", + join(workspace, "src/extension.mjs"), + "url.split", + "link.split", + ]); + expect(status, output).toBe(1); + expect(output).toContain("Not caught"); + expect(sourceOf(workspace)).toBe(source); + }), + 60_000, + ); + it.skipIf(process.platform === "win32")( "restores the file when it is stopped mid-run", withWorkspace(async (workspace) => { - // Passes at once on the baseline and hangs once the plant is in. + // Hangs while the plant is in. const hang = 'require("fs").readFileSync("src/extension.mjs", "utf8").includes("shift") && setTimeout(() => {}, 30_000)'; const child = spawn( From b458b61e62267ddc6f376d3fd6c03b185c6eafe8 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 23:22:02 +0200 Subject: [PATCH 12/14] test: keep plant backups after a failed restore, always run the baseline The exit handler's second restore deleted the backups that a refused restore had just pointed at. A plant that was not caught skipped the baseline, so an e2e run left main.js built from the plant. --- scripts/plant.mjs | 11 +++++++---- tests/tooling/plant.test.ts | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/scripts/plant.mjs b/scripts/plant.mjs index b11d1ca2..31a6d379 100644 --- a/scripts/plant.mjs +++ b/scripts/plant.mjs @@ -23,6 +23,7 @@ const files = new Map(); const backups = fs.mkdtempSync(path.join(os.tmpdir(), "podnotes-plant-")); /** @type {import("node:child_process").ChildProcess | undefined} */ let child; +let keepBackups = false; /** Restores every planted file, unless one changed during the run. */ function restore() { @@ -31,6 +32,7 @@ function restore() { ); if (changed.length > 0) { files.clear(); + keepBackups = true; process.stderr.write( `${changed.join(", ")} changed during the run, so nothing was restored. Originals: ${backups}\n`, ); @@ -40,7 +42,7 @@ function restore() { for (const [file, { original }] of files) fs.writeFileSync(file, original); if (files.size > 0) process.stderr.write(`Restored ${[...files.keys()].join(", ")}.\n`); files.clear(); - fs.rmSync(backups, { recursive: true, force: true }); + if (!keepBackups) fs.rmSync(backups, { recursive: true, force: true }); return true; } @@ -135,15 +137,16 @@ async function main() { } const { code, output } = await run(command); if (!restore()) return; + // Restored, the command must pass, or a failure proves nothing. A build in it + // also replaces anything built from the plant. + const baseline = await run(command); const caught = output .split(/\r?\n/) .filter((line) => /^\s*FAIL\s/.test(line) && line.trimEnd().endsWith(` > ${testName}`)); if (code === 0 || caught.length === 0) { throw new Error(`Not caught: "${testName}" is not a failed test in the run's summary.`); } - // Restored, the command must pass, or the failure proves nothing. A build in it - // also replaces anything built from the plant. - if ((await run(command)).code !== 0) { + if (baseline.code !== 0) { throw new Error("The command also fails without the plant, so its failure proves nothing."); } process.stderr.write(`Caught by:\n${caught.map((line) => line.trim()).join("\n")}\n`); diff --git a/tests/tooling/plant.test.ts b/tests/tooling/plant.test.ts index 698429fa..669a3018 100644 --- a/tests/tooling/plant.test.ts +++ b/tests/tooling/plant.test.ts @@ -2,6 +2,7 @@ import { spawn, spawnSync } from "node:child_process"; import { mkdirSync, mkdtempSync, + readdirSync, readFileSync, realpathSync, rmSync, @@ -151,6 +152,30 @@ describe("npm run plant", () => { 60_000, ); + it( + "keeps the backups and the file when the file changed during the run", + withWorkspace((workspace) => { + const { status, output } = runPlant(workspace, [ + "reads the extension", + "src/extension.mjs", + ".pop()", + ".shift()", + "--", + process.execPath, + "-e", + 'require("fs").writeFileSync("src/extension.mjs", "edited")', + ]); + expect(status, output).toBe(1); + expect(sourceOf(workspace)).toBe("edited"); + const backups = /Originals: (\S+)/.exec(output)?.[1] ?? ""; + expect( + readdirSync(backups).map((name) => readFileSync(join(backups, name), "utf8")), + ).toEqual([source]); + rmSync(backups, { recursive: true }); + }), + 60_000, + ); + it.skipIf(process.platform === "win32")( "restores the file when it is stopped mid-run", withWorkspace(async (workspace) => { From 476b5be6e24f8c118cb3ff9f68b05ffb6523965d Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 24 Oct 2026 23:22:02 +0200 Subject: [PATCH 13/14] test: stop test:e2e on an interrupt during its build An interrupt during the build returned a null exit code, which exited 0, and a signal to the wrapper alone still reset the instance. Only a busy lock now reads as another run. --- scripts/obsidian-e2e.mjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/obsidian-e2e.mjs b/scripts/obsidian-e2e.mjs index eb9143ff..2df3bf80 100644 --- a/scripts/obsidian-e2e.mjs +++ b/scripts/obsidian-e2e.mjs @@ -88,6 +88,7 @@ async function runTests(args) { lockRoot: path.join(os.tmpdir(), "podnotes-e2e-runs"), onBusy: "fail", }).catch((/** @type {Error} */ error) => { + if (!error.message.includes(" is locked by ")) throw error; throw new Error( `Another npm run test:e2e is running in this worktree. Wait for it to finish. (${error.message})`, ); @@ -95,7 +96,8 @@ async function runTests(args) { for (const signal of FORWARDED_SIGNALS) process.on(signal, onSignal); try { const [built] = await once(spawn("npm", ["run", "build"], { stdio: "inherit" }), "close"); - if (built !== 0) return built; + if (interrupted) return 128 + os.constants.signals[interrupted]; + if (built !== 0) return built ?? 1; // Manual work and interrupted runs leave fixtures, layout and errors in the // vault that fail later tests. const stopped = await runObsidianE2ECli(["stop"]); From 9ad2ab6d20635e56da38ba2dcc0e2a29d00d8ea0 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 25 Oct 2026 00:03:20 +0200 Subject: [PATCH 14/14] test: let an interrupted plant wait for its command to clean up plant exited at once on a signal, so a test:e2e child died on a closed pipe before it released its run lock, and the next run was refused. It now waits for the command to close, and a second signal exits at once. test:e2e also releases its lock when stopping the instance throws. --- scripts/obsidian-e2e.mjs | 7 +++++-- scripts/plant.mjs | 11 +++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/scripts/obsidian-e2e.mjs b/scripts/obsidian-e2e.mjs index 2df3bf80..0eaf139f 100644 --- a/scripts/obsidian-e2e.mjs +++ b/scripts/obsidian-e2e.mjs @@ -147,8 +147,11 @@ async function runTests(args) { } finally { for (const signal of FORWARDED_SIGNALS) process.off(signal, onSignal); await fsp.rm(results, { force: true }); - await runObsidianE2ECli(["stop"]); - await lock.release(); + try { + await runObsidianE2ECli(["stop"]); + } finally { + await lock.release(); + } } } diff --git a/scripts/plant.mjs b/scripts/plant.mjs index 31a6d379..f4d2db1e 100644 --- a/scripts/plant.mjs +++ b/scripts/plant.mjs @@ -49,14 +49,21 @@ function restore() { process.stdout.on("error", () => {}); process.stderr.on("error", () => {}); process.on("exit", restore); +let interrupted = false; for (const signal of /** @type {NodeJS.Signals[]} */ (["SIGINT", "SIGTERM", "SIGHUP"])) { process.on(signal, () => { - child?.kill(signal); + const exit = () => process.exit(128 + os.constants.signals[signal]); + if (interrupted) exit(); + interrupted = true; restore(); process.stderr.write( "Interrupted: a build in the run may hold the plant. Run npm run build.\n", ); - process.exit(128 + os.constants.signals[signal]); + // The command cleans up after itself, test:e2e its instance and lock, so wait for it. + if (child && child.exitCode === null && child.signalCode === null) { + child.once("close", exit); + child.kill(signal); + } else exit(); }); }