Skip to content

test: pin the delayed-blunderbuss and paginated-OWNERS contracts (consolidates #555, #558, #559, #561, #562, #564-#567) - #589

Merged
mrbobbytables merged 10 commits into
mainfrom
test/blunderbuss-delay-contracts
Oct 26, 2026
Merged

mrbobbytables merged 10 commits into
mainfrom
test/blunderbuss-delay-contracts

Conversation

@mrbobbytables

@mrbobbytables mrbobbytables commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

Description

Consolidates the nine open PRs that pin the blunderbuss.delay feature from #530 and the paginated OWNERS-group contract from #486 into one branch, plus the harness wiring #571 had to leave out of tide.test.ts and sweep.test.ts while those files were occupied. Every original commit is cherry-picked unchanged (author and DCO sign-off preserved); they applied without conflicts. No src/ or dist/ change.

Original Commit Pins
#555 e51eec6 bundle: later timeline page's ready_for_review; team member on page two approves / is selected (fake GitHub paginates teams)
#558 63b5282 unit: inclusive due-window bounds (sweep.ts:214/224), timeline-read failure
#559 82a7759 bundle: a second delayed sweep leaves the first run's reviewers alone (sweep.ts:208-211)
#561 e3239c9 unit + bundle: delayed-reviewers OWNERS guard on a base branch without OWNERS (sweep.ts:80 false arm)
#562 e7f93b6 unit + bundle: requested_reviewers/requested_teams ?? fallbacks on a bare listing (sweep.ts:208)
#564 b371357 unit: OWNERS group read through the real paginator; fail-closed when a later page cannot be fetched (owners.ts:153-162)
#565 25e2974 unit: the delayed-reviewers catch in its Error and String(e) arms (sweep.ts:85)
#566 6526644 unit: ignore_drafts: false arm (sweep.ts:218)
#567 0cd8c9b unit + bundle + docs: no catch-up after missed schedule runs; a longer sweep.lookback is what recovers it
— (new) tide.test.ts / sweep.test.ts: noMergeQueue + failTestOnUnhandledRequests; sweep's OWNERS scenarios serve GET /branches/master so they no longer run through the "could not read the tip" fallback

Overlaps reviewed and kept as distinct contracts: #558 (timeline read fails) and #565 (review read fails, non-Error rejection) exercise different arms of the same catch; #555 (bundle) and #564 (unit) pin pagination at different layers, with #564 adding the fail-closed case.

Fixes #560
Refs #554 (P2 "Add cross-request contracts for the new features"), #570 (deferred wiring — the fallback warnings those two files used to emit are now zero)

Testing

  • npm run lint — passes (one pre-existing jsdoc/check-param-names warning at cc.ts:81, unrelated)
  • npm run test:coverage — 179 files / 2370 passed, 75 skipped; 100 / 99.76 / 100 / 100 (from 99.92 / 99.34 / 100 / 99.91 on main)
  • npx vitest run on the six touched bundle files — 35 passed
  • npx vitest run __tests__/cronJobTest/sweep.test.ts __tests__/plugins/tide.test.ts — 153 passed; grep -cE 'could not read the (tip|merge queue)' on the output: 0
  • npm run test:coverage:e2e — left to the e2e-coverage CI job (no dist/ change)

Checklist

  • I ran npm run all to lint and build my code (lint and unit coverage; no build-affecting change)
  • I ran npm run pack and committed dist/ — not applicable, no src/ change
  • Commits are signed off (DCO)
  • Note any new dependencies these changes bring in — none
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (docs/configuration.md via the test(sweep): pin the no-catch-up recovery of delayed reviewers after missed schedule runs #567 commit)
  • My changes generates no new warnings (please describe new warnings if unavoidable)

/kind cleanup

mrbobbytables and others added 10 commits October 26, 2026 14:36
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>
…d 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>
…equested 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>
…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>
… 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>
…ers 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>
…d 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>
…eep.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>
…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>
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>
@mrbobbytables

Copy link
Copy Markdown
Member Author

/kind cleanup

@github-actions github-actions Bot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed needs-kind labels Oct 26, 2026
@mrbobbytables
mrbobbytables merged commit bc6e6b4 into main Oct 26, 2026
6 checks passed
@mrbobbytables
mrbobbytables deleted the test/blunderbuss-delay-contracts branch October 26, 2026 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[quality] sweep.ts delayed-reviewers OWNERS guard (line 80 false arm) is covered by neither the unit nor the bundle suite

1 participant