test(uncheck): rewrite the suite as e2e tests per command at 100% coverage

[dinwwwh]

Requested by dinwwwh · project thread

Before: 118 tests across 4 files, most of them calling commands in-process or internal functions (plan, resolvePaths, argvBatches, resolveBin) on generated files, at about 96% coverage.

After: 398 end-to-end tests in 35 files, plus 2 that are skipped when run as root. There is one folder per command: tests/uncheck, tests/staged, tests/prepare, tests/hooks/install and tests/hooks/run. Every test runs the real CLI as a separate process on a copy of a real project, either a single package (tests/fixtures/single) or a monorepo (tests/fixtures/monorepo). Scenarios run in both shapes. pnpm run test:coverage now requires 100% statements, branches, functions and lines, and the suite meets that on Node 22, 24 and 26.

How:

• tests/_shared/project.ts copies a fixture into a temp folder and installs the tools and uncheck into its node_modules. It also links the monorepo packages and runs git init.
  • The installed uncheck bin runs src/bin.ts directly with Node type stripping. tests/_shared/register.mjs only adds support for extensionless imports, folder imports and JSON imports.
  • The hooks that prepare and hooks install write (npx --no uncheck …) run the same installed bin, so real git commit and sh -c runs exercise the source under test.
• Coverage comes from those child processes. c8 wraps vitest run and maps V8 coverage straight onto the .ts sources. @vitest/coverage-v8 is removed because it cannot see child processes. src/types.ts holds only types and is excluded.
• The fixtures pass the repo's own uncheck check. tsconfig.json excludes them from the root project.
• Code that no real CLI run can reach is simplified rather than unit tested:
  • argvBatches returns no batch for an empty argument list, which replaces the guard in gitEach.
  • One argument limit of 30,000 characters, the Windows one, now applies on every platform. Other platforms just run a few more batches.
  • hooks run always has a ✘ summary line when a check fails, so its empty-string fallback is gone.
• Two /* c8 ignore next */ comments remain:
  • tsc's case-sensitivity flag depends on the platform. Its test asserts whichever behavior the current platform has.
  • The error.message fallback in platformMessage is kept as a safeguard for a PlatformError with no cause.
• Also fixed: a trailing space after ▶ <tool> when colors are on and the tool gets no arguments.

The suite takes about 4.5 minutes on 4 cores, up from about 45 seconds, because every run is a real process.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BS5DRMnG7TWyjw7w9DVKpr

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/uncheck@18

commit: 3890b8d

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[pullfrog]

ℹ️ No critical issues — the e2e rewrite is thorough and the coverage gate is legitimate. Two minor suggestions inline, plus one note on where the 100% branch target is pushing the source.

Reviewed changes

• E2e harness — tests/_shared/project.ts copies a real fixture, installs the real tools and a fake uncheck package whose bin imports src/bin.ts, and git inits; register.mjs supplies Node type-stripping plus extensionless/folder/JSON resolution.
• Per-command suites — 398 tests under tests/uncheck, tests/staged, tests/prepare, tests/hooks, each spawning the CLI as a child process and asserting exact stdout/stderr and on-disk/index state.
• Coverage gate — test:coverage switched to c8 --all --check-coverage --100 over packages/*/src/**; @vitest/coverage-v8 removed and c8 added; vitest.config.ts and tsconfig.json updated for the fixtures.
• Source simplifications — staged.ts gitEach, tool.ts argvBatches/MAX_ARGV_LENGTH, uncheck.ts ▶ line, hooks/run.ts summary, errors.ts platformMessage, tsc.ts CASE_FLAGS.
• Deleted unit suites — command.test.ts, files.test.ts, fixture.ts, tool.test.ts, tsc.test.ts replaced by the above.

I verified the coverage gate actually measures child-process execution (NODE_V8_COVERAGE propagates, --all fails unexecuted files, type-strip preserves offsets), that each removed guard is currently unreachable, and that the assertions are exact rather than loose. One self-referential assertion and the platformMessage cast are the only findings.

ℹ️ The 100% branch gate is being met partly by deleting defensive branches

Two source fallbacks were removed and the coverage gate now forbids replacing them with a test, since the guarded case is unreachable from the CLI. The spawner always attaches an Error cause and a failed run always ends on a ✘ line, so both removals are safe today — but they trade production robustness for a metric, and the gate makes it structurally hard to keep such guards in the future. Flagging so the maintainer can confirm this is the intended policy before it sets precedent.

Technical details

# Coverage target vs. defensive branches

## Affected sites
- `packages/uncheck/src/errors.ts:27` — `(error.cause as Error).message` replaced the `cause instanceof Error ? ... : error.message` fallback; the fallback branch was the only uncovered one.
- `packages/uncheck/src/commands/hooks/run.ts:104` — `.at(-1)!` replaced `.at(-1) ?? ''`.
- `packages/uncheck/src/checks/tsc.ts:464-466` — the platform-specific branch is kept but hidden behind `/* c8 ignore next */` rather than exercised.

## Required outcome
Decide whether "reach 100% branches" may drive deletion of defensive code, or whether such branches should be retained with an explicit `/* c8 ignore */`. No code change is required if the policy is intentional.

## Open questions for the human
- Is `--check-coverage --100` meant to hold for every future PR, including platform-only branches?
- The suite went from ~45s to ~4.5min on 4 cores and now runs on Node 22/24/26; is that CI budget acceptable long-term?

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (default — pick a model for stronger reviews) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Since the prior pullfrog review (96fe2be), one commit landed (3890b8d) and addresses both inline findings:

• Restored the platformMessage fallback — errors.ts:26-29 again returns error.message for a PlatformError whose cause is not an Error, marked /* c8 ignore next */ so the unreachable branch does not break the 100% gate.
• Hardened the Cursor config assertion — tests/hooks/install/agents.test.ts:46-48 now compares against a literal config instead of re-stringifying the file the code just wrote.

Both prior threads were addressed and resolved, and the delta introduces no new issues. The remaining body-level note from the prior review (the 100% branch gate pushing deletion of defensive branches) was explicitly "no code change required if intentional," and the one still-uncovered fallback is now retained behind /* c8 ignore next */.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (default — pick a model for stronger reviews) ｜ 𝕏}