Repository navigation
test(harness): fail a unit test on any msw request no handler served; serve the merge-queue state and base-branch tip the exposed tests were falling back from - #571
Conversation
failOnUnhandledRequest only rejects the request; code under test that catches the rejection (readQueueState's direct-merge fallback, baseBranchTip's base.sha fallback, the configuration probes) let the test pass with the request silently unserved. failTestOnUnhandledRequests records every request:unhandled event and fails the test in afterEach, with .allow() for a test that leaves one unserved on purpose. Wiring it into the msw-backed unit files surfaced tests that ran through those fallbacks: the merge queue state query was unserved in every merge-path test (now answered by noMergeQueue, the isMergeQueueEnabled: false GitHub reports for a branch without a queue), the base branch tip was unserved in two tree-failure probes (now branchHandler()), one /test outsider test skipped the configuration probes and one ownersLabel error probe left the approve handler's default-branch tree read unserved. __tests__/plugins/tide.test.ts and __tests__/cronJobTest/sweep.test.ts are not wired yet; see #570. Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
Important Held for human review by the hive's ACMM level gate. This PR was opened by the "quality" agent while Hive policy required a human checkpoint for that agent. Agent PRs are held at ACMM L3–L5; an Hive will keep the |
|
/kind cleanup 🐝 Hive Agent: — hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
…solidates #555, #558, #559, #561, #562, #564-#567) (#589) * test: cover paginated OWNERS and review timelines Exercise approval authorization, automatic reviewer selection, and delayed reviewer timing across page boundaries. Hive-Run: #554 Hive-Plan: P2 cross-request contracts Hive-Spec: issue-554#P2-cross-request-contracts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: mrbobbytables <mrbobbytables@users.noreply.github.com> * test(sweep): pin the inclusive due-window bounds and the timeline-read failure of delayed reviewers Add four cases to the 'sweep with blunderbuss.delay' block of __tests__/cronJobTest/sweep.test.ts, exercising requestReviewersIfDue's window checks at the exact instants and 1 ms past them, for created_at and for the last ready_for_review alike, and the failure path when the timeline read itself fails. Each boundary case fails on a one-character mutant of sweep.ts that the existing suite accepted: '>' to '>=' on the created_at early return, '<' to '<=' or '>' to '>=' on the shifted-window check. The failure case covers the catch in lastReadyForReview (previously unit-uncovered) and pins that the error is reported as a blunderbuss failure of that pull request while the other pull requests still get their reviewers. Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> * test(sweep): pin that a second delayed sweep leaves the first run's requested reviewers alone The delayed-blunderbuss bundle suite modelled a pull that already had reviewers only as a static fixture. This drives dist/index.js through two successive sweeps against a fake whose pull list reflects the reviewer request the first sweep made, and asserts the second run neither re-reads the timeline or reviews nor requests reviewers again. Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> * test(sweep): pin the delayed-reviewers OWNERS guard on a base branch without OWNERS files Both suites left the false arm of the branchHasOwners guard ahead of requestReviewersIfDue in src/cronJobs/sweep.ts with zero hits: every delayed-blunderbuss case served an OWNERS file on master. Add one unit case and one dist/index.js scenario, each in its own file, that configure blunderbuss.delay, serve an empty master tree and a due pull request, and assert the tree is probed once while the timeline, reviews and requested_reviewers endpoints are never touched. Both fail when the guard is replaced by `|| true`. Fixes #560 Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> * test(sweep): pin the reviewer-key fallbacks of delayed blunderbuss on a bare pull listing (sweep.ts:208) requestReviewersIfDue reads requested_reviewers ?? [] and requested_teams ?? [] off the pull list entry, but no unit or bundle test listed a delayed-only pull request without those keys, leaving both fallback arms unreached in coverage/ and coverage-e2e/. Serve a due pull with the keys removed on both harnesses and assert reviewers are still requested. Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> * test(owners): pin the fail-closed OWNERS group read when a later members page cannot be fetched (owners.ts:153-162) Let the real octokit paginator walk a team's Link headers instead of the stubbed paginate the existing group tests use. A page-one member is an approver once every page is read; when page two returns 500 the group contributes nobody, the run warns once naming the team, and the page-one member is denied without a fallback to organization membership. Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> * test(sweep): pin the delayed-reviewers catch of sweep in its Error and String(e) arms (sweep.ts:85) Adds __tests__/cronJobTest/sweepDelayFailures.test.ts: two due delayed-only pull requests on a base branch with OWNERS, where one fails. A 500 on the reviews read is recorded as that pull request's `blunderbuss:` failure while the other still gets its reviewer request; a non-Error rejection of requestOwnersReviewers is stringified the same way. Before: coverage/lcov.info BRDA:85,5,0,0 / BRDA:85,5,1,0 (neither arm hit by the unit suite). After: both 1; sweep.ts branches 92.3 -> 94.87 %. Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> * test(sweep): pin the ignore_drafts=false arm of delayed reviewers (sweep.ts:218) With blunderbuss.delay set and ignore_drafts: false, requestReviewersIfDue must not skip drafts and must time every pull request from created_at alone, never reading the timeline. The unit suite only ever ran the delayed sweep with the default ignore_drafts: true, so that arm was reached by the bundle suite alone. Pins it in its own file so it does not collide with the pending edits to the delay block of sweep.test.ts. Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> * test(sweep): pin the no-catch-up recovery of delayed reviewers after missed schedule runs After the schedule misses runs for longer than sweep.lookback, a pull request whose review clock expired before now - delay - lookback is neither listed, read nor requested by the delayed-reviewers step, and a longer sweep.lookback is what recovers it. Pinned in a unit suite (__tests__/cronJobTest/sweepDelayOutage.test.ts) and against dist/index.js (__tests__/bundle/blunderbussDelayOutage.test.ts); the blunderbuss.delay paragraph of docs/configuration.md now records the expectation. Refs #554 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> * test(harness): fail tide and sweep unit tests on unserved requests Completes the #571 wiring that was deferred while #557 and #558 occupied these files: both servers now answer the merge-queue read with noMergeQueue and assert at the end of each test that no request went unserved; sweep's OWNERS scenarios serve the base-branch tip instead of running through the 'could not read the tip' fallback. Refs #570 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Bob Killen <bkillen@linuxfoundation.org> --------- Signed-off-by: mrbobbytables <mrbobbytables@users.noreply.github.com> Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Signed-off-by: Bob Killen <bkillen@linuxfoundation.org> Co-authored-by: mrbobbytables <mrbobbytables@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
Test Improvement
failOnUnhandledRequestonly rejects a request no handler serves; code under test that catches the rejection lets the test pass with the request silently unserved. This PR adds the missing end-of-test assertion for the unit suite and fixes the 158 tests (15 files) it exposed onmain@e3119ad.__tests__/testUtils.tsfailTestOnUnhandledRequests(server): records msw'srequest:unhandledper test (listener re-attached inbeforeEach, so anafterEachthat callsserver.events.removeAllListeners()cannot disarm it), fails inafterEachlisting the requests;.allow()is the explicit per-test allowance for a deliberate probe;seen()/check()expose the state for the self-test.noMergeQueue:mergeQueueGraphql({ enabled: false }).handler, theisMergeQueueEnabled: falseGitHub reports for a base branch without a merge queue.__tests__/unhandledRequests.test.ts(new): pins the helper — a swallowed unserved request is recorded and failscheck(), a served request and a deliberateResponse.error()are not counted,allow()lasts one test, and the listener is re-armed afterremoveAllListeners().58 msw-backed unit files (every file outside
__tests__/bundle/exceptplugins/tide.test.tsandcronJobTest/sweep.test.ts, see Related Issue): one line,utils.failTestOnUnhandledRequests(server), aftersetupServer(...).What the assertion exposed, and how each is served now
POST /graphqlmerge-queue state query, unserved in every tide/sweep/lgtm merge-path test and reaching the direct merge viareadQueueState's warning fallback (src/utils/mergeQueue.ts:88-93) →utils.noMergeQueueas the file-level default in 11 files (cronJobTest/lgtm,issueCommentTest/approve,label/lgtm,issueCommentTest/trigger,cronJobTest/handleCronJob,plugins/approveReview,plugins/approveEvents,pullReqTest/onPrLgtm,cronJobTest/labelSync,plugins/requireMatchingLabel,utils/ownersAuth).queue?.enabled === true(src/plugins/tide.ts:194) is the only consumer, so the outcome is unchanged; only the fallback warning disappears.GET /branches/master, unserved and falling back topull.base.sha(src/utils/pullRequestOwners.ts:83-86) →branchHandler()in theownersAuth/pullRequestOwnerstree-failure probes.trigger.test.ts"/test › refuses an outsider" →...utils.noOrgOrRepoConfigExcept(), as its/retestsibling already has.ownersLabel.test.tserror probe →utils.defaultBranchTree()for the approve handler's concurrent OWNERS-presence read.The fallback branches keep their dedicated tests (
utils/mergeQueue.test.ts:116-124,plugins/tideMergeQueue.test.ts:261,utils/pullRequestOwners.test.ts:290); no.allow()was needed in the existing suite.Coverage evidence
npm run test:coverageon this branch: 168 files / 2336 tests pass, 99.92 % lines / 99.35 % branches / 100 % functions — identical tomain@e3119ad.npm run lint: clean apart from the pre-existingsrc/issueComment/cc.ts:81jsdoc warning onmain.git merge-treewith test(tide): pin the lgtm gate re-check after an unknown-mergeability retry (evaluateMerge, tide.ts:198-206) #557, test(sweep): pin the inclusive due-window bounds and the timeline-read failure of delayed reviewers #558, test(sweep): pin the delayed-reviewers OWNERS guard on a base branch without OWNERS files (sweep.ts:80) #561, test(sweep): pin the reviewer-key fallbacks of delayed blunderbuss on a bare pull listing (sweep.ts:208) #562, test(sweep): pin the delayed-reviewers catch of sweep in its Error and String(e) arms (sweep.ts:85) #565, test(sweep): pin the ignore_drafts=false arm of delayed reviewers (requestReviewersIfDue, sweep.ts:218) #566, test(sweep): pin the no-catch-up recovery of delayed reviewers after missed schedule runs #567 reports no conflict. New files those PRs add (sweepDelay*.test.ts,blunderbussDelay*.test.ts) create their own servers and are not wired here; a one-line follow-up can add them once they land.__tests__/bundle/is untouched — that side is test(bundle): record the fake GitHub API's unmatched requests and assert on them outside the application #569.Related Issue
Fixes #570 (remaining: wiring
__tests__/plugins/tide.test.tsand__tests__/cronJobTest/sweep.test.ts, left untouched because held PRs #557 and #558 occupy those files — the hive refuses an overlapping-files PR; one-line follow-up once they land)Refs #554 (umbrella: this is the unit-suite half of "Record unexpected API traffic and assert on it"; the bundle half is #569, the other sections stay open there)
Filed by quality agent (hold-gated mode). Human review required.
— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88 requested_by=@mrbobbytables