Add headless Node test runner (@srhazi/gooey-test/headless) #1
Loading…
Reference in a new issue
No description provided.
Delete branch "wip/headless-runner"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adds a headless Node test runner to
@srhazi/gooey-test, ported from pigraph'sci/test-headless.mjs(per review on the Gooey PR).@srhazi/gooey-test/headless+ CLIgooey-test-headless(dist/headless/cli.mjs).runHeadless(options)— parameterized:cwd,include/testFiles,viteConfigFile/viteArgs,host,port,strictPort,timeoutMs,browser,keepOpenOnFailure,remoteDebuggingPort,artifactsDir.keepOpenOnFailure— leaves the browser + Vite running and prints the testrunner URL.remoteDebuggingPort— launches Chromium with--remote-debugging-portfor DevTools.@playwright/testis a devDependency + optional peer, never bundled into the browser artifacts).dist/rebuilt and committed (as the repo pattern does).Verified locally:
node dist/headless/cli.mjs src/happypath.test.ts→PASS: 11 test(s) …exit 0.--keep-open-on-failure --remote-debugging-port 9333→ process stays alive;GET /json/versionresponds.@ -0,0 +52,4 @@viteConfigFile?: string;/** Extra arguments passed through to the Vite CLI. */viteArgs?: string[];/** Milliseconds to wait for Vite to print its URL. Defaults to 30000. */30s is way too long, this should be 5 seconds
Done in
717f99f—viteStartupTimeoutMsnow defaults to 5000ms (therunHeadlessdefault, the doc comment, the CLI--help, and the README).@ -6,0 +34,4 @@### Options| Option | Default | Description |Please show this table instead as the typescript type of the options passed to
runHeadless, with the commandline flags called out.i.e.
Also, I think we need an
excludeso that we can avoidnode_modulesdirectories. Ideallyincludehas a default of**/*.test.{ts,tsx}andexcludehas a default of**/node_modules/**.Done in
717f99f— the options table is now aninterface HeadlessOptions { ... }block with each CLI flag called out inline, plus the newexcludeoption (default**/node_modules/**) andincludedefault changed to**/*.test.{ts,tsx}.While doing this, review caught a hole: a caller-supplied
excludereplaced the default, which could letnode_modulesbe walked (the exclude regexes only match directory paths, so they can't catchnode_modules/**files).node_modulesis now always pruned (938b663), and the README documents the include/exclude scan scope.@ -6,0 +39,4 @@| `cwd` | `process.cwd()` | Directory tests and Vite run from. || `testFiles` | _discovered_ | Explicit test files, relative to `cwd`. || `include` | `src/**/*.test.{ts,tsx}` | Glob(s) used to discover test files. || `testrunnerUrl` | consumer's `node_modules/@srhazi/gooey-test/dist/testrunner/testrunner.html` | Full testrunner URL (without the `#fragment`). |Do we need
testrunnerUrlas an option? I don't think we should support custom test runners (yet)Done in
717f99f—testrunnerUrland its--urlflag are removed; the runner always resolves its own testrunner URL. (HeadlessResult.testRunnerUrlremains, now just the resolved URL.)@ -6,0 +48,4 @@| `viteStartupTimeoutMs` | `30000` | How long to wait for Vite to print its URL. || `timeoutMs` | `60000` | How long to wait for the testrunner to finish. || `browser` | `chromium` | `chromium`, `firefox`, or `webkit`. || `keepOpenOnFailure` | `false` | On failure, leave the browser + Vite running and print the testrunner URL. |Note: this should be called
--debug-failuresanddebugFailuresWhen
false, this should print a message on stderr telling users they may pass--debug-failuresto pause on test failure with instructions for how to connect with Chrome DevTools.Done in
717f99f— renamed todebugFailures/--debug-failures. When a run fails (or times out) with it off, a hint is printed to stderr pointing at--debug-failuresand how to attach Chrome DevTools; the wording also names the programmaticdebugFailures: true(938b663).⏸ Resume notes — session paused
Pausing here. This is what is not inferable from the MR/diff.
Cross-repo role
ci/test-headless.mjsin favour of@srhazi/gooey-test/headless; wiring Gooey'stest:headlessscript is pending this package landing/publishing.Environment / dev loop (not in the repo)
/Users/sufian/development/2025/gooey-test-headless(self-containednode_modules; chromium installed vianpx playwright install chromium).s/dist— do not runs/clean(it runs interactivegit clean -i).node dist/headless/cli.mjs --cwd /Users/sufian/development/2026/gooey-identity --port 5173 --strict-port <20 non-perf files>→
PASS: 523 tests / 20 files.⚠️ Main-checkout state
/Users/sufian/development/2025/gooey-testhas uncommitted changes (yours) that overlap this branch ontsconfig.json,s/dist,src/lib/index.ts— expect a small rebase/conflict when you commit them.stash@{0}(WIP on main) exists there; drop it when ready.Packaging decisions worth remembering
@playwright/testis a devDependency + optional peerDependency;@types/nodeis a devDependency (kept out of the browser bundle via a separate Node build entry).sufian+orchestrator@gmail.com.▶ Revision pushed — ready for re-review
Addressed all four review comments on
wip/headless-runner(per-thread replies below).Commits
717f99f— address the four comments: 5s Vite startup timeout; README options table →interface HeadlessOptionswith CLI flags called out;excludeadded (default**/node_modules/**) andincludedefault**/*.test.{ts,tsx};testrunnerUrl/--urlremoved;keepOpenOnFailure→debugFailures/--debug-failureswith a stderr hint on failure.938b663— fix found in review: a caller-suppliedexcludereplaced the default, sonode_modulescould be walked (the exclude regexes only match directory paths and can't catchnode_modules/**files).node_modulesis now always pruned, and the README documents the include/exclude scan scope.Verification
node dist/headless/cli.mjs src/happypath.test.ts→PASS: 11 test(s), exit 0.include+exclude), re-run by the orchestrator:node dist/headless/cli.mjs --cwd /Users/sufian/development/2026/gooey-identity --port 5173 --strict-port --exclude 'src/performance.test.tsx'→
PASS: 523 test(s) passed across 20 file(s), exit 0.tsc -p .andtsc -p tsconfig.headless.jsonclean;dist/rebuilt and committed (only the headless artifacts changed —dist/lib/*byte-identical at the existing es2015 target).Not publishing — npm is yours. Over to you for re-review.
@ -6,3 +91,4 @@## UsageTBDCan you please update these to briefly describe:
Will do — I'll rewrite this section to cover, concretely: (1) running headless (
gooey-test-headless/runHeadless), (2) running the runner in a host browser (start Vite and open the runner entry), and (3) debugging a failure in Chrome DevTools (--debug-failures+--remote-debugging-port 9222). (2) depends on #129 — the entry URL shouldn't be a hand-builtnode_modules/...path.Implemented in
4d6f577/ce4e762(pushed). New## Running testssection covers: headless (multiple package-manager invocations), host browser (open the runner URL served by Vite), and DevTools debugging (Chromium--remote-debugging-port; Firefoxabout:debugging; WebKit Safari's Develop menu /WEBKIT_INSPECTOR_SERVER;PWDEBUG=1fallback).@ -6,0 +20,4 @@Run it from the command line:```npx gooey-test-headless [options] [test-file ...]What options do we have here in terms of how our headless tool is named?
I'm not quite sure what capabilities npm/npx binaries that are distributed as packages are, and personally am a bit wary of npx.
Naming/distribution options:
bin(gooey-test-headless). After install it's on PATH inside package scripts:"test:headless": "gooey-test-headless", or directly atnode_modules/.bin/gooey-test-headless. No npx needed.@srhazi/gooey-test/headlessand let the user scriptrunHeadless(...). Most explicit, least magic.gooey-test,gt) — cosmetic.On npx:
npx gooey-test-headlessresolves the local bin when the package is installed, but if it isn't it will fetch from the registry (network + version/typosquat risk). Since consumers must have@srhazi/gooey-testinstalled anyway, I'd document the package-script / local-bin form and not lead with npx (or usenpx --no-install). Preference on the bin name?Implemented in
4d6f577/ce4e762(pushed). Bin renamed togooey-test. Docs show the directnode_modules/.bin/gooey-test, an npm script, yarn, pnpm, andnpx --no-install(never auto-fetches) — plus the programmatic form.@ -0,0 +39,4 @@return parseArgs({args: argv,allowPositionals: true,options: {Can we have short names options for all of these except: cwd, strict-port, vite-arg, vite-startup-timeout?
Will add short flags for everything except cwd / strict-port / vite-arg / vite-startup-timeout. Proposed:
-iinclude,-xexclude,-pport,-Hhost,-cconfig,-ttimeout,-bbrowser,-ddebug-failures,-rremote-debugging-port,-aartifacts-dir (-his already help). Flag any letters you dislike.Implemented in
4d6f577/ce4e762(pushed). Short flags:-i -x -p -H -c -t -b -d -r -a(-his help); none for cwd / strict-port / vite-arg / vite-startup-timeout.@ -0,0 +131,4 @@// Browser + Vite keep the event loop alive on purpose; do not exit.return;}process.exitCode = result.exitCode;Huh, does this cause returning from the main function to exit? I don't see how this differs from the early return when
result.keptOpenis trueYou're right — returning from
main()doesn't exit; Node exits when the event loop drains. The two paths differ only in the exit code: in the non-keptOpenpathrunHeadless'sfinallyhas already closed the browser + Vite, so the loop drains and Node exits withprocess.exitCode; in thekeptOpenpath we return before settingprocess.exitCode, so it stays 0 (we don't want to signal failure while deliberately leaving the browser running). The comment is misleading — I'll simplify toprocess.exitCode = result.keptOpen ? 0 : result.exitCode;and drop the early return.Implemented in
4d6f577/ce4e762(pushed). Nowprocess.exitCode = result.keptOpen ? 0 : result.exitCode;(early return dropped). You were right that the loop wasn't draining: thePromise.racetimeout timer was never cleared, so every run hung ~60s — fixed ince4e762(a PASS run is now ~2s).@ -0,0 +155,4 @@return source;}function globToRegExp(pattern: string): RegExp {Shouldn't we be using the standard node
fsglobfunction? Why are we building regexp for this?Good question.
fs.glob/fs.promises.globonly landed in Node 22 (we build with--target=node18), and it does no brace expansion (*.test.{ts,tsx}), which is why this hand-rolled matcher exists. I'd rather replace it than keep it. Options:picomatch/tinyglobby) used for both matching and directory pruning;enginesto Node >=22 and use the built-in glob + a brace-expansion step (or drop braces).The hand-rolled matcher is also where the
node_moduleshole came from, so I lean (a). Which do you prefer?Implemented in
4d6f577/ce4e762(pushed). Deleted the hand-rolled matcher; discovery now uses Node 22fs/promisesglobwith a string-arrayexclude(braces supported natively).engines.nodecorrected to>=22.17.0ince4e762— arrayexcludeneeds ≥22.14 and 22.14–22.16 emit an ExperimentalWarning.@ -0,0 +206,4 @@}}function findTestFiles(If for some reason
globdoesn't work because we've chosen include/exclude specific details, we can change our options to work differently (like specifying a target directory/file and finding all*.test.ts{,x}files in there to run)Agreed that's a viable simplification (point at a dir/file and find
*.test.ts{,x}). I'd keepinclude/excludeglobs — familiar, matches other runners, and needed for thenode_modules/build-output cases — and fix/replace the matcher per #125. If you'd rather go the target-dir route (fewer knobs), happy to; your call.Implemented in
4d6f577/ce4e762(pushed). Keptinclude/excludeglobs (now via the built-in glob) rather than switching to the target-dir model — say the word if you'd still prefer that.@ -0,0 +391,4 @@/*** Resolve the testrunner URL that Vite should serve for the given project.** Preference order:This library was written rapidly. What's the best way we could pre-package a "test runner" so it Just Works™ when using headless mode or a hosted browser mode (i.e. hit vite directly, the entry point would be the test runner)
This is the important one. Today the runner is a hand-built URL into the consumer's
node_modules/.../testrunner.html#/file1:/file2, which is why it's fragile. Proposed "Just Works" mechanism, shared by both modes:@srhazi/gooey-test/vitethat (a) serves the runner entry at a stable path (e.g./@gooey-test) and (b) injects a virtual module listing discovered test files (import.meta.glob), so no#/...file list is needed;vite dev;include/exclude) lives in one place (the plugin).I'd write this up as a short design and land it as a follow-up, not in this first PR. Sound good?
@ -0,0 +506,4 @@return await import('@playwright/test');} catch (err) {throw new Error('Unable to load @playwright/test. Install it in your project (npm install --save-dev @playwright/test) and run `npx playwright install chromium`.\n' +I'm assuming we should also note that other browsers may be needed?
Yes — I'll broaden it to install the browser(s) you actually use, e.g.
npx playwright install chromium firefox webkit, and note only the one passed via--browseris needed.Implemented in
4d6f577/ce4e762(pushed). Message now suggestsnpx playwright install chromium firefox webkitand notes only the--browserone is needed.@ -0,0 +637,4 @@const outcome = await Promise.race([page.waitForSelector('.testrunner.testrunner--pass', {I'm planning on doing some visual changes, would it be ok to use data- attributes to target those instead of using CSS class names for identifying items in the DOM? (class is for visual style, data- is for structure & behavior)
Yes please —
data-*for structure/behavior and classes for styling is exactly right. Note the headless runner currently keys off classes (.testrunner--pass/fail,.testfile--fail .testfile__status,.test--fail .test__link,.test--fail pre) plus the existingdata-tests-*summary attrs. I'll migrate the runner to stabledata-*hooks as part of the visual refresh so your class changes can't break it. Proposed hooks:data-testrunner-status,data-test-status,data-test-name,data-test-error,data-file-status— confirm/rename.@ -0,0 +661,4 @@if (outcome === 'pass') {const testCount = summary.testsPassed;const fileCount = summary.filesPassed;log(How much work would it take to improve the output of the headless runner so it shows only as much information as needed?
<TEST | PASS | FAIL> <path/to/test.test.tsx> <test-name>using terminal escape codes to update TEST (running, yellow), PASS (test passed, green), FAIL (test failed, red) updated as the active test file is run. We'll need to truncate atCOLSso wrapping does not occur (so it's all on one line in the terminal).(Note: this doesn't need to happen in this first PR, we could follow-up with this in our todo list)
Love it — and agreed it's a follow-up, not this PR. Captured: (dense/auto, non-tty) failures only; (standard, tty) a single updating line
<TEST|PASS|FAIL> <path> <name>with ANSI colors, truncated to$COLUMNS; (verbose) a section per test + console output in color. Added to the todo list.@ -0,0 +729,4 @@log(`Open the test runner at: ${testRunnerUrl}`);if (options.remoteDebuggingPort !== undefined) {log(`DevTools: devtools://devtools/bundled/inspector.html?ws=localhost:${options.remoteDebuggingPort}/... (http://localhost:${options.remoteDebuggingPort})`Is there an equivalent for Firefox and Safari? It'd be extra helpful to point to all of those.
Good call. Chrome DevTools (CDP) is Chromium-only; I'll document the equivalents: Firefox via
about:debugging+ the Firefox DevTools protocol, WebKit/Safari via Safari's Develop menu / Web Inspector. I'll verify the exact Playwright-launched steps for each before writing them, and mention Playwright's own inspector (PWDEBUG=1) as a cross-browser fallback.Implemented in
4d6f577/ce4e762(pushed). DevTools guidance is now browser-aware. Caveat: Firefox/WebKit couldn't be launched here (installed Playwright 1.64 wants browser revisions that aren't present), so that text is source-verified only — install the browsers to exercise it.Please respond to all comments
▶ Headless-runner follow-ups pushed — ready for re-review
Addressed the new review comments (per-thread replies below). Two commits on
wip/headless-runner:4d6f577— per your decisions:fs/promisesglob(native braces) with a string-arrayexclude.gooey-test, with README invocations for npm script /node_modules/.bin/ yarn / pnpm /npx --no-install(never auto-installs) / programmatic.-i -x -p -H -c -t -b -d -r -a(all except cwd/strict-port/vite-arg/vite-startup-timeout).about:debugging; WebKit Safari inspector) + broadened browser-install message.## Running testssection (headless / host-browser / debugging);--remote-debugging-portis Chromium-only.ce4e762— review fixes:engines.node→>=22.17.0(arrayexcludeneeds ≥22.14; 22.14–22.16 warn);.nvmrc→ 22.17.0.Promise.racetimeout timer was never cleared, so every run (pass or fail) waited the fulltimeoutMsbefore exiting. Now cleared/unref'd.localhost:9222when no debugging port was opened; README accuracy nits.Verification
node dist/headless/cli.mjs src/happypath.test.ts→ PASS 11, ~2s (was 61s).--cwd .../gooey-identity --strict-port --exclude 'src/performance.test.tsx') → PASS 523 / 20, ~7s.--timeout 3000on a hanging test) → exit 2 in ~3s.tscclean on both configs;dist/rebuilt and committed.Deferred (per your notes):
data-*hooks (comment 133) → visual-refresh work; headless output modes (135) → follow-up; the@srhazi/gooey-test/vite"Just Works" entry (129) → follow-up design.Not publishing — npm is yours.
▶ Smoke test + README fixes pushed (head
8c883ff)I manually verified every README instruction in a throwaway
node:22-bookworm-slimcontainer, then turned that into a durable, Dockerized smoke test, and fixed what it found. Three commits:4e30f19—smoke/harness +s/smoke: a non-gooey-test-based (plain bash + tiny node helpers) end-to-end test that runs the README flows across npm / yarn-classic / pnpm, plus direct/npm run/npx --no-install/programmatic, the hosted runner, failure artifacts,--debug-failures,--remote-debugging-port, and the option surface.smoke/run.shpacks the repo, builds a cached CI image (Playwright browsers preinstalled) and runs it.5caf962— fixes the harness caught:resolveVite()rannode node_modules/.bin/vite; pnpm's.bin/viteis a/bin/shcmd-shim, so it threw. Now resolves Vite's real JS bin from itspackage.json.Local:and the URL in ANSI SGR escapes (even on a pipe underCI=1), so the startup URL regex never matched → every run timed out. AddedstripAnsi.vite,@srhazi/gooeypeer,--with-deps), hosted#-fragment convention, one-package-manager caveat.8c883ff— review follow-ups: loosened the@srhazi/gooeypeer to^0.25.0(the README's unpinned install conflicted with the exact0.25.0pin while latest is 0.25.2); the smoke now installs the README's exact command (so 0.25.2 is exercised); made thenode_modules-prune assertion meaningful;stripAnsion the whole buffer; added the Node ≥ 22.17 requirement; extra--host/--artifacts-dir/ hosted test-file checks.Verification:
./s/smoke→ 63 passed / 0 failed (re-run by the orchestrator, exit 0). Before the fixes the same harness caught 31 passed / 27 failed — including the pnpm failure and the Vite ≥5 ANSI timeout — so it's a meaningful regression guard.dist/rebuilt;tscclean; no leftover containers/processes.Run it with
./s/smoke(Docker required; no npm publish/auth involved — it uses a localnpm pack).vite— declare an (optional) peer and/or own it via a@srhazi/gooey-test/viteplugin #2