diff --git a/CHANGELOG.md b/CHANGELOG.md index c9cce649..3b0cd6cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ Each release of Research-Kit, newest first. The version is `KIT_VERSION` in ## Unreleased +- selftest refuses each unmatched group before starting tests, including when another requested group matches (#332). + - The README opens with an accessible workflow preview and linked CI, license, last-commit and stars badges. A contributor guide explains setup, verification, issue labels and the existing commit/PR protocol. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ca0813fa..52cab6b2 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -286,7 +286,7 @@ maps to exit code — `PASS` 0, `FAIL`/`REOPEN` 1, `INCOMPLETE` 2, `BLOCKED` 3, and a **usage error exits 2 from every subcommand**, printing the usage block to stderr; `test/support-policy.test.mjs` pins that by running the CLI. It writes exactly one thing, only when asked: `fi-validate --report `), -`selftest.mjs` (the whole suite, queued by the harness — which **awaits** every test, +`selftest.mjs` (every requested group must match a test file before any test starts; an unmatched group is named and refused with exit 2, including beside a valid group; #332. CLI regressions isolate child report paths from the parent suite; the whole suite, queued by the harness — which **awaits** every test, so `ok` means the assertions settled; ADR-0021. It runs from the repository root whatever the caller's cwd, resolving a relative `RESEARCH_KIT_RESULT_FILE` against the caller's folder first; started anywhere else, it crashed at import before any test ran - ADR-0071); `RESEARCH_KIT_ALLOW_UNSUP=1` lets a LOCAL run whose only blockers are UNSUP tests exit 0, ending "NOT a full pass" and writing `unsupportedWaived` into the result file, and is ignored when `CI` is set (2026-09-30, ADR-0108); a block whose only blockers are unsupported tests ends "No test failed ... this is not a red suite", naming the opt-in, where it said "A red suite stops work" - the protocol's stop-the-line sentence, which stopped a break-test at its baseline on a host with no Chromium (2026-10-02); below the Node floor it refuses before any test runs, exit 2, naming the floor and the fix - on Node 20 it ran and reported 28 red tests each saying "this kit needs 22 or newer" (2026-10-02; `RESEARCH_KIT_TEST_NODE_VERSION` is the harness's seam); the result file names the failing tests (`failed: [labels]`) beside their count, so two runs compare as sets - a count cannot show one test starting to fail while another starts to pass (break-test pass 5, 2026-10-04); a `RESEARCH_KIT_RESULT_FILE` that cannot be written is named on stderr and a green run exits 2 for it, where it exited 0 and left CI's summary reading "crashed before reporting" beside a green step (2026-10-02); it gives the run a scratch home and removes every `RESEARCH_KIT_*` variable but the ones that describe the host or the run (the result file, the unsupported opt-in, the browser, the gate timeout, the live opt-ins, the harness's `RESEARCH_KIT_TEST_*` seams), and the vendor keys, before the first test (2026-10-02, ADR-0123) - the tests that spawn kit commands read the operator's machine config, deployed kit and CLI login, so a `strict` posture config turned 18 tests red and this container's `http-keyless` config hid two cli tests red on any host whose Firecrawl CLI is installed and not logged in; a test file that throws while it is imported is reported as a named FAIL by `importTestFiles` in the harness, and the rest of the suite still runs and counts - one such file used to abort the whole run with no count); it tries the temp folder once before the run and, if it cannot be used, names it and the TMPDIR fix at the top and under a red summary - an unwritable one gave 541 EACCES failures and no cause; a closed stdout no longer kills the run - `| head -1` printed an unhandled EPIPE and exited 1 while the suite was still going, so a green run read as broken (2026-09-28); outside a repository checkout - a deployed kit - it refuses with exit 2 and points at `doctor`, where 80 tests had failed on a healthy install, ADR-0080; a temp folder that does not exist yet is created by its probe and by the harness's `tempDir` through `createTempFolder`, which creates each missing ancestor with its own non-recursive mkdir and records only a mkdir that succeeded, each at the front of the list so it stays deepest first at every moment (a sort after the loop was skipped by a mkdir that threw part-way, and an ancestor already recorded was left behind: 2026-10-04, second review) - recording from the existence check before it left a window in which another process created the same chain and this run removed that process's empty folder (2026-10-04, external review) - and removes them with the scratch at exit by a non-recursive rmdir that refuses anything non-empty - the run had left the created chain behind, empty, `/nonexistent/dir/xyz` for that TMPDIR and an empty folder inside the checkout for a relative one: break-test pass 4, F3, 2026-10-04). diff --git a/research-kit/README.md b/research-kit/README.md index 4bc12369..27f8d04f 100644 --- a/research-kit/README.md +++ b/research-kit/README.md @@ -563,7 +563,7 @@ node research-kit/bin/selftest.mjs # all of it node research-kit/bin/selftest.mjs gate hook # just these files ``` -1789 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means +1792 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means the assertions settled (ADR-0021), and each test is raced against a watchdog (`RESEARCH_KIT_TEST_TIMEOUT`, default 60s). diff --git a/research-kit/bin/selftest.mjs b/research-kit/bin/selftest.mjs index f5f2e85e..cd78474e 100644 --- a/research-kit/bin/selftest.mjs +++ b/research-kit/bin/selftest.mjs @@ -45,8 +45,14 @@ if (!nodeFloor.ok) { process.exit(2); } -const files = listFiles(dir) - .filter((name) => name.endsWith('.test.mjs')) +const testFiles = listFiles(dir).filter((name) => name.endsWith('.test.mjs')); +const unmatched = positional.filter((needle) => !testFiles.some((name) => name.includes(needle))); +if (unmatched.length) { + process.stderr.write(`selftest: ${unmatched.map((name) => JSON.stringify(name)).join(', ')} matches no test file in ${dir}; groups are named by file, e.g. warc for warc.test.mjs\n`); + process.exit(2); +} + +const files = testFiles .filter((name) => !positional.length || positional.some((needle) => name.includes(needle))) .sort(); diff --git a/research-kit/test/selftest-cli.test.mjs b/research-kit/test/selftest-cli.test.mjs new file mode 100644 index 00000000..a5ddd0d6 --- /dev/null +++ b/research-kit/test/selftest-cli.test.mjs @@ -0,0 +1,39 @@ +// The suite must run every requested group, or refuse before any tests start. +import { spawnSync } from 'node:child_process'; +import { test, describe, assert, tempDir, path, KIT_ROOT, fs } from './harness.mjs'; + +describe('selftest-cli'); + +function runGroups(groups, env = process.env) { + const childEnv = { ...env }; + delete childEnv.RESEARCH_KIT_RESULT_FILE; + return spawnSync(process.execPath, [path.join(KIT_ROOT, 'bin', 'selftest.mjs'), ...groups], { + cwd: tempDir('rk-selftest-groups-'), env: childEnv, encoding: 'utf8', windowsHide: true, timeout: 60000, + }); +} + +test('unknown groups are named and refused, alone or mixed with valid groups', () => { + for (const groups of [['no-such-group'], ['warc', 'no-such-group'], ['no-such-group', 'warc']]) { + const result = runGroups(groups); + assert.equal(result.status, 2, `${result.stdout}\n${result.stderr}`); + assert.match(result.stderr, /no-such-group/); + assert.match(result.stderr, /matches no test file/); + assert.doesNotMatch(result.stdout, /^ok\s|all tests passed|passed, \d+ failed/m); + } +}); + +test('multiple valid groups still run every matching test file', () => { + const result = runGroups(['warc', 'similarity']); + assert.equal(result.status, 0, `${result.stdout}\n${result.stderr}`); + assert.match(result.stdout, /^ok\s+warc >/m); + assert.match(result.stdout, /^ok\s+similarity >/m); + assert.match(result.stdout, /all tests passed/); +}); + +test('regression subprocesses cannot overwrite the parent suite report', () => { + const file = path.join(tempDir('rk-parent-report-'), 'parent-report.json'); + fs.writeFileSync(file, 'PARENT REPORT\n'); + const result = runGroups(['warc', 'similarity'], { ...process.env, RESEARCH_KIT_RESULT_FILE: file }); + assert.equal(result.status, 0, `${result.stdout}\n${result.stderr}`); + assert.equal(fs.readFileSync(file, 'utf8'), 'PARENT REPORT\n'); +});