Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,22 @@ Semver applies from 1.0.0. A breaking change to a documented API needs a major

## [Unreleased]

Nothing yet.
### Fixed

- **A gate step whose suite executed nothing reports as skipped, never as passed** (#434). `x verify`
printed `✓ e2e 46ms` and `"skipped": false` over the one `e2eTest` a scaffold writes, which
`test.skip`s itself until the app registers a browser driver — so a customer's green gate included
a lane that had never run, and nothing in the table or in `--json` said so. `bun test` exits 0 over
an all-skipped file, but it prints its own counts and `packages/cli/src/test-counts.ts` was already
reading them: the rule is now `tests.ran === 0` in `packages/cli/src/verify-run.ts`, read once and
answered two ways — a step the committed `x.verify.json` requires stays the `X_VERIFY_SUITE_VANISHED`
failure it already was, and a step no floor requires is a skip beside `roadmap`'s, counted apart
from the passes in the summary line. A missing browser driver is a skip rather than an error code
because the framework ships no browser by design (`packages/scraping/src/cdp-port.ts`: the app
installs `puppeteer-core`, the CLI asks for it) — unlike Bun's own floor, which is `X_BUN_VERSION`
and a refusal. `StepResult.tests` now reaches `--json` and the human line, so `- e2e found 1
test(s) and every one skipped itself` is distinguishable from `- roadmap`, which has no suite at
all.

## 20.1.0 - 2026-09-12

Expand Down
9 changes: 6 additions & 3 deletions packages/cli/src/cmd-verify.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,15 @@ describe('unit · x verify', () => {
});
});

