charles/claude-hooks

Fork 0

test(coverage): forgejo-api + cleanup tests; migrate cleanup consumers off mock.module #103

Merged

charles merged 2 commits from test/coverage-cleanup-forgejo-api into main

2026-04-19 20:35:21 +00:00

claude-desktop commented

2026-04-19 20:23:06 +00:00

Collaborator

Summary

Audit follow-up (from PR #102). Two untested modules flagged — forgejo-api.ts (226 lines) and cleanup.ts (40 lines). Writing cleanup.test.ts required solving a Bun-specific issue first: mock.module is process-global, and four webhook test files mocked ./cleanup to record calls, which blocked any isolated test for cleanup itself.

This PR adds tests for both modules and migrates the four consumers onto a tiny test seam on cleanup.ts so mock.module("./cleanup") disappears from the codebase.

Changes

`src/forgejo-api.test.ts` (new, 28 tests)

Covers auth header / content-type / URL construction / AbortSignal timeout; happy paths for all 9 exported helpers; 404 / 403 / 500 / network error / malformed JSON / non-array response all map to documented defaults. Uses the existing fetch-spy harness — no mock.module.

`src/cleanup.ts` + `src/cleanup.test.ts` (new, 9 tests)

cleanup.ts gains an impl object holding its three external dependencies (deriveIssueBranch, releaseWorktree, dropAllForIssue) plus two optional observation hooks (onCleanupIssue, onCleanupBranch, undefined in prod). The two public functions now call through impl.* and fire the hooks on entry. Zero runtime cost in prod (nullish check + indirect call); no behavior change.

cleanup.test.ts covers both public functions: empty-agent no-op, per-agent release + one session drop, single-agent release failure is swallowed without skipping the rest, session-drop failure doesn't throw.

Consumer migration — four webhook test files

webhook.test.ts / webhook-handlers.test.ts / webhook-assign-dedup.test.ts / webhook-post-merge.test.ts — replace mock.module("./cleanup", …) with:

Observe calls via impl.onCleanupIssue / impl.onCleanupBranch
Suppress real side-effects by swapping impl.releaseWorktree / impl.dropAllForIssue to no-ops
Restore impl in afterAll so the seam stays clean for files that run after

Net result: mock.module("./cleanup") appears zero times in the repo.

Why the seam

Bun's mock.module registers in a process-global registry. Once a test file calls mock.module("./cleanup", …), every other test file that runs after sees that stub — even ones that had nothing to do with cleanup. Writing an isolated cleanup.test.ts while four consumers were globally replacing the module was structurally impossible without this change.

The impl pattern:

Keeps mocking scoped to the test file that does it (mutate-and-restore on a plain object — no process-global side effect)
Doubles as a documented seam operators can point at when writing new cleanup tests
Adds 8 lines to production code (the object literal + two optional hook call sites)

Test plan

just qa — 313 pass, 0 fail (+37 net new tests: 28 forgejo-api, 9 cleanup)
All four consumer test files still pass with the same coverage they had before
grep -r 'mock.module("./cleanup"' src/ returns no matches

🤖 Generated with Claude Code

## Summary Audit follow-up (from PR #102). Two untested modules flagged — `forgejo-api.ts` (226 lines) and `cleanup.ts` (40 lines). Writing `cleanup.test.ts` required solving a Bun-specific issue first: `mock.module` is process-global, and four webhook test files mocked `./cleanup` to record calls, which blocked any isolated test for cleanup itself. This PR adds tests for both modules and migrates the four consumers onto a tiny test seam on `cleanup.ts` so `mock.module("./cleanup")` disappears from the codebase. ## Changes ### `src/forgejo-api.test.ts` (new, 28 tests) Covers auth header / content-type / URL construction / AbortSignal timeout; happy paths for all 9 exported helpers; 404 / 403 / 500 / network error / malformed JSON / non-array response all map to documented defaults. Uses the existing fetch-spy harness — no `mock.module`. ### `src/cleanup.ts` + `src/cleanup.test.ts` (new, 9 tests) `cleanup.ts` gains an `impl` object holding its three external dependencies (`deriveIssueBranch`, `releaseWorktree`, `dropAllForIssue`) plus two optional observation hooks (`onCleanupIssue`, `onCleanupBranch`, `undefined` in prod). The two public functions now call through `impl.*` and fire the hooks on entry. Zero runtime cost in prod (nullish check + indirect call); no behavior change. `cleanup.test.ts` covers both public functions: empty-agent no-op, per-agent release + one session drop, single-agent release failure is swallowed without skipping the rest, session-drop failure doesn't throw. ### Consumer migration — four webhook test files `webhook.test.ts` / `webhook-handlers.test.ts` / `webhook-assign-dedup.test.ts` / `webhook-post-merge.test.ts` — replace `mock.module("./cleanup", …)` with: - Observe calls via `impl.onCleanupIssue` / `impl.onCleanupBranch` - Suppress real side-effects by swapping `impl.releaseWorktree` / `impl.dropAllForIssue` to no-ops - Restore `impl` in `afterAll` so the seam stays clean for files that run after Net result: `mock.module("./cleanup")` appears zero times in the repo. ## Why the seam Bun's `mock.module` registers in a process-global registry. Once a test file calls `mock.module("./cleanup", …)`, every other test file that runs after sees that stub — even ones that had nothing to do with cleanup. Writing an isolated `cleanup.test.ts` while four consumers were globally replacing the module was structurally impossible without this change. The `impl` pattern: - Keeps mocking scoped to the test file that does it (mutate-and-restore on a plain object — no process-global side effect) - Doubles as a documented seam operators can point at when writing new cleanup tests - Adds 8 lines to production code (the object literal + two optional hook call sites) ## Test plan - [x] `just qa` — 313 pass, 0 fail (+37 net new tests: 28 forgejo-api, 9 cleanup) - [x] All four consumer test files still pass with the same coverage they had before - [x] `grep -r 'mock.module("./cleanup"' src/` returns no matches 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-desktop added 1 commit

2026-04-19 20:23:07 +00:00

test(forgejo-api): add unit tests for auth, happy paths, and error swallowing

qa / qa (pull_request) Successful in 2m34s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

4053907060

`src/forgejo-api.ts` had no tests — every HTTP call silently returned
null / [] / false / "" on 4xx/5xx or network errors, with no regression
guard against dropped auth headers, forgotten timeouts, or URL mistakes.

Adds 28 tests covering:

- Auth header: every request carries `Authorization: token <token>`.
- Content-Type: JSON-body requests set `application/json`.
- URL construction: /api/v1 prefix, encoded base ref for branch filters.
- Timeout: the fetch init carries an AbortSignal.
- Happy paths: all 9 exported helpers return parsed bodies / statuses
  when the upstream is healthy.
- Error paths: 404, 403, 500, malformed JSON, network errors, and
  non-array responses all map to the documented defaults.
- repoHasWorkflows short-circuits on a malformed repo string
  (no slash) without making a fetch call.

Uses the fetch-spy harness already established in webhook.test.ts
(install once at module top, register handlers per test) — no
`mock.module` calls, so no cross-file leakage risk.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

charles added 1 commit

2026-04-19 20:31:10 +00:00

test(cleanup): add 9 cleanup tests + migrate consumers off mock.module

qa / qa (pull_request) Successful in 2m35s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

850364d198

Bun's `mock.module` registers process-globally and leaks stubs to every
test file that runs after. The four webhook tests all mocked ./cleanup
just to record calls — which blocked any cleanup.test.ts from working
at all (either no impl export, or a replaced cleanupIssue that ignored
the test stubs).

Fix: add a tiny test seam to cleanup.ts and move every ./cleanup test
consumer onto it.

- `src/cleanup.ts` adds an `impl` object holding the three external
  dependencies (deriveIssueBranch, releaseWorktree, dropAllForIssue)
  plus two optional observation hooks (onCleanupIssue, onCleanupBranch,
  `undefined` by default). `cleanupIssue` / `cleanupBranch` now go
  through `impl.*` and fire the hooks on entry. Zero runtime cost in
  prod (nullish-check plus indirect call); no behavior change.

- `src/cleanup.test.ts` (new, 9 tests) covers both exported functions:
  no-op on empty agent list, per-agent release + one session drop, a
  single agent's release failure is swallowed without skipping the
  others, session-drop failure doesn't throw.

- webhook.test.ts / webhook-handlers.test.ts / webhook-assign-dedup.test.ts
  / webhook-post-merge.test.ts: drop `mock.module("./cleanup", …)`.
  Observe calls via `impl.onCleanup{Issue,Branch}` and suppress the
  internal side-effects by swapping `impl.releaseWorktree` /
  `impl.dropAllForIssue` to no-ops. Each file restores `impl` in
  `afterAll` so the seam stays clean for subsequent files.

Results: 313 pass, 0 fail (was 304 pre-refactor; +9 net new).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

claude-desktop changed title from ~~test(forgejo-api): add unit tests for auth, happy paths, and error swallowing~~ to test(coverage): forgejo-api + cleanup tests; migrate cleanup consumers off mock.module

2026-04-19 20:31:32 +00:00

charles merged commit 9dd3bbdc55 into main

2026-04-19 20:35:21 +00:00

charles deleted branch test/coverage-cleanup-forgejo-api

2026-04-19 20:35:21 +00:00

charles referenced this pull request from a commit

2026-04-19 20:35:23 +00:00

test(coverage): forgejo-api + cleanup tests; migrate cleanup consumers off mock.module (#103)

claude-desktop referenced this pull request

2026-04-19 20:57:44 +00:00

Agents: migrate boss / dev / reviewer to container mode + give them their plugin sets #76