fix: ignore fully skipped startup failures (#1170)

Author: panva

lib/pr_checker.js

@@ -467,43 +467,54 @@ export default class PRChecker {
       }
 
       if (!GITHUB_SUCCESS_CONCLUSIONS.includes(conclusion)) {
-        hasFailures = true;
+        const runs = checkRuns?.nodes ?? [];
+        const allRunsSkipped = runs.length > 0 &&
+          runs.length === checkRuns.totalCount &&
+          runs.every((checkRun) => checkRun.status === 'COMPLETED' &&
+            checkRun.conclusion === 'SKIPPED');
+
+        // GitHub can report a STARTUP_FAILURE for a workflow even though all
+        // of its conditional jobs were skipped. There is no failed check in
+        // that case, so the suite should not make the commit unlandable.
+        if (conclusion === 'STARTUP_FAILURE' && allRunsSkipped) {
+          continue;
+        }
 
-        // If we have detailed checkRuns, show specific failing jobs
-        if (checkRuns && checkRuns.nodes && checkRuns.nodes.length > 0) {
-          for (const checkRun of checkRuns.nodes) {
-            if (checkRun.status === 'COMPLETED' &&
-                !GITHUB_SUCCESS_CONCLUSIONS.includes(checkRun.conclusion)) {
-              if (checkRun.conclusion === 'CANCELLED') {
-                cancelledJobs.push({
-                  name: checkRun.name,
-                  conclusion: checkRun.conclusion,
-                  url: checkRun.detailsUrl
-                });
-              } else {
-                failedJobs.push({
-                  name: checkRun.name,
-                  conclusion: checkRun.conclusion,
-                  url: checkRun.detailsUrl
-                });
-              }
+        hasFailures = true;
+        let reportedFailure = false;
+
+        // If we have detailed checkRuns, show specific failing jobs.
+        for (const checkRun of runs) {
+          if (checkRun.status === 'COMPLETED' &&
+              !GITHUB_SUCCESS_CONCLUSIONS.includes(checkRun.conclusion)) {
+            reportedFailure = true;
+            if (checkRun.conclusion === 'CANCELLED') {
+              cancelledJobs.push({
+                name: checkRun.name,
+                conclusion: checkRun.conclusion,
+                url: checkRun.detailsUrl
+              });
+            } else {
+              failedJobs.push({
+                name: checkRun.name,
+                conclusion: checkRun.conclusion,
+                url: checkRun.detailsUrl
+              });
             }
           }
-        } else {
-          // Fallback to check suite level information if no checkRuns
-          if (conclusion === 'CANCELLED') {
-            cancelledJobs.push({
-              name: GITHUB_ACTIONS_APP,
-              conclusion,
-              url: null
-            });
-          } else {
-            failedJobs.push({
-              name: GITHUB_ACTIONS_APP,
-              conclusion,
-              url: null
-            });
-          }
+        }
+
+        // Fall back to the suite when its failed conclusion is not reflected
+        // by an individual check run.
+        if (!reportedFailure) {
+          const failures = conclusion === 'CANCELLED'
+            ? cancelledJobs
+            : failedJobs;
+          failures.push({
+            name: GITHUB_ACTIONS_APP,
+            conclusion,
+            url: null
+          });
         }
       }
     }

lib/queries/PR.gql

@@ -38,6 +38,7 @@ query PR($prid: Int!, $owner: String!, $repo: String!) {
                 conclusion,
                 status,
                 checkRuns(first: 40) {
+                  totalCount
                   nodes {
                     name
                     status

test/unit/graphql_queries.test.js

@@ -20,6 +20,7 @@ describe('GraphQL queries', () => {
       headCommitQuery,
       /checkSuites\(first: 100, filterBy: \{ appId: 15368 \}\)/);
     assert.match(headCommitQuery, /checkRuns\(first: 40\)/);
+    assert.match(headCommitQuery, /checkRuns\(first: 40\) \{\s+totalCount/);
     assert.match(headCommitQuery, /status \{\s+state\s+\}/);
     assert.doesNotMatch(headCommitQuery, /\bapp\s*\{/);
     assert.doesNotMatch(commitsQuery, /checkSuites/);

test/unit/pr_checker.test.js

@@ -1860,6 +1860,88 @@ describe('PRChecker', () => {
       cli.assertCalledWith(expectedLogs);
     });
 
+    it('should ignore startup failures with only skipped check runs',
+      async() => {
+        const cli = new TestCLI();
+
+        const expectedLogs = {
+          ok: [
+            ['Last GitHub CI successful']
+          ]
+        };
+
+        const commits = [{
+          commit: {
+            checkSuites: {
+              nodes: [{
+                status: 'COMPLETED',
+                conclusion: 'STARTUP_FAILURE',
+                checkRuns: {
+                  totalCount: 2,
+                  nodes: [
+                    {
+                      name: 'stale-comment',
+                      status: 'COMPLETED',
+                      conclusion: 'SKIPPED'
+                    },
+                    {
+                      name: 'notable-change',
+                      status: 'COMPLETED',
+                      conclusion: 'SKIPPED'
+                    }
+                  ]
+                }
+              }]
+            }
+          }
+        }];
+        const data = Object.assign({}, baseData, { commits });
+
+        const checker = new PRChecker(cli, data, {}, testArgv);
+
+        const status = await checker.checkCI();
+        assert(status);
+        cli.assertCalledWith(expectedLogs);
+      });
+
+    it('should report a suite failure when check runs are incomplete',
+      async() => {
+        const cli = new TestCLI();
+
+        const expectedLogs = {
+          error: [
+            ['1 GitHub CI job(s) failed:'],
+            ['  - github-actions: STARTUP_FAILURE']
+          ]
+        };
+
+        const commits = [{
+          commit: {
+            checkSuites: {
+              nodes: [{
+                status: 'COMPLETED',
+                conclusion: 'STARTUP_FAILURE',
+                checkRuns: {
+                  totalCount: 2,
+                  nodes: [{
+                    name: 'stale-comment',
+                    status: 'COMPLETED',
+                    conclusion: 'SKIPPED'
+                  }]
+                }
+              }]
+            }
+          }
+        }];
+        const data = Object.assign({}, baseData, { commits });
+
+        const checker = new PRChecker(cli, data, {}, testArgv);
+
+        const status = await checker.checkCI();
+        assert(!status);
+        cli.assertCalledWith(expectedLogs);
+      });
+
     it('should succeed if commit status succeeded', async() => {
       const cli = new TestCLI();