diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index dece3e344983..605b01f0a206 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -211,13 +211,47 @@ describe("evaluatePullRequestWatch", () => { ); }); - it("reports each failure at once, even while another check never finishes", () => { + it("reports each failure at once, and again once the other checks finish", () => { const bot = check("CodeRabbit", "pending"); const first = detail({ checks: [check("lint", "failure"), check("test", "pending"), bot] }); const lint = evaluatePullRequestWatch(watch(), first, noRemarks); assert.deepEqual(lint.changes, [{ kind: "checks-failed", failed: [check("lint", "failure")] }]); assert.deepEqual(evaluatePullRequestWatch(lint.next, first, noRemarks).changes, []); + const finished = detail({ + checks: [check("lint", "failure"), check("test", "success"), check("CodeRabbit", "success")], + }); + const settled = evaluatePullRequestWatch(lint.next, finished, noRemarks); + assert.deepEqual(settled.changes, [ + { kind: "checks-failed", failed: [check("lint", "failure")] }, + ]); + assert.deepEqual(evaluatePullRequestWatch(settled.next, finished, noRemarks).changes, []); + const late = detail({ checks: [...finished.checks, check("docs", "pending")] }); + const lateRun = evaluatePullRequestWatch(settled.next, late, noRemarks); + const lateDone = detail({ checks: [...finished.checks, check("docs", "success")] }); + assert.deepEqual(evaluatePullRequestWatch(lateRun.next, lateDone, noRemarks).changes, []); + const held = { ...check("test", "action-required"), url: run(3) }; + const heldRead = detail({ checks: [check("lint", "failure"), held] }); + const hold = evaluatePullRequestWatch(lint.next, heldRead, noRemarks); + assert.deepEqual(hold.changes, [{ kind: "checks-awaiting-approval", checks: [held] }]); + const ran = { ...check("test", "success"), url: `${run(3)}/job/1` }; + const heldDone = detail({ checks: [check("lint", "failure"), ran] }); + assert.deepEqual(evaluatePullRequestWatch(hold.next, heldDone, noRemarks).changes, [ + { kind: "checks-failed", failed: [check("lint", "failure")] }, + ]); + const gate = { ...check("CI", "action-required"), url: run(4) }; + const gateRead = evaluatePullRequestWatch(watch(), detail({ checks: [gate] }), noRemarks); + const omitted = detail({ checks: [check("lint", "failure")] }); + const lintOnly = evaluatePullRequestWatch(gateRead.next, omitted, noRemarks); + assert.deepEqual(lintOnly.changes, [ + { kind: "checks-failed", failed: [check("lint", "failure")] }, + ]); + const gateRan = { ...check("e2e", "success"), url: `${run(4)}/job/1` }; + const gateDone = detail({ checks: [check("lint", "failure"), gateRan] }); + assert.deepEqual(evaluatePullRequestWatch(lintOnly.next, gateDone, noRemarks).changes, [ + { kind: "checks-failed", failed: [check("lint", "failure")] }, + ]); + const second = detail({ checks: [check("lint", "failure"), check("test", "failure"), bot] }); const test = evaluatePullRequestWatch(lint.next, second, noRemarks); assert.deepEqual(test.changes, [{ kind: "checks-failed", failed: [check("test", "failure")] }]); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 046c48e1858b..8e1c00cd2b25 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -61,8 +61,9 @@ export function evaluatePullRequestWatch( const checks = detail.checks.filter((check) => check.placeholder !== true); const failed = checks.filter(isFailedCheck); const awaiting = checks.filter((check) => check.status === "action-required"); + const owedBefore = owedChecks; + const newlyFailed = failed.filter((check) => !failedChecks.includes(check.name)); if (checks.length > 0) { - const newlyFailed = failed.filter((check) => !failedChecks.includes(check.name)); if (newlyFailed.length > 0) changes.push({ kind: "checks-failed", failed: newlyFailed }); if (!approvalAnnounced && awaiting.length > 0) { changes.push({ kind: "checks-awaiting-approval", checks: awaiting }); @@ -136,6 +137,18 @@ export function evaluatePullRequestWatch( changes.push({ kind: "checks-passed", count: checks.length, required: false }); owedChecks = []; } + // A failed gate never reports passed, so it owes one wake when the checks still running finish. + if (!passedNow && failed.length > 0) { + const unfinished = checks + .filter((check) => check.status === "pending" || check.status === "action-required") + .map((check) => check.name); + const holding = unfinished.length > 0 || awaitingApproval.length > 0; + if (!holding && owedBefore.length > 0 && newlyFailed.length === 0) { + changes.push({ kind: "checks-failed", failed }); + } + const owing = newlyFailed.length > 0 || owedBefore.length > 0; + owedChecks = owing && holding ? [...unfinished, ...awaitingApproval] : []; + } passed = passedNow; passedChecks = passedNow ? gateNames : []; }