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)
This commit is contained in:
Tom Boucher
2026-05-27 22:59:26 -04:00
committed by GitHub
parent e4aca8dab0
commit f5f51b5b49
3 changed files with 45 additions and 18 deletions

View File

@@ -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.

View File

@@ -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 {

View File

@@ -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');
});
});