// The floor is the claim; without one there is nothing to be measured against, and a repo that
// never committed one is not ratcheted in either direction.
test('with no floor, an all-skipped suite stays the honest pass it was', async () => {
// The floor is the claim; without one there is nothing to be measured against, so nothing is
// FAILED here. It is still not a pass: a suite that executed nothing is the same "nothing to
// check" a step with no files reports, and issue #434 is what a green check over it costs.
test('with no floor, an all-skipped suite is a skip rather than a pass', async () => {
await withFloor(undefined, async (root) => {
const result = await runVerify([...green, allSkipped], { ...ctx, root });
expect(result.ok).toBe(true);
expect(result.data).toMatchObject({ failed: [], skipped: ['live'] });
expect(result.steps?.find((step) => step.name === 'live')?.skipped).toBe(true);
});
});

Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/e2e-driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ export const e2eFixtures = (page: PageLike, browser: E2eBrowserPage): E2eFixture
* declaration `driverFixtures()` registered — the ordinary way a driver arrives, last registration
* wins — so every `test('…', async ({ page }) => …)` in the suite gets a browser. `useE2eDriver`
* is the other half: it is what makes `hasE2eDriver()` answer true and stops `e2eTest` becoming a
* `test.skip` a green gate reports over.
* `test.skip` — which the gate now reports as a SKIPPED step rather than the green check it
* printed until #434, and which a repo whose `x.verify.json` names `e2e` gets red for.
*
* `budget`, `signIn` and `deploy` are deliberately NOT registered here. Each needs something a
* page cannot supply — byte counts off a built `dist/`, an app's own sign-in route, a second build
Expand Down
8 changes: 8 additions & 0 deletions packages/cli/src/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,14 @@ const CATALOG = {
// The `errors` step's own coverage, in `output`: a scan without a parser reads most fix lines
// and not all of them, and a step that reports findings alone claims a completeness it lacks.
'cli.verify.fixCoverage': 'checked {checked} fix line(s), could not read {unreadable}',
// Why a skipped step is a dash when the step can say so: the suite exists and not one of its
// tests ran. Each test's own name carries the reason it skipped, and `bun test` prints only the
// counts — so the line says how many and `x test <type>` prints the names.
'cli.verify.allSkipped': 'found {skipped} test(s) and every one skipped itself',
// The other shape the same counts can take: a suite that reported no test at all, skipped ones
// included. Its own line and not `allSkipped` with a zero, because the repair differs — there is
// no skipped test to read a reason off, so the files the step selected hold no test.
'cli.verify.ranNothing': 'found no test to run in the file(s) it selected',
'cli.verify.serial': 'serial',
'cli.verify.workers': '{workers} workers',
'cli.env.checked': '{count} declared variable(s), all present and valid',
Expand Down
22 changes: 22 additions & 0 deletions packages/cli/src/output.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,28 @@ describe('unit · renderHuman says how a step was run', () => {
expect(skipped).toContain('- unit');
});

// #434: the whole point of reporting a zero-run suite as a skip is that a reader can tell it
// from a step with no suite at all, and the two shapes the counts take are two different
// repairs — read a skipped test's name, or find out why the files hold no test.
test('a skipped step that ran a suite says which kind of skip it is', () => {
const line = (tests: StepResult['tests']): string =>
renderHuman({
ok: true,
command: 'verify',
summary: 'ok',
steps: [
stepResult({ name: 'e2e', skipped: true, ...(tests === undefined ? {} : { tests }) }),
],
});
expect(line({ ran: 0, skipped: 1 })).toContain(msg('cli.verify.allSkipped', { skipped: 1 }));
expect(line({ ran: 0, skipped: 0 })).toContain(msg('cli.verify.ranNothing'));
expect(line({ ran: 0, skipped: 0 })).not.toContain(
msg('cli.verify.allSkipped', { skipped: 0 }),
);
// A step with no suite — `roadmap` — has nothing to say beyond the dash.
expect(line(undefined).split('\n')[0]?.trim()).toBe('- e2e 40ms');
});

test('a step with no workers field says nothing either', () => {
expect(
renderHuman({ ok: true, command: 'verify', summary: 'ok', steps: [stepResult({})] }),
Expand Down
30 changes: 29 additions & 1 deletion packages/cli/src/output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import { ERROR_DOCS_URL, renderThrowable, singleLine, stringField } from '@ultimat3/core';
import { msg } from './messages';
import type { TestCounts } from './test-counts';

export interface Finding {
readonly code: string;
Expand All @@ -24,6 +25,13 @@ export interface StepResult {
readonly output?: string;
/** Worker processes the step used; `1` means it ran serially. Absent for a non-test step. */
readonly workers?: number;
/**
* What the step's suite executed. Absent for a step that spawned no test process, which is NOT
* the same state as `{ ran: 0 }` — a step with no suite and a suite whose every test skipped
* itself both report `skipped`, and this is the only thing that tells a reader which one it is
* looking at (#434).
*/
readonly tests?: TestCounts;
}

export type JsonValue =
Expand Down Expand Up @@ -164,10 +172,27 @@ const width = (step: StepResult): string => {
return ` ${step.workers === 1 ? msg('cli.verify.serial') : msg('cli.verify.workers', { workers: step.workers })}`;
};

/**
* Why a step is a dash, when the step itself can say. `- roadmap` is "there is nothing here to
* check"; a step that spawned a suite and executed none of it is a different state, which is what
* issue #434 reported as a green check — so a step carrying counts always says which it is, in one
* of the two shapes its counts can take. The reason each test skipped lives in that test's own
* NAME — `bun test` prints only the counts — so the line points at the suite and `x test <type>`
* is what prints the names.
*/
const why = (step: StepResult): string => {
const tests = step.tests;
if (step.skipped !== true || tests === undefined) return '';
if (tests.skipped === 0) return ` ${msg('cli.verify.ranNothing')}`;
return ` ${msg('cli.verify.allSkipped', { skipped: tests.skipped })}`;
};

export function renderHuman(result: CommandResult, verbose = false): string {
const out: string[] = [];
for (const step of result.steps ?? []) {
out.push(` ${mark(step)} ${step.name.padEnd(18)} ${step.durationMs}ms${width(step)}`);
out.push(
` ${mark(step)} ${step.name.padEnd(18)} ${step.durationMs}ms${width(step)}${why(step)}`,
);
for (const finding of step.findings) out.push(renderFinding(finding, ' '));
// NOT escaped, and that is the one exception: `output` is this process's own captured
// subprocess stdout — `bun test`'s colour is the reason a human reads it at all, and it is
Expand Down Expand Up @@ -196,6 +221,9 @@ export function renderJson(result: CommandResult): string {
skipped: step.skipped === true,
findings: step.findings,
...(step.workers === undefined ? {} : { workers: step.workers }),
// The counts the human line's `why()` renders, as numbers: a `--json` reader deciding whether
// a skipped lane is missing a suite or missing a prerequisite needs the same fact CI's log has.
...(step.tests === undefined ? {} : { tests: step.tests }),
// A FAILED step carries its captured stdout, exactly as the human renderer prints it. CI runs
// `--json`, and without this the log said only "one or more unit tests failed" with a generic
// fix line — the failing test's name and its assertion diff existed and were thrown away, so
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/templates/scaffold-repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,9 @@ const biome = (): string => `{
*
* Two remain absent. `e2e` has a scaffolded file and it is an `e2eTest` — `test.skip` until the app
* registers a browser driver, so the step would run zero tests and fail the ratchet on the
* scaffold's own placeholder. `contract-diff` needs a committed `x.manifest.json`, which
* scaffold's own placeholder. Its absence is visible rather than silent: a suite that executed
* nothing reports as a SKIPPED step, not the green check it printed until #434, so the app's own
* gate line says `- e2e` and names it in the summary until a driver makes it run. `contract-diff` needs a committed `x.manifest.json`, which
* `x manifest` writes later. Each joins the list in the commit that makes the app's gate run it.
*/
const SCAFFOLD_FLOOR: readonly VerifyStepName[] = [
Expand Down
84 changes: 83 additions & 1 deletion packages/cli/src/verify-run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import { tmpdir } from 'node:os';
// why: Bun exposes no path-join primitive; Bun.file and import() take one already joined.
import { join } from 'node:path';
import { msg } from './messages';
import { exitCodeFor } from './output';
import { exitCodeFor, renderHuman, renderJson } from './output';
import { VERIFY_FLOOR_FILE } from './verify-floor';
import { runVerify } from './verify-run';
import type { VerifyContext, VerifyStep } from './verify-step';
import { resetTestDiscovery, TEST_STEPS } from './verify-tests';

/** The banner a narrowed run carries, from the catalog that renders it — never a second literal. */
const NOT_A_GATE_RUN = msg('cli.verify.notAGateRun', { summary: '' }).trim();
Expand Down Expand Up @@ -169,3 +170,84 @@ describe('skips are counted apart from passes, and named', () => {
expect(result.data).toMatchObject({ failed: ['drift'], skipped: ['e2e'] });
});
});

// Issue #434: `x verify`'s `e2e` step printed `✓ e2e 46ms` and `"skipped": false` over a suite
// whose only test skipped itself, because `bun test` exits 0 on a skip and the exit code was the
// only thing the step's `ok` was read from. A gate that cannot tell a lane that ran from a lane
// that did not is the one failure the step table exists to prevent, and it is the SAME question
// the floor already answers one layer up (`X_VERIFY_SUITE_VANISHED`): the counts, not the code.
describe('a suite that executed nothing is a skip, never a pass', () => {
// Bun's own summary for the scaffold's one e2e test on a box with no browser driver, verbatim.
// The reason lives in the test's NAME, which bun does not print — so the counts are the only
// channel, and this is exactly the output the gate had to read to be wrong about it.
const ALL_SKIPPED = ' 0 pass\n 1 skip\n 0 fail\nRan 1 test across 1 file. [46.00ms]';
const ONE_RAN = ' 1 pass\n 0 skip\n 0 fail\nRan 1 test across 1 file. [46.00ms]';

const suite =
(stdout: string): VerifyContext['runner'] =>
async (command) => ({
command,
code: 0,
ok: true,
stdout,
stderr: '',
durationMs: 46,
});

/** The real `e2e` step, over a root holding the file the scaffold writes for a new route. */
const e2eOver = async (stdout: string, root: string) => {
await Bun.write(join(root, 'apps/web/app/posts/page.e2e.test.ts'), '// e2e\n');
resetTestDiscovery();
const step = TEST_STEPS.find((candidate) => candidate.name === 'e2e');
if (step === undefined) return expect.unreachable('the e2e step is registered');
return runVerify([...STEPS.slice(0, 2), step], { root, runner: suite(stdout) });
};

const inTempRoot = async (run: (root: string) => Promise<void>): Promise<void> => {
const root = await mkdtemp(join(tmpdir(), 'x-verify-e2e-'));
try {
await run(root);
} finally {
resetTestDiscovery();
await rm(root, { recursive: true, force: true });
}
};

test('the step reports itself skipped, the way roadmap does', async () => {
await inTempRoot(async (root) => {
const result = await e2eOver(ALL_SKIPPED, root);
const e2e = result.steps?.find((step) => step.name === 'e2e');
expect(e2e?.skipped).toBe(true);
expect(dataOf(result.data).skipped).toEqual(['e2e']);
});
});

// Both renderers, because `--json` is what CI reads and the check mark is what a human reads,
// and the defect was visible in each.
test('neither renderer claims it passed', async () => {
await inTempRoot(async (root) => {
const result = await e2eOver(ALL_SKIPPED, root);
expect(result.summary).toContain('1 skipped: e2e');
expect(renderHuman(result)).toContain('- e2e');
expect(renderHuman(result)).not.toContain('✓ e2e');
const payload = JSON.parse(renderJson(result)) as {
readonly steps: readonly { readonly name: string; readonly skipped: boolean }[];
};
expect(payload.steps.find((step) => step.name === 'e2e')).toMatchObject({
skipped: true,
ok: true,
tests: { ran: 0, skipped: 1 },
});
});
});

// The other side of the enumeration: this may not turn a suite that ran into a skip.
test('one test that actually ran keeps the step a pass', async () => {
await inTempRoot(async (root) => {
const result = await e2eOver(ONE_RAN, root);
expect(result.steps?.find((step) => step.name === 'e2e')?.skipped).not.toBe(true);
expect(dataOf(result.data).skipped).toEqual([]);
expect(result.ok).toBe(true);
});
});
});
17 changes: 13 additions & 4 deletions packages/cli/src/verify-run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,29 @@ export async function runVerify(
findings: [findingOf(error, step.name)],
}),
);
// A suite that executed nothing did not run, whatever its exit code says: `bun test` exits 0
// over an all-skipped file, so the counts are the only channel that can tell the two apart.
// ONE definition of "nothing ran", read twice, because the floor decides which of the two
// things it means — exactly as it already does for a step whose `applies` said no.
const tests = outcome.tests;
const nothingRan = tests !== undefined && tests.ran === 0;
const required = floorRequires(floor, step.name);
// A step the floor requires whose suite executed nothing is the same vanished suite as a step
// with no files at all — the run just had to finish before it could be seen. Appended to the
// step's own findings so `data.failed`, the counts and every gate reading this table carry it.
const vanished =
floorRequires(floor, step.name) && outcome.tests !== undefined && outcome.tests.ran === 0
? [skippedSuiteFinding(step.name, outcome.tests.skipped)]
: [];
const vanished = nothingRan && required ? [skippedSuiteFinding(step.name, tests.skipped)] : [];
results.push({
name: step.name,
ok: outcome.ok && vanished.length === 0,
durationMs: Math.round(performance.now() - started),
// Without a floor to require it, a suite that ran nothing is a SKIP and not a pass (#434):
// the `e2e` step printed `✓ e2e 46ms` over its one skipped test, which is the one thing a
// step table may never do — a reader cannot tell a lane that ran from a lane that did not.
skipped: nothingRan && !required,
findings: [...outcome.findings, ...vanished],
...(outcome.output === undefined ? {} : { output: outcome.output }),
...(outcome.workers === undefined ? {} : { workers: outcome.workers }),
...(tests === undefined ? {} : { tests }),
});
}
const failedSteps = results.filter((step) => !step.ok).map((step) => step.name);
Expand Down
15 changes: 8 additions & 7 deletions packages/testing/src/test-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,14 @@ let e2eDriver: ((name: string, body: E2eBody) => void) | undefined;
* Register the browser-backed driver. Without one, `e2eTest` skips loudly — the skipped test's own
* NAME carries the reason and the command that would build what it drives.
*
* What the gate then reports is a **pass over an all-skipped suite**, and that is stated here
* rather than claimed away: this used to say "`x verify` reports the step as skipped rather than
* green", which nothing implements. The step shells out to `bun test`, `bun test` exits 0 on a
* skip, and an exit code is the only channel between the two — the driver is registered inside the
* CHILD process, so the step cannot ask. Closing it means a channel the step can read, which is a
* design decision and not a docstring. `As of 2026-08` there are zero registered drivers, so every
* `e2eTest` in the tree is a skip; the framework's own `e2e` suites use plain `bun:test`.
* The gate's rule over that skip: a step whose suite reports `ran === 0` is SKIPPED, unless the
* repo's `x.verify.json` requires the step — then it is `X_VERIFY_SUITE_VANISHED`
* (`packages/cli/src/verify-run.ts`). Never a pass either way. This block used to say the opposite
* and argue for it — that an exit code was the only channel, so the gate could not tell — which
* `packages/cli/src/test-counts.ts` had already disproved by reading `bun test`'s own counts (#434).
*
* `As of 2026-08` there are zero registered drivers, so every `e2eTest` in the tree is a skip; the
* framework's own `e2e` suites use plain `bun:test`.
*/
export function useE2eDriver(driver: (name: string, body: E2eBody) => void): void {
e2eDriver = driver;
Expand Down