Repository navigation
test(sweep): pin the inclusive due-window bounds and the timeline-read failure of delayed reviewers - #558
test(sweep): pin the inclusive due-window bounds and the timeline-read failure of delayed reviewers#558hivecommons-hive[bot] wants to merge 1 commit into
Conversation
…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>
|
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 |
|
Another agent tried to open a PR from |
There was a problem hiding this comment.
Read the diff against src/cronJobs/sweep.ts at merge-base, not just the test file. The four new cases match the real boundary semantics:
sweep.ts:214(if (start > due.to)) andsweep.ts:224(if (start < due.from || start > due.to)) are inclusive on both ends, exactly as the "opened exactly at the end/start of the due window" cases assert. ThetimelineReadsexpectations are also right: a PR failing the line-214 check returns beforelastReadyForReview, while one failing only line 224 is read first — the tests distinguish the two correctly.- The
ready_for_reviewcase exercises thestart = readypromotion atsweep.ts:219-222against the same bounds. - The 500-timeline case matches the throw at
sweep.ts:244wrapped into the failure aggregate atsweep.ts:102-104, including the exactsweep: 1 pull request(s) failed: #1 (blunderbuss: could not read the timeline: HttpError: Server Error)message, andrequested = [2]confirms the per-PR catch atsweep.ts:83-86isolates the failure.
The lookback: 30m / delay: 1h shifted window keeps all fixtures on the delayed-only path (updated_at < since at sweep.ts:69), so no candidate-path crosstalk. Test-only change, no behavior touched, CI green. Looks correct to me — leaving the hold/merge call to a maintainer per the repo's hold-gated flow.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
|
/kind cleanup 🐝 Hive Agent: — hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
Test Improvement
Adds four cases to the
sweep with blunderbuss.delayblock of__tests__/cronJobTest/sweep.test.ts, for the Delayed-review boundaries item of #554 ("cover both exact shifted-window boundaries and timestamps immediately outside them") and the one unit-uncovered branch left insrc/cronJobs/sweep.ts.Files/functions claimed:
__tests__/cronJobTest/sweep.test.tsonly (servegains an optional config argument; four newitcases). Source under test:requestReviewersIfDueandlastReadyForReviewinsrc/cronJobs/sweep.ts. Disjoint from #553 (auth.ts), #555 (__tests__/bundle/*), #556 (e2e floor) and #557 (tide.test.ts).due.to, and 1 ms latercreated = now-60m/now-60m+1mscreated_atalone (no timeline read)due.from, and 1 ms earliercreated = now-90m/now-90m-1ms(listed because updated inside the window)ready_for_reviewatdue.to,due.to+1ms,due.from,due.from-1ms#1 (blunderbuss: could not read the timeline: HttpError: Server Error); #2 still gets reviewersThe boundary cases use
sweep.lookback: 30mwithdelay: 1h, so the due window[now-90m, now-60m]ends before the candidate window starts and every pull request takes the delayed-reviewers path only.Why these, per #554's "justify by mutation, not percentage"
Each one-character mutant below was applied to
src/cronJobs/sweep.tsand run against the wholesweep.test.ts; the listed new case is the only one that fails, so the prior 42 cases accepted it:start > due.to→>=(early return, line 214)start < due.from→<=(line 224)start > due.to→>=(line 224)catchinlastReadyForReview→events = [](line 244)Coverage evidence
npm run test:coverageonmain@e3119ad(2330 tests, 99.92 % lines / 99.35 % branches):sweep.ts98.33 / 92.3, uncovered85,244. With this PR line 244 (the timelinecatch) is covered; 85 (non-Error throw in the delayed loop) is untouched here.npm run test:coverage:e2eonmain@e3119ad(528 tests, 100 % / 99.46 %):sweep.ts100 / 94.87, uncovered80-85,208— line 244 is already exercised by the bundle suite, so this was a unit-only gap, and the boundary instants add no new lines or branches in either run by design.Verification
npx vitest run __tests__/cronJobTest/sweep.test.ts— 46 passednpx eslint __tests__/cronJobTest/sweep.test.ts— cleanRelated Issue
Refs #554 — ticks only the "Delayed-review boundaries" box of the P2 cross-request section. The remaining boxes (OWNERS pagination failure, successive sweeps, scheduling outage recovery, and the P1 CI/release/harness sections) are tracked there and in #555/#556/#557; the tracker itself must not be closed by a single test PR.
Filed by quality agent (hold-gated mode). Human review required.
— hive: agent=quality backend=copilot model=claude-fable-5.1 requested_by=@mrbobbytables