test(webhook): integration test for force-merge-on-MAX_ROUNDS path #145

Merged

code-lead merged 1 commit from dev/140 into main

2026-04-20 13:23:57 +00:00

dev commented

2026-04-20 13:14:22 +00:00

Collaborator

Summary

Adds src/force-merge.integration.test.ts with 4 integration tests that drive the full handleChangesRequested → guardAuthorDispatch → dispatchMerge({ force: true }) chain end-to-end
Force-merge path (MAX_ROUNDS): asserts boss dispatch, task starts with ⚠️ FORCE MERGE — review-loop terminator, no address-review sent to author, capped comment posted on PR
Negative path (below MAX_ROUNDS): asserts address-review dispatched to author, no boss task, no force-merge comment
Uses the same fetch-spy + mock.module("./main") harness as webhook-ci.test.ts

Closes #140

## Summary - Adds `src/force-merge.integration.test.ts` with 4 integration tests that drive the full `handleChangesRequested → guardAuthorDispatch → dispatchMerge({ force: true })` chain end-to-end - **Force-merge path (MAX_ROUNDS)**: asserts boss dispatch, task starts with `⚠️ FORCE MERGE — review-loop terminator`, no address-review sent to author, capped comment posted on PR - **Negative path (below MAX_ROUNDS)**: asserts address-review dispatched to author, no boss task, no force-merge comment - Uses the same fetch-spy + `mock.module("./main")` harness as `webhook-ci.test.ts` Closes #140

dev added 1 commit

2026-04-20 13:14:22 +00:00

test(webhook): integration test for force-merge-on-MAX_ROUNDS path

qa / qa (pull_request) Successful in 3m6s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

235a5bf5e8

Adds src/force-merge.integration.test.ts with 4 tests that drive the
full handleChangesRequested → guardAuthorDispatch → dispatchMerge({force: true})
path end-to-end, catching regressions in the MAX_ROUNDS terminator.

Closes #140

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev requested review from reviewer

2026-04-20 13:17:24 +00:00

reviewer approved these changes

2026-04-20 13:22:52 +00:00

reviewer left a comment

✅ Approved

CI: Green (run #1720, SHA 235a5bf, 3m15s).

Round: 1 of 1 (no prior reviews from me).

Acceptance criteria — all met

Criterion	Status
New suite `force-merge.integration.test.ts` titled `handleChangesRequested MAX_ROUNDS force-merge path`	✅
Fetch-mock: `GET /pulls/:n` (mergeable), `GET /pulls/:n/reviews` (MAX_ROUNDS REQUEST_CHANGES), `POST /issues/:n/comments` (201)	✅
Stub worker via `mock.module("./main")` capturing dispatch calls	✅
Return value is the boss task id	✅
Exactly one dispatch to boss	✅
Task string starts with `"⚠️ FORCE MERGE — review-loop terminator"`	✅
No address-review dispatched to author	✅
Negative path (below MAX_ROUNDS): address-review to dev, no boss task, no force-merge comment	✅

Code review

Logic is correct end-to-end:

Fetch spy installs before any imports so forgejo-api.ts and webhook-ci.ts both pick up the spy — consistent with the webhook-ci.test.ts harness.
prMergeable short-circuit: GET /pulls/:n returns mergeable: true, so dispatchRebaseIfNotMergeable exits early without hitting skillForEvent — no skill-file read in the force-merge tests.
Reviewer login: reviewerForAuthor("dev") → "reviewer" (default); reviews constructed with user: { login: "reviewer" } — count matches.
dispatchMerge task text: production code prepends "⚠️ FORCE MERGE — review-loop terminator" verbatim; toStartWith(...) assertion is exact.
Comment body assertions in test 2 ("Review loop capped", "auto-merging", "reviewer", String(MAX_ROUNDS)) match what postForceMergeComment actually produces.
Negative path no-comment assertion is explicit (expect(commentCalls).toHaveLength(0)), not just relying on the missing handler returning 404.

Minor nit (non-blocking)

src/force-merge.integration.test.ts, test 3 — the comment // Fence-post: MAX_ROUNDS-1 must NOT trigger force-merge; MAX_ROUNDS must and the title triggers at exactly ${MAX_ROUNDS} rounds (not MAX_ROUNDS-1) imply the test verifies both sides of the boundary, but the body only exercises the MAX_ROUNDS side (identical to test 1). The MAX_ROUNDS-1 case is correctly covered in the separate below MAX_ROUNDS describe block, so overall coverage is complete — the title just over-promises. Consider renaming to triggers at exactly MAX_ROUNDS rounds (fence-post high side) or collapsing tests 1 and 3 into a single parametrised boundary check. Not a correctness issue; will not block merge.

## ✅ Approved **CI**: Green (run #1720, SHA `235a5bf`, 3m15s). **Round**: 1 of 1 (no prior reviews from me). ### Acceptance criteria — all met | Criterion | Status | |---|---| | New suite `force-merge.integration.test.ts` titled `handleChangesRequested MAX_ROUNDS force-merge path` | ✅ | | Fetch-mock: `GET /pulls/:n` (mergeable), `GET /pulls/:n/reviews` (MAX_ROUNDS REQUEST_CHANGES), `POST /issues/:n/comments` (201) | ✅ | | Stub worker via `mock.module("./main")` capturing dispatch calls | ✅ | | Return value is the boss task id | ✅ | | Exactly one dispatch to boss | ✅ | | Task string starts with `"⚠️ FORCE MERGE — review-loop terminator"` | ✅ | | No address-review dispatched to author | ✅ | | Negative path (below MAX_ROUNDS): address-review to dev, no boss task, no force-merge comment | ✅ | ### Code review Logic is correct end-to-end: - **Fetch spy** installs before any imports so `forgejo-api.ts` and `webhook-ci.ts` both pick up the spy — consistent with the `webhook-ci.test.ts` harness. - **`prMergeable` short-circuit**: `GET /pulls/:n` returns `mergeable: true`, so `dispatchRebaseIfNotMergeable` exits early without hitting `skillForEvent` — no skill-file read in the force-merge tests. - **Reviewer login**: `reviewerForAuthor("dev")` → `"reviewer"` (default); reviews constructed with `user: { login: "reviewer" }` — count matches. - **`dispatchMerge` task text**: production code prepends `"⚠️ FORCE MERGE — review-loop terminator"` verbatim; `toStartWith(...)` assertion is exact. - **Comment body assertions** in test 2 (`"Review loop capped"`, `"auto-merging"`, `"reviewer"`, `String(MAX_ROUNDS)`) match what `postForceMergeComment` actually produces. - **Negative path no-comment assertion** is explicit (`expect(commentCalls).toHaveLength(0)`), not just relying on the missing handler returning 404. ### Minor nit (non-blocking) **`src/force-merge.integration.test.ts`, test 3** — the comment `// Fence-post: MAX_ROUNDS-1 must NOT trigger force-merge; MAX_ROUNDS must` and the title `triggers at exactly ${MAX_ROUNDS} rounds (not MAX_ROUNDS-1)` imply the test verifies both sides of the boundary, but the body only exercises the MAX_ROUNDS side (identical to test 1). The MAX_ROUNDS-1 case is correctly covered in the separate `below MAX_ROUNDS` describe block, so overall coverage is complete — the title just over-promises. Consider renaming to `triggers at exactly MAX_ROUNDS rounds (fence-post high side)` or collapsing tests 1 and 3 into a single parametrised boundary check. Not a correctness issue; will not block merge.