fix: escape Playwright file filters; skip type-only Playwright imports; pytest probe and summary read hardening

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-31 18:47:40 +00:00
1 parent 9bb5fd9a4a
commit de9b69038a
4 files changed
+32 -16

No files matched your search

+1 -1
View File
@@ -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 {files}`); 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 single-line `import type … from '@playwright/test'` does not count (a multi-line or inline type import still does), 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 {files}`); 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) |
| `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`) |
+20 -8
View File
@@ -120,8 +120,8 @@ export function dartRunner(repoRoot: string, root: string, env: NodeJS.ProcessEn
function pytestMissing(repoRoot: string, env: NodeJS.ProcessEnv): string | null {
const python = Bun.which('python3', { PATH: env.PATH ?? '' });
if (!python) return 'python3';
const r = spawnSync(python, ['-c', 'import importlib.util, sys; sys.exit(importlib.util.find_spec("pytest") is None)'], { cwd: repoRoot, env, stdio: 'ignore', timeout: 10_000 });
return r.status === 1 ? 'pytest' : null;
const r = spawnSync(python, ['-c', 'import importlib.util, sys; sys.exit(3 if importlib.util.find_spec("pytest") is None else 0)'], { cwd: repoRoot, env, stdio: 'ignore', timeout: 10_000 });
return r.status === 3 ? 'pytest' : null;
}
/**
@@ -174,8 +174,18 @@ function packageRunner(repoRoot: string, dir: string, env: NodeJS.ProcessEnv): {
return { runner: `${PM_EXEC[pm]} ${tool} {files}` };
}
/** A JS/TS test that imports Playwright's runner at the start of a line (a string or a fixture body that mentions it mid-line, or an `import type`, does not count). */
const PLAYWRIGHT_IMPORT = /^[\t ]*(?:import\b(?![\t ]+type\b)[^'"`\n]*?\bfrom|\}[\t ]*from|import|(?:const|let|var)\b[^=\n]*=[\t ]*require\()[\t ]*['"]@playwright\/test['"]/m;
/**
* Whether a JS/TS test imports a value from Playwright's runner in a statement that starts a line: `import … from '@playwright/test'`
* (one line or several), a bare import, or `require`. A type-only import (`import type …`, or every specifier `type X`) and a string or
* fixture body that mentions the import mid-line do not count.
*/
function importsPlaywright(text: string): boolean {
if (/^[\t ]*(?:import[\t ]*|(?:const|let|var)\b[^=\n]*=[\t ]*require\([\t ]*)['"]@playwright\/test['"]/m.test(text)) return true;
for (const m of text.matchAll(/^[\t ]*import[\t ]+(type[\t ]+)?([^'"`;]*?)\s*\bfrom[\t ]*['"]@playwright\/test['"]/gm)) {
if (!m[1] && m[2]!.replace(/[{}]/g, '').split(',').some(s => s.trim() && !/^type\s/.test(s.trim()))) return true;
}
return false;
}
/** 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;
@@ -224,7 +234,7 @@ export function planRuns(ctx: PregateContext, files: readonly string[]): RunPlan
continue;
}
const pkg = /\.[cm]?[jt]sx?$/.test(f) ? nearestRoot(ctx.repoRoot, f, 'package.json', seenPackage) : null;
if (pkg !== null && PLAYWRIGHT_IMPORT.test(readRepoFile(ctx.repoRoot, f) ?? '')) { add(playwright, pkg, under(pkg, f)); continue; }
if (pkg !== null && importsPlaywright(readRepoFile(ctx.repoRoot, f) ?? '')) { add(playwright, pkg, under(pkg, f)); continue; }
if (pkg) {
if (!pkgRunners.has(pkg)) pkgRunners.set(pkg, packageRunner(ctx.repoRoot, pkg, ctx.env));
const r = pkgRunners.get(pkg);
@@ -302,10 +312,10 @@ function parseCargo(output: string): RunnerSummary | null {
return { ran: lines.reduce((n, m) => n + Number(m[1]), 0) + failed, failed };
}
/** playwright test: its `N failed`, `N interrupted`, `N flaky`, `N passed` and `N skipped` lines, each on its own line with test names between them; skipped tests did not run. */
/** playwright test: the last `N failed`, `N interrupted`, `N flaky`, `N passed` and `N skipped` lines (its closing summary, with test names between them; a test's own output earlier does not count); skipped tests did not run. */
function parsePlaywright(output: string): RunnerSummary | null {
if (!/^\s*\d+ (?:passed|failed|flaky|interrupted|skipped)\b/m.test(output)) return null;
const count = (word: string) => Number(new RegExp(`^\\s*(\\d+) ${word}\\b`, 'm').exec(output)?.[1] ?? 0);
const count = (word: string) => Number([...output.matchAll(new RegExp(`^\\s*(\\d+) ${word}\\b`, 'gm'))].pop()?.[1] ?? 0);
const failed = count('failed') + count('interrupted');
return { ran: count('passed') + count('flaky') + failed, failed };
}
@@ -334,7 +344,9 @@ interface LocalRun { exit: number | null; outcome: Supervised['outcome']; signal
const quote = (f: string) => `'${f.replace(/'/g, `'\\''`)}'`;
async function runOnce(ctx: PregateContext, env: NodeJS.ProcessEnv, g: RunGroup, files: string[], log: string, counter: Counter): Promise<LocalRun> {
const joined = files.map(quote).join(' ');
// playwright test reads each file argument as a case-insensitive regex: escape it and anchor it to a path segment, so `a[1].spec.ts` runs that file, not `a1.spec.ts`.
const args = /(?<![\w-])playwright\s+test(?![\w-])/.test(g.runner) ? files.map(f => `(?:^|/)${f.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}$`) : files;
const joined = args.map(quote).join(' ');
const run = g.runner.replace('{files}', () => joined);
const command = g.cwd ? `(cd ${quote(g.cwd)} && ${run})` : run;
const shown = g.cwd ? files.map(f => `${g.cwd}/${f}`) : files;
+6 -4
View File
@@ -176,7 +176,7 @@ describe('environment failures are reported apart from diff failures', () => {
describe('a detected pytest runner needs pytest importable by python3', () => {
const PY_TEST = { 'tests/test_alpha.py': 'import unittest\n\nclass T(unittest.TestCase):\n def test_a(self):\n self.assertEqual(1 + 1, 2)\n' };
/** A directory holding only a `python3` whose `-c` probe exits `probeExit` (0: pytest importable, 1: not). */
/** A directory holding only a `python3` whose `-c` probe exits `probeExit` (0: pytest importable, 3: not, anything else: the probe itself failed). */
const python3Shim = (repo: FixtureRepo, probeExit: number): string => {
const dir = path.join(repo.root, `py-${probeExit}`);
fs.mkdirSync(dir, { recursive: true });
@@ -192,7 +192,9 @@ describe('a detected pytest runner needs pytest importable by python3', () => {
const repo = makeFixtureRepo({ pregate: null, baseFiles: PY_TEST }); repos.push(repo);
const files = ['tests/test_alpha.py'];
expect(planRuns(ctxFor(repo, python3Shim(repo, 0)), files)).toEqual({ groups: [{ runner: 'python3 -m pytest {files}', cwd: '', files }], unclaimed: [], missing: [], noPubspec: [] });
expect(planRuns(ctxFor(repo, python3Shim(repo, 1)), files)).toEqual({ groups: [], unclaimed: files, missing: ['pytest'], noPubspec: [] });
expect(planRuns(ctxFor(repo, python3Shim(repo, 3)), files)).toEqual({ groups: [], unclaimed: files, missing: ['pytest'], noPubspec: [] });
// A probe that crashes (a repo-root module shadowing importlib exits 1) is not read as a missing pytest: the run decides.
expect(planRuns(ctxFor(repo, python3Shim(repo, 1)), files).groups).toEqual([{ runner: 'python3 -m pytest {files}', cwd: '', files }]);
expect(planRuns(ctxFor(repo, path.join(repo.root, 'empty-path')), files)).toEqual({ groups: [], unclaimed: files, missing: ['python3'], noPubspec: [] });
});
@@ -202,7 +204,7 @@ describe('a detected pytest runner needs pytest importable by python3', () => {
test('a machine without pytest is PREGATE_ENVIRONMENT naming pytest, never PREGATE_LANE_FAILED', () => {
const repo = makeFixtureRepo({ pregate: null, baseFiles: PY_TEST }); repos.push(repo);
commit(repo, { 'tests/test_alpha.py': `${PY_TEST['tests/test_alpha.py']}# touched\n` });
const r = runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests', '--only', 'lanes'], { PATH: `${python3Shim(repo, 1)}:${process.env.PATH ?? ''}` });
const r = runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests', '--only', 'lanes'], { PATH: `${python3Shim(repo, 3)}:${process.env.PATH ?? ''}` });
expect(r.err).toContain('(PREGATE_ENVIRONMENT)');
expect(r.err).not.toContain('PREGATE_LANE_FAILED');
expect(r.out).not.toContain('unexpected: python3');
@@ -241,7 +243,7 @@ describe('Playwright without its browsers is the machine, not the diff', () => {
const c = check(repo, 'lanes');
expect([c.status, c.code, c.cause, c.environment.kind]).toEqual(['incomplete', 'PREGATE_ENVIRONMENT', 'environment', 'missing-binary']);
expect(c.environment.evidence).toContain('Playwright browsers not installed (npx playwright install)');
expect(c.lines.join('\n')).toContain('playwright test --reporter=list');
expect(c.lines.join('\n')).toContain("playwright test --reporter=list '(?:^|/)tests/e2e/home\\.spec\\.ts$'");
runBin(repo, 'gstack-pregate', args, npxShim(repo, ASSERTION));
const d = check(repo, 'lanes');
expect([d.status, d.code]).toEqual(['fail', 'PREGATE_LANE_FAILED']);
+5 -3
View File
@@ -311,6 +311,8 @@ describe('literals (pure)', () => {
expect(parseSummary(' 137 passed (1.1m)', 'npx playwright test {files}')).toEqual({ ran: 137, failed: 0 });
expect(parseSummary(' 3 failed\n [e2e] › a.spec.ts:9:1 › one\n 1 interrupted\n 2 did not run', 'npx playwright test {files}')).toEqual({ ran: 4, failed: 4 });
expect(parseSummary(' 5 skipped', 'npx playwright test {files}')).toEqual({ ran: 0, failed: 0 });
// A test's own output that looks like a count is not the summary: the last count lines are.
expect(parseSummary(' 0 passed\n 1 failed\n [e2e] › a.spec.ts:9:1 › one\n 4 passed (2.0s)', 'npx playwright test {files}')).toEqual({ ran: 5, failed: 1 });
expect(parseSummary('ok')).toEqual({ ran: null, failed: null });
});
test('config: unknown keys, secrets in warn, a runner without {files} and an uncontained glob are PREGATE_CONFIG_INVALID', () => {
@@ -790,7 +792,7 @@ describe('nested JS packages run their own runner from their own directory', ()
expect(planRuns(ctx, ['app/src/sum.test.ts']).groups).toEqual([{ runner: 'npx vitest run {files}', cwd: 'app', files: ['src/sum.test.ts'] }]);
});
/** `env` with a `python3` first on PATH whose `-c` probe exits `probeExit` (0: pytest importable, 1: not). */
/** `env` with a `python3` first on PATH whose `-c` probe exits `probeExit` (0: pytest importable, 3: not). */
const withPython = (repo: FixtureRepo, env: { PATH: string; BASH_ENV: string }, probeExit: number) => {
const dir = path.join(repo.root, `py-${probeExit}`);
fs.mkdirSync(dir, { recursive: true });
@@ -813,7 +815,7 @@ describe('nested JS packages run their own runner from their own directory', ()
expect(planRuns(ctx, ['scripts/doku-gate.test.mjs', 'test/run.bats'])).toEqual({ groups: [
{ runner: 'node --test {files}', cwd: '', files: ['scripts/doku-gate.test.mjs'] },
], unclaimed: ['test/run.bats'], missing: [], noPubspec: [] });
expect(planRuns(ctxFor(repo, withPython(repo, pmShim(repo), 1)), mixed)).toEqual({ groups: [
expect(planRuns(ctxFor(repo, withPython(repo, pmShim(repo), 3)), mixed)).toEqual({ groups: [
{ runner: 'node --test {files}', cwd: '', files: ['scripts/doku-gate.test.mjs', 'scripts/version-gate.test.mjs'] },
], unclaimed: ['scripts/test_assets.py'], missing: ['pytest'], noPubspec: [] });
});
@@ -846,7 +848,7 @@ describe('nested JS packages run their own runner from their own directory', ()
write(repo.work, 'package.json', JSON.stringify({ name: 'fixture', devDependencies: { vitest: '^3.2.0', '@playwright/test': '^1.48.0' } }) + '\n');
write(repo.work, 'tests/e2e/home.spec.ts', "import { expect, test } from '@playwright/test';\ntest('home', async ({ page }) => expect(page).toBeTruthy());\n");
write(repo.work, 'app/e2e/app.spec.ts', "import {\n test,\n} from \"@playwright/test\";\ntest('app', () => {});\n");
write(repo.work, 'test/util.test.js', "import { test } from 'vitest';\nimport type { Page } from '@playwright/test';\nconst spec = \"import { test } from '@playwright/test';\";\ntest('fixture text and a type import only', () => {});\n");
write(repo.work, 'test/util.test.js', "import { test } from 'vitest';\nimport type { Page } from '@playwright/test';\nimport type {\n Locator,\n} from '@playwright/test';\nimport { type BrowserContext } from '@playwright/test';\nconst spec = \"import { test } from '@playwright/test';\";\ntest('fixture text and type imports only', () => {});\n");
expect(planRuns(ctx, ['tests/e2e/home.spec.ts', 'test/util.test.js', 'app/src/sum.test.ts', 'app/e2e/app.spec.ts'])).toEqual({ groups: [
{ runner: 'npx vitest run {files}', cwd: '', files: ['test/util.test.js'] },
{ runner: 'pnpm exec vitest run {files}', cwd: 'app', files: ['src/sum.test.ts'] },