Skip to content

tests: unify test execution with Vitest#61

Merged
benvinegar merged 2 commits into
mainfrom
bentlegen/unified-vitest-runner
Feb 19, 2026
Merged

tests: unify test execution with Vitest#61
benvinegar merged 2 commits into
mainfrom
bentlegen/unified-vitest-runner

Conversation

@benvinegar

@benvinegar benvinegar commented Feb 18, 2026

Copy link
Copy Markdown
Member

Summary

  • switch repo test scripts to a unified Vitest runner (npm test, npm run test:js, npm run test:shell, npm run test:coverage)
  • add vitest.config.mjs and new wrapper suites under test/:
    • test/legacy-node-tests.test.mjs runs existing node:test suites (tool-guard, bridge security, extension scanner)
    • test/shell-scripts.test.mjs runs shell-script suites (safe-bash + redact-logs)
    • test/security-audit.test.mjs adds focused fixture-based checks for bin/security-audit.sh
  • migrate selected suites to Vitest directly (pi/extensions/heartbeat.test.mjs, pi/extensions/memory.test.mjs, control-plane/server.test.mjs)
  • improve control-plane/server.test.mjs startup diagnostics when the child server exits before readiness
  • update CI test job to use the unified runner (npm test + npm run test:coverage)
  • update docs (README.md, AGENTS.md) and control-agent memory note to reflect the new test commands
  • add root dev dependencies required for unified execution (vitest, @vitest/coverage-v8, express)

Validation

  • npm test
  • npm run test:js
  • npm run test:shell
  • npm run test:coverage
  • npm run lint
  • npm run typecheck

@greptile-apps

greptile-apps Bot commented Feb 18, 2026

Copy link
Copy Markdown

Greptile Summary

Unifies all test execution under a single Vitest runner, replacing the previous bin/test.sh shell orchestrator and node --test direct invocations. Three test suites (heartbeat, memory, server) are migrated directly to Vitest imports, while legacy node:test suites and shell scripts are wrapped via Vitest child-process runners under test/. A new fixture-based security-audit.test.mjs adds targeted coverage for bin/security-audit.sh. CI is simplified to npm ci + npm test.

  • vitest.config.mjs added with explicit test file list, 120s timeouts, and serial execution (fileParallelism: false)
  • control-plane/server.test.mjs improved with stderr capture and early-exit diagnostics on child server failure
  • express hoisted to root devDependencies so control-plane/server.mjs resolves it without a sub-package npm ci
  • c8 replaced by @vitest/coverage-v8
  • Several docs not included in this PR still reference bin/test.sh: CONTRIBUTING.md, .github/PULL_REQUEST_TEMPLATE.md, bin/ci/setup-ubuntu.sh, bin/ci/setup-arch.sh, and bin/baudbot CLI

Confidence Score: 4/5

  • This PR is safe to merge — test infrastructure only, no production code changes, all 122 tests pass.
  • Well-structured migration from shell-based test orchestration to Vitest. All tests pass (122 across 6 files). The only concern is incomplete doc updates — several files outside the changeset still reference the old bin/test.sh runner, which creates inconsistency but doesn't break anything since bin/test.sh still exists.
  • No files in the changeset require special attention. Files outside the changeset (CONTRIBUTING.md, .github/PULL_REQUEST_TEMPLATE.md, bin/ci/setup-ubuntu.sh, bin/ci/setup-arch.sh, bin/baudbot) still reference the old bin/test.sh and should be updated in a follow-up.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Simplified to npm ci + npm test + coverage. Removed sub-package installs (slack-bridge, control-plane) which is safe since express is now a root devDependency and other sub-package tests only use Node built-ins.
