mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-11 19:20:26 +01:00
fix: graph checkout ignores the tree's export attributes; the tracer runs the free runner's own command and sandbox
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
6b4e6c5f05
commit
8750d33ef0
4 files changed
+45
-24
No files matched your search
+4
-4
@@ -6,7 +6,7 @@ GST-167 is the plan to skip a free test on a pull request only when main already
|
||||
|
||||
`scripts/test-impact.ts` is a Bun CLI:
|
||||
|
||||
- `trace [--out <dir>] <test-file>...` runs each file with `bun test` under strace and writes its read set to `<dir>` (default `.context/test-impact/traces`). It needs Linux with strace installed.
|
||||
- `trace [--out <dir>] <test-file>...` runs each file under strace with the free runner's own `bun test` arguments and shard sandbox, and writes its read set to `<dir>` (default `.context/test-impact/traces`). It needs Linux with strace installed.
|
||||
- `key --tree <ref> [--base <ref>] [--traces <dir>] [--json] <test-file>...` prints each file's key at `<ref>`, or why it has none.
|
||||
- `explain [--tree <ref>] [--base <ref>] [--traces <dir>] <test-file>` prints every input of one file's key and why it is or isn't eligible.
|
||||
|
||||
@@ -18,12 +18,12 @@ The recorder runs `strace -f -y -qq -v` with `openat, open, execve, execveat, ge
|
||||
|
||||
## What the key covers
|
||||
|
||||
`lib/test-impact/key.ts` reads everything at the given tree through git, never from the working tree. The key is a sha256 over:
|
||||
`lib/test-impact/key.ts` reads everything at the given tree through git, never from the working tree. The graph runs over a checkout of that tree made through a private index, so the tree's own `.gitattributes` can't hide files the way `export-ignore` would in `git archive`. The key is a sha256 over:
|
||||
|
||||
- each traced path's bytes (read with `git cat-file --batch`), or that it is a directory, or that it is absent;
|
||||
- the names in each traced directory listing, at the tree;
|
||||
- the test's forward import-graph closure from `lib/pregate/import-graph.ts`, up to 12 hops;
|
||||
- `bun.lock`, every `tsconfig*.json` and the `dependencies` in `.gstack/pregate.json`;
|
||||
- `bun.lock`, `bunfig.toml`, every `tsconfig*.json` and the `dependencies` in `.gstack/pregate.json`;
|
||||
- the key code itself: the tracer, the key, the CLI, the free-test runner and planner (`scripts/test-free-shards.ts`) with every repo module it imports (shard engine, HOME guard, CI health, Windows curation, the paid/free split, the state root), the import graph and `lib/pregate/lane-selection.ts`;
|
||||
- the bytes of each executable outside the repo the trace ran (git, bash, tar and so on), read on the machine computing the key;
|
||||
- the runtime (platform, architecture, Bun version) and the values of `CI`, `GITHUB_ACTIONS`, `TZ`, `LANG`, `LC_ALL`, `NODE_ENV`, `EVALS` and `EVALS_TIER`.
|
||||
@@ -39,4 +39,4 @@ A file is ineligible, so it always runs, when:
|
||||
- its import-graph walk reaches the 12-hop cap. The walk fails closed: a deeper chain makes the test ineligible instead of cutting the closure short;
|
||||
- the tree changes any key-code file against the base. Then every file runs.
|
||||
|
||||
Limits: strace can't see which environment variables a test reads, so only the allowlist above is in the key, and a test that depends on another variable has to read it through a file the trace sees or stay off the reuse path. The recorder runs `bun test` directly, not inside the free runner's per-shard sandbox, so PR 3 should record traces through the runner. Timing assertions, flaky history and the shard's execution grouping are for PR 3 and PR 4, which have the receipts to check them against. Windows has no tracer, so the Windows lane stays full.
|
||||
Limits: strace can't see which environment variables a test reads, so only the allowlist above is in the key, and a test that depends on another variable has to read it through a file the trace sees or stay off the reuse path. The recorder reuses the runner's arguments and sandbox but not its home-write guard, which only watches and changes nothing a test reads. Timing assertions, flaky history and the shard's execution grouping are for PR 3 and PR 4, which have the receipts to check them against. Windows has no tracer, so the Windows lane stays full.
|
||||
+13
-10
@@ -8,7 +8,7 @@
|
||||
* - listed: the sorted entry names of every repo directory the trace listed;
|
||||
* - graph: the forward import-graph closure of the test (lib/pregate/import-graph.ts),
|
||||
* capped at MAX_HOPS hops; a closure that reaches the cap is ineligible, never truncated;
|
||||
* - global: bun.lock, every tsconfig*.json and the pregate `dependencies`;
|
||||
* - global: bun.lock, bunfig.toml, every tsconfig*.json and the pregate `dependencies`;
|
||||
* - code: KEY_CODE_FILES, the runner, planner, shard engine, graph and key code;
|
||||
* - tool: the bytes of each executable outside the repo the trace ran (git, bash, tar…), read on this machine;
|
||||
* - runtime and env: the runtime manifest and ENV_ALLOWLIST values.
|
||||
@@ -58,8 +58,8 @@ export interface KeyOptions {
|
||||
now?: number; runtime?: Record<string, string>; env?: Record<string, string | undefined>;
|
||||
}
|
||||
|
||||
function git(repoRoot: string, args: string[], input?: Buffer | string): Buffer {
|
||||
const r = spawnSync('git', args, { cwd: repoRoot, input, timeout: 300_000, maxBuffer: 1024 * 1024 * 1024 });
|
||||
function git(repoRoot: string, args: string[], input?: Buffer | string, env?: NodeJS.ProcessEnv): Buffer {
|
||||
const r = spawnSync('git', args, { cwd: repoRoot, input, env, timeout: 300_000, maxBuffer: 1024 * 1024 * 1024 });
|
||||
if (r.status !== 0) throw new Error(`git ${args.join(' ')} failed: ${r.error?.message ?? r.stderr?.toString().trim() ?? r.status}`);
|
||||
return r.stdout;
|
||||
}
|
||||
@@ -104,19 +104,22 @@ export function catText(repoRoot: string, treeish: string, rel: string): string
|
||||
return r.status === 0 ? r.stdout : null;
|
||||
}
|
||||
|
||||
/** A tree-ish exported to a fresh temp directory with every file staged, so the import graph (git ls-files) sees exactly that tree. */
|
||||
/**
|
||||
* A tree-ish checked out into a fresh temp directory with every file staged, so the import graph (git ls-files) sees exactly
|
||||
* that tree. It goes through a private index and checkout-index, never `git archive`, whose export-ignore and export-subst
|
||||
* attributes would let the tree itself hide files from the graph.
|
||||
*/
|
||||
export function materializeTree(repoRoot: string, treeish: string): string {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-test-impact-tree-'));
|
||||
const tar = `${dir}.tar`;
|
||||
const env = { ...process.env, GIT_INDEX_FILE: `${dir}.index` };
|
||||
try {
|
||||
git(repoRoot, ['archive', '--format=tar', '-o', tar, treeish]);
|
||||
const x = spawnSync('tar', ['-xf', tar, '-C', dir], { timeout: 300_000 });
|
||||
if (x.status !== 0) throw new Error(`tar -xf failed: ${x.stderr?.toString().trim()}`);
|
||||
git(repoRoot, ['read-tree', treeish], undefined, env);
|
||||
git(repoRoot, ['checkout-index', '-a', '-f', `--prefix=${dir}/`], undefined, env);
|
||||
git(dir, ['init', '-q']);
|
||||
git(dir, ['add', '-A', '-f']);
|
||||
return dir;
|
||||
} catch (error) { fs.rmSync(dir, { recursive: true, force: true }); throw error; }
|
||||
finally { fs.rmSync(tar, { force: true }); }
|
||||
finally { fs.rmSync(env.GIT_INDEX_FILE, { force: true }); }
|
||||
}
|
||||
|
||||
/** The runtime manifest a key binds: platform, architecture and Bun version. */
|
||||
@@ -187,7 +190,7 @@ export function computeKeys(opts: KeyOptions): KeyReport {
|
||||
const envNow = envDigests(opts.env ?? process.env);
|
||||
const runtime = opts.runtime ?? runtimeManifest();
|
||||
const shared: KeyInput[] = [
|
||||
...[...treeIdx.entries.keys()].filter(p => p === 'bun.lock' || TSCONFIG.test(p)).map(p => ({ kind: 'global' as const, path: p, digest: '' })),
|
||||
...[...treeIdx.entries.keys()].filter(p => p === 'bun.lock' || p === 'bunfig.toml' || TSCONFIG.test(p)).map(p => ({ kind: 'global' as const, path: p, digest: '' })),
|
||||
{ kind: 'global', path: `${PREGATE_CONFIG_PATH}#dependencies`, digest: sha(JSON.stringify(dependencies)) },
|
||||
...KEY_CODE_FILES.map(p => ({ kind: 'code' as const, path: p, digest: '' })),
|
||||
...Object.entries(runtime).sort().map(([k, v]) => ({ kind: 'runtime' as const, path: k, digest: v })),
|
||||
|
||||
+13
-8
@@ -3,7 +3,8 @@
|
||||
* test-impact — trace free tests and compute their reuse keys (GST-167 PR 2; docs/test-impact.md).
|
||||
*
|
||||
* scripts/test-impact.ts trace [--out <dir>] <test-file>...
|
||||
* Run each file with `bun test` under strace (Linux) and write its read set to <dir>
|
||||
* Run each file under strace (Linux) exactly as the free runner does (its bun test arguments and shard
|
||||
* sandbox) and write its read set to <dir>
|
||||
* (default .context/test-impact/traces).
|
||||
* scripts/test-impact.ts key --tree <ref> [--base <ref>] [--traces <dir>] [--json] <test-file>...
|
||||
* Print each file's key at <ref>, or why it is ineligible. With --base, the key code is
|
||||
@@ -17,7 +18,9 @@ import { spawnSync } from 'node:child_process';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { bunIsolationArgs } from '../lib/bun-isolation';
|
||||
import { randomUUID } from 'node:crypto';
|
||||
import { createShardSandbox } from './lib/shard-engine';
|
||||
import { buildShardArgs, disableGitAutoMaintenance } from './test-free-shards';
|
||||
import * as localKey from '../lib/test-impact/key';
|
||||
import type { KeyOptions, KeyReport } from '../lib/test-impact/key';
|
||||
import { STRACE_FLAGS, TRACE_VERSION, envDigests, parseStrace, tracePath, type TraceRecord } from '../lib/test-impact/trace';
|
||||
@@ -55,20 +58,22 @@ function trace(root: string, argv: string[]): number {
|
||||
const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: root, encoding: 'utf8', timeout: 30_000 });
|
||||
let failed = 0;
|
||||
for (const file of files.map(f => relTo(root, f))) {
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-test-impact-run-'));
|
||||
const sandbox = createShardSandbox('gstack-test-impact-run-', process.env, { realpath: true });
|
||||
const env = { ...sandbox.env, BROWSE_STATE_FILE: path.join(sandbox.stateDir, '.gstack', 'browse.json'), GSTACK_FREE_SHARD_ID: randomUUID(), BUN_RUNTIME_TRANSPILER_CACHE_PATH: '0' };
|
||||
disableGitAutoMaintenance(env);
|
||||
const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-test-impact-log-'));
|
||||
try {
|
||||
const log = path.join(logDir, 'strace.txt');
|
||||
const r = spawnSync('strace', [...STRACE_FLAGS, '-o', log, '--', process.execPath, ...bunIsolationArgs(), 'test', path.join(root, file), '--timeout=30000', '--max-concurrency=1'],
|
||||
{ cwd: root, env: { ...process.env, TMPDIR: tmp, TMP: tmp, TEMP: tmp, BUN_RUNTIME_TRANSPILER_CACHE_PATH: '0' }, timeout: 600_000, maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'pipe'] });
|
||||
const set = parseStrace(fs.existsSync(log) ? fs.readFileSync(log, 'utf8') : '', { repoRoot: root, home: os.homedir(), tmpDir: tmp, pathDirs: (process.env.PATH ?? '').split(path.delimiter) });
|
||||
const record: TraceRecord = { version: TRACE_VERSION, test: file, tracedAt: new Date().toISOString(), commit: head.status === 0 ? head.stdout.trim() : null, exitCode: r.status, runtime: localKey.runtimeManifest(), env: envDigests(process.env), ...set };
|
||||
const r = spawnSync('strace', [...STRACE_FLAGS, '-o', log, '--', process.execPath, ...buildShardArgs([file], { rootDir: root })],
|
||||
{ cwd: root, env, timeout: 600_000, maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'pipe'] });
|
||||
const set = parseStrace(fs.existsSync(log) ? fs.readFileSync(log, 'utf8') : '', { repoRoot: root, home: os.homedir(), tmpDir: sandbox.stateDir, pathDirs: (process.env.PATH ?? '').split(path.delimiter) });
|
||||
const record: TraceRecord = { version: TRACE_VERSION, test: file, tracedAt: new Date().toISOString(), commit: head.status === 0 ? head.stdout.trim() : null, exitCode: r.status, runtime: localKey.runtimeManifest(), env: envDigests(env), ...set };
|
||||
fs.writeFileSync(tracePath(out, file), JSON.stringify(record, null, 2) + '\n');
|
||||
if (r.status !== 0) failed++;
|
||||
const verdict = set.ineligible.length ? `ineligible: ${set.ineligible.join('; ')}` : 'eligible';
|
||||
console.log(`traced ${file}: exit ${r.status ?? r.signal}, ${set.paths.length} paths, ${Object.keys(set.dirs).length} listed dirs, ${verdict} → ${path.relative(root, tracePath(out, file))}`);
|
||||
} finally {
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
fs.rmSync(sandbox.stateDir, { recursive: true, force: true });
|
||||
fs.rmSync(logDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -80,6 +80,17 @@ describe('test-impact key: what changes it', () => {
|
||||
expect(keyAt(repo, 'HEAD', t).key).not.toBe(before.key);
|
||||
});
|
||||
|
||||
test('a branch .gitattributes export-ignore cannot hide a graph input', () => {
|
||||
const t = 'test/a.test.ts';
|
||||
const repo = fixture({ 'lib/hidden.ts': 'export const x = 1;\n', [t]: "import { x } from '../lib/hidden';\nimport { test } from 'bun:test';\ntest('a', () => x);\n" });
|
||||
traceFrom(repo, t);
|
||||
const before = keyAt(repo, 'main', t);
|
||||
commit(repo, { '.gitattributes': 'lib/hidden.ts export-ignore\n', 'lib/hidden.ts': 'export const x = 2;\n' });
|
||||
const after = keyAt(repo, 'HEAD', t);
|
||||
expect(after.inputs).toContainEqual(expect.objectContaining({ kind: 'graph', path: 'lib/hidden.ts' }));
|
||||
expect(after.key).not.toBe(before.key);
|
||||
});
|
||||
|
||||
test('a file added under a directory the test listed changes the key', () => {
|
||||
const t = 'test/listing.test.js';
|
||||
const repo = fixture({ 'test/data/a.txt': 'a\n', 'test/data/b.txt': 'b\n', [t]: `${NODE_TEST}test('lists', () => require('fs').readdirSync(require('path').join(__dirname, 'da' + 'ta')));\n` });
|
||||
@@ -221,11 +232,13 @@ describe('scripts/test-impact.ts', () => {
|
||||
});
|
||||
|
||||
const hasStrace = process.platform === 'linux' && spawnSync('strace', ['-V'], { timeout: 10_000 }).status === 0;
|
||||
test.if(hasStrace)('trace (real strace): a spawned bin that sources a sibling records the sibling, and editing it changes the key', () => {
|
||||
test.if(hasStrace)('trace (real strace, the runner\'s own command): a spawned bin that sources a sibling and the bunfig preload are recorded, and editing the sibling changes the key', () => {
|
||||
const t = 'test/tool2.test.ts';
|
||||
const repo = fixture({
|
||||
'bin/tool2': '#!/bin/bash\ncd "$(dirname "$0")"\nsource ./tool2-lib.sh\ngreet\n',
|
||||
'bin/tool2-lib.sh': 'greet() { echo hi; }\n',
|
||||
'bunfig.toml': '[test]\npreload = ["./test/setup.ts"]\n',
|
||||
'test/setup.ts': 'export {};\n',
|
||||
[t]: "import { expect, test } from 'bun:test';\nimport { spawnSync } from 'node:child_process';\nimport * as path from 'node:path';\ntest('greets', () => expect(spawnSync(path.join(import.meta.dir, '..', 'bin', 'tool2'), { encoding: 'utf8', timeout: 10000 }).stdout.trim()).toBe('hi'));\n",
|
||||
});
|
||||
fs.chmodSync(path.join(repo.work, 'bin/tool2'), 0o755);
|
||||
@@ -237,7 +250,7 @@ describe('scripts/test-impact.ts', () => {
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.out).toContain(`traced ${t}: exit 0`);
|
||||
const rec = readTrace(traces, t)!;
|
||||
expect(rec.paths).toEqual(expect.arrayContaining([t, 'bin/tool2', 'bin/tool2-lib.sh']));
|
||||
expect(rec.paths).toEqual(expect.arrayContaining([t, 'bin/tool2', 'bin/tool2-lib.sh', 'bunfig.toml', 'test/setup.ts']));
|
||||
expect(rec.ineligible).toEqual([]);
|
||||
const env = { ...process.env, ...repo.env };
|
||||
const runtime = { platform: process.platform, arch: process.arch, bun: Bun.version };
|
||||
|
||||
Reference in new issue
Block a user