Commit Graph

6 Commits

Author SHA1 Message Date
Tom Boucher
7fa5eb7e63 fix(3597): resolve CR threads — drop substring-assertion guidance, use $testDir
Two CodeRabbit threads from PR #3649 review:

- docs/TESTING-SUITES.md:75 — removed the "stable message substring"
  fallback from the error-assertion guidance. Project rule (per the
  no-source-grep lint and lint-no-source-grep.cjs) is structured/typed
  checks only — err.code, JSON fields, enums. Substring matching
  re-introduces the exact prose-coupling we banned.

- scripts/run-tests.cjs:106 — the "no test files found" error now
  reports the resolved testDir variable instead of the hardcoded
  'tests/' string, so when GSD_TEST_DIR points elsewhere the message
  names the actual directory the harness searched.

Local: docker gsd-test-summary 11224/0 on holodeck.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 10:44:51 -04:00
Tom Boucher
52f23ac0a0 fix(3597): chunk node --test spawn to survive Windows CreateProcess limit
Windows CreateProcess caps lpCommandLine at 32,767 chars. The original
`execFileSync(node, ['--test', ...546 paths])` exceeded that on every
Windows runner and exited within ~70ms with no test output. Linux/macOS
allow ~2 MB ARG_MAX so the same call worked there.

`scripts/run-tests.cjs` now splits selected files into chunks that keep
each spawn's argv under 28,000 chars (operator-overridable via
RUN_TESTS_MAX_CMDLINE_CHARS), runs them sequentially, and reports the
first non-zero exit. Cross-platform regression test forces chunking with
a low ceiling and asserts the `run-tests: chunk N/M …` stderr marker.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 09:25:36 -04:00
Tom Boucher
e82876fe45 feat(3597): split test suites and add Node 22/24/26 OS matrix
scripts/run-tests.cjs gains `--suite <name>` filtering using a filename
suffix convention (`*.security.test.cjs`, `*.integration.test.cjs`, …).
Files with no marker are `unit` (the default fast lane); files with a
marker land in the matching suite. No `--suite` flag preserves the prior
behavior of running every test (backcompat for `npm test` and
`npm run test:coverage`).

New package scripts wire the suites to stable entrypoints:
test:unit, test:integration, test:install, test:security, test:slow,
test:coverage:unit, test:coverage:all. Unknown suite → exit 2 with the
list of valid suites; empty suite → exit 0 with a stderr notice so empty
lanes (e.g. `security` before adversarial tests land) don't gate CI.

CI matrix grows from `ubuntu × {22,24}` + a single macOS lane to
`{ubuntu, macos, windows} × {22, 24, 26}`. `fail-fast: false` so one
lane failure doesn't cancel siblings. Node 26 is `continue-on-error`
until actions/setup-node stabilises that image. PR CI runs unit +
integration + security on every cell; `install` and `slow` only on
`main` push. A dedicated `coverage` job runs `test:coverage:unit` on
ubuntu/Node 24 and uploads the report.

Grouping policy lives in docs/TESTING-SUITES.md with a pointer from
CONTRIBUTING.md. New harness test covers arg parsing, filter selection,
empty-suite behavior, and failure propagation.

Closes #3597.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 08:34:09 -04:00
Tom Boucher
2703422be8 refactor(tests): standardize to node:assert/strict and t.after() per CONTRIBUTING.md (#1675)
* refactor(tests): standardize to node:assert/strict and t.after() per CONTRIBUTING.md

- Replace require('node:assert') with require('node:assert/strict') across
  all 73 test files to enforce strict equality (no type coercion)
- Replace try/finally cleanup blocks with t.after() hooks in core.test.cjs
  and hooks-opt-in.test.cjs per the test lifecycle standards
- Utility functions in codex-config and security-scan retain try/finally
  as that is appropriate for per-function resource guards, not lifecycle hooks

Closes #1674

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* perf(tests): add --test-concurrency=4 to test runner for parallel file execution

Node.js --test-concurrency controls how many test files run as parallel child
processes. Set to 4 by default, configurable via TEST_CONCURRENCY env var.
Fixes tests at a known level rather than inheriting os.availableParallelism()
which varies across CI environments.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(security): allowlist verify.test.cjs in prompt-injection scanner

tests/verify.test.cjs uses <human>...</human> as GSD phase task-type
XML (meaning "a human should verify this step"), which matches the
scanner's fake-message-boundary pattern for LLM APIs. This is a
false positive — add it to the allowlist alongside the other test files
that legitimately contain injection-adjacent patterns.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-04 14:29:03 -04:00
Lex Christopherson
02a5319777 fix(ci): propagate coverage env in cross-platform test runner
The run-tests.cjs child process now inherits NODE_V8_COVERAGE from the
parent so c8 collects coverage data. Also restores npm scripts to use
the cross-platform runner for both test and test:coverage commands.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-27 10:07:02 -06:00
Lex Christopherson
ccb8ae1d18 fix(ci): cross-platform test runner for Windows glob expansion
npm scripts pass `tests/*.test.cjs` to node/c8 as a literal string on
Windows (PowerShell/cmd don't expand globs). Adding `shell: bash` to CI
steps doesn't help because c8 spawns node as a child process using the
system shell. Use a Node script to enumerate test files cross-platform.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-27 10:00:26 -06:00