golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(3582): stop the cold-tree guard from racing the builders it guards against (#3665)

Browse Source 
The guard added in #3656 asserts that this test leaves the real repo hooks/ directory
alone. It compared a RAW listing before and after — and just failed on the runner:

    the real repo hooks/ directory listing must be unchanged by this test
    -   '.dist-staging-20858',
        'dist',
        ...

Nothing was wrong with the test's own behaviour. A CONCURRENT scripts/build-hooks.js —
nine test files invoke it from before() hooks — created hooks/.dist-staging-20858 inside
the comparison window. The assertion assumed the shared hooks/ directory is stable for the
duration of a test, which is precisely the assumption this line of work exists to
disprove. The race-detector raced.

The intent is right and is kept: this test must not add or remove anything in the repo.
Only the comparison changes — both snapshots are now filtered through shouldCopyHookEntry,
the same rule the fixture itself uses, so transient build scratch that is not this test's
doing and is excluded from the fixture anyway no longer registers as a difference.

Proven by execution: with a .dist-staging dir injected mid-window the filtered listings
compare equal, while the unfiltered listings provably differ by exactly that entry — so
the old comparison would have failed and the new one is immune rather than merely quieter.
The injected directory is removed afterwards and hooks/ is confirmed byte-identical.

Checked for the same shape elsewhere: this is the only raw listing comparison of the live
hooks/ directory in the file or the repo. The second title in the failure output is the
describe() wrapper around this same test, not a sibling.

Refs #3582

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-19 09:00:39 -04:00
committed by GitHub
parent 1bf73d957b
commit 249c586a40
1 changed files with 23 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
tests/gsd-check-update-worker-platform-gate.test.cjs
View File

@@ -163,7 +163,7 @@ describe('worker delegates the npm spawn (does not re-open the gate, #498)', ()
const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const { buildColdInstallTree, REPO_ROOT } = require('./helpers/cold-runtime-lib-fixture.cjs');
const { buildColdInstallTree, REPO_ROOT, shouldCopyHookEntry } = require('./helpers/cold-runtime-lib-fixture.cjs');
const { cleanup } = require('./helpers.cjs');
describe('cold-runtime-lib-fixture.cjs: #3582 tolerates a concurrent hooks/.dist-staging-<pid> dir', () => {
@@ -198,7 +198,21 @@ describe('worker delegates the npm spawn (does not re-open the gate, #498)', ()
path.join(fakeBinDir, 'ensure-runtime-build.cjs'),
);
const realHooksBefore = fs.readdirSync(path.join(REPO_ROOT, 'hooks')).sort();
// Filtered by shouldCopyHookEntry — the same predicate the fixture
// itself uses to decide what to copy. Up to nine other test files'
// before() hooks concurrently invoke scripts/build-hooks.js, which
// creates/removes hooks/.dist-staging-<pid> at unpredictable times, so
// a RAW (unfiltered) before/after listing comparison of the live
// hooks/ dir is itself racy — it can observe a sibling build's
// transient staging dir appear or vanish between the two snapshots and
// fail with no real defect. Filtering both snapshots the same way the
// fixture does preserves the actual intent (this test adds/removes no
// REAL entry in the repo's hooks/) while tolerating scratch that is
// not this test's doing and is excluded from the fixture anyway.
const realHooksBefore = fs
.readdirSync(path.join(REPO_ROOT, 'hooks'))
.filter(shouldCopyHookEntry)
.sort();
const cold = buildColdInstallTree({ repoRoot: fakeRepoRoot });
t.after(cold.cleanup);
@@ -216,11 +230,16 @@ describe('worker delegates the npm spawn (does not re-open the gate, #498)', ()
'fixture must still contain the representative hooks/lib/ subdir file',
);
const realHooksAfter = fs.readdirSync(path.join(REPO_ROOT, 'hooks')).sort();
const realHooksAfter = fs
.readdirSync(path.join(REPO_ROOT, 'hooks'))
.filter(shouldCopyHookEntry)
.sort();
assert.deepEqual(
realHooksAfter,
realHooksBefore,
'the real repo hooks/ directory listing must be unchanged by this test',
'the real repo hooks/ directory listing, filtered by shouldCopyHookEntry, must be unchanged by ' +
'this test (transient hooks/.dist-staging-<pid> entries from concurrent build-hooks.js runs are ' +
'excluded from the comparison since this test does not own them)',
);
});
});