vitest.config.mjs New Vitest config with explicit test file list, generous timeouts for shell-spawning tests, and serial execution to prevent port conflicts. Clean and appropriate.
control-plane/server.test.mjs Migrated from node:test to Vitest (before/afterbeforeAll/afterAll). Added stderr capture and early-exit handler for better diagnostics when the child server fails to start.
test/legacy-node-tests.test.mjs New Vitest wrapper that runs legacy node --test suites (tool-guard, bridge security, extension scanner) as child processes. Clean bridge pattern with good error reporting.
test/shell-scripts.test.mjs New Vitest wrapper for shell test scripts (baudbot-safe-bash, redact-logs). Same clean pattern as legacy-node-tests wrapper.
test/security-audit.test.mjs New fixture-based Vitest suite for security-audit.sh. Uses temp directories with proper cleanup, handles EADDRINUSE gracefully for port 7890. Tests permission fixing, SLACK_ALLOWED_USERS detection.
package.json Replaced bin/test.sh scripts with vitest run commands. Swapped c8 for @vitest/coverage-v8, added express and vitest as root devDependencies.
pi/extensions/heartbeat.test.mjs Import swap from node:test to vitest. Run comment updated. No logic changes.
pi/extensions/memory.test.mjs Import swap from node:test to vitest. Run comment updated. No logic changes.
AGENTS.md Updated "Running Tests" section to reference npm test / vitest.config.mjs. Repo layout section still lists test.sh as "runs all test suites" which is potentially confusing.
README.md Updated test commands section to reference npm test, npm run test:js, npm run test:shell, and npm run test:coverage.
pi/skills/control-agent/memory/repos.md Updated example comment to reference npm test (unified Vitest runner) instead of bin/test.sh.
package-lock.json Lock file updated to reflect new devDependencies (vitest, @vitest/coverage-v8, express) and removal of c8.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["npm test"] --> B["vitest run --config vitest.config.mjs"]
    B --> C["pi/extensions/heartbeat.test.mjs\n(Vitest native — 54 tests)"]
    B --> D["pi/extensions/memory.test.mjs\n(Vitest native — 44 tests)"]
    B --> E["control-plane/server.test.mjs\n(Vitest native — 16 tests)"]
    B --> F["test/legacy-node-tests.test.mjs\n(wrapper — 3 tests)"]
    B --> G["test/shell-scripts.test.mjs\n(wrapper — 2 tests)"]
    B --> H["test/security-audit.test.mjs\n(fixture-based — 3 tests)"]

    F -->|"spawnSync node --test"| F1["tool-guard.test.mjs"]
    F -->|"spawnSync node --test"| F2["security.test.mjs"]
    F -->|"spawnSync node --test"| F3["scan-extensions.test.mjs"]

    G -->|"spawnSync bash"| G1["baudbot-safe-bash.test.sh"]
    G -->|"spawnSync bash"| G2["redact-logs.test.sh"]

    H -->|"spawnSync bash"| H1["bin/security-audit.sh"]

    style A fill:#4CAF50,color:#fff
    style B fill:#2196F3,color:#fff
    style C fill:#E1BEE7
    style D fill:#E1BEE7
    style E fill:#E1BEE7
    style F fill:#FFE0B2
    style G fill:#FFE0B2
    style H fill:#FFE0B2
Loading

Last reviewed commit: 8fceb6d

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

13 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread AGENTS.md
```

Add new test files to `bin/test.sh` — don't scatter test invocations across CI or docs.
Add new test files to `vitest.config.mjs` (and shell wrappers under `test/` as needed) — don't scatter test invocations across CI or docs.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale bin/test.sh references elsewhere

This line correctly says to add new test files to vitest.config.mjs, but several other files still reference the old bin/test.sh runner and were not updated in this PR:

  • CONTRIBUTING.md (lines 13–20) — still documents bin/test.sh commands
  • .github/PULL_REQUEST_TEMPLATE.md (line 7) — checklist says "Tests pass (bin/test.sh)"
  • bin/ci/setup-ubuntu.sh:72 and bin/ci/setup-arch.sh:61 — droplet CI scripts still call bash bin/test.sh
  • bin/baudbot:252 — the CLI test subcommand still delegates to bin/test.sh

Per the repo convention ("When changing behavior, update all docs"), these should be updated to reference npm test / vitest.config.mjs.

Context Used: Context from dashboard - CLAUDE.md (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: AGENTS.md
Line: 126

Comment:
**Stale `bin/test.sh` references elsewhere**

This line correctly says to add new test files to `vitest.config.mjs`, but several other files still reference the old `bin/test.sh` runner and were not updated in this PR:

- `CONTRIBUTING.md` (lines 13–20) — still documents `bin/test.sh` commands
- `.github/PULL_REQUEST_TEMPLATE.md` (line 7) — checklist says "Tests pass (`bin/test.sh`)"
- `bin/ci/setup-ubuntu.sh:72` and `bin/ci/setup-arch.sh:61` — droplet CI scripts still call `bash bin/test.sh`
- `bin/baudbot:252` — the CLI `test` subcommand still delegates to `bin/test.sh`

Per the repo convention ("When changing behavior, update all docs"), these should be updated to reference `npm test` / `vitest.config.mjs`.

**Context Used:** Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=301c5d5e-f39f-40b0-952d-acd5ddc604f4))

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — I updated the stale docs references in this PR:\n- CONTRIBUTING.md now uses npm test / npm run test:*\n- .github/PULL_REQUEST_TEMPLATE.md now checks npm test\n\nFor the bin/ci/* scripts and bin/baudbot, those still intentionally delegate to bin/test.sh as a compatibility wrapper in admin-managed bin/ paths.\n\nResponded by pi-coding-agent using openai/gpt-5.

@benvinegar benvinegar force-pushed the bentlegen/unified-vitest-runner branch from 56c5fb7 to 96c25a4 Compare February 19, 2026 04:18
@socket-security

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​vitest/​coverage-v8@​4.0.18991006998100
Addedvitest@​4.0.18961007999100

View full report

@benvinegar benvinegar merged commit 722c95a into main Feb 19, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant