Skip to content

Test coverage: close the remaining gap (91.3% → 96%+) #522

Description

@rhuanbarreto

Baseline

Merged cross-platform coverage from the latest main run (run 30183883990, commit bcb086f — includes the #512 / ARCH-025 test-idiom refactor, which left coverage numerically unchanged), combining the coverage-linux and coverage-windows artifacts exactly as the CI coverage-report action does (a line counts as covered if either platform hits it):

  • Line coverage: 91.3% (8,354 / 9,155) — CI threshold is 90%
  • 801 uncovered lines across 55 of 86 source files

Per-directory breakdown

Directory Coverage Missed Found
src/helpers 90.9% 401 4,411
src/engine 93.8% 148 2,378
src/commands/adr 86.3% 100 728
src/commands 90.0% 81 814
src/commands/plugin 79.3% 52 251
src/commands/adr/domain 87.9% 16 132
src/formats 98.7% 2 150
src/commands/session-context 99.7% 1 291

Coverage math

Target Additional lines to cover
93% ~160
95% ~343
96% ~435
97% ~526

What the uncovered code actually is

Every uncovered region in the top 21 files (642 of the 801 missed lines) was read and categorized. Almost nothing is inherently untestable — the gaps cluster into five patterns:

  1. Pure output-formatting branches never exercised (reporter severity/verbose branches, --json output paths) — no mocking needed, just more test cases.
  2. Interactive prompt paths (inquirer flows behind isTTY checks) — testable with the mock.module("inquirer", ...) pattern already used elsewhere in the suite.
  3. Subprocess/network error paths (Bun.spawn throwing, git clone failing, download/extract failures) — testable by mocking Bun.spawn/fetch, both already-established patterns in tests/.
  4. Filesystem error paths (EACCES, corrupted SQLite DB, malformed .rules.ts) — testable with crafted fixtures.
  5. Platform-gated branches (darwin-arm64 artifact selection, macOS VS Code path, WSL success path) — coverable on any runner by testing them idiomatically: parameterize the pure functions over platform/arch and drive all combos with test.each()/describe.each() per ARCH-025 (.archgate/adrs/ARCH-025-idiomatic-bun-test-parametrization-and-matchers.md, merged in fix(tests): replace bun:test anti-patterns with idiomatic patterns #512), or make platform detection mockable following the patterns the suite already uses (withBunWhich, os.homedir mocking, test.skipIf()). The CI aggregate only merges Linux + Windows runs, so skipIf-gated darwin tests never count — parametrization is the path that actually moves the aggregate.

Phase 1 — Quick wins, no new test infra (~196 lines → ~93.4%)

All S-effort: existing mirror test files, existing mocking patterns, mostly new cases.

  • src/engine/reporter.ts (48) — add CheckResult fixtures with status: "error", suppressionWarnings, briefingWarnings, unparsedAdrs, and verbose: true to tests/engine/reporter.test.ts. Pure formatting, console.log capture already in place.
  • src/commands/adr/create.ts (31) — test the interactive branch by mocking inquirer.prompt (reuse the established mock.module idiom) and invoking without --title/--domain.
  • src/helpers/session-context-opencode.ts (24) — nonexistent DB for listOpencodeSessions, garbage-bytes DB file (open failure), corrupted schema (query failure), zero-row session table. Existing SQLite harness covers all four.
  • src/helpers/repo-probe.ts (18) — per-host mockFetch(200, <schema-invalid body>) cases and mockFetch(500) fallthrough cases; same mock-fetch helper already in the file.
  • src/helpers/plugin-install.ts (17) — pre-seed ~/.cursor/hooks.json (valid with prior archgate hook + user hook; and invalid JSON) in the existing installCursorPlugin tests to cover mergeCursorHooks.
  • src/commands/adr/import.ts (16) — empty-source early return; interactive confirm accepted/declined via mocked inquirer.prompt without --yes.
  • src/helpers/update-check.ts (13) — pre-seed fresh cache file (TTL skip), null tag, already-up-to-date compare, thrown fetch (outer catch), isTTY: true path.
  • src/commands/adr/domain/remove.ts (10) — --json variants of the existing success and not-registered tests.
  • src/engine/context.ts (10) — buildReviewContext(tempDir, { runChecks: true }) with a rules: true ADR + companion .rules.ts fixture so the real runChecks path executes (today's test uses rules: false, so it always hits the EMPTY_SUMMARY fallback).
  • src/helpers/git.ts (9) — non-Windows mirror of the existing Windows installGit test: mock Bun.which → null and Bun.spawn → exit 0/1 to cover the brew/apt success and UserError paths.

Ratchet: raise min-coverage in code-pull-request.yml from 90 → 92 when Phase 1 lands.

Phase 2 — Mocking-heavy paths (~322 lines → ~96.9% cumulative)

M-effort: mostly Bun.spawn / module mocks and TTY stubbing; helpers are already DI-friendly.

  • src/commands/upgrade.ts (66) — mock resolveCommand/Bun.spawn for package-manager detection branches; mock binary-upgrade helpers for upgradeBinary/runExternalUpgrade; stub stderr.isTTY for download-progress rendering; cover the version-compare-failed and dispatch branches in upgrade-action.test.ts.
  • src/engine/loader.ts (59) — direct unit test for blockedToRuleResult; readdirSync EACCES mock for the unreadable-dir branch; crafted .rules.ts fixtures that pass syntax scanning but fail dynamic import, and one with a schema-invalid export.
  • src/commands/plugin/install.ts (49) — cursor case via mocked installCursorPlugin; per-editor forced failures to exercise printManualInstructions; stdin.isTTY stub + mocked detectEditors/promptEditorSelection for the interactive branch.
  • src/commands/adr/sync.ts (47) — mocked shallowClone returning temp dirs missing adrs/ or specific files; TTY + mocked inquirer loop for keep/take/skip; rmSync-throw for cleanup; --json --yes combined output branch.
  • src/helpers/registry.ts (43) — shallowClone against a local file:// bare-repo fixture (pattern exists in tests/helpers/git.test.ts / git-files.test.ts), including failure + cleanup-rethrow; detectTarget invalid-target fixtures (no mocking).
  • src/helpers/credential-store.ts (30) — rehabilitate the skipped store-file-helper round-trip test (git config credential.helper "store --file=<tmp>" with the existing GIT_CONFIG_GLOBAL isolation); mock Bun.spawn with a never-resolving stdout for the timeout race.
  • src/helpers/repo.ts (19) — temp git init repo with no remote and no origin/HEAD symref (use _resetRepoContextCache); direct parseRemoteUrl("") / single-segment cases; Bun.spawn-throw for the capture/check catches.
  • src/commands/review-context.ts (9) — engineer real truncation (many changed files with small maxFiles, long ADR prose) in the git-backed temp project and spy on logWarn.

Ratchet: raise min-coverage 92 → 95 when Phase 2 lands.

Phase 3 — Platform branches & telemetry (~124 lines) + long tail (~159 lines)

Platform-gated branches get covered idiomatically, following the patterns already in the suite plus the test.each()/describe.each() parametrization idiom now codified in ARCH-025 (merged in #512) — not excluded:

  • src/helpers/binary-upgrade.ts (23) — tar path-traversal guard + extraction-failure throws via crafted .tar.gz fixtures (the Windows zip fixture pattern in the existing test already does this). For the darwin-arm64 branch: getArtifactInfo is a pure function — parameterize it over (platform, arch) and cover all platform/arch combos with a test.each() table on any runner.
  • src/helpers/vscode-settings.ts (16) — the macOS and WSL-success branches are unreachable today because isMacOS()/isWSL() are called directly. Make platform detection mockable (namespace import or injected facade, mirroring the os.homedir mocking already in vscode-settings.test.ts), then describe.each() the platform matrix: win32, macOS, WSL-with-winHome, WSL-fallback, plain Linux.
  • src/helpers/telemetry.ts (85) — the one genuine policy call. The source says these paths are "intentionally uncovered, validated via dashboard." Two options: (a) mock posthog-node via mock.module and run with NODE_ENV !== "test" to inspect captured payloads — covers property builders, trackEvent, and the fetch/shutdown catch paths; or (b) keep the policy but make it explicit with coverage-ignore comments so the intent is visible instead of ambient. Recommendation: (a) for getStaticProperties/getCommonProperties/trackEvent (payload correctness is worth testing — the env detectors are pure functions over Bun.env and fit a test.each() table of CI-provider/shell fixtures), (b) only if the team wants to keep the dashboard-validation policy.
  • Long tail (~159 lines across ~34 files, each ≤ 12 missed lines) — platform.ts, ast-support.ts, adr-import.ts, init-project.ts, stack-detect.ts, prompt.ts, etc. Batch as "good first issue"-style additions; each is a handful of assertions in an existing mirror test file.

Ratchet: consider 95 → 96 once Phase 3 decisions are settled.

Notes

  • All percentages are against the merged Linux+Windows aggregate — single-platform numbers understate coverage for files with platform branches (platform.ts, credential-store.ts, vscode-settings.ts), so any local verification should replicate the merge or be compared against the CI coverage-report artifact.
  • Platform-specific code is not an excuse for exclusions: pure functions get parameterized over (platform, arch) and covered via test.each() tables (ARCH-025); impure platform detection gets a mockable seam following existing suite patterns. The CI aggregate merges Linux + Windows only, so test.skipIf(platform !== "darwin") tests never contribute — parametrized tests that run everywhere are what move the number.
  • With platform branches testable, the realistic ceiling after all three phases is ~98%+; the only intentional-exclusion candidate left is telemetry, and only if the "validated via dashboard" policy is kept.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions