Skip to content

tests: unify test execution with Vitest #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
benvinegar merged 2 commits into from
Feb 19, 2026
Merged

tests: unify test execution with Vitest #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2b95ab
tests: unify runner on vitest and add security wrappers
benvinegar Feb 18, 2026
96c25a4
docs: update test command references to npm scripts
benvinegar Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

## Checklist

- [ ] Tests pass (`bin/test.sh`)
- [ ] Tests pass (`npm test`)
- [ ] Lint passes (`npm run lint`)
- [ ] Docs updated (if behavior changed)
- [ ] Security audit passes (`bin/security-audit.sh --deep`) — if touching security code
17 changes: 5 additions & 12 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,13 @@ jobs:
node-version: "22"

- name: Install dependencies
run: |
npm ci
cd slack-bridge && npm ci
cd ../control-plane && npm ci

- name: Run JS tests with coverage
run: npm run test:coverage
run: npm ci

- name: Run shell tests
run: bin/test.sh shell
- name: Run unified test suite
run: npm test

# security-audit.sh checks live system state (running services, firewall,
# /proc mounts) that doesn't exist in CI. Run it locally instead:
# cd bin && bash security-audit.test.sh
- name: Run coverage suite
run: npm run test:coverage

secret-scan:
runs-on: ubuntu-latest
Expand Down
15 changes: 9 additions & 6 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,20 @@ tmux new-window -n baudbot 'sudo -u baudbot_agent ~/runtime/start.sh'
## Running Tests

```bash
# All tests (10 suites)
bin/test.sh
# All tests (unified Vitest runner)
npm test

# Only JS/TS tests
bin/test.sh js
npm run test:js

# Only shell tests
bin/test.sh shell
# Only shell/security script tests
npm run test:shell

# JS/TS coverage
npm run test:coverage
```

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.

@greptile-apps greptile-apps Bot Feb 18, 2026

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.

@benvinegar benvinegar Feb 18, 2026

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.


## Conventions

Expand Down
13 changes: 8 additions & 5 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,17 @@ npm install
## Running Tests

```bash
# All tests (10 suites)
bin/test.sh
# All tests (unified Vitest runner)
npm test

# JS/TS only
bin/test.sh js
npm run test:js

# Shell only
bin/test.sh shell
# Shell/security script tests only
npm run test:shell

# Coverage
npm run test:coverage

# Lint + typecheck
npm run lint && npm run typecheck
Expand Down
15 changes: 9 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,17 @@ See [SECURITY.md](SECURITY.md) for full threat model, trust boundaries, and know
## Tests

```bash
# All tests across 10 suites
bin/test.sh
# All tests (unified Vitest runner)
npm test

# JS/TS only
bin/test.sh js
# JS/TS suites only
npm run test:js

# Shell only
bin/test.sh shell
# Shell/security script suites only
npm run test:shell

# JS/TS coverage
npm run test:coverage

# Lint + typecheck
npm run lint && npm run typecheck
Expand Down
25 changes: 17 additions & 8 deletions control-plane/server.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/**
* Control plane server tests.
*
* Uses Node's built-in test runner. Run with:
* node --test server.test.mjs
* Run with Vitest:
* npx vitest run control-plane/server.test.mjs
*/

import { describe, it, before, after } from "node:test";
import { describe, it, beforeAll, afterAll } from "vitest";
import assert from "node:assert/strict";

// ── Test helpers ────────────────────────────────────────────────────────────
Expand All @@ -28,9 +28,14 @@ async function startServer({ token, port } = {}) {
stdio: ["ignore", "pipe", "pipe"],
});

let stderr = "";
child.stderr.on("data", (chunk) => {
stderr += chunk.toString();
});

// Wait for server to be ready
await new Promise((resolve, reject) => {
const timeout = setTimeout(() => reject(new Error("server start timeout")), 5000);
const timeout = setTimeout(() => reject(new Error(`server start timeout${stderr ? `: ${stderr.trim()}` : ""}`)), 5000);
child.stdout.on("data", (chunk) => {
if (chunk.toString().includes("listening")) {
clearTimeout(timeout);
Expand All @@ -41,6 +46,10 @@ async function startServer({ token, port } = {}) {
clearTimeout(timeout);
reject(err);
});
child.on("exit", (code, signal) => {
clearTimeout(timeout);
reject(new Error(`server exited before ready (code=${code}, signal=${signal})${stderr ? `: ${stderr.trim()}` : ""}`));
});
});

const base = `http://127.0.0.1:${p}`;
Expand All @@ -61,10 +70,10 @@ async function startServer({ token, port } = {}) {
describe("control-plane (no auth)", () => {
let server;

before(async () => {
beforeAll(async () => {
server = await startServer();
});
after(() => server?.close());
afterAll(() => server?.close());

it("GET /health returns 200", async () => {
const res = await server.fetch("/health");
Expand Down Expand Up @@ -128,10 +137,10 @@ describe("control-plane (with auth)", () => {
let server;
const TOKEN = "test-token-abc123";

before(async () => {
beforeAll(async () => {
server = await startServer({ token: TOKEN });
});
after(() => server?.close());
afterAll(() => server?.close());

it("GET /health works without auth", async () => {
const res = await server.fetch("/health");
Expand Down
Loading