mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 14:34:42 +01:00
fix(pregate): recognise bats test files so lanes does not fail in bats repos (#3161)
gstack-pregate never treated a .bats file as a test, so in a repository whose tests are bats files the lanes check failed every run with PREGATE_NO_LANE: a .gstack/pregate.json dependency pointing at tests/foo.bats selected nothing, a selection manifest printing .bats paths yielded 0 files, and a bats run that executed no tests could not trip the zero-run rule. Recognise .bats in the test-file pattern and in the manifest-output parser, and read the bats TAP plan and pretty summary (as a last resort, after every existing runner's summary) so a run that reports zero tests is a failure. Fixes #3161. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
953a62a724
commit
d355c116b1
5 files changed
+82
-8
No files matched your search
@@ -36,8 +36,17 @@ export function detectRunner(ctx: PregateContext, files: readonly string[]): str
|
||||
|
||||
export interface RunnerSummary { ran: number | null; failed: number | null }
|
||||
|
||||
/** The runner's own summary line: bun, node --test, vitest, jest, pytest. null when not recognised. */
|
||||
export function parseSummary(output: string): RunnerSummary {
|
||||
/** bats: the pretty `N tests, M failures` line or the TAP plan `1..N`. */
|
||||
function parseBats(output: string): RunnerSummary | null {
|
||||
let m: RegExpExecArray | null;
|
||||
if ((m = /^\s*(\d+)\s+tests?,\s+(\d+)\s+failures?/m.exec(output))) return { ran: Number(m[1]), failed: Number(m[2]) };
|
||||
if ((m = /^1\.\.(\d+)\s*$/m.exec(output))) return { ran: Number(m[1]), failed: (output.match(/^not ok\b/gm) ?? []).length };
|
||||
return null;
|
||||
}
|
||||
|
||||
/** The runner's own summary line: bun, node --test, vitest, jest, pytest, bats. A bats runner's summary is tried first; for any other runner bats output is a last resort. `ran: null` when not recognised. */
|
||||
export function parseSummary(output: string, runner = ''): RunnerSummary {
|
||||
if (/(?<![\w-])bats(?![\w-])/.test(runner)) { const b = parseBats(output); if (b) return b; }
|
||||
let m: RegExpExecArray | null;
|
||||
if ((m = /^\s*(?:#|ℹ)\s*tests\s+(\d+)/m.exec(output))) return { ran: Number(m[1]), failed: Number(/^\s*(?:#|ℹ)\s*fail\s+(\d+)/m.exec(output)?.[1] ?? 0) };
|
||||
if ((m = /Ran\s+(\d+)\s+tests?\s+across/m.exec(output))) return { ran: Number(m[1]), failed: Number(/^\s*(\d+)\s+fail\b/m.exec(output)?.[1] ?? 0) };
|
||||
@@ -46,7 +55,7 @@ export function parseSummary(output: string): RunnerSummary {
|
||||
if ((m = /Tests\s+(?:(\d+)\s+failed\s*\|\s*)?(\d+)\s+passed/.exec(output))) return { ran: Number(m[2]) + Number(m[1] ?? 0), failed: Number(m[1] ?? 0) };
|
||||
if (/no tests ran/.test(output) || /No tests found/i.test(output)) return { ran: 0, failed: 0 };
|
||||
if ((m = /(?:(\d+)\s+failed,?\s*)?(\d+)\s+passed/.exec(output))) return { ran: Number(m[2]) + Number(m[1] ?? 0), failed: Number(m[1] ?? 0) };
|
||||
return { ran: null, failed: null };
|
||||
return parseBats(output) ?? { ran: null, failed: null };
|
||||
}
|
||||
|
||||
interface LocalRun { exit: number | null; ran: number | null; failed: number | null; log: string; tail: string[]; command: string }
|
||||
@@ -56,7 +65,7 @@ function runOnce(ctx: PregateContext, env: NodeJS.ProcessEnv, runner: string, fi
|
||||
const r = spawnSync('bash', ['-c', `set -o pipefail; ${command}`], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: ctx.timeoutMs, maxBuffer: 256 * 1024 * 1024, env: { ...env, CI: env.CI ?? '1' } });
|
||||
const output = `${r.stdout ?? ''}\n${r.stderr ?? ''}`;
|
||||
fs.appendFileSync(log, `$ ${command}\n${output}\n`);
|
||||
const summary = parseSummary(output);
|
||||
const summary = parseSummary(output, runner);
|
||||
return { exit: r.status, ...summary, log, tail: output.trim().split('\n').slice(-25), command };
|
||||
}
|
||||
|
||||
|
||||
@@ -20,7 +20,7 @@ import * as path from 'node:path';
|
||||
import { matchesAny } from '../ship-policy';
|
||||
|
||||
export const SOURCE_EXT = /\.(?:[cm]?[jt]sx?|py|rb|sh|bash|go|rs)$/;
|
||||
export const TEST_FILE_RE = /(?:^|\/)(?:test|tests|spec|__tests__)\/.*\.(?:test|spec)\.[cm]?[jt]sx?$|(?:^|\/)[^/]*\.(?:test|spec)\.[cm]?[jt]sx?$|(?:^|\/)test_[^/]*\.py$|_test\.(?:py|go|rb)$|_spec\.rb$/;
|
||||
export const TEST_FILE_RE = /(?:^|\/)(?:test|tests|spec|__tests__)\/.*\.(?:test|spec)\.[cm]?[jt]sx?$|(?:^|\/)[^/]*\.(?:test|spec)\.[cm]?[jt]sx?$|(?:^|\/)test_[^/]*\.py$|_test\.(?:py|go|rb)$|_spec\.rb$|(?:^|\/)[^/]*\.bats$/;
|
||||
|
||||
export interface Edge { from: string; via: 'import' | 'literal' | 'convention' | 'declared' | 'generated' }
|
||||
export interface Graph {
|
||||
|
||||
@@ -39,7 +39,7 @@ export function laneFileSet(ctx: PregateContext, lane: Lane): string[] | '?' | '
|
||||
if (!ctx.allowRepoCommands) { ctx.explain.push(`lane ${lane.id}: selection manifest \`${manifest}\` not run (pass --allow-repo-commands); treated as unknown`); return files.size ? [...files] : '?'; }
|
||||
const r = spawnSync('bash', ['-c', `set -o pipefail; ${manifest}`], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: Math.min(ctx.timeoutMs, 300_000), maxBuffer: 64 * 1024 * 1024, env: ctx.env });
|
||||
if (r.status !== 0) { ctx.explain.push(`lane ${lane.id}: selection manifest exited ${r.status}; treated as unknown`); return '?'; }
|
||||
for (const line of (r.stdout ?? '').split('\n')) { const m = /((?:[\w.-]+\/)*[\w.-]+\.(?:test|spec)\.[cm]?[jt]sx?|(?:[\w.-]+\/)*test_[\w.-]+\.py)/.exec(line.trim()); if (m) files.add(m[1]!); }
|
||||
for (const line of (r.stdout ?? '').split('\n')) { const m = /((?:[\w.-]+\/)*[\w.-]+\.bats(?![\w./-])|(?:[\w.-]+\/)*[\w.-]+\.(?:test|spec)\.[cm]?[jt]sx?|(?:[\w.-]+\/)*test_[\w.-]+\.py)/.exec(line.trim()); if (m) files.add(m[1]!); }
|
||||
ctx.explain.push(`lane ${lane.id}: ${files.size} files from manifest \`${manifest}\``);
|
||||
return [...files];
|
||||
}
|
||||
|
||||
@@ -79,7 +79,7 @@ jobs:
|
||||
run: node --test \${{ matrix.files }}
|
||||
`;
|
||||
|
||||
export function makeFixtureRepo(opts: { registry?: Record<string, unknown> | null; pregate?: Record<string, unknown> | null; prefix?: string } = {}): FixtureRepo {
|
||||
export function makeFixtureRepo(opts: { registry?: Record<string, unknown> | null; pregate?: Record<string, unknown> | null; prefix?: string; baseFiles?: Record<string, string> } = {}): FixtureRepo {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), opts.prefix ?? 'gstack-pregate-fx-'));
|
||||
const origin = path.join(root, 'origin.git');
|
||||
const work = path.join(root, 'work');
|
||||
@@ -122,6 +122,7 @@ export function makeFixtureRepo(opts: { registry?: Record<string, unknown> | nul
|
||||
write(work, '.github/workflows/windows.yml', WORKFLOW_WINDOWS);
|
||||
if (opts.registry !== null) write(work, '.gstack/generated.json', JSON.stringify(opts.registry ?? BASE_REGISTRY, null, 2) + '\n');
|
||||
if (opts.pregate !== null) write(work, '.gstack/pregate.json', JSON.stringify(opts.pregate ?? BASE_PREGATE, null, 2) + '\n');
|
||||
for (const [rel, text] of Object.entries(opts.baseFiles ?? {})) write(work, rel, text);
|
||||
git('add', '-A');
|
||||
git('commit', '-q', '-m', 'base');
|
||||
git('remote', 'add', 'origin', origin);
|
||||
|
||||
@@ -11,13 +11,15 @@
|
||||
*/
|
||||
import { afterEach, describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { EXIT } from '../lib/headless-artifacts';
|
||||
import { changedLiterals, constantsIn } from '../lib/pregate/checks/literals';
|
||||
import { parseSummary } from '../lib/pregate/checks/lanes';
|
||||
import { validateConfig } from '../lib/pregate/config';
|
||||
import { buildGraph, testsFor } from '../lib/pregate/import-graph';
|
||||
import { buildGraph, isTestFile, testsFor } from '../lib/pregate/import-graph';
|
||||
import { CHECK_IDS } from '../lib/pregate/index';
|
||||
import { laneFileSet } from '../lib/pregate/lane-selection';
|
||||
import { parseWorkflow } from '../lib/pregate/workflows';
|
||||
import { BASE_PREGATE, cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture';
|
||||
|
||||
@@ -48,6 +50,68 @@ describe('workflow lanes (parser over .github/workflows)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('bats tests (#3161)', () => {
|
||||
const bats = (run: string) => parseWorkflow('b.yml', `jobs:\n t:\n runs-on: ubuntu-24.04\n steps:\n - run: ${run}\n`, 'linux')[0]!;
|
||||
|
||||
test('isTestFile and testsFor recognise .bats, and only a terminal .bats', () => {
|
||||
for (const f of ['tests/foo.bats', 'test/a/b.bats', 'foo.bats']) expect(isTestFile(f)).toBe(true);
|
||||
for (const f of ['tests/foo.bats.md', 'x.bats.sh', 'batsfile', 'tests/foo.bats.xml']) expect(isTestFile(f)).toBe(false);
|
||||
const repo = fixture({ baseFiles: { 'scripts/foo.sh': '#!/bin/sh\ntrue\n', 'tests/foo.bats': '@test "x" { true; }\n' } });
|
||||
const g = buildGraph(repo.work, { declarations: { 'scripts/**': ['tests/foo.bats'] } });
|
||||
expect(testsFor(g, 'scripts/foo.sh').tests).toEqual(['tests/foo.bats']);
|
||||
});
|
||||
|
||||
test('a selection manifest printing .bats paths yields them; .bats.md and .bats.xml are not taken', () => {
|
||||
const ctx: any = { config: { selection: { 'b.yml/t': "printf 'tests/foo.bats\\ntests/x/bar.bats\\ntests/foo.bats.md\\ntests/foo.bats.xml\\ntests/foo.test.js.bats\\ntests/foo.spec.ts.bats\\n'" } }, allowRepoCommands: true, repoRoot: os.tmpdir(), timeoutMs: 30_000, env: process.env, explain: [] };
|
||||
expect((laneFileSet(ctx, bats('bats tests/')) as string[]).sort()).toEqual(['tests/foo.bats', 'tests/foo.spec.ts.bats', 'tests/foo.test.js.bats', 'tests/x/bar.bats']);
|
||||
});
|
||||
|
||||
test('parseSummary reads the bats TAP plan and the pretty summary', () => {
|
||||
expect(parseSummary('1..1\nok 1 x')).toEqual({ ran: 1, failed: 0 });
|
||||
expect(parseSummary('1..0')).toEqual({ ran: 0, failed: 0 });
|
||||
expect(parseSummary('1..2\nok 1 a\nnot ok 2 b')).toEqual({ ran: 2, failed: 1 });
|
||||
expect(parseSummary('3 tests, 1 failure')).toEqual({ ran: 3, failed: 1 });
|
||||
expect(parseSummary('1 test, 0 failures')).toEqual({ ran: 1, failed: 0 });
|
||||
// A bats-looking line inside output that ends with another runner's summary must not mask it.
|
||||
expect(parseSummary('1..0\n===== 1 passed in 0.1s =====')).toEqual({ ran: 1, failed: 0 });
|
||||
expect(parseSummary('0 tests, 0 failures\n===== 1 passed in 0.1s =====')).toEqual({ ran: 1, failed: 0 });
|
||||
expect(parseSummary('1..0\n===== 1 passed in 0.1s =====', 'pytest {files}')).toEqual({ ran: 1, failed: 0 });
|
||||
// With a bats runner the bats summary wins over stray words from test output.
|
||||
for (const runner of ['bats {files}', 'CI=1 bats {files}', './node_modules/.bin/bats {files}']) {
|
||||
expect(parseSummary('1..1\n# No tests found\nok 1 works', runner)).toEqual({ ran: 1, failed: 0 });
|
||||
expect(parseSummary('1..0\n# 1 passed', runner)).toEqual({ ran: 0, failed: 0 });
|
||||
}
|
||||
for (const runner of ['npx bats {files}', 'pnpm exec bats {files}', 'bats {files}', 'CI=1 bats {files}', './node_modules/.bin/bats {files}', 'make bats']) expect(parseSummary('1..1\nok 1 No tests found', runner)).toEqual({ ran: 1, failed: 0 });
|
||||
// Runners that merely contain the letters keep the generic behaviour.
|
||||
for (const runner of ['bats-core {files}', 'node scripts/run-bats.js {files}', 'pytest {files}']) expect(parseSummary('1..1\nok 1 No tests found', runner)).toEqual({ ran: 0, failed: 0 });
|
||||
});
|
||||
|
||||
test('end to end: a declared dependency selects the .bats file and runs it with the bats runner; a runner reporting 0 tests is ZERO-RUN', () => {
|
||||
const repo = fixture({
|
||||
pregate: { ...BASE_PREGATE, runner: 'bats {files}', dependencies: { ...BASE_PREGATE.dependencies, 'scripts/**': ['tests/foo.bats'] } },
|
||||
baseFiles: {
|
||||
'tests/foo.bats': '@test "x" { true; }\n',
|
||||
'scripts/foo.sh': '#!/bin/sh\ntrue\n',
|
||||
},
|
||||
});
|
||||
commit(repo, { 'scripts/foo.sh': '#!/bin/sh\n# touched\ntrue\n' });
|
||||
expect(repo.git('diff', '--name-only', 'main...HEAD')).toBe('scripts/foo.sh');
|
||||
const stub = (tap: string) => { const dir = path.join(repo.root, 'bin-stub'); fs.mkdirSync(dir, { recursive: true }); fs.writeFileSync(path.join(dir, 'bats'), `#!/bin/sh\nprintf '%s\\n' '${tap}'\n`, { mode: 0o755 }); return { PATH: `${dir}:${process.env.PATH ?? ''}` }; };
|
||||
const ok = runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests', '--explain'], stub('1..1\nok 1 x'));
|
||||
expect(ok.status).toBe(EXIT.ok);
|
||||
expect(ok.err).not.toContain('PREGATE_NO_LANE');
|
||||
// The fixture's own Linux lane is remote on a non-Linux host, so the verdict may be requires-remote; the selected file and runner are what matter here.
|
||||
expect(['pass', 'requires-remote']).toContain(check(repo, 'lanes').status);
|
||||
expect(ok.out).toContain("ran: bats 'tests/foo.bats'");
|
||||
const zero = runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests'], stub('1..0'));
|
||||
expect(zero.status).toBe(EXIT.fail);
|
||||
expect(zero.out).toMatch(/^tests {6}lanes {5}fail .*ZERO-RUN/m);
|
||||
const masked = runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests'], stub('1..0\n# 1 passed'));
|
||||
expect(masked.status).toBe(EXIT.fail);
|
||||
expect(masked.out).toMatch(/^tests {6}lanes {5}fail .*ZERO-RUN/m);
|
||||
});
|
||||
});
|
||||
|
||||
describe('import graph (static imports ∪ repo-path literals ∪ bin convention ∪ declarations)', () => {
|
||||
test('imports, a joined literal read, the bin/<name> convention, a declared dependency and a deleted file all map; an orphan does not', () => {
|
||||
const repo = fixture();
|
||||
|
||||
Reference in new issue
Block a user