mirror of
https://github.com/garrytan/gstack.git
synced 2026-08-21 13:37:14 +02:00
test: revert GSTACK_HOME injection in the free runner — shared mutable state
The first full run under the strict runner surfaced 12 failures with one root cause: injecting a single throwaway GSTACK_HOME per invocation made 6,900 tests share a MUTABLE scratch home. gstack-config tests wrote keys into it; relink and update-check tests then read them (e.g. relink saw skill_prefix left behind by a config test and produced prefixed names). All 12 pass when run directly. TMPDIR isolation stays (mkdtemp inside it is still per-call unique). Tests needing GSTACK_HOME isolation mkdtemp their own per test — the repo convention — and hermetic-env covers E2E children. The env-dump pin now asserts GSTACK_HOME passes through UNTOUCHED so the injection can't come back. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
0a292dc063
commit
39715addff
@@ -446,12 +446,14 @@ function shardEpilogue(outcome: FreeShardOutcome, totalShards: number): string {
|
||||
* so tests can pin the summary-missing => failure backstop; fake passing
|
||||
* commands must print a synthetic `Ran N tests across M files. [Xms]` line.
|
||||
*
|
||||
* Per-shard state isolation: each spawned child gets its own throwaway
|
||||
* GSTACK_HOME and TMPDIR (TEMP/TMP on Windows) so shards — and the bun
|
||||
* --parallel workers inside the full-suite invocation — can't contend on the
|
||||
* operator's real ~/.gstack or trip over each other's temp files. Tests that
|
||||
* mkdtemp their own state dirs are unaffected: this only moves the DEFAULT
|
||||
* location. The throwaway dirs are removed when the shard finishes.
|
||||
* Per-shard temp isolation: each spawned child gets its own throwaway TMPDIR
|
||||
* (TEMP/TMP on Windows) so shards can't trip over each other's temp files.
|
||||
* Deliberately NOT GSTACK_HOME: injecting one shared scratch home for a whole
|
||||
* invocation made 6,900 tests share a MUTABLE state dir — config tests wrote
|
||||
* keys into it and relink/update-check tests then read them (measured: 12
|
||||
* cross-contamination failures on the first full run). Tests that need
|
||||
* GSTACK_HOME isolation mkdtemp their own per test — the repo convention —
|
||||
* and the hermetic-env machinery covers E2E children.
|
||||
*/
|
||||
export async function runFreeShard(
|
||||
files: string[],
|
||||
@@ -482,11 +484,8 @@ export async function runFreeShard(
|
||||
|
||||
const env = { ...(options.env ?? process.env) };
|
||||
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-free-shard-'));
|
||||
const gstackHome = path.join(stateDir, 'gstack-home');
|
||||
const childTmp = path.join(stateDir, 'tmp');
|
||||
fs.mkdirSync(gstackHome);
|
||||
fs.mkdirSync(childTmp);
|
||||
env.GSTACK_HOME = gstackHome;
|
||||
env.TMPDIR = childTmp;
|
||||
env.TEMP = childTmp;
|
||||
env.TMP = childTmp;
|
||||
|
||||
@@ -253,15 +253,18 @@ describe('test-free-shards: strict shard execution', () => {
|
||||
expect(lines.some((l) => /^\[test:free\] shard 7\/20: 0 files, 0s, pass$/.test(l))).toBe(true);
|
||||
});
|
||||
|
||||
test('each spawned shard gets its own throwaway GSTACK_HOME and TMPDIR, removed after the run', async () => {
|
||||
test('spawned shard gets throwaway TMPDIR but NEVER an injected GSTACK_HOME', async () => {
|
||||
// GSTACK_HOME injection was tried and reverted: one shared scratch home
|
||||
// per invocation made 6,900 tests share MUTABLE state — config tests
|
||||
// wrote keys that relink/update-check tests then read (12 measured
|
||||
// cross-contamination failures). This pin keeps the regression out.
|
||||
const captureDir = fs.mkdtempSync(path.join(os.tmpdir(), 'free-shard-env-'));
|
||||
const dump = path.join(captureDir, 'env.json');
|
||||
try {
|
||||
const script =
|
||||
`const fs = require("fs");`
|
||||
+ `fs.writeFileSync(${JSON.stringify(dump)}, JSON.stringify({`
|
||||
+ ` home: process.env.GSTACK_HOME, tmp: process.env.TMPDIR,`
|
||||
+ ` homeExists: fs.existsSync(process.env.GSTACK_HOME || ""),`
|
||||
+ ` home: process.env.GSTACK_HOME ?? null, tmp: process.env.TMPDIR,`
|
||||
+ ` tmpExists: fs.existsSync(process.env.TMPDIR || "") }));`
|
||||
+ `console.log(${JSON.stringify(SUMMARY_1)});`;
|
||||
const outcome = await runFreeShard(['env-dump'], 1, 1, {
|
||||
@@ -271,13 +274,13 @@ describe('test-free-shards: strict shard execution', () => {
|
||||
});
|
||||
expect(outcome.status).toBe('passed');
|
||||
const seen = JSON.parse(fs.readFileSync(dump, 'utf8'));
|
||||
expect(seen.home).toContain('gstack-free-shard-');
|
||||
expect(seen.homeExists).toBe(true);
|
||||
// GSTACK_HOME passes through untouched (whatever the parent had, incl. unset).
|
||||
expect(seen.home).toBe(process.env.GSTACK_HOME ?? null);
|
||||
// TMPDIR is a per-shard throwaway, cleaned up once the shard finishes.
|
||||
expect(seen.tmp).toContain('gstack-free-shard-');
|
||||
expect(seen.tmpExists).toBe(true);
|
||||
expect(seen.home).not.toBe(process.env.GSTACK_HOME ?? '');
|
||||
expect(seen.tmp).not.toBe(process.env.TMPDIR ?? '');
|
||||
// The throwaway state dir is cleaned up once the shard finishes.
|
||||
expect(fs.existsSync(seen.home)).toBe(false);
|
||||
expect(fs.existsSync(seen.tmp)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(captureDir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user