mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-04 15:57:33 +01:00
fix(pregate): a declared runner gets only the files its tool runs (GST-1331) (#3986)
* fix(pregate): a declared runner gets only the files its tool runs (GST-1331)
A set runner such as `bun test {files}` was handed every selected test,
including *_test.py files: alone bun exits 1 (PREGATE_LANE_FAILED), and
beside a TS test it skips them silently while the lane counts them as run.
The declared runner now takes the files its tool runs by extension (JS/TS
for bun, node, deno, vitest, jest, mocha, ava, playwright; .py for pytest
or python; every file for any other tool), and the lanes detail names each
file it cannot run as not run and not counted as covered.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* fix: pre-landing review fixes
Review-Finding: p1/specialist-batched#2 reader=testing
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* fix: recognise test commands, not interpreters, in a declared runner
Review-Finding: p1/outside-voice#2 reader=outside-voice
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
---------
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
a66d2f460d
commit
ca90f0ff2d
4 files changed
+76
-7
No files matched your search
@@ -0,0 +1 @@
|
||||
- **A declared test runner only gets the files it can run.** With `runner: "bun test {files}"` in `.gstack/pregate.json`, pregate used to hand every selected test to bun, `*_test.py` files included. If only Python files were selected, bun exited 1 and the lane failed (PREGATE_LANE_FAILED). If a TS test was selected too, bun skipped the Python files without saying so, and they were counted as run. Now the local lane gives a declared runner only the files its tool runs, judged by extension: JS/TS for `bun test`, `node --test`, vitest, jest and similar, and `.py` for pytest. A custom dispatcher such as `node scripts/run-tests.js {files}` still gets every file. The lanes detail names each file it left out as not run and not counted as covered. When no runner is set, tests are still split by language as before. (GST-1331)
|
||||
+1
-1
@@ -74,7 +74,7 @@ the byte-pinned `/ship` renders in `test/fixtures/golden/`).
|
||||
|
||||
| Key | Meaning |
|
||||
|---|---|
|
||||
| `runner` | the local test command with `{files}` (gstack: `bun test --timeout=30000 {files}`, the 30 s per-test timeout CI uses); unset, it is detected (bun, vitest or jest for JS/TS files, pytest when python3 can import it (else `pytest` or `python3` is named missing), `node --test`, and `flutter test`/`dart test` per Dart package root, through fvm when that package or an ancestor pins it with `.fvmrc` or `.fvm/fvm_config.json` and fvm is on PATH; `cargo test -p <name>` per Rust crate, run from the root of the workspace the crate is a member of (Cargo's `members` globs, `exclude` and `package.workspace`), else the crate directory; a JS/TS test whose nearest package.json is a nested package declaring vitest or jest (a dependency or its `test` script) runs from that package, with package-relative paths, through the package manager its `packageManager` field or the nearest lockfile up to the repo root names: `pnpm exec`, `yarn`, `bun x`, else `npx`; a JS/TS test file that imports `@playwright/test` at the start of a line runs as `playwright test --reporter=list` from its nearest package through that same package manager, whatever the unit runner is, a type-only import (`import type …`, or one whose every specifier is `type X`) does not count, file arguments are passed as escaped regexes anchored to the path (Playwright reads them as regexes), and a run where every test fails and Playwright says its browser is not installed is `PREGATE_ENVIRONMENT` (a partial install, where some tests pass, stays the diff's failure); every other test keeps the root detection, split by language when no one runner fits them all, so `.mjs` and `.py` root tests each get their own runner); nothing detected, a Dart SDK, cargo or package manager missing from PATH, a `.dart` file with no `pubspec.yaml` above it, a `.rs` file outside any crate, or one in a crate whose name Cargo would reject (anything but letters, digits, `_` and `-`), is `incomplete` (the runnable groups still run, so a real failure still fails) |
|
||||
| `runner` | the local test command with `{files}` (gstack: `bun test --timeout=30000 {files}`, the 30 s per-test timeout CI uses); set, the local lane gives it only the selected files its tool runs, judged by extension (JS/TS files for `bun test`, `node --test`, `deno test`, vitest, jest, mocha, ava or playwright; `.py` files for pytest; each matched as a whole word, so `npx vitest run` counts and `tests/node/run.sh` does not; every file for any other command, such as a `node scripts/run-tests.js` dispatcher, or one naming both), and the lanes detail names each file it cannot run as not run and not counted as covered, so a `*_test.py` never reaches a local `bun test` (no Python lane runs beside a declared runner; the remote gate and `test_backend` commands still receive the whole selection); unset, it is detected (bun, vitest or jest for JS/TS files, pytest when python3 can import it (else `pytest` or `python3` is named missing), `node --test`, and `flutter test`/`dart test` per Dart package root, through fvm when that package or an ancestor pins it with `.fvmrc` or `.fvm/fvm_config.json` and fvm is on PATH; `cargo test -p <name>` per Rust crate, run from the root of the workspace the crate is a member of (Cargo's `members` globs, `exclude` and `package.workspace`), else the crate directory; a JS/TS test whose nearest package.json is a nested package declaring vitest or jest (a dependency or its `test` script) runs from that package, with package-relative paths, through the package manager its `packageManager` field or the nearest lockfile up to the repo root names: `pnpm exec`, `yarn`, `bun x`, else `npx`; a JS/TS test file that imports `@playwright/test` at the start of a line runs as `playwright test --reporter=list` from its nearest package through that same package manager, whatever the unit runner is, a type-only import (`import type …`, or one whose every specifier is `type X`) does not count, file arguments are passed as escaped regexes anchored to the path (Playwright reads them as regexes), and a run where every test fails and Playwright says its browser is not installed is `PREGATE_ENVIRONMENT` (a partial install, where some tests pass, stays the diff's failure); every other test keeps the root detection, split by language when no one runner fits them all, so `.mjs` and `.py` root tests each get their own runner); nothing detected, a Dart SDK, cargo or package manager missing from PATH, a `.dart` file with no `pubspec.yaml` above it, a `.rs` file outside any crate, or one in a crate whose name Cargo would reject (anything but letters, digits, `_` and `-`), is `incomplete` (the runnable groups still run, so a real failure still fails) |
|
||||
| `dependencies` | glob → test files for subprocess targets, runtime reads and generated inputs the import graph cannot see; `[]` declares "no lane needed" (docs, licences). A `[]` that matches a deleted test does not skip it: the deleted test still runs its owning suite, and a file naming it that `[]` covers (a durations list) counts as covered. With this key present, even empty, the policy [declares lanes](#lane-requirements) |
|
||||
| `empty_lanes` | extra globs for the docs/no-tests empty lane. Any Markdown file already qualifies with no pin file when no test depends on it (the same import graph as other files: a quoted path, a `join()` of path segments, an import, or a `dependencies` entry). reStructuredText and AsciiDoc count as Markdown here. README, CHANGELOG, CONTRIBUTING, LICENSE, AGENTS and CLAUDE qualify even without `.md`, and so does licence text under any vendored name (`roboto-LICENSE.txt`, `LICENSE-MIT`, `licenses/*.txt`). Source code and files under a test directory never qualify. A test that reads the file still selects that test |
|
||||
| `selection` | `workflow.yml/job` → a command printing the lane's file list, for matrices a planner computes (gstack: `scripts/test-free-shards.ts --list`) |
|
||||
|
||||
@@ -197,11 +197,18 @@ function importsPlaywright(text: string): boolean {
|
||||
/** Playwright's own text when its browsers are not installed on this machine. */
|
||||
const PLAYWRIGHT_NO_BROWSERS = /^.*(?:Executable doesn't exist at|download new browsers).*$/m;
|
||||
|
||||
/** The run plan: the groups that can run, the files no runner claims, tools missing from PATH, and `.dart` files with no pubspec.yaml above. */
|
||||
export interface RunPlan { groups: RunGroup[]; unclaimed: string[]; missing: string[]; noPubspec: string[] }
|
||||
/** The run plan: the groups that can run, the files no runner claims, tools missing from PATH, `.dart` files with no pubspec.yaml above, and the files a declared runner cannot run (not run, not counted as covered). */
|
||||
export interface RunPlan { groups: RunGroup[]; unclaimed: string[]; missing: string[]; noPubspec: string[]; dropped?: string[] }
|
||||
|
||||
/** The files a declared runner's test tool runs, by extension: JS/TS for `bun test`, `node --test`, `deno test`, vitest, jest, mocha, ava and playwright; `.py` for pytest. A tool counts as a whole command word (`./node_modules/.bin/jest` does, `tests/node/run.sh` does not). Null for any other command (a dispatcher such as `node scripts/run-tests.js` may run any language), or one naming both kinds: it takes every file. */
|
||||
function declaredRunnerFiles(runner: string): RegExp | null {
|
||||
const js = /(?:^|[\s;&|(])(?:\S*\/)?(?:vitest|jest|mocha|ava|playwright|bun\s+test|node\s+--test|deno\s+test)(?=$|[\s;&|)])/.test(runner);
|
||||
const py = /(?:^|[\s;&|(])(?:\S*\/)?pytest(?=$|[\s;&|)])/.test(runner);
|
||||
return js === py ? null : js ? /\.[cm]?[jt]sx?$/ : /\.py$/;
|
||||
}
|
||||
|
||||
/**
|
||||
* The invocations that run `files`. A declared runner takes every file. Otherwise
|
||||
* The invocations that run `files`. A declared runner takes every file its tool runs (`declaredRunnerFiles`); the rest are `dropped`. Otherwise
|
||||
* Dart tests run from their own package root (a sub-package's imports resolve
|
||||
* against its own pubspec); Rust tests run per crate as `cargo test -p <name>`
|
||||
* from the crate's workspace root; JS/TS tests owned by a nested package that
|
||||
@@ -214,7 +221,12 @@ export interface RunPlan { groups: RunGroup[]; unclaimed: string[]; missing: str
|
||||
* crate whose name Cargo would reject, no detected runner) are `unclaimed`: the lane is `incomplete`, never a pass.
|
||||
*/
|
||||
export function planRuns(ctx: PregateContext, files: readonly string[]): RunPlan {
|
||||
if (ctx.config.runner) return { groups: [{ runner: ctx.config.runner, cwd: '', files: [...files] }], unclaimed: [], missing: [], noPubspec: [] };
|
||||
if (ctx.config.runner) {
|
||||
const runs = declaredRunnerFiles(ctx.config.runner);
|
||||
const taken = runs ? files.filter(f => runs.test(f)) : [...files];
|
||||
const dropped = files.filter(f => !taken.includes(f));
|
||||
return { groups: taken.length ? [{ runner: ctx.config.runner, cwd: '', files: taken }] : [], unclaimed: [], missing: [], noPubspec: [], ...(dropped.length ? { dropped } : {}) };
|
||||
}
|
||||
const groups: RunGroup[] = [];
|
||||
const dart = new Map<string, string[]>();
|
||||
const cargo = new Map<string, { name: string; cwd: string; files: string[] }>();
|
||||
@@ -543,6 +555,8 @@ async function runSelected(ctx: PregateContext, sel: Selection, base: CheckResul
|
||||
// Runnable groups run even when other files are unclaimed: a real failure is reported as one, and an all-green run is still incomplete.
|
||||
const plan = planRuns(ctx, sel.localTests);
|
||||
const gap = plan.unclaimed.length ? planGap(ctx, plan, sel.localTests.length) : null;
|
||||
const dropped = plan.dropped ? `${plan.dropped.length} selected file(s) the declared runner cannot run (not run, not counted as covered): ${plan.dropped.join(' ')}` : '';
|
||||
if (!plan.groups.length && dropped) return { ...base, status: sel.remote.length ? 'requires-remote' : 'pass', detail: `no selected file is one the declared runner can run; ${dropped}` };
|
||||
if (!plan.groups.length) return { ...base, status: 'incomplete', ...gap, ...(gap?.environment ? { lines: [...lines, environmentLine(gap.environment)] } : {}) };
|
||||
const { groups } = plan;
|
||||
const total = groups.reduce((n, g) => n + g.files.length, 0);
|
||||
@@ -550,7 +564,7 @@ async function runSelected(ctx: PregateContext, sel: Selection, base: CheckResul
|
||||
if (runtime.refusal) return { ...base, ...runtime.refusal, cause: 'environment', environment: launchCause('runtime', runtime.refusal.detail), lines: [`runtime: ${runtime.label}`, environmentLine(launchCause('runtime', runtime.refusal.detail))] };
|
||||
const r = await runLocal(ctx, runtime.env, groups);
|
||||
const adapters = [...new Set(groups.map(g => adapterFor(g.runner)).filter(Boolean))];
|
||||
const runLines = [...lines, `runtime: ${runtime.label}`, `progress: ${adapters.join(', ') || 'none (no progress adapter for this runner; serial files and heartbeats only)'}`, `ran: ${r.command.length > 200 ? `${r.command.slice(0, 200)}…` : r.command}`, `log: ${r.log}`, ...r.tail.map(l => ` ${l}`)];
|
||||
const runLines = [...lines, ...(dropped ? [`not run: ${dropped}`] : []), `runtime: ${runtime.label}`, `progress: ${adapters.join(', ') || 'none (no progress adapter for this runner; serial files and heartbeats only)'}`, `ran: ${r.command.length > 200 ? `${r.command.slice(0, 200)}…` : r.command}`, `log: ${r.log}`, ...r.tail.map(l => ` ${l}`)];
|
||||
const withRun = { ...base, lines: runLines, inputs: { ...inputs, command: sha256(r.command), runtime: runtime.label } };
|
||||
if (r.exit === null) {
|
||||
const why = unfinished(ctx, r, total);
|
||||
@@ -568,7 +582,7 @@ async function runSelected(ctx: PregateContext, sel: Selection, base: CheckResul
|
||||
return { ...withRun, status: 'fail', cause: 'diff', code: 'PREGATE_LANE_FAILED', detail: `local lane exited ${r.exit} (${r.failed ?? '?'} failed of ${r.ran ?? '?'} ran; ${total} files; pipefail)`, fix: `read ${r.log}, fix the in-branch failure, rerun gstack-pregate --stage tests` };
|
||||
}
|
||||
if (r.ran === 0) return { ...withRun, status: 'fail', cause: 'diff', code: 'PREGATE_LANE_FAILED', detail: `ZERO-RUN: the runner reported 0 tests${r.zero ? ` in ${r.zero}` : ''} (${total} selected file(s))`, fix: 'a bad selector, a missing file or a skipped describe; exit 0 is not a pass' };
|
||||
const detail = `${r.ran ?? 'count unavailable'} tests ran in ${total} file(s)${r.serial ? ` (${r.serial} serial)` : ''}, exit 0${sel.remote.length ? `; ${sel.remote.length} requires-remote obligation(s)` : ''}`;
|
||||
const detail = `${r.ran ?? 'count unavailable'} tests ran in ${total} file(s)${r.serial ? ` (${r.serial} serial)` : ''}, exit 0${dropped ? `; ${dropped}` : ''}${sel.remote.length ? `; ${sel.remote.length} requires-remote obligation(s)` : ''}`;
|
||||
if (gap) return { ...withRun, status: 'incomplete', ...gap, detail: `${detail}; ${gap.detail}`, ...(gap.environment ? { lines: [...runLines, environmentLine(gap.environment)] } : {}) };
|
||||
return { ...withRun, status: sel.remote.length ? 'requires-remote' : 'pass', detail };
|
||||
}
|
||||
|
||||
@@ -1030,3 +1030,57 @@ describe('nested JS packages run their own runner from their own directory', ()
|
||||
expect(r.status).toBe(EXIT.ok);
|
||||
}, 60_000);
|
||||
});
|
||||
|
||||
describe('a declared runner gets only the files its tool runs', () => {
|
||||
const BUN_APP = {
|
||||
'lib/sum.ts': 'export const sum = (a: number, b: number) => a + b;\n',
|
||||
'test/sum.test.ts': "import { expect, test } from 'bun:test';\nimport { sum } from '../lib/sum';\ntest('sum', () => expect(sum(1, 2)).toBe(3));\n",
|
||||
'tests/real-outcome/agent_test.py': "import subprocess\n\ndef test_agent():\n assert 'lib/sum.ts'\n",
|
||||
};
|
||||
const ctxWith = (repo: FixtureRepo, runner: string) => ({ repoRoot: repo.work, config: { ...DEFAULT_CONFIG, runner }, env: process.env }) as unknown as PregateContext;
|
||||
|
||||
// Value: protects=a declared JS runner never receives a Python test (bun exits 1 on "filters did not match any test files", so the
|
||||
// lane went PREGATE_LANE_FAILED on gstack-evals #196 and #200), and the files it cannot run are named, not counted as covered;
|
||||
// fails_when=a declared runner takes every selected file again; why_new=only detected runners split by language; seam=none;
|
||||
// no_claim=a Python lane (pytest never runs under a declared JS runner)
|
||||
test('a declared bun runner plans JS/TS only and names the dropped .py files; pytest takes .py only; an unknown tool takes every file', () => {
|
||||
const repo = fixture({ baseFiles: BUN_APP });
|
||||
const files = ['test/sum.test.ts', 'test/resize_test.py', 'tests/real-outcome/agent_test.py'];
|
||||
expect(planRuns(ctxWith(repo, 'bun test --timeout=30000 {files}'), files)).toEqual({ groups: [{ runner: 'bun test --timeout=30000 {files}', cwd: '', files: ['test/sum.test.ts'] }], unclaimed: [], missing: [], noPubspec: [], dropped: ['test/resize_test.py', 'tests/real-outcome/agent_test.py'] });
|
||||
expect(planRuns(ctxWith(repo, 'python3 -m pytest {files}'), files).groups).toEqual([{ runner: 'python3 -m pytest {files}', cwd: '', files: ['test/resize_test.py', 'tests/real-outcome/agent_test.py'] }]);
|
||||
expect(planRuns(ctxWith(repo, 'bash scripts/run-tests.sh {files}'), files)).toEqual({ groups: [{ runner: 'bash scripts/run-tests.sh {files}', cwd: '', files }], unclaimed: [], missing: [], noPubspec: [] });
|
||||
expect(planRuns(ctxWith(repo, 'tests/node/run.sh {files}'), files).groups[0]!.files).toEqual(files);
|
||||
expect(planRuns(ctxWith(repo, 'node scripts/run-tests.js {files}'), files).groups[0]!.files).toEqual(files);
|
||||
expect(planRuns(ctxWith(repo, 'python3 scripts/run-tests.py {files}'), files).groups[0]!.files).toEqual(files);
|
||||
expect(planRuns(ctxWith(repo, 'node --test {files}'), files).dropped).toEqual(['test/resize_test.py', 'tests/real-outcome/agent_test.py']);
|
||||
expect(planRuns(ctxWith(repo, 'bun test {files} && python3 -m pytest {files}'), files).dropped).toBeUndefined();
|
||||
expect(planRuns(ctxWith(repo, './node_modules/.bin/jest {files}'), files).dropped).toEqual(['test/resize_test.py', 'tests/real-outcome/agent_test.py']);
|
||||
expect(planRuns(ctxWith(repo, 'bun test {files}'), ['test/resize_test.py'])).toEqual({ groups: [], unclaimed: [], missing: [], noPubspec: [], dropped: ['test/resize_test.py'] });
|
||||
});
|
||||
|
||||
// Value: protects=the reported runs end to end under a set `bun test {files}` runner: beside a TS test, a changed *_test.py and a .py
|
||||
// test selected by a literal match are no longer handed to bun (which skips them silently, so the lane counted 3 files for 1 run),
|
||||
// and a .py-only selection no longer runs bun at all (bun exits 1 when no file is a bun test: PREGATE_LANE_FAILED, gstack-evals
|
||||
// #196 and #200); both name the .py files; fails_when=bun is handed a .py file again; why_new=no fixture declared a JS runner beside
|
||||
// Python tests; seam=none; no_claim=TS tests that spawn python3 count as coverage of the .py files (they do not)
|
||||
test('a set bun runner with a changed *_test.py and a literal-match .py runs bun without any .py file', () => {
|
||||
const repoWith = (files: Record<string, string>) => {
|
||||
const repo = fixture({ pregate: { ...BASE_PREGATE, runner: 'bun test {files}' }, baseFiles: BUN_APP });
|
||||
commit(repo, { '.github/workflows/free-tests.yml': null, '.github/workflows/windows.yml': null }, 'no workflows');
|
||||
repo.git('push', '-q', 'origin', 'HEAD:main');
|
||||
repo.git('fetch', '-q', 'origin');
|
||||
commit(repo, files);
|
||||
return pregate(repo, ['--stage', 'tests', '--only', 'lanes']);
|
||||
};
|
||||
const mixed = repoWith({ 'lib/sum.ts': 'export const sum = (a: number, b: number) => b + a;\n', 'test/resize_test.py': 'def test_resize():\n assert True\n' });
|
||||
const ran = /^\s*lanes: ran: .*$/m.exec(mixed.out)?.[0] ?? '';
|
||||
expect(ran).toContain('test/sum.test.ts');
|
||||
expect(ran).not.toContain('.py');
|
||||
expect(mixed.out).toMatch(/^tests {6}lanes {5}pass .*1 tests ran in 1 file\(s\), exit 0; 2 selected file\(s\) the declared runner cannot run \(not run, not counted as covered\): test\/resize_test\.py tests\/real-outcome\/agent_test\.py/m);
|
||||
expect(mixed.status).toBe(EXIT.ok);
|
||||
const pyOnly = repoWith({ 'test/resize_test.py': 'def test_resize():\n assert True\n' });
|
||||
expect(pyOnly.out).not.toMatch(/lanes: ran: /);
|
||||
expect(pyOnly.out).toMatch(/^tests {6}lanes {5}pass .*no selected file is one the declared runner can run; 1 selected file\(s\) the declared runner cannot run \(not run, not counted as covered\): test\/resize_test\.py/m);
|
||||
expect(pyOnly.status).toBe(EXIT.ok);
|
||||
}, 60_000);
|
||||
});
|
||||
Reference in new issue
Block a user