Both independent reviews (Codex GPT-5.6 cross-family, and a cold Opus agent in an isolated worktree) returned BLOCKED. Both independently confirmed the CI path itself is sound — neither found a route that publishes an image on which the suite never ran — so every finding is about the guard's reach, plus one factual error in the prose. THE STRUCTURAL ONE. The guard asserted a `needs:` edge EXISTS, never that it is load bearing. Since this change deletes the in-image run, that edge is the only remaining layer, so `continue-on-error: true`, `if: false`, a job-level `if:`, `npm test … || true`, a pipe into `tee`, and `set +e` each certified a publish over a red suite with every assertion green. `test_the_gating_suite_run_is_NOT_ADVISORY` closes all six. A filter written into `web/package.json`'s script body was invisible at the call site: `"test": "vitest --exclude x"` with a workflow saying `npm test -- --run` is a filtered gating run reading as clean — the removed defect, one level down. `vitest_scripts()` now derives each script's own narrowing arguments and `suite_args` prepends them. PARSER REACH, every case measured rather than argued. `shlex.split` yields `lint&&npm` as one token, so unspaced `&&` and `;` re-adds were invisible; `shlex` in punctuation_chars mode splits them. Added: `sh -c` payload expansion, `npm --prefix`/`npx -p` flag skipping, `xargs`, heredoc bodies as DATA (a `cat > f <<'EOF' … npm test … EOF` block counted as a real run), `ADD`/JSON-form/no-trailing-slash `COPY` in `carries_spa_source`, and redirections no longer read as spec filters. `--root` and `--config` moved to the narrowing set: both change which specs vitest collects. A FACTUAL ERROR, in five places including the mutation `expect`: "the build context is `web/` + `design-system/`, so there is no `.git`". The context is the repository root (`context: .`) and `.dockerignore` does not exclude `.git`. The true statement is about the STAGE, which copies only those two directories. The conclusion survives — bookworm slim has no git binary either — but a reader who checked would have found `.git` in the context and concluded the note was stale. ONE FINDING WAS MINE, from the mutant battery rather than from either review, and it is the reason the battery exists: `failure_suppressions` tokenised the whole multi-line `run:` body at once. A newline is not a shell separator, so a realistic two-line step — the `ci-step-ran.sh` marker line, then the suite — merged into ONE segment whose head was the marker script, and three suppression mutants passed while my single-line unit test was green. It now works per logical line, and the regression test uses the two-line shape. 17 mutants, 0 missed, each caught by the intended assertion; baseline green. The `docs/guard-inventory.md` residual list is rewritten as MEASURED reach — the previous one was wrong rather than merely short, which cold review rightly called worse than silence. Refs: #887 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019T79beF1Ufid3dXju4yqkF
101 lines
5.3 KiB
TypeScript
101 lines
5.3 KiB
TypeScript
import { execFileSync } from 'node:child_process';
|
|
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
|
|
import { tmpdir } from 'node:os';
|
|
import { join } from 'node:path';
|
|
import { afterAll, describe, expect, it } from 'vitest';
|
|
import { type GitRunner, resolveTrackedSourceFiles } from './trackedSourceFiles';
|
|
|
|
/**
|
|
* SEPARATE FILE because these are the only tests in the SPA suite that need the `git` BINARY but NO
|
|
* CHECKOUT — they build their own repository (ersatztv#819). That is not the same as saying the
|
|
* guard needs only a checkout: it needs a checkout AND, through it, the binary. Measured — with
|
|
* `.git` present and git off `PATH`, `pageSizeCallSites.guard.test.ts` still fails with the
|
|
* plugin's refusal. Reading the two exclusions as checkout-versus-binary is what broke the Docker
|
|
* stage once already; see `docker/Dockerfile`, which retracts that framing.
|
|
*
|
|
* Keeping these apart from `trackedSourceFiles.test.ts` confines the git prerequisite to one file:
|
|
* its thirteen injected-runner siblings run with no git and no repository. The suite as a whole is
|
|
* therefore run only where git is present. `docker/Dockerfile`'s web-build stage is not such a
|
|
* place — `node:22-bookworm-slim` ships no git at all — and since ersatztv#887 it does not run the
|
|
* suite there rather than naming the files that cannot run, which was a list nothing derived.
|
|
*
|
|
* Do NOT make these skip when git is missing. A check that answers "could not tell" by passing is a
|
|
* permanent no-op, which is the whole failure mode this plugin exists to avoid.
|
|
*/
|
|
describe('resolveTrackedSourceFiles against a REAL repository (#819)', () => {
|
|
// Every test in the sibling `trackedSourceFiles.test.ts` injects a fake `GitRunner`, which proves
|
|
// the derivation's SHAPE and never that
|
|
// it works against git. `scripts/tests/test_guard_populations_derive_from_git.py` makes exactly
|
|
// this point about its own primitive — "It proves the mechanism by EXECUTING it rather than by
|
|
// recognising its shape — no monkeypatching, no stand-in for git" — and the population this
|
|
// plugin feeds is asserted for completeness, so it earns the same treatment.
|
|
const repos: string[] = [];
|
|
|
|
afterAll(() => {
|
|
for (const dir of repos) {
|
|
rmSync(dir, { recursive: true, force: true });
|
|
}
|
|
});
|
|
|
|
function repoWithOneTrackedAndOneUntracked(): string {
|
|
const dir = mkdtempSync(join(tmpdir(), 'etv-819-'));
|
|
repos.push(dir);
|
|
const git = (...args: string[]) => execFileSync('git', ['-C', dir, ...args], { encoding: 'utf8' });
|
|
|
|
git('init', '-q');
|
|
mkdirSync(join(dir, 'src'), { recursive: true });
|
|
writeFileSync(join(dir, 'src', 'tracked.ts'), 'export const a = 1;\n');
|
|
writeFileSync(join(dir, 'src', 'untracked.ts'), 'export const b = 2;\n');
|
|
// STAGED, not committed. `ls-files` reads the INDEX, so a commit adds nothing — and committing
|
|
// makes the fixture depend on the developer's global git config: under a `commit.gpgsign = true`
|
|
// that many people set, `git commit` fails and these tests go red on their machine while CI
|
|
// stays green. That is the very shape #819 exists to remove, so it must not be reintroduced by
|
|
// #819's own tests. `git add` is unaffected by signing.
|
|
git('add', 'src/tracked.ts');
|
|
return dir;
|
|
}
|
|
|
|
it('really separates a tracked file from an identical untracked sibling', () => {
|
|
const result = resolveTrackedSourceFiles(repoWithOneTrackedAndOneUntracked());
|
|
|
|
expect(result.tracked).toEqual(['src/tracked.ts']);
|
|
expect(result.others).toEqual(['src/untracked.ts']);
|
|
expect(result.absentFromDisk).toEqual([]);
|
|
});
|
|
|
|
it('really reports a tracked file deleted from the working tree as absent', () => {
|
|
const dir = repoWithOneTrackedAndOneUntracked();
|
|
rmSync(join(dir, 'src', 'tracked.ts'));
|
|
|
|
const result = resolveTrackedSourceFiles(dir);
|
|
|
|
expect(result.tracked).toEqual(['src/tracked.ts']);
|
|
expect(result.absentFromDisk).toEqual(['src/tracked.ts']);
|
|
});
|
|
|
|
// Residual: this asserts the message the MISSING-BINARY path also emits, so in an environment with
|
|
// no git it passes for the wrong reason. Not silent overall — its two siblings fail ENOENT there,
|
|
// loudly. Since ersatztv#887 no such environment runs this suite: `docker/Dockerfile` stopped
|
|
// running it rather than excluding the files that cannot run, so the residual is now reachable
|
|
// only on a developer machine with no git installed.
|
|
it('really throws outside a repository, rather than returning an empty population', () => {
|
|
const notARepo = mkdtempSync(join(tmpdir(), 'etv-819-bare-'));
|
|
repos.push(notARepo);
|
|
mkdirSync(join(notARepo, 'src'), { recursive: true });
|
|
|
|
// `GIT_CEILING_DIRECTORIES` stops git walking up into an enclosing repository — without it this
|
|
// test depends on where TMPDIR happens to point, and reports the wrong throw when TMPDIR sits
|
|
// inside a checkout.
|
|
const ceiling: GitRunner = (args) =>
|
|
execFileSync('git', [...args], {
|
|
encoding: 'utf8',
|
|
// Same stderr suppression `runGitDefault` sets, for the same reason: without it git's own
|
|
// `fatal: not a git repository` prints on every run of this file.
|
|
stdio: ['ignore', 'pipe', 'pipe'],
|
|
env: { ...process.env, GIT_CEILING_DIRECTORIES: tmpdir() }
|
|
});
|
|
|
|
expect(() => resolveTrackedSourceFiles(notARepo, ceiling)).toThrow(/could not read the git index/);
|
|
});
|
|
});
|