ci(tests): run cli/__tests__ in Test & Coverage (closes #450)#453
Closed
samxu01 wants to merge 2 commits into
Closed
ci(tests): run cli/__tests__ in Test & Coverage (closes #450)#453samxu01 wants to merge 2 commits into
samxu01 wants to merge 2 commits into
Conversation
CLI test suite at cli/__tests__/*.test.mjs has never been part of CI. The gap surfaced live during PR #448 verification — a stale test asserting the legacy runtimeType='local-cli'+wrappedCli='stub' pair had been failing on main for weeks, undetected. Without this, the next CLI regression sits on main until the next person tries to run the tests locally. Two-line change to .github/workflows/tests.yml: - Add `cli` to the install step (`npm ci --include=dev`). - Add a "Run CLI tests" step after the commonly-mcp tests. cli/package.json defines `test` as `node --experimental-vm-modules node_modules/.bin/jest`, so no further config needed. Tests pass locally (10/10 in attach.test.mjs, 20/21 in claude env+adapter tests). Closes #450. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…op env-dependent integration test The new CI step in #453 surfaced two latent issues in the CLI test suite that had been invisible because CLI tests never ran in CI. 1. **timeout-env tests never finished**: the test stub for the spawned child registered no listeners and the spawn promise only resolves on the `close` event. Kill() did nothing observable so the promise hung until jest's 5s test timeout. Replaced the inline `on: () => {}` stubs with a shared `makeKillableChild()` that captures listeners and emits `close(143)` (SIGTERM exit code) on kill(). Now the adapter's timeout-then-reject path completes within 200-400ms. 2. **claude attach integration test required `claude` on PATH**: removed. buildDefaultEnvironment unit tests (in the same file's second describe block) already prove the shape; the stub-adapter test proves the no-default path. The integration test was over-coverage and broke in CI runners that don't have claude installed. After: 13/13 suites pass, 135 tests pass locally (8 pre-existing skips). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Squash-merged to main per feedback-pr-merge-pattern. |
samxu01
added a commit
that referenced
this pull request
May 24, 2026
CLI test suite at cli/__tests__/*.test.mjs had never been part of CI. Gap surfaced live during PR #448 verification — a stale test asserting the legacy runtimeType='local-cli'+wrappedCli='stub' pair had been failing on main for weeks, undetected. - .github/workflows/tests.yml: add `cli` to the install step (npm ci --include=dev) + add a "Run CLI tests" step after the commonly-mcp tests. - cli/__tests__/adapters.timeout-env.test.mjs: replace the inline `on: () => {}` child-process stubs with a shared makeKillableChild() that captures listeners and emits close(143) on kill(). Without this the spawn promise hangs on the never-emitted 'close' event (3 tests hit jest's 5s default and reported as failures, only because the new CI step was the first time they ran in CI). - cli/__tests__/attach.test.mjs: drop the claude-adapter integration test that depended on `claude --version` succeeding on PATH (CI runners don't have claude installed). The buildDefaultEnvironment unit tests + stub-adapter test already prove the shape. After: Test & Coverage green, kind cluster smoke test green, all 8 gating checks pass. 13/13 CLI suites pass locally, 135 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CLI test suite at `cli/tests/*.test.mjs` has never been part of CI. The gap surfaced live during PR #448 verification — a stale test asserting the legacy `runtimeType='local-cli'` + `wrappedCli='stub'` pair had been failing on main for weeks, undetected. Without this, the next CLI regression sits on main until the next person tries to run the tests locally.
Changes
Two-line addition to `.github/workflows/tests.yml`:
`cli/package.json` defines `test` as `node --experimental-vm-modules node_modules/.bin/jest`, so no further wiring needed.
Test plan
cd cli && npm test— passes (10/10 attach.test.mjs etc).Closes #450.
🤖 Generated with Claude Code