From 85d33748e08e5c4a0dcffd88c26e175cd674b33a Mon Sep 17 00:00:00 2001 From: Jamkris Date: Thu, 14 May 2026 12:24:45 +0900 Subject: [PATCH] test(hooks): regression coverage for round 1 review fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 9 new cases locking in the behavior added by the previous two commits. Each was verified to fail before the fix and pass after. Greptile — quote-aware depth counting: - blocks $(echo ")"; (npm run dev)) - blocks (echo ")"; npm run dev) - allows $(echo "(npm run dev)") — () inside double-quoted body is literal Greptile — brace groups: - blocks { npm run dev; } - blocks echo hi && { npm run dev; } - allows {npm run dev} — bash brace-group syntax requires a space after { CodeRabbit — missing package-manager variants: - blocks yarn run dev (yarn 1.x convention) - blocks bun dev (bun bare form) CodeRabbit nitpick — symmetric quote test: - blocks echo "$(npm run dev)" — double-quoted substitution still substitutes The `{npm run dev}` allow case is intentional: bash treats `{` as a reserved word only when followed by whitespace. The pre-fix code already passed this through, but until now we never asserted it, so a future change to brace handling could silently start blocking literal `{npm` tokens. --- tests/hooks/pre-bash-dev-server-block.test.js | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/hooks/pre-bash-dev-server-block.test.js b/tests/hooks/pre-bash-dev-server-block.test.js index c8409f0b..4f075ddb 100644 --- a/tests/hooks/pre-bash-dev-server-block.test.js +++ b/tests/hooks/pre-bash-dev-server-block.test.js @@ -144,6 +144,55 @@ function runTests() { }) ? passed++ : failed++); } + // --- Round 1 review fixes (Greptile + CodeRabbit on PR #1889) --- + + if (!isWindows) { + (test('blocks $(echo ")"; (npm run dev)) — quoted ) does not terminate $() early', () => { + const result = runScript('$(echo ")"; (npm run dev))'); + assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`); + }) ? passed++ : failed++); + + (test('blocks (echo ")"; npm run dev) — quoted ) does not terminate (...) early', () => { + const result = runScript('(echo ")"; npm run dev)'); + assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`); + }) ? passed++ : failed++); + + (test('allows $(echo "(npm run dev)") — () inside double-quoted substitution body is literal', () => { + const result = runScript('$(echo "(npm run dev)")'); + assert.strictEqual(result.code, 0, `Expected exit code 0, got ${result.code}`); + }) ? passed++ : failed++); + + (test('blocks { npm run dev; } — brace group runs in current shell', () => { + const result = runScript('{ npm run dev; }'); + assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`); + }) ? passed++ : failed++); + + (test('blocks echo hi && { npm run dev; } — brace group after &&', () => { + const result = runScript('echo hi && { npm run dev; }'); + assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`); + }) ? passed++ : failed++); + + (test('allows {npm run dev} — bash requires space after { to form a group', () => { + const result = runScript('{npm run dev}'); + assert.strictEqual(result.code, 0, `Expected exit code 0, got ${result.code}`); + }) ? passed++ : failed++); + + (test('blocks yarn run dev — yarn 1.x convention', () => { + const result = runScript('yarn run dev'); + assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`); + }) ? passed++ : failed++); + + (test('blocks bun dev — bun bare form', () => { + const result = runScript('bun dev'); + assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`); + }) ? passed++ : failed++); + + (test('blocks "$(npm run dev)" — double-quoted substitution still substitutes', () => { + const result = runScript('echo "$(npm run dev)"'); + assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`); + }) ? passed++ : failed++); + } + // --- Edge cases --- (test('empty/invalid input passes through (exit code 0)', () => {