From 94130e99682606c85402acf56e107b4416d31039 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 1 Jun 2026 09:45:58 -0400 Subject: [PATCH] test(#425): replace source-grep assertions with behavioral coverage (#573) Remove three source-grep describe blocks from bug-1834 and bug-2136 test files that anchored on byte-offset windows in install.js source. The same regressions are already fully covered by the E2E behavioral tests (Section 1 in bug-1834, Part 4 in bug-2136) that invoke the actual installer and inspect the installed files directly. Co-authored-by: Claude Sonnet 4.6 --- tests/bug-1834-sh-hooks-installed.test.cjs | 55 ----------------- tests/bug-2136-sh-hook-version.test.cjs | 69 ---------------------- 2 files changed, 124 deletions(-) diff --git a/tests/bug-1834-sh-hooks-installed.test.cjs b/tests/bug-1834-sh-hooks-installed.test.cjs index cfca61666..38a552700 100644 --- a/tests/bug-1834-sh-hooks-installed.test.cjs +++ b/tests/bug-1834-sh-hooks-installed.test.cjs @@ -137,58 +137,3 @@ describe('#1834: installer deploys .sh hooks alongside .js hooks', () => { } }); }); - -// ───────────────────────────────────────────────────────────────────────────── -// 2. Source-level correctness: install.js copies non-.js files -// ───────────────────────────────────────────────────────────────────────────── - -describe('#1834: install.js source handles .sh files in the hook copy loop', () => { - let src; - - before(() => { - src = fs.readFileSync(INSTALL_SCRIPT, 'utf-8'); - }); - - test('hook copy loop has an else branch for non-.js files', () => { - // The loop must handle files that are not .js — specifically .sh hooks. - // The v1.32.0 bug was that only the if(entry.endsWith('.js')) branch - // existed; non-.js files (i.e. .sh hooks) were silently skipped. - // - // Find the hook copy loop by anchoring on its unique context: the - // configDirReplacement variable is declared only once in install.js, - // right before the entry.endsWith('.js') branch. - const anchorPhrase = 'configDirReplacement'; - const anchorIdx = src.indexOf(anchorPhrase); - assert.ok(anchorIdx !== -1, 'hook copy loop anchor (configDirReplacement) not found in install.js'); - // Extract a window large enough to contain the if/else block (≈1500 chars) - const region = src.slice(anchorIdx, anchorIdx + 1500); - assert.ok( - region.includes("entry.endsWith('.js')"), - "install.js hook copy loop must check entry.endsWith('.js')" - ); - assert.ok( - region.includes('} else {') || region.includes('else {'), - 'hook copy loop must have an else branch to handle .sh and other non-.js files — ' + - 'without it, .sh hooks are silently skipped (root cause of #1834)' - ); - }); - - test('.sh chmod is applied in the non-.js branch', () => { - // Verify the else branch sets chmod for .sh files. - // Without this, .sh hooks exist but are not executable. - assert.ok( - src.includes("entry.endsWith('.sh')"), - "install.js must check entry.endsWith('.sh') to apply chmod after copying" - ); - }); - - test('.sh hooks are listed in expectedShHooks warning check', () => { - // The post-copy verification must check each expected .sh hook. - for (const hook of SH_HOOKS) { - assert.ok( - src.includes(hook), - `install.js must reference '${hook}' in its post-copy verification` - ); - } - }); -}); diff --git a/tests/bug-2136-sh-hook-version.test.cjs b/tests/bug-2136-sh-hook-version.test.cjs index 40d6c95cf..e3b1e99a6 100644 --- a/tests/bug-2136-sh-hook-version.test.cjs +++ b/tests/bug-2136-sh-hook-version.test.cjs @@ -194,75 +194,6 @@ describe('bug #2136 part 2: stale-hook detector handles bash comment syntax', () }); }); -// ───────────────────────────────────────────────────────────────────────────── -// Part 3a: install.js bundled path substitutes {{GSD_VERSION}} in .sh hooks -// ───────────────────────────────────────────────────────────────────────────── - -describe('bug #2136 part 3a: install.js bundled path substitutes {{GSD_VERSION}} in .sh hooks', () => { - let src; - - before(() => { - src = fs.readFileSync(INSTALL_SCRIPT, 'utf8'); - }); - - test('.sh branch in bundled hook copy loop reads file and substitutes GSD_VERSION', () => { - // Anchor on configDirReplacement — unique to the bundled-hooks path. - const anchorIdx = src.indexOf('configDirReplacement'); - assert.ok(anchorIdx !== -1, 'bundled hook copy loop anchor (configDirReplacement) not found'); - - // Window large enough for the if/else block - const region = src.slice(anchorIdx, anchorIdx + 2000); - - assert.ok( - region.includes("entry.endsWith('.sh')"), - "bundled hook copy loop must check entry.endsWith('.sh')" - ); - assert.ok( - region.includes('GSD_VERSION'), - 'bundled .sh branch must reference GSD_VERSION substitution. Without this, ' + - 'installed .sh hooks contain the literal "{{GSD_VERSION}}" placeholder and ' + - 'bash hook staleness becomes permanently undetectable after future updates' - ); - // copyFileSync on a .sh file would skip substitution — ensure we read+write instead - const shBranchIdx = region.indexOf("entry.endsWith('.sh')"); - const shBranchRegion = region.slice(shBranchIdx, shBranchIdx + 400); - assert.ok( - shBranchRegion.includes('readFileSync') || shBranchRegion.includes('writeFileSync'), - 'bundled .sh branch must read the file (readFileSync) to perform substitution, ' + - 'not copyFileSync directly (which skips template expansion)' - ); - }); -}); - -// ───────────────────────────────────────────────────────────────────────────── -// Part 3b: install.js Codex path also substitutes {{GSD_VERSION}} in .sh hooks -// ───────────────────────────────────────────────────────────────────────────── - -describe('bug #2136 part 3b: install.js Codex path substitutes {{GSD_VERSION}} in .sh hooks', () => { - let src; - - before(() => { - src = fs.readFileSync(INSTALL_SCRIPT, 'utf8'); - }); - - test('.sh branch in Codex hook copy block substitutes GSD_VERSION', () => { - // Anchor on codexHooksSrc — unique to the Codex path. - const anchorIdx = src.indexOf('codexHooksSrc'); - assert.ok(anchorIdx !== -1, 'Codex hook copy block anchor (codexHooksSrc) not found'); - - const region = src.slice(anchorIdx, anchorIdx + 2000); - - assert.ok( - region.includes("entry.endsWith('.sh')"), - "Codex hook copy block must check entry.endsWith('.sh')" - ); - assert.ok( - region.includes('GSD_VERSION'), - 'Codex .sh branch must substitute {{GSD_VERSION}}. The bundled path was fixed ' + - 'but Codex installs a separate copy of the hooks from hooks/dist that also needs stamping' - ); - }); -}); // ───────────────────────────────────────────────────────────────────────────── // Part 4: End-to-end — installed .sh hooks have stamped version, not placeholder