Add headless Node test runner (@srhazi/gooey-test/headless) #1

Merged
sufian merged 11 commits from wip/headless-runner into main 2026-10-12 03:53:10 +00:00
Member

Adds a headless Node test runner to @srhazi/gooey-test, ported from pigraph's ci/test-headless.mjs (per review on the Gooey PR).

  • New subpath export @srhazi/gooey-test/headless + CLI gooey-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-port for DevTools.
  • Playwright/Node kept out of the browser bundle (separate Node build entry; @playwright/test is 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.
  • Failure path → non-zero exit + failing test/reason + screenshot/DOM artifact.
  • --keep-open-on-failure --remote-debugging-port 9333 → process stays alive; GET /json/version responds.
Adds a **headless** Node test runner to `@srhazi/gooey-test`, ported from pigraph's `ci/test-headless.mjs` (per review on the Gooey PR). - New subpath export **`@srhazi/gooey-test/headless`** + CLI `gooey-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-port` for DevTools. - Playwright/Node kept **out of the browser bundle** (separate Node build entry; `@playwright/test` is 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. - Failure path → non-zero exit + failing test/reason + screenshot/DOM artifact. - `--keep-open-on-failure --remote-debugging-port 9333` → process stays alive; `GET /json/version` responds.
Ports the Playwright-driven in-browser testrunner harness into the package as a
parameterized Node module + CLI, exposed via the ./headless subpath export.
Playwright is kept external (dev + optional peer dep) and never reaches the
browser bundle.
The headless runner counted .test--pass/.testfile--pass in the DOM, but the
testrunner collapses passing test files, so only the last few open files were
counted (e.g. 69 tests / 4 files for Gooey's 21-file suite).

The testrunner now exposes state-derived data-* attributes on .testrunner
(tests/files: total/passed/failed) and runHeadless reads those instead.
The default package-relative /@fs URL did not serve the testrunner's assets,
iframes and test files reliably for a consumer (e.g. Gooey reported 69 tests / 4
files instead of the real 523 / 20). Prefer the consumer's installed
<cwd>/node_modules/@srhazi/gooey-test/dist/testrunner/testrunner.html, which is
inside the Vite root and serves without server.fs.allow tweaks. Fall back to the
package's own dist when it is inside the Vite root (self-test), then /@fs.
@ -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. */
Owner

30s is way too long, this should be 5 seconds

30s is way too long, this should be 5 seconds
Author
Member

Done in 717f99f — viteStartupTimeoutMs now defaults to 5000ms (the runHeadless default, the doc comment, the CLI --help, and the README).

Done in `717f99f` — `viteStartupTimeoutMs` now defaults to **5000ms** (the `runHeadless` default, the doc comment, the CLI `--help`, and the README).
sufian marked this conversation as resolved
README.md Outdated
@ -6,0 +34,4 @@
### Options
| Option | Default | Description |
Owner

Please show this table instead as the typescript type of the options passed to runHeadless, with the commandline flags called out.

i.e.

interface Options {
  include?: string; // (--include) [default: `src/**/*.test.{ts,tsx}`] -- Glob(s) used to discover test files
  ...etc...
}

Also, I think we need an exclude so that we can avoid node_modules directories. Ideally include has a default of **/*.test.{ts,tsx} and exclude has a default of **/node_modules/**.

Please show this table instead as the typescript type of the options passed to `runHeadless`, with the commandline flags called out. i.e. ``` interface Options { include?: string; // (--include) [default: `src/**/*.test.{ts,tsx}`] -- Glob(s) used to discover test files ...etc... } ``` Also, I think we need an `exclude` so that we can avoid `node_modules` directories. Ideally `include` has a default of `**/*.test.{ts,tsx}` and `exclude` has a default of `**/node_modules/**`.
Author
Member

Done in 717f99f — the options table is now an interface HeadlessOptions { ... } block with each CLI flag called out inline, plus the new exclude option (default **/node_modules/**) and include default changed to **/*.test.{ts,tsx}.

While doing this, review caught a hole: a caller-supplied exclude replaced the default, which could let node_modules be walked (the exclude regexes only match directory paths, so they can't catch node_modules/** files). node_modules is now always pruned (938b663), and the README documents the include/exclude scan scope.

Done in `717f99f` — the options table is now an `interface HeadlessOptions { ... }` block with each CLI flag called out inline, plus the new `exclude` option (default `**/node_modules/**`) and `include` default changed to `**/*.test.{ts,tsx}`. While doing this, review caught a hole: a caller-supplied `exclude` *replaced* the default, which could let `node_modules` be walked (the exclude regexes only match directory paths, so they can't catch `node_modules/**` *files*). `node_modules` is now always pruned (`938b663`), and the README documents the include/exclude scan scope.
sufian marked this conversation as resolved
README.md Outdated
@ -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`). |
Owner

Do we need testrunnerUrl as an option? I don't think we should support custom test runners (yet)

Do we need `testrunnerUrl` as an option? I don't think we should support custom test runners (yet)
Author
Member

Done in 717f99f — testrunnerUrl and its --url flag are removed; the runner always resolves its own testrunner URL. (HeadlessResult.testRunnerUrl remains, now just the resolved URL.)

Done in `717f99f` — `testrunnerUrl` and its `--url` flag are removed; the runner always resolves its own testrunner URL. (`HeadlessResult.testRunnerUrl` remains, now just the *resolved* URL.)
README.md Outdated
@ -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. |
Owner

Note: this should be called --debug-failures and debugFailures

When false, this should print a message on stderr telling users they may pass --debug-failures to pause on test failure with instructions for how to connect with Chrome DevTools.

Note: this should be called `--debug-failures` and `debugFailures` When `false`, this should print a message on stderr telling users they may pass `--debug-failures` to pause on test failure with instructions for how to connect with Chrome DevTools.
Author
Member

Done in 717f99f — renamed to debugFailures / --debug-failures. When a run fails (or times out) with it off, a hint is printed to stderr pointing at --debug-failures and how to attach Chrome DevTools; the wording also names the programmatic debugFailures: true (938b663).

Done in `717f99f` — renamed to `debugFailures` / `--debug-failures`. When a run fails (or times out) with it off, a hint is printed to **stderr** pointing at `--debug-failures` and how to attach Chrome DevTools; the wording also names the programmatic `debugFailures: true` (`938b663`).
sufian marked this conversation as resolved
Author
Member

⏸ Resume notes — session paused

Pausing here. This is what is not inferable from the MR/diff.

Cross-repo role

  • This package's headless runner is what Gooey PR #1 now depends on. Gooey removed its own ci/test-headless.mjs in favour of @srhazi/gooey-test/headless; wiring Gooey's test:headless script is pending this package landing/publishing.

Environment / dev loop (not in the repo)

  • Worktree: /Users/sufian/development/2025/gooey-test-headless (self-contained node_modules; chromium installed via npx playwright install chromium).
  • Build using the direct steps in s/dist — do not run s/clean (it runs interactive git clean -i).
  • Reference consumer run:
    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-test has uncommitted changes (yours) that overlap this branch on tsconfig.json, s/dist, src/lib/index.ts — expect a small rebase/conflict when you commit them.
  • A stash@{0} (WIP on main) exists there; drop it when ready.

Packaging decisions worth remembering

  • @playwright/test is a devDependency + optional peerDependency; @types/node is a devDependency (kept out of the browser bundle via a separate Node build entry).
  • Commit author on this branch is sufian+orchestrator@gmail.com.
## ⏸ Resume notes — session paused Pausing here. This is what is **not** inferable from the MR/diff. ### Cross-repo role - This package's headless runner is what **Gooey PR #1** now depends on. Gooey removed its own `ci/test-headless.mjs` in favour of `@srhazi/gooey-test/headless`; wiring Gooey's `test:headless` script is **pending this package landing/publishing**. ### Environment / dev loop (not in the repo) - Worktree: **`/Users/sufian/development/2025/gooey-test-headless`** (self-contained `node_modules`; chromium installed via `npx playwright install chromium`). - Build using the **direct steps in `s/dist`** — do **not** run `s/clean` (it runs interactive `git clean -i`). - Reference consumer run: `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-test` has **uncommitted changes (yours)** that overlap this branch on `tsconfig.json`, `s/dist`, `src/lib/index.ts` — expect a small rebase/conflict when you commit them. - A `stash@{0}` (`WIP on main`) exists there; drop it when ready. ### Packaging decisions worth remembering - `@playwright/test` is a **devDependency + optional peerDependency**; `@types/node` is a devDependency (kept out of the browser bundle via a separate Node build entry). - Commit author on this branch is `sufian+orchestrator@gmail.com`.
Author
Member

▶ 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 HeadlessOptions with CLI flags called out; exclude added (default **/node_modules/**) and include default **/*.test.{ts,tsx}; testrunnerUrl/--url removed; keepOpenOnFailure → debugFailures/--debug-failures with a stderr hint on failure.
  • 938b663 — fix found in review: a caller-supplied exclude replaced the default, so node_modules could be walked (the exclude regexes only match directory paths and can't catch node_modules/** files). node_modules is now always pruned, and the README documents the include/exclude scan scope.

Verification

  • Self-test: node dist/headless/cli.mjs src/happypath.test.ts → PASS: 11 test(s), exit 0.
  • Full gooey suite (exercises the new default 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 . and tsc -p tsconfig.headless.json clean; 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.

## ▶ 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 HeadlessOptions` with CLI flags called out; `exclude` added (default `**/node_modules/**`) and `include` default `**/*.test.{ts,tsx}`; `testrunnerUrl`/`--url` removed; `keepOpenOnFailure` → `debugFailures`/`--debug-failures` with a **stderr** hint on failure. - `938b663` — fix found in review: a caller-supplied `exclude` replaced the default, so `node_modules` could be walked (the exclude regexes only match directory paths and can't catch `node_modules/**` files). `node_modules` is now **always** pruned, and the README documents the include/exclude scan scope. **Verification** - Self-test: `node dist/headless/cli.mjs src/happypath.test.ts` → `PASS: 11 test(s)`, exit 0. - Full gooey suite (exercises the new default `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 .` and `tsc -p tsconfig.headless.json` clean; `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 @@
## Usage
TBD
Owner

Can you please update these to briefly describe:

  • How to run a headless test?
  • How to run a test using the host browser?
  • How to debug a test failure using Chrome DevTools?
Can you please update these to briefly describe: - How to run a headless test? - How to run a test using the host browser? - How to debug a test failure using Chrome DevTools?
Author
Member

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-built node_modules/... path.

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-built `node_modules/...` path.
Author
Member

Implemented in 4d6f577 / ce4e762 (pushed). New ## Running tests section covers: headless (multiple package-manager invocations), host browser (open the runner URL served by Vite), and DevTools debugging (Chromium --remote-debugging-port; Firefox about:debugging; WebKit Safari's Develop menu / WEBKIT_INSPECTOR_SERVER; PWDEBUG=1 fallback).

Implemented in `4d6f577` / `ce4e762` (pushed). New `## Running tests` section covers: **headless** (multiple package-manager invocations), **host browser** (open the runner URL served by Vite), and **DevTools debugging** (Chromium `--remote-debugging-port`; Firefox `about:debugging`; WebKit Safari's Develop menu / `WEBKIT_INSPECTOR_SERVER`; `PWDEBUG=1` fallback).
sufian marked this conversation as resolved
README.md Outdated
@ -6,0 +20,4 @@
Run it from the command line:
```
npx gooey-test-headless [options] [test-file ...]
Owner

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.

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.
Author
Member

Naming/distribution options:

  1. Keep the bin (gooey-test-headless). After install it's on PATH inside package scripts: "test:headless": "gooey-test-headless", or directly at node_modules/.bin/gooey-test-headless. No npx needed.
  2. No bin — expose only @srhazi/gooey-test/headless and let the user script runHeadless(...). Most explicit, least magic.
  3. Rename the bin (e.g. gooey-test, gt) — cosmetic.

On npx: npx gooey-test-headless resolves 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-test installed anyway, I'd document the package-script / local-bin form and not lead with npx (or use npx --no-install). Preference on the bin name?

Naming/distribution options: 1. **Keep the `bin`** (`gooey-test-headless`). After install it's on PATH inside package scripts: `"test:headless": "gooey-test-headless"`, or directly at `node_modules/.bin/gooey-test-headless`. No npx needed. 2. **No bin** — expose only `@srhazi/gooey-test/headless` and let the user script `runHeadless(...)`. Most explicit, least magic. 3. **Rename the bin** (e.g. `gooey-test`, `gt`) — cosmetic. On npx: `npx gooey-test-headless` resolves 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-test` installed anyway, I'd document the package-script / local-bin form and not lead with npx (or use `npx --no-install`). Preference on the bin name?
Author
Member

Implemented in 4d6f577 / ce4e762 (pushed). Bin renamed to gooey-test. Docs show the direct node_modules/.bin/gooey-test, an npm script, yarn, pnpm, and npx --no-install (never auto-fetches) — plus the programmatic form.

Implemented in `4d6f577` / `ce4e762` (pushed). Bin renamed to **`gooey-test`**. Docs show the direct `node_modules/.bin/gooey-test`, an npm script, yarn, pnpm, and `npx --no-install` (never auto-fetches) — plus the programmatic form.
sufian marked this conversation as resolved
@ -0,0 +39,4 @@
return parseArgs({
args: argv,
allowPositionals: true,
options: {
Owner

Can we have short names options for all of these except: cwd, strict-port, vite-arg, vite-startup-timeout?

Can we have short names options for all of these except: cwd, strict-port, vite-arg, vite-startup-timeout?
Author
Member

Will add short flags for everything except cwd / strict-port / vite-arg / vite-startup-timeout. Proposed: -i include, -x exclude, -p port, -H host, -c config, -t timeout, -b browser, -d debug-failures, -r remote-debugging-port, -a artifacts-dir (-h is already help). Flag any letters you dislike.

Will add short flags for everything except cwd / strict-port / vite-arg / vite-startup-timeout. Proposed: `-i` include, `-x` exclude, `-p` port, `-H` host, `-c` config, `-t` timeout, `-b` browser, `-d` debug-failures, `-r` remote-debugging-port, `-a` artifacts-dir (`-h` is already help). Flag any letters you dislike.
Author
Member

Implemented in 4d6f577 / ce4e762 (pushed). Short flags: -i -x -p -H -c -t -b -d -r -a (-h is help); none for cwd / strict-port / vite-arg / vite-startup-timeout.

Implemented in `4d6f577` / `ce4e762` (pushed). Short flags: `-i -x -p -H -c -t -b -d -r -a` (`-h` is 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;
Owner

Huh, does this cause returning from the main function to exit? I don't see how this differs from the early return when result.keptOpen is true

Huh, does this cause returning from the main function to exit? I don't see how this differs from the early return when `result.keptOpen` is true
Author
Member

You'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-keptOpen path runHeadless's finally has already closed the browser + Vite, so the loop drains and Node exits with process.exitCode; in the keptOpen path we return before setting process.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 to process.exitCode = result.keptOpen ? 0 : result.exitCode; and drop the early return.

You'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-`keptOpen` path `runHeadless`'s `finally` has already closed the browser + Vite, so the loop drains and Node exits with `process.exitCode`; in the `keptOpen` path we return *before* setting `process.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 to `process.exitCode = result.keptOpen ? 0 : result.exitCode;` and drop the early return.
Author
Member

Implemented in 4d6f577 / ce4e762 (pushed). Now process.exitCode = result.keptOpen ? 0 : result.exitCode; (early return dropped). You were right that the loop wasn't draining: the Promise.race timeout timer was never cleared, so every run hung ~60s — fixed in ce4e762 (a PASS run is now ~2s).

Implemented in `4d6f577` / `ce4e762` (pushed). Now `process.exitCode = result.keptOpen ? 0 : result.exitCode;` (early return dropped). You were right that the loop wasn't draining: the `Promise.race` timeout timer was never cleared, so **every run hung ~60s** — fixed in `ce4e762` (a PASS run is now ~2s).
sufian marked this conversation as resolved
@ -0,0 +155,4 @@
return source;
}
function globToRegExp(pattern: string): RegExp {
Owner

Shouldn't we be using the standard node fs glob function? Why are we building regexp for this?

Shouldn't we be using the standard node `fs` `glob` function? Why are we building regexp for this?
Author
Member

Good question. fs.glob/fs.promises.glob only 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:

  • (a) a tiny, battle-tested glob dep (picomatch/tinyglobby) used for both matching and directory pruning;
  • (b) bump engines to Node >=22 and use the built-in glob + a brace-expansion step (or drop braces).

The hand-rolled matcher is also where the node_modules hole came from, so I lean (a). Which do you prefer?

Good question. `fs.glob`/`fs.promises.glob` only 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: - (a) a tiny, battle-tested glob dep (`picomatch`/`tinyglobby`) used for both matching and directory pruning; - (b) bump `engines` to Node >=22 and use the built-in glob + a brace-expansion step (or drop braces). The hand-rolled matcher is also where the `node_modules` hole came from, so I lean (a). Which do you prefer?
Author
Member

Implemented in 4d6f577 / ce4e762 (pushed). Deleted the hand-rolled matcher; discovery now uses Node 22 fs/promises glob with a string-array exclude (braces supported natively). engines.node corrected to >=22.17.0 in ce4e762 — array exclude needs ≥22.14 and 22.14–22.16 emit an ExperimentalWarning.

Implemented in `4d6f577` / `ce4e762` (pushed). Deleted the hand-rolled matcher; discovery now uses Node 22 `fs/promises` `glob` with a string-array `exclude` (braces supported natively). `engines.node` corrected to `>=22.17.0` in `ce4e762` — array `exclude` needs ≥22.14 and 22.14–22.16 emit an ExperimentalWarning.
sufian marked this conversation as resolved
@ -0,0 +206,4 @@
}
}
function findTestFiles(
Owner

If for some reason glob doesn'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)

If for some reason `glob` doesn'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)
Author
Member

Agreed that's a viable simplification (point at a dir/file and find *.test.ts{,x}). I'd keep include/exclude globs — familiar, matches other runners, and needed for the node_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.

Agreed that's a viable simplification (point at a dir/file and find `*.test.ts{,x}`). I'd keep `include`/`exclude` globs — familiar, matches other runners, and needed for the `node_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.
Author
Member

Implemented in 4d6f577 / ce4e762 (pushed). Kept include/exclude globs (now via the built-in glob) rather than switching to the target-dir model — say the word if you'd still prefer that.

Implemented in `4d6f577` / `ce4e762` (pushed). Kept `include`/`exclude` globs (now via the built-in glob) rather than switching to the target-dir model — say the word if you'd still prefer that.
sufian marked this conversation as resolved
@ -0,0 +391,4 @@
/**
* Resolve the testrunner URL that Vite should serve for the given project.
*
* Preference order:
Owner

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 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)
Author
Member

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:

  • ship a Vite plugin @srhazi/gooey-test/vite that (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;
  • hosted browser mode = open that URL in the consumer's vite dev;
  • headless mode = start Vite with the plugin and navigate to the same URL;
  • discovery (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?

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: - ship a Vite plugin **`@srhazi/gooey-test/vite`** that (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; - **hosted browser mode** = open that URL in the consumer's `vite dev`; - **headless mode** = start Vite with the plugin and navigate to the same URL; - discovery (`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?
sufian marked this conversation as resolved
@ -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' +
Owner

I'm assuming we should also note that other browsers may be needed?

I'm assuming we should also note that other browsers may be needed?
Author
Member

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 --browser is 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 `--browser` is needed.
Author
Member

Implemented in 4d6f577 / ce4e762 (pushed). Message now suggests npx playwright install chromium firefox webkit and notes only the --browser one is needed.

Implemented in `4d6f577` / `ce4e762` (pushed). Message now suggests `npx playwright install chromium firefox webkit` and notes only the `--browser` one is needed.
sufian marked this conversation as resolved
@ -0,0 +637,4 @@
const outcome = await Promise.race([
page
.waitForSelector('.testrunner.testrunner--pass', {
Owner

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)

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)
Author
Member

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 existing data-tests-* summary attrs. I'll migrate the runner to stable data-* 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.

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 existing `data-tests-*` summary attrs. I'll migrate the runner to stable `data-*` 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.
sufian marked this conversation as resolved
@ -0,0 +661,4 @@
if (outcome === 'pass') {
const testCount = summary.testsPassed;
const fileCount = summary.filesPassed;
log(
Owner

How much work would it take to improve the output of the headless runner so it shows only as much information as needed?

  • In the most dense mode (or when stdout is not a tty), I only want output on test failure or on
  • In the standard mode (when stdout is a tty), I want a single line formatted with <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 at COLS so wrapping does not occur (so it's all on one line in the terminal).
  • In verbose mode we print a section for each test as well as console logs/errors/warnings (using different colors)

(Note: this doesn't need to happen in this first PR, we could follow-up with this in our todo list)

How much work would it take to improve the output of the headless runner so it shows only as much information as needed? - In the most dense mode (or when stdout is not a tty), I only want output on test failure or on - In the standard mode (when stdout is a tty), I want a single line formatted with `<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 at `COLS` so wrapping does not occur (so it's all on one line in the terminal). - In verbose mode we print a section for each test as well as console logs/errors/warnings (using different colors) (Note: this doesn't need to happen in this first PR, we could follow-up with this in our todo list)
Author
Member

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.

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.
sufian marked this conversation as resolved
@ -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})`
Owner

Is there an equivalent for Firefox and Safari? It'd be extra helpful to point to all of those.

Is there an equivalent for Firefox and Safari? It'd be extra helpful to point to all of those.
Author
Member

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.

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.
Author
Member

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.

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.
sufian marked this conversation as resolved
sufian left a comment

Please respond to all comments

Please respond to all comments
Author
Member

▶ 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:
    • Glob → Node 22 built-ins. Deleted the hand-rolled matcher; discovery now uses fs/promises glob (native braces) with a string-array exclude.
    • Bin renamed to gooey-test, with README invocations for npm script / node_modules/.bin / yarn / pnpm / npx --no-install (never auto-installs) / programmatic.
    • Short flags -i -x -p -H -c -t -b -d -r -a (all except cwd/strict-port/vite-arg/vite-startup-timeout).
    • Browser-aware debugging (Chromium CDP; Firefox about:debugging; WebKit Safari inspector) + broadened browser-install message.
    • New ## Running tests section (headless / host-browser / debugging); --remote-debugging-port is Chromium-only.
  • ce4e762 — review fixes:
    • engines.node → >=22.17.0 (array exclude needs ≥22.14; 22.14–22.16 warn); .nvmrc → 22.17.0.
    • Fixed a 60-second hang: the Promise.race timeout timer was never cleared, so every run (pass or fail) waited the full timeoutMs before exiting. Now cleared/unref'd.
    • DevTools hint no longer points at a dead localhost:9222 when no debugging port was opened; README accuracy nits.

Verification

  • Self-test node dist/headless/cli.mjs src/happypath.test.ts → PASS 11, ~2s (was 61s).
  • Full gooey suite (--cwd .../gooey-identity --strict-port --exclude 'src/performance.test.tsx') → PASS 523 / 20, ~7s.
  • Timeout path (--timeout 3000 on a hanging test) → exit 2 in ~3s.
  • tsc clean 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.

## ▶ 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: - **Glob → Node 22 built-ins.** Deleted the hand-rolled matcher; discovery now uses `fs/promises` `glob` (native braces) with a string-array `exclude`. - **Bin renamed to `gooey-test`**, with README invocations for npm script / `node_modules/.bin` / yarn / pnpm / `npx --no-install` (never auto-installs) / programmatic. - **Short flags** `-i -x -p -H -c -t -b -d -r -a` (all except cwd/strict-port/vite-arg/vite-startup-timeout). - **Browser-aware debugging** (Chromium CDP; Firefox `about:debugging`; WebKit Safari inspector) + broadened browser-install message. - New `## Running tests` section (headless / host-browser / debugging); `--remote-debugging-port` is Chromium-only. - **`ce4e762`** — review fixes: - **`engines.node` → `>=22.17.0`** (array `exclude` needs ≥22.14; 22.14–22.16 warn); `.nvmrc` → 22.17.0. - **Fixed a 60-second hang:** the `Promise.race` timeout timer was never cleared, so *every* run (pass or fail) waited the full `timeoutMs` before exiting. Now cleared/unref'd. - DevTools hint no longer points at a dead `localhost:9222` when no debugging port was opened; README accuracy nits. **Verification** - Self-test `node dist/headless/cli.mjs src/happypath.test.ts` → PASS 11, **~2s** (was 61s). - Full gooey suite (`--cwd .../gooey-identity --strict-port --exclude 'src/performance.test.tsx'`) → **PASS 523 / 20**, ~7s. - Timeout path (`--timeout 3000` on a hanging test) → exit 2 in ~3s. - `tsc` clean 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.
Author
Member

▶ Smoke test + README fixes pushed (head 8c883ff)

I manually verified every README instruction in a throwaway node:22-bookworm-slim container, 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.sh packs the repo, builds a cached CI image (Playwright browsers preinstalled) and runs it.
  • 5caf962 — fixes the harness caught:
    • pnpm bug: resolveVite() ran node node_modules/.bin/vite; pnpm's .bin/vite is a /bin/sh cmd-shim, so it threw. Now resolves Vite's real JS bin from its package.json.
    • Vite ≥5 bug: Vite wraps Local: and the URL in ANSI SGR escapes (even on a pipe under CI=1), so the startup URL regex never matched → every run timed out. Added stripAnsi.
    • README install steps corrected (vite, @srhazi/gooey peer, --with-deps), hosted #-fragment convention, one-package-manager caveat.
  • 8c883ff — review follow-ups: loosened the @srhazi/gooey peer to ^0.25.0 (the README's unpinned install conflicted with the exact 0.25.0 pin while latest is 0.25.2); the smoke now installs the README's exact command (so 0.25.2 is exercised); made the node_modules-prune assertion meaningful; stripAnsi on 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; tsc clean; no leftover containers/processes.

Run it with ./s/smoke (Docker required; no npm publish/auth involved — it uses a local npm pack).

## ▶ Smoke test + README fixes pushed (head `8c883ff`) I manually verified every README instruction in a throwaway `node:22-bookworm-slim` container, 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.sh` packs the repo, builds a cached CI image (Playwright browsers preinstalled) and runs it. - **`5caf962`** — fixes the harness caught: - **pnpm bug**: `resolveVite()` ran `node node_modules/.bin/vite`; pnpm's `.bin/vite` is a `/bin/sh` cmd-shim, so it threw. Now resolves Vite's real JS bin from its `package.json`. - **Vite ≥5 bug**: Vite wraps `Local:` and the URL in ANSI SGR escapes (even on a pipe under `CI=1`), so the startup URL regex never matched → every run timed out. Added `stripAnsi`. - README install steps corrected (`vite`, `@srhazi/gooey` peer, `--with-deps`), hosted `#`-fragment convention, one-package-manager caveat. - **`8c883ff`** — review follow-ups: loosened the `@srhazi/gooey` peer to `^0.25.0` (the README's unpinned install conflicted with the exact `0.25.0` pin while latest is 0.25.2); the smoke now installs the README's **exact** command (so 0.25.2 is exercised); made the `node_modules`-prune assertion meaningful; `stripAnsi` on 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; `tsc` clean; no leftover containers/processes. Run it with `./s/smoke` (Docker required; no npm publish/auth involved — it uses a local `npm pack`).
sufian merged commit 127135f5fc into main 2026-10-12 03:53:10 +00:00
sufian deleted branch wip/headless-runner 2026-10-12 03:53:11 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
legend/gooey-test!1
No description provided.