test: replace mock-heavy CLI tests with pure unit + real-I/O tests#115
Conversation
Three of the worst mock-only tests are now honest:
1. first-run.test.ts — was 100% vi.mock("node:fs"). Replaced with
mkdtempSync against the real filesystem; exercises actual 0o600/0o700
mode bits, real EACCES on read-only dirs, and the round-trip between
isFirstRun + markFirstRun. Adds an optional configDir param to both
helpers (default = CONFIG_DIR), so production behavior is unchanged.
2. api.test.ts — was vi.stubGlobal("fetch", …). Replaced with a real
http.createServer in beforeAll; every test now exercises the real
fetch / Authorization-header serialization / JSON body / response-
header reading, including the X-Straude-Refreshed-Token rotation and
the 401-retry path. The only remaining mock is auth.saveConfig
(boundary, not faked behavior).
3. ccusage-install.test.ts — extracted the install-command decision into
a new pure helper pickInstallCommand({ hasBun }) and unit-tested it
with zero mocks. Trimmed the orchestration test to branches that
genuinely depend on process state (PATH, isatty).
Also extracted resolvePushDateRange() from pushCommand as a pure
function, unit-tested across all six branches (--date / codex repair /
--days / smart-sync / no last_push_date / future date / boundary).
CLI test count: 240 → 265 (+25 pure unit tests, 7 mock-only tests retired).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Why
Audit of the test suite from #114's PR review:
Same pattern in
api.test.ts(stubbedglobalThis.fetch) andccusage-install.test.ts(mocked everything around the install decision).What changed
Three of the worst offenders are now honest, plus one preventive extraction.
1.
first-run.test.ts— real fsBefore:
vi.mock("node:fs")and assertions on the call shape (e.g. "we called writeFileSync with this path").After:
mkdtempSync+ real I/O. New tests assert:0o600for the marker,0o700for the dir)EACCESon achmod 0o500dirmkdir -psemantics for nested config dirsisFirstRunandmarkFirstRunRequired a tiny additive change:
isFirstRun(configDir?)andmarkFirstRun(configDir?)accept an optional override (default =CONFIG_DIR). Production behavior unchanged.2.
api.test.ts— real http serverBefore:
vi.stubGlobal('fetch', mockFetch).After:
http.createServerinbeforeAll, planned-response queue, request recording. Every test exercises the real:fetchHTTP/1.1 wire formatres.headers.get()X-Straude-Refreshed-Tokenrotation and 401-retry pathsOnly remaining mock is
auth.saveConfig(we don't want test runs writing to~/.straude/config.json). That's a true boundary — captured-and-asserted, not faked-and-forgotten.3.
ccusage-install.test.ts— pure helper extractedExtracted
pickInstallCommand({ hasBun }): { cmd, args, manager }as a pure function. The decision logic (`bun add -g ccusage` vs `npm install -g ccusage`) is now unit-tested inpick-install-command.test.tswith zero mocks. The orchestration test inccusage-install.test.tsis trimmed to branches that genuinely depend on process state (PATH, isatty).4.
resolvePushDateRange— pure helper extracted (preventive)The complex date-range logic in
pushCommand(six branches: `--date` / codex repair / `--days` / smart-sync / fresh install / future date / boundary) is now a pure function. Newresolve-push-date-range.test.tscovers all six branches with zero mocks.Stats
first-run.test.tsapi.test.tsccusage-install.test.tsTest plan
Out of scope (real follow-ups)
🤖 Generated with Claude Code