From f5f51b5b49e69a84b2bb6eada0a97f122a16fa11 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 27 May 2026 22:59:26 -0400 Subject: [PATCH] fix(#408): align ci-test-scope smoke handling with #395 changeset (drop unconditional injection; unit fallback) (#420) - Remove `DEFAULT_SMOKE_TESTS` and `WINDOWS_SMOKE_TESTS` constants (now dead after the unconditional injection block is dropped) - Drop the `addAll(targeted, DEFAULT_SMOKE_TESTS)` / `addAll(windows, WINDOWS_SMOKE_TESTS)` block from the `codeChanged` branch - When `codeChanged && targetedTests.length === 0`, push `'unit'` as the fallback suite token - Two new regression tests in `tests/ci-test-scope.test.cjs` covering the no-injection and unit-fallback contracts (bug #408) --- .../408-ci-test-scope-smoke-alignment.md | 5 +++ scripts/ci-test-scope.cjs | 24 ++++--------- tests/ci-test-scope.test.cjs | 34 +++++++++++++++++++ 3 files changed, 45 insertions(+), 18 deletions(-) create mode 100644 .changeset/408-ci-test-scope-smoke-alignment.md diff --git a/.changeset/408-ci-test-scope-smoke-alignment.md b/.changeset/408-ci-test-scope-smoke-alignment.md new file mode 100644 index 000000000..d37c1e4be --- /dev/null +++ b/.changeset/408-ci-test-scope-smoke-alignment.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 408 +--- +`ci-test-scope.cjs` now matches the #370/#395 changeset: drops the unconditional `DEFAULT_SMOKE_TESTS` injection on code-change, and falls back to the `unit` suite when the affected selection is empty. diff --git a/scripts/ci-test-scope.cjs b/scripts/ci-test-scope.cjs index 56a21bba3..f085bf1bd 100644 --- a/scripts/ci-test-scope.cjs +++ b/scripts/ci-test-scope.cjs @@ -5,20 +5,6 @@ const { execFileSync } = require('child_process'); const { existsSync, readdirSync, appendFileSync } = require('fs'); const { join } = require('path'); -const DEFAULT_SMOKE_TESTS = [ - 'tests/command-contract.test.cjs', - 'tests/commands.test.cjs', - 'tests/core.test.cjs', - 'tests/package-manifest.test.cjs', -]; - -const WINDOWS_SMOKE_TESTS = [ - 'tests/hardcoded-paths.test.cjs', - 'tests/windows-robustness.test.cjs', - 'tests/windows-test-parity-guard.test.cjs', - 'tests/workflow-shell-pinning.test.cjs', -]; - const RULES = [ { name: 'workflow automation', @@ -257,12 +243,14 @@ function classify(files) { } } - if (codeChanged) { - addAll(targeted, DEFAULT_SMOKE_TESTS); - addAll(windows, WINDOWS_SMOKE_TESTS); + const targetedTests = existingTests([...targeted].sort()); + + // When code changed but no rule matched any changed file, fall back to the + // unit suite so the targeted lane always runs something meaningful (#408). + if (codeChanged && targetedTests.length === 0) { + targetedTests.push('unit'); } - const targetedTests = existingTests([...targeted].sort()); const windowsTests = existingTests([...new Set([...windows, ...targetedTests.filter(t => /windows|path|shell|workflow|install|hook/i.test(t))])].sort()); return { diff --git a/tests/ci-test-scope.test.cjs b/tests/ci-test-scope.test.cjs index cb7a8c05c..cec4be1ec 100644 --- a/tests/ci-test-scope.test.cjs +++ b/tests/ci-test-scope.test.cjs @@ -68,4 +68,38 @@ describe('ci-test-scope.cjs', () => { // allow-test-rule: CLI usage banner presence is a user-facing contract. assert.match(r.stderr, /Usage:/); }); + + // bug-408: unconditional DEFAULT_SMOKE_TESTS injection removed; unit fallback added + test('bug-408: code change with matched rules produces exactly the rule-selected tests (no smoke list appended)', () => { + // commands/ matches the "command definitions" rule only — no smoke list should be added + const result = scopeFor(['commands/gsd/plan-phase.md']); + assert.strictEqual(result.code_changed, true); + const expectedTests = [ + 'tests/command-contract.test.cjs', + 'tests/command-routing-hub.test.cjs', + 'tests/commands.test.cjs', + 'tests/phase-command-router.test.cjs', + 'tests/roadmap-command-router.test.cjs', + ]; + // Every expected test must be present + for (const t of expectedTests) { + assert.ok(result.targeted_tests.includes(t), `expected ${t} in targeted_tests`); + } + // No DEFAULT_SMOKE_TESTS files should be injected beyond what the rule selects. + // The former smoke list contained package-manifest.test.cjs and core.test.cjs — + // neither is in the "command definitions" rule, so they must not appear. + assert.ok(!result.targeted_tests.includes('tests/core.test.cjs'), + 'tests/core.test.cjs must NOT be unconditionally injected for command changes'); + assert.ok(!result.targeted_tests.includes('tests/package-manifest.test.cjs'), + 'tests/package-manifest.test.cjs must NOT be unconditionally injected for command changes'); + }); + + test('bug-408: code change with no rule match falls back to unit suite token', () => { + // A plain source file that matches no RULES entry but is under get-shit-done/ (code path) + const result = scopeFor(['get-shit-done/src/some-util.js']); + assert.strictEqual(result.code_changed, true); + // allow-test-rule: the unit-fallback contract is the exact subject of bug #408. + assert.deepStrictEqual(result.targeted_tests, ['unit'], + 'targeted_tests must be [\'unit\'] when code changed but no rule matched'); + }); });