Skip to content

Commit 589a335

Browse files
authored
[BE] Make DrCI report workflows pending approvals (#7722)
Lots of 1st time contributors are confused by `✅ No Failures` status when no CI has been run on CI yet This would replace it with `⚠️ 2 Awaiting Approval`
1 parent d654ffa commit 589a335

File tree

2 files changed

+106
-2
lines changed

2 files changed

+106
-2
lines changed

torchci/pages/api/drci/drci.ts

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ export async function updateDrciComments(
212212
flakyJobs,
213213
brokenTrunkJobs,
214214
unstableJobs,
215+
awaitingApprovalJobs,
215216
relatedJobs,
216217
relatedIssues,
217218
relatedInfo,
@@ -230,6 +231,7 @@ export async function updateDrciComments(
230231
FLAKY: flakyJobs,
231232
BROKEN_TRUNK: brokenTrunkJobs,
232233
UNSTABLE: unstableJobs,
234+
AWAITING_APPROVAL: awaitingApprovalJobs,
233235
};
234236

235237
const failureInfo = constructResultsComment(
@@ -238,6 +240,7 @@ export async function updateDrciComments(
238240
flakyJobs,
239241
brokenTrunkJobs,
240242
unstableJobs,
243+
awaitingApprovalJobs,
241244
relatedJobs,
242245
relatedIssues,
243246
relatedInfo,
@@ -638,6 +641,7 @@ export function constructResultsComment(
638641
flakyJobs: RecentWorkflowsData[],
639642
brokenTrunkJobs: RecentWorkflowsData[],
640643
unstableJobs: RecentWorkflowsData[],
644+
awaitingApprovalJobs: RecentWorkflowsData[],
641645
relatedJobs: Map<number, RecentWorkflowsData>,
642646
relatedIssues: Map<number, IssueData[]>,
643647
relatedInfo: Map<number, string>,
@@ -671,6 +675,7 @@ export function constructResultsComment(
671675
const pendingIcon = `:hourglass_flowing_sand:`;
672676
const successIcon = `:white_check_mark:`;
673677
const failuresIcon = `:x:`;
678+
const warningIcon = `:warning:`;
674679
const noneFailing = `No Failures`;
675680
const significantFailures = `${newFailedJobs.length} New ${pluralize(
676681
"Failure",
@@ -685,30 +690,37 @@ export function constructResultsComment(
685690
unrelatedFailureCount
686691
)}`;
687692
const pendingJobs = `${pending} Pending`;
693+
const awaitingApprovalMsg = `${awaitingApprovalJobs.length} Awaiting Approval`;
688694

689695
const hasAnyFailing = failing > 0;
690696
const hasSignificantFailures = newFailedJobs.length > 0;
691697
const hasCancelledFailures = cancelledJobs.length > 0;
692698
const hasPending = pending > 0;
693699
const hasUnrelatedFailures = unrelatedFailureCount > 0;
700+
const hasAwaitingApproval = awaitingApprovalJobs.length > 0;
694701

695702
let icon = "";
696703
if (hasSignificantFailures || hasCancelledFailures) {
697704
icon = failuresIcon;
705+
} else if (hasAwaitingApproval) {
706+
icon = warningIcon;
698707
} else if (hasPending) {
699708
icon = pendingIcon;
700709
} else {
701710
icon = successIcon;
702711
}
703712

704713
let title_messages = [];
714+
if (hasAwaitingApproval) {
715+
title_messages.push(awaitingApprovalMsg);
716+
}
705717
if (hasSignificantFailures) {
706718
title_messages.push(significantFailures);
707719
}
708720
if (hasCancelledFailures) {
709721
title_messages.push(cancelledFailures);
710722
}
711-
if (!hasAnyFailing) {
723+
if (!hasAnyFailing && !hasAwaitingApproval) {
712724
title_messages.push(noneFailing);
713725
}
714726
if (hasPending) {
@@ -735,10 +747,29 @@ export function constructResultsComment(
735747
}
736748
output += ":";
737749

738-
if (!hasAnyFailing) {
750+
if (!hasAnyFailing && !hasAwaitingApproval) {
739751
output += `\n:green_heart: Looks good so far! There are no failures yet. :green_heart:`;
740752
}
741753

754+
if (awaitingApprovalJobs.length) {
755+
output += constructResultsJobsSections(
756+
hudBaseUrl,
757+
owner,
758+
repo,
759+
prNumber,
760+
`AWAITING APPROVAL`,
761+
`The following ${
762+
awaitingApprovalJobs.length > 1 ? "workflows need" : "workflow needs"
763+
} approval before CI can run`,
764+
awaitingApprovalJobs,
765+
"",
766+
false,
767+
relatedJobs,
768+
relatedIssues,
769+
relatedInfo
770+
);
771+
}
772+
742773
if (newFailedJobs.length) {
743774
output += constructResultsJobsSections(
744775
hudBaseUrl,
@@ -897,6 +928,7 @@ export async function getWorkflowJobsStatuses(
897928
flakyJobs: RecentWorkflowsData[];
898929
brokenTrunkJobs: RecentWorkflowsData[];
899930
unstableJobs: RecentWorkflowsData[];
931+
awaitingApprovalJobs: RecentWorkflowsData[];
900932
relatedJobs: Map<number, RecentWorkflowsData>;
901933
relatedIssues: Map<number, IssueData[]>;
902934
relatedInfo: Map<number, string>;
@@ -907,6 +939,7 @@ export async function getWorkflowJobsStatuses(
907939
const brokenTrunkJobs: RecentWorkflowsData[] = [];
908940
const unstableJobs: RecentWorkflowsData[] = [];
909941
const failedJobs: RecentWorkflowsData[] = [];
942+
const awaitingApprovalJobs: RecentWorkflowsData[] = [];
910943

911944
// This map holds the list of the base failures for broken trunk jobs or the similar
912945
// failures for flaky jobs
@@ -917,6 +950,15 @@ export async function getWorkflowJobsStatuses(
917950
const relatedInfo: Map<number, string> = new Map();
918951

919952
for (const job of prInfo.jobs) {
953+
// Check for workflows awaiting approval (first-time contributors need maintainer approval)
954+
if (
955+
job.conclusion === "action_required" ||
956+
job.conclusion === "startup_failure"
957+
) {
958+
awaitingApprovalJobs.push(job);
959+
continue;
960+
}
961+
920962
if (isPending(job)) {
921963
pending++;
922964
if (isUnstableJob(job as any, unstableIssues)) {
@@ -1114,6 +1156,7 @@ export async function getWorkflowJobsStatuses(
11141156
flakyJobs,
11151157
brokenTrunkJobs,
11161158
unstableJobs,
1159+
awaitingApprovalJobs,
11171160
relatedJobs,
11181161
relatedIssues,
11191162
relatedInfo,

torchci/test/drci.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,20 @@ const unstableB = getDummyJob({
226226
runnerName: "dummy",
227227
});
228228

229+
// Workflow awaiting approval from maintainer
230+
const awaitingApprovalA = getDummyJob({
231+
name: "pull / linux-build",
232+
conclusion: "action_required",
233+
completed_at: "2022-07-13 19:34:03",
234+
html_url: "a",
235+
head_sha: "abcdefg",
236+
id: 1,
237+
pr_number: 1001,
238+
failure_lines: [],
239+
failure_captures: [],
240+
runnerName: "",
241+
});
242+
229243
const sev = genIssueData({
230244
number: 1,
231245
state: "open",
@@ -251,6 +265,7 @@ function constructResultsCommentHelper({
251265
flakyJobs = [],
252266
brokenTrunkJobs = [],
253267
unstableJobs = [],
268+
awaitingApprovalJobs = [],
254269
sha = "random sha",
255270
merge_base = "random_merge_base_sha",
256271
merge_base_date = "2023-08-08 06:03:21",
@@ -264,6 +279,7 @@ function constructResultsCommentHelper({
264279
flakyJobs?: RecentWorkflowsData[];
265280
brokenTrunkJobs?: RecentWorkflowsData[];
266281
unstableJobs?: RecentWorkflowsData[];
282+
awaitingApprovalJobs?: RecentWorkflowsData[];
267283
sha?: string;
268284
merge_base?: string;
269285
merge_base_date?: string;
@@ -278,6 +294,7 @@ function constructResultsCommentHelper({
278294
flakyJobs,
279295
brokenTrunkJobs,
280296
unstableJobs,
297+
awaitingApprovalJobs,
281298
new Map(),
282299
new Map(),
283300
new Map(),
@@ -692,6 +709,50 @@ describe("Update Dr. CI Bot Unit Tests", () => {
692709
).toBeTruthy();
693710
});
694711

712+
test("test awaiting approval workflows show in comment", async () => {
713+
const failureInfoComment = constructResultsCommentHelper({
714+
pending: 0,
715+
awaitingApprovalJobs: [awaitingApprovalA],
716+
});
717+
718+
const expectToContain = [
719+
":warning:",
720+
"1 Awaiting Approval",
721+
"AWAITING APPROVAL",
722+
"workflow needs approval before CI can run",
723+
awaitingApprovalA.name,
724+
];
725+
const expectNotToContain = ["No Failures", ":green_heart:"];
726+
727+
expect(
728+
expectToContain.every((s) => failureInfoComment.includes(s!))
729+
).toBeTruthy();
730+
expect(
731+
expectNotToContain.every((s) => !failureInfoComment.includes(s!))
732+
).toBeTruthy();
733+
});
734+
735+
test("test awaiting approval with failures shows both", async () => {
736+
const failureInfoComment = constructResultsCommentHelper({
737+
pending: 1,
738+
failedJobs: [failedA],
739+
awaitingApprovalJobs: [awaitingApprovalA],
740+
});
741+
742+
const expectToContain = [
743+
"1 Awaiting Approval",
744+
"1 New Failure",
745+
"1 Pending",
746+
"AWAITING APPROVAL",
747+
awaitingApprovalA.name,
748+
failedA.name,
749+
];
750+
751+
expect(
752+
expectToContain.every((s) => failureInfoComment.includes(s!))
753+
).toBeTruthy();
754+
});
755+
695756
test("test merge base time shows up in results comment", async () => {
696757
const failureInfoComment = constructResultsCommentHelper({
697758
sha: "sha",

0 commit comments

Comments
 (0)