fix(test): isolate cli-e2e from shared mini-repo fixture (#954)

Deterministic fix for the Windows-flaky pipeline-graph-golden test.

Root cause
  cli-e2e.test.ts wrote into the SHARED fixture directory
  (test/fixtures/mini-repo/) — git init, analyze run that creates
  AGENTS.md, CLAUDE.md, .claude/, .gitnexus/. When pipeline-graph-golden
  ran in parallel, its `cpSync` of the source directory could capture
  the mid-flight pollution before cli-e2e's afterAll cleanup fired.
  macOS/Ubuntu won the race often enough that the flake presented as
  Windows-only.

Fix
  cli-e2e now copies mini-repo into a fresh `mkdtemp`'d parent whose
  basename is `mini-repo` (preserving `--repo mini-repo` CLI lookup by
  basename), runs git-init there, and rm's the whole tmpdir in afterAll.
  The shared fixture source is never touched.

  Fallout from the cwd change: bare `--import tsx` specifiers (2
  spawnSync + 1 spawn) can't resolve `tsx` from an os.tmpdir cwd where
  there is no node_modules. Switched them to the already-existing
  `tsxImportUrl` (absolute file:// URL to the tsx loader), matching
  the `runCliOutsideProject` pattern that was already set up for this
  exact case.

  Updated the "MINI_REPO is inside the project tree" comment in the
  `status on non-indexed repo` test — MINI_REPO is now in os.tmpdir,
  so the rationale for using a separate throwaway tmp git repo is
  different (but still valid: previous tests in the suite create
  MINI_REPO/.gitnexus, which findRepo() would pick up).

  Also updated pipeline-graph-golden's comment explaining WHY it
  copies to tmp — it's now defense-in-depth rather than a necessity,
  so a future test that adds files to the source can't silently
  regress the golden.

Verification
  - 5x consecutive `cli-e2e + pipeline-graph-golden` runs: 20/20 pass
    (deterministic)
  - 3x full suite including pipeline.test: 27/27 pass
  - test/fixtures/mini-repo/ post-run contents: only `src/` —
    zero pollution from any test
  - macOS/Ubuntu behavior unchanged (they were passing; tmpdir
    isolation is purely additive)
This commit is contained in:
Gergő Magyar 2026-04-18 12:54:59 +01:00 • committed by GitHub
parent 043a37a20e
commit 3cfd0d23df
2 changed files with 54 additions and 35 deletions

View file

@ -20,7 +20,22 @@ import { createRequire } from 'module';
const testDir = path.dirname(fileURLToPath(import.meta.url));
const repoRoot = path.resolve(testDir, '../..');
const cliEntry = path.join(repoRoot, 'src/cli/index.ts');
const MINI_REPO = path.resolve(testDir, '..', 'fixtures', 'mini-repo');
const FIXTURE_SRC = path.resolve(testDir, '..', 'fixtures', 'mini-repo');
// `MINI_REPO` is a *per-run temp copy* of the fixture, not the shared
// source. Writing into the shared source races with other suites that
// ingest it read-only (pipeline-graph-golden, pipeline.test) — those
// suites copy the source to their own tmp dir but the copy happens at
// `beforeAll`, so if this suite's analyze has already created AGENTS.md
// / CLAUDE.md / .claude/ in the source when the other suite's cpSync
// runs, the pollution is captured before the isolation kicks in.
//
// The deterministic fix: this suite never touches the shared source.
// `beforeAll` copies the fixture to a fresh mkdtemp'd directory whose
// basename is `mini-repo` (so `--repo mini-repo` lookup by basename
// still works), `afterAll` rms the parent tmpdir.
let MINI_REPO: string;
let tmpParent: string;
// Absolute file:// URL to tsx loader — needed when spawning CLI with cwd
// outside the project tree (bare 'tsx' specifier won't resolve there).
@ -31,40 +46,39 @@ const tsxPkgDir = path.dirname(_require.resolve('tsx/package.json'));
const tsxImportUrl = pathToFileURL(path.join(tsxPkgDir, 'dist', 'loader.mjs')).href;
beforeAll(() => {
// Copy the fixture into an isolated tmpdir named `mini-repo` so that the
// `--repo mini-repo` CLI arg (which matches by basename) still works.
tmpParent = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-cli-e2e-'));
MINI_REPO = path.join(tmpParent, 'mini-repo');
fs.cpSync(FIXTURE_SRC, MINI_REPO, { recursive: true });
// Initialize mini-repo as a git repo so the CLI analyze command
// can run the full pipeline (it requires a .git directory).
const gitDir = path.join(MINI_REPO, '.git');
if (!fs.existsSync(gitDir)) {
spawnSync('git', ['init'], { cwd: MINI_REPO, stdio: 'pipe' });
spawnSync('git', ['add', '-A'], { cwd: MINI_REPO, stdio: 'pipe' });
spawnSync('git', ['commit', '-m', 'initial commit'], {
cwd: MINI_REPO,
stdio: 'pipe',
env: {
...process.env,
GIT_AUTHOR_NAME: 'test',
GIT_AUTHOR_EMAIL: 'test@test',
GIT_COMMITTER_NAME: 'test',
GIT_COMMITTER_EMAIL: 'test@test',
},
});
}
spawnSync('git', ['init'], { cwd: MINI_REPO, stdio: 'pipe' });
spawnSync('git', ['add', '-A'], { cwd: MINI_REPO, stdio: 'pipe' });
spawnSync('git', ['commit', '-m', 'initial commit'], {
cwd: MINI_REPO,
stdio: 'pipe',
env: {
...process.env,
GIT_AUTHOR_NAME: 'test',
GIT_AUTHOR_EMAIL: 'test@test',
GIT_COMMITTER_NAME: 'test',
GIT_COMMITTER_EMAIL: 'test@test',
},
});
});
afterAll(() => {
// Clean up all files/dirs created by analyze (git init, .gitnexus output,
// AI context files, skill files, .gitignore) so parallel tests like
// pipeline-graph-golden see a pristine fixture.
for (const entry of ['.git', '.gitnexus', '.claude', 'AGENTS.md', 'CLAUDE.md', '.gitignore']) {
const fullPath = path.join(MINI_REPO, entry);
if (fs.existsSync(fullPath)) {
fs.rmSync(fullPath, { recursive: true, force: true });
}
// Entire tmp copy goes away — no selective cleanup needed. The shared
// `test/fixtures/mini-repo/` source was never touched.
if (tmpParent) {
fs.rmSync(tmpParent, { recursive: true, force: true });
}
});
function runCli(command: string, cwd: string, timeoutMs = 15000) {
return spawnSync(process.execPath, ['--import', 'tsx', cliEntry, command], {
return spawnSync(process.execPath, ['--import', tsxImportUrl, cliEntry, command], {
cwd,
encoding: 'utf8',
timeout: timeoutMs,
@ -84,7 +98,7 @@ function runCli(command: string, cwd: string, timeoutMs = 15000) {
* can pass flags (e.g. --help) or omit a command entirely.
*/
function runCliRaw(extraArgs: string[], cwd: string, timeoutMs = 15000) {
return spawnSync(process.execPath, ['--import', 'tsx', cliEntry, ...extraArgs], {
return spawnSync(process.execPath, ['--import', tsxImportUrl, cliEntry, ...extraArgs], {
cwd,
encoding: 'utf8',
timeout: timeoutMs,
@ -190,9 +204,11 @@ describe('CLI end-to-end', () => {
}
it('status on non-indexed repo reports not indexed', () => {
// MINI_REPO is inside the project tree so findRepo() walks up and
// finds the parent project's .gitnexus. Use an isolated temp git
// repo to guarantee no .gitnexus exists anywhere in the path.
// Even though MINI_REPO is now in an isolated tmpdir, previous tests
// in this suite may have created MINI_REPO/.gitnexus via analyze,
// and findRepo() walks up so any `.gitnexus` along the path still
// counts. This test needs a GUARANTEED pristine repo to assert the
// "not indexed" output, so it mints its own throwaway tmp git repo.
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-noindex-'));
try {
spawnSync('git', ['init'], { cwd: tmpDir, stdio: 'pipe' });
@ -410,7 +426,7 @@ describe('CLI end-to-end', () => {
process.execPath,
[
'--import',
'tsx',
tsxImportUrl,
cliEntry,
'cypher',
'MATCH (n) RETURN n LIMIT 500',
@ -469,7 +485,7 @@ describe('CLI end-to-end', () => {
return new Promise<void>((resolve, reject) => {
const child = spawn(
process.execPath,
['--import', 'tsx', cliEntry, 'eval-server', '--port', '0', '--idle-timeout', '3'],
['--import', tsxImportUrl, cliEntry, 'eval-server', '--port', '0', '--idle-timeout', '3'],
{
cwd: MINI_REPO,
stdio: ['ignore', 'pipe', 'pipe'],

View file

@ -132,9 +132,12 @@ describe('pipeline graph golden', () => {
let tmpDir: string;
beforeAll(async () => {
// Copy the fixture to a temp directory so parallel tests (cli-e2e)
// that create AGENTS.md / CLAUDE.md / .claude/ in the shared fixture
// don't pollute the golden snapshot.
// Copy the fixture to a temp directory as defense-in-depth against
// parallel tests writing into the shared fixture source. cli-e2e
// was the historical offender — it now copies to its own tmpdir
// (see test/integration/cli-e2e.test.ts `beforeAll`) — but this
// cpSync stays as a belt-and-suspenders guarantee that any future
// test adding files to the source won't pollute the golden snapshot.
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-golden-'));
fs.cpSync(FIXTURE_SRC, tmpDir, { recursive: true });