From 777ed3840555423327c68215ce394c320e5bcef9 Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Fri, 11 Sep 2026 14:35:34 +0200 Subject: [PATCH] Fail an unloadable test file on its own instead of the whole run A test file that could not be imported killed the process before any test ran. It now becomes a failing test, so the existing stats and exit-code paths report it and healthy files still run. Under --watch, rerun() swaps in that failing test rather than keeping the stale passing subtree. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RaXmFiBg3UGUMuGFXjn5Ki --- src/env/node.js | 81 +++++++++++++++++++++++++++++-------------------- tests/run.js | 51 +++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 33 deletions(-) diff --git a/src/env/node.js b/src/env/node.js index 31f591a..d7e7bd8 100644 --- a/src/env/node.js +++ b/src/env/node.js @@ -25,6 +25,40 @@ const filenamePatterns = { exclude: /^index/, }; +/** + * A test that reports why its file could not be loaded, so one unloadable file + * fails on its own instead of taking down the whole run. + * @param {Error} err + * @param {string} name + */ +function failedTest (err, name) { + return { + name, + run: () => { + throw err; + }, + }; +} + +/** + * Import a test file, or produce a failing test in its place if it cannot be loaded. + * @param {URL} url + * @param {string} base - Directory to label the file against, the same one loaded files are labeled against. + */ +async function importTests (url, base) { + // Before the import, not after: a file that fails to load must still get its directory watched, + // or fixing it is never noticed. Explicit rather than via the resolve hook, which needs Node ≥ 22.15. + loadedFiles.add(url.href); + + try { + let module = await import(url); + return module.default ?? Object.values(module); + } + catch (err) { + return failedTest(err, path.relative(base, fileURLToPath(url))); + } +} + async function getTestsIn (dir) { let filenames = fs .readdirSync(dir) @@ -34,20 +68,7 @@ async function getTestsIn (dir) { let cwd = process.cwd(); let paths = filenames.map(name => path.resolve(cwd, dir, name)); - return ( - await Promise.all( - paths.map(path => { - path = pathToFileURL(path); - loadedFiles.add(path.href); - return import(path).then( - module => module.default ?? Object.values(module), - err => { - console.error(`Error importing tests from ${path}:`, err); - }, - ); - }), - ) - ).flat(); + return (await Promise.all(paths.map(path => importTests(pathToFileURL(path), dir)))).flat(); } // AbortSignal (not a plain boolean) because runAll() shallow-copies options per child — @@ -183,17 +204,18 @@ async function rerun (options, urls) { continue; } - // Import before mutating stats — on error, keep the old subtree intact let uncached = new URL(url); uncached.searchParams.set("htest", version); - let module; + let test; try { - module = await import(uncached.href); + let module = await import(uncached.href); + test = module.default ?? Object.values(module); } catch (err) { - console.error(`Error importing ${url}:`, err); - continue; + // Swap in a failing test rather than keeping the stale passing one, + // which would leave a file that just broke showing green. + test = failedTest(err, old.test.file.label); } // Subtract old stats, swap in the new subtree @@ -206,8 +228,6 @@ async function rerun (options, urls) { (currentRoot.timeTakenAsync ?? 0) - old.timeTakenAsync; } - let test = module.default ?? Object.values(module); - if (Object.isExtensible(test)) { test.file = old.test.file; } @@ -319,6 +339,7 @@ export default { let tests; let isDirectory = fs.statSync(location, { throwIfNoEntry: false })?.isDirectory(); + let base = isDirectory ? location : path.dirname(location); if (isDirectory) { // Directory provided, fetch all files tests = await getTestsIn(location); @@ -329,12 +350,7 @@ export default { let modules = globSync(location).flatMap(paths => { // Convert paths to imported modules paths = getType(paths) === "string" ? [paths] : paths; - return paths.map(p => { - p = path.resolve(process.cwd(), p); - p = pathToFileURL(p); - loadedFiles.add(p.href); - return import(p).then(m => m.default ?? Object.values(m)); - }); + return paths.map(p => importTests(pathToFileURL(path.resolve(process.cwd(), p)), base)); }); tests = (await Promise.all(modules)).flat(); } @@ -344,16 +360,15 @@ export default { // Tag each module's default with its source file. Re-imports return the cached namespace — no I/O. await Promise.all( [...loadedFiles].map(async url => { - let module = await import(url); - let test = module.default ?? module; + // loadedFiles holds transitive imports too, so a failure here was already surfaced — + // by this file's own failing test, or by the test file that imported it. + let module = await import(url).catch(() => null); + let test = module?.default ?? module; if (test && typeof test === "object" && Object.isExtensible(test) && !test.file) { let fileUrl = new URL(url); fileUrl.search = ""; test.file = { - label: path.relative( - isDirectory ? location : path.dirname(location), - fileURLToPath(url), - ), + label: path.relative(base, fileURLToPath(url)), path: fileUrl.href, }; } diff --git a/tests/run.js b/tests/run.js index 5473923..5c660e4 100644 --- a/tests/run.js +++ b/tests/run.js @@ -2,6 +2,33 @@ import Test from "../src/classes/Test.js"; import TestResult from "../src/classes/TestResult.js"; import BubblingEventTarget from "../src/classes/BubblingEventTarget.js"; +// In run() rather than beforeAll(): a throwing beforeAll skips its tests, and a skipped test +// cannot fail CI. Memoized because both assertions read one child run. +let unloadable; +function runUnloadable () { + return (unloadable ??= (async () => { + let { spawnSync } = await import("node:child_process"); + let { mkdtempSync, writeFileSync, rmSync } = await import("node:fs"); + let { tmpdir } = await import("node:os"); + let { join } = await import("node:path"); + + // ESM, because a CJS syntax error escapes as an unhandled rejection Node never hands us + let dir = mkdtempSync(join(tmpdir(), "htest-unloadable-")); + writeFileSync(join(dir, "package.json"), `{ "type": "module" }`); + writeFileSync(join(dir, "good.js"), `export default { tests: [{ run: () => 1, expect: 1 }] };`); + writeFileSync(join(dir, "broken.js"), `import missing from "./nope.js";\nexport default { tests: [] };`); + + let result = spawnSync( + process.execPath, + [join(process.cwd(), "bin/htest.js"), dir, "--ci"], + { encoding: "utf8" }, + ); + rmSync(dir, { recursive: true }); + + return result; + })()); +} + export default { name: "Run tests", tests: [ @@ -80,6 +107,30 @@ export default { }, expect: true, }, + { + name: "A test file that cannot be imported", + description: "It fails on its own instead of taking down the run, so healthy siblings still report.", + skip: typeof globalThis.process === "undefined", + async run () { + let { stdout, stderr } = await runUnloadable(); + + // A failing run prints its tree to stderr, a crashing one its stack — assert across both. + return stdout + stderr; + }, + check: (actual, expect) => actual.includes(expect), + tests: [ + { + name: "Still runs the healthy files, and counts the broken one", + description: "The denominator matters: dropping the broken file would report 1/1 and look fine.", + expect: "1/2 PASS", + }, + { + name: "Names the file that failed to load", + description: "Matching the row, not the output: the error text below it mentions the path either way.", + expect: "FAIL broken.js", + }, + ], + }, { name: "Aborting", tests: [