Skip to content

test: split unit tests into node:test suite and add coverage #267

@robertsLando

Description

@robertsLando

Sub-issue of #235.

Problem

test/test-XX-*/ is advertised as the e2e suite, but several tests are actually in-process unit tests — they require('../../lib-es5/...') directly and assert against pure functions without ever spawning the pkg CLI or producing a binary. Examples found on main:

  • test-00-sea-picker/main.js — tests pickMatchingHostTargetIndex in lib/sea.ts.
  • test-48-common/main.js — tests path/snapshot helpers in lib/common.ts.
  • test-50-config-parse/main.js — tests config + compress-type parsing; uses an ad-hoc in-process t() harness.

Lumping these in with the real e2e suite:

  • pays the per-directory process-spawn cost for trivial assertions,
  • mixes "lib regression" with "build regression" under one runner,
  • makes it hard to add many small, fast unit tests without polluting the e2e list,
  • and we have zero coverage visibility today — no c8, no NODE_V8_COVERAGE, no --experimental-test-coverage anywhere in the repo or CI.

Proposal

Phase 1 — new unit suite on node:test

  • Add test/unit/ with files using the built-in node:test runner. No new runtime deps — we're already on Node >= 22.
  • Add scripts:
    • yarn test:unitnode --test --test-reporter=spec test/unit
    • yarn test:unit:watchnode --test --watch test/unit
  • Migrate the existing unit-in-disguise tests into test/unit/ as *.test.ts files (loaded via esbuild-register, the same mechanism DEV=true already uses for the CLI):
    • test-00-sea-pickertest/unit/sea-picker.test.ts
    • test-48-commontest/unit/common.test.ts
    • test-50-config-parsetest/unit/config-parse.test.ts
  • Remove the old directories once migrated and update any exclusion lists in test/test.js.

First unit-test targets beyond the migration (pure-ish modules, no filesystem/network):

Phase 2 — coverage across unit + e2e (the tricky part)

The pkg CLI runs as a child process under the e2e suite (utils.pkg.syncspawn('node', ['lib-es5/bin.js', …])), so a coverage tool that only wraps the test harness sees nothing interesting. Node's NODE_V8_COVERAGE env var is the right primitive: it's honoured by every child process that inherits it — exactly what we need. test/utils.js's utils.pkg.sync already spreads ...process.env into the spawned CLI's env, so the pkg subprocess will emit coverage with zero code changes the moment the var is set upstream.

The open question is what reports over the resulting V8 JSON dump.

Option A — node:test built-in coverage (zero-dep, preferred long-term). node --test --experimental-test-coverage --test-reporter=lcov gathers V8 coverage itself and supports --test-coverage-include=lib/** / --test-coverage-exclude. When enabled, Node sets NODE_V8_COVERAGE internally, so even subprocess coverage is captured. Fits the unit suite perfectly.

The snag is the e2e half: test/test.js is a custom harness, not a node:test run, so the built-in coverage machinery doesn't wrap it. Two sub-options:

  • A1. Set NODE_V8_COVERAGE=coverage/e2e manually on the test.js harness, then stitch together a reporter (roll a ~100 LOC merger, or wait for Phase 3 to migrate e2e onto node:test so the built-in reporter covers everything).
  • A2. Defer unified coverage until Phase 3 lands — ship unit coverage alone first.

Option B — c8 as a thin reporter over V8 dumps (pragmatic short-term). Adds c8 as a devDependency; it wraps NODE_V8_COVERAGE, merges per-process coverage, and reports. Scripts:

  • yarn coverage:unitc8 --reporter=text --reporter=lcov node --test test/unit
  • yarn coverage:e2ec8 --reporter=text --reporter=lcov --clean=false node test/test.js node22 no-npm (--clean=false so unit + e2e accumulate in the same dir).
  • yarn coverage → runs both, then c8 report over the merged coverage/tmp dir.

Add a .c8rc.json with include: ["lib/**"], exclude: ["lib-es5/**", "test/**", "prelude/**", "**/*.d.ts"], all: true.

Recommendation: start with Option B (c8) because it's the only off-the-shelf merger that bridges the current custom e2e harness to V8 output. Once Phase 3 migrates e2e to node:test, revisit and drop c8 in favour of Option A's built-in reporter. Worst case: we keep c8 long-term — it's ~50 KB, no runtime impact, and does one thing well.

Known caveats (apply to either option):

  • SEA binaries embed V8 bytecode; coverage from inside a packaged binary won't map back to lib/ source. That's fine — we only care about coverage of the build process (lib/bin.ts and its imports), not of the snapshot runtime.
  • prelude/bootstrap.js runs inside the packaged binary; exclude it from coverage targets.
  • Worker-thread bootstrap forks another Node process — it inherits env, so coverage should propagate, but needs a sanity check.
  • CI: upload the merged lcov.info as a workflow artifact. Wiring Codecov / Coveralls is a follow-up, out of scope here.

Phase 3 — stretch: migrate e2e to node:test

Rewrite test/test.js as a node:test entrypoint that discovers test/e2e/**/*.test.js. Benefits: one runner, consistent reporter, built-in --concurrency, and free --test-shard support for CI parallelism (which #241 touches). Each current test-XX-*/main.js becomes a test('name', async (t) => { … }) block. Bonus: unlocks Option A in Phase 2 — built-in coverage across the board, c8 can go away. Non-trivial refactor, so broken out of Phase 1 to keep Phase 1 shippable. If it grows further, split into its own sub-issue.

Acceptance criteria

  • yarn test:unit runs the unit suite on node:test; no new runtime deps.
  • The three identified unit-in-disguise tests are migrated and their test-XX-*/ directories removed.
  • yarn coverage produces a merged lcov report covering both unit and e2e runs.
  • Coverage baseline for lib/ is recorded in the implementing PR's description so we can see it grow over time.
  • CI runs the unit suite on every PR (cheap) and the full coverage job on-label or nightly (expensive — full e2e).
  • (Phase 3, optional) e2e migrated to node:test, or a separate sub-issue filed to track it. When this lands, revisit Phase 2 and drop c8 in favour of the built-in coverage reporter.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions