From 7f4cc8dc2ca0193a8616b97a41bc2192a635e1b2 Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Sun, 21 Jun 2026 11:43:18 +0800 Subject: [PATCH 1/2] docs(#96): document test hygiene convention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests must point CCXRAY_HOME at a throwaway temp dir with their own synthetic index.ndjson and never read the real ~/.ccxray — the fallback that made PR #94's usage e2e tests pass locally but fail in empty-home CI. Adds docs/testing.md (4 rules, canonical pattern from usage.test.js, the $HOME-vs-CCXRAY_HOME distinction incl. the puppeteer Chrome-cache caveat) and a short Test Hygiene section + pointer in CLAUDE.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 8 +++ docs/testing.md | 134 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 docs/testing.md diff --git a/CLAUDE.md b/CLAUDE.md index f3249c4..e196d79 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -35,6 +35,14 @@ CCXRAY_HOME=/tmp/ccxray-smoke-$$ ccxray --port 5602 --no-browser - For browser verification use browser-harness (CDP/Chrome), not cmux-browser (WKWebView has SSE and JS eval issues) - `BU_CDP_URL=http://127.0.0.1:` — point browser-harness at a self-launched Chrome with `--remote-debugging-port=` to skip the manual "Allow remote debugging" dialog +## Test Hygiene + +`docs/testing.md` documents how the suite is run and the isolation rules every test must follow. In short: any test that touches storage or spawns the CLI/server must point `CCXRAY_HOME` at a throwaway temp dir with its own synthetic `index.ndjson` — never read the real `~/.ccxray`, never embed real logs/usernames/paths. `test/usage.test.js` is the canonical pattern. + +Before pushing, confirm the suite passes against an empty home: `CCXRAY_HOME=$(mktemp -d) npm test`. CI runs the suite with an empty `CCXRAY_HOME` as a backstop, so a test that reads the real `~/.ccxray` (the PR #94 failure class) fails the build. + +**Maintenance rule**: when you add a test that reads `CCXRAY_HOME`, depends on `$HOME`, or introduces a new fixture shape, keep `docs/testing.md` accurate — especially the `$HOME` vs `CCXRAY_HOME` distinction (scrubbing `$HOME` broadly breaks the puppeteer browser e2e tests). + ## Wire Protocol Documentation `docs/wire-protocol-reference.md` documents observable wire-level differences between Claude Code (Anthropic Messages API) and Codex (OpenAI Responses API). Every field is tagged with a confidence level (`contractual`, `obs-stable`, `obs-fragile`) and a version range. diff --git a/docs/testing.md b/docs/testing.md new file mode 100644 index 0000000..5f997f4 --- /dev/null +++ b/docs/testing.md @@ -0,0 +1,134 @@ +# Testing + +How ccxray's test suite is run, and the hygiene rules every test must follow. + +```bash +npm test # node --test test/*.test.js +node --test test/usage.test.js # one file +``` + +No build step, no test framework beyond Node's built-in `node:test`. + +## Test hygiene + +A test must produce the same result on the author's machine and on a clean CI +runner. The failure mode this section exists to prevent: a test silently reads +the developer's real data, passes locally, and fails (or worse, passes for the +wrong reason) in CI. + +This actually happened — PR #94's `usage` CLI e2e tests defaulted `CCXRAY_HOME` +to `~/.ccxray`. They passed locally because the author's home had logs; CI's +home was empty, `usage` exited 1, and 11 assertions failed. The same fallback +also risked leaking the runner's username and home path into recorded data. + +The rules below make isolation the default, not an afterthought. + +### 1. Isolate `CCXRAY_HOME` + +ccxray reads logs, the hub lockfile, and secrets from `CCXRAY_HOME` (default +`~/.ccxray`). Any test that invokes the CLI/server or touches storage **must** +point `CCXRAY_HOME` at a throwaway temp dir and write its own synthetic +`logs/index.ndjson`. Never read the real `~/.ccxray`. + +For in-process tests, set it before requiring any module that captures it at +load time: + +```js +process.env.CCXRAY_HOME = fs.mkdtempSync(path.join(os.tmpdir(), 'ccxray-foo-')); +// ...then require the modules under test +``` + +For tests that spawn the CLI, pass it in the child env instead of mutating the +parent process: + +```js +execFileSync(process.execPath, ['server/index.js', 'usage'], + { env: { ...process.env, CCXRAY_HOME: FIX_HOME } }); +``` + +### 2. No real data in fixtures + +Fixtures contain only synthetic session ids, cwds, and titles — never real +logs, project names, usernames, or home paths. Build them as literals; don't +copy a slice of your own `~/.ccxray`. + +If a test needs to exercise `~` expansion, set a throwaway `$HOME` for that +single test — don't resolve against the real `os.homedir()`. Note this is +narrow: see the `$HOME` caveat below before scrubbing `$HOME` broadly. + +### 3. CI-equivalent check + +Before pushing, confirm the suite passes against an empty home: + +```bash +CCXRAY_HOME=$(mktemp -d) npm test +``` + +If a test forgot to isolate, it inherits this empty `CCXRAY_HOME`, finds no +logs, and fails — the same `~/.ccxray`-dependency that bit PR #94. This checks +the isolation condition CI enforces (see below); CI additionally runs the +Node 20/22 matrix, so a green local run covers isolation but not the matrix. + +### 4. Clean up + +Remove temp dirs when the process exits, so repeated runs don't fill `/tmp`: + +```js +process.on('exit', () => { try { fs.rmSync(FIX_HOME, { recursive: true, force: true }); } catch {} }); +``` + +Use `finally` instead for dirs scoped to a single test. + +## Canonical pattern + +`test/usage.test.js` is the reference. Copy its setup: + +```js +const FIX_HOME = fs.mkdtempSync(path.join(os.tmpdir(), 'ccxray-usage-test-')); +fs.mkdirSync(path.join(FIX_HOME, 'logs'), { recursive: true }); +fs.writeFileSync(path.join(FIX_HOME, 'logs', 'index.ndjson'), + FIXTURE.map(e => JSON.stringify(e)).join('\n') + '\n'); +process.on('exit', () => { try { fs.rmSync(FIX_HOME, { recursive: true, force: true }); } catch {} }); + +const cli = (...args) => execFileSync(process.execPath, + ['server/index.js', 'usage', ...args], + { env: { ...process.env, CCXRAY_HOME: FIX_HOME }, timeout: 10000 }).toString(); +``` + +``` + real ~/.ccxray ✗ (never read by tests) + │ + ▼ + [ test process ] ── CCXRAY_HOME ──▶ /tmp/ccxray-test-XXXX/logs/index.ndjson + (synthetic, deterministic, cleaned on exit) +``` + +## `$HOME` vs `CCXRAY_HOME` + +These are different layers of isolation — don't conflate them: + +- **`CCXRAY_HOME`** is ccxray's own data dir. Isolate it in every storage/CLI + test (rule 1). The full-suite CI-equivalent check scrubs this one. +- **`$HOME`** is the toolchain's cache dir. The puppeteer-based browser e2e + tests (`test/rebuild-index.e2e.test.js`, `test/dashboard-codex-e2e.test.js`) + launch a real Chrome whose binary lives at `$HOME/.cache/puppeteer`. + Scrubbing `$HOME` for the whole suite breaks them with "Could not find + Chrome" — that's a missing toolchain cache, not a hygiene violation. + +So: scrub `CCXRAY_HOME` for the whole suite; only set a throwaway `$HOME` for a +specific non-browser test that needs to assert `~` expansion. + +## CI enforcement + +`.github/workflows/ci.yml` runs the suite with `CCXRAY_HOME` pointed at a fresh +empty dir under `$RUNNER_TEMP`. This guarantees two things: no test can read the +runner's real `~/.ccxray`, and every test starts from an empty log dir — so a +test that skips rule 1 and reads logs it didn't create finds none and fails, +which is exactly the PR #94 failure class. + +This is a backstop, not full per-test isolation: a test that writes into the +shared home and reads its own data back could still pass without isolating, and +a shared home can introduce order-dependence between tests. Rule 1 (each test +makes its own temp home) is the real guard; CI just stops the real-data +dependency from going unnoticed. `$HOME` is left untouched so puppeteer's Chrome +cache stays intact. It costs nothing extra — it doesn't re-run the suite. From 2e8543b3a07612ea67b42733540173dca763e9dc Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Sun, 21 Jun 2026 11:43:18 +0800 Subject: [PATCH 2/2] ci(#96): run tests against an empty CCXRAY_HOME MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a test step that points CCXRAY_HOME at a fresh empty dir under $RUNNER_TEMP, as a backstop against the PR #94 failure class: a test that reads the real ~/.ccxray now finds no logs and fails the build. $HOME is left untouched so puppeteer's Chrome cache stays intact. CCXRAY_HOME is set at the step (not job) level via the $RUNNER_TEMP shell var — the runner context is not available in jobs..env. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 94bdae8..b078ae4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,4 +19,11 @@ jobs: node-version: ${{ matrix.node-version }} cache: npm - run: npm ci - - run: npm test + # Run against an empty CCXRAY_HOME so a test that forgets to isolate + # (rule 1 in docs/testing.md) can't read the real ~/.ccxray and starts + # from an empty log dir. $HOME is left untouched so puppeteer's Chrome + # cache (~/.cache/puppeteer) stays intact for the browser e2e tests. + - name: Run tests with isolated empty CCXRAY_HOME + run: | + mkdir -p "$RUNNER_TEMP/empty-ccxray" + CCXRAY_HOME="$RUNNER_TEMP/empty-ccxray" npm test