Merge pull request #4122 from github/henrymercer/friendly-potato

Don't record an overlay status when the job was cancelled

Author: henrymercer

init/action.yml

@@ -164,6 +164,13 @@ inputs:
       [Internal] The ID of the check run, as provided by the Actions runtime environment. Do not set this value manually.
     default: ${{ job.check_run_id }}
     required: false
+  job-status:
+    description: >-
+      [Internal] The status of the job, as provided by the Actions runtime environment. This is how the
+      post step learns whether the job as a whole succeeded, failed, or was cancelled. Do not set this
+      value manually.
+    default: ${{ job.status }}
+    required: false
 outputs:
   codeql-path:
     description: The path of the CodeQL binary used for analysis

lib/entry-points.js

@@ -15,6 +15,7 @@ import { getRunnerLogger } from "./logging";
 import { OverlayDatabaseMode } from "./overlay/overlay-database-mode";
 import * as overlayStatus from "./overlay/status";
 import { parseRepositoryNwo } from "./repository";
+import { JobStatus } from "./status-report";
 import {
   createFeatures,
   createTestConfig,
@@ -58,6 +59,7 @@ test.serial("init-post action with debug mode off", async (t) => {
       createTestConfig({ debugMode: false }),
       parseRepositoryNwo("github/codeql-action"),
       createFeatures([]),
+      "success",
       getRunnerLogger(true),
     );
 
@@ -80,6 +82,7 @@ test.serial("init-post action with debug mode on", async (t) => {
       createTestConfig({ debugMode: true }),
       parseRepositoryNwo("github/codeql-action"),
       createFeatures([]),
+      "success",
       getRunnerLogger(true),
     );
 
@@ -375,6 +378,7 @@ test.serial(
         }),
         parseRepositoryNwo("github/codeql-action"),
         createFeatures([Feature.OverlayAnalysisStatusSave]),
+        "success",
         getRunnerLogger(true),
       );
 
@@ -443,6 +447,7 @@ test.serial(
         }),
         parseRepositoryNwo("github/codeql-action"),
         createFeatures([]),
+        "success",
         getRunnerLogger(true),
       );
 
@@ -480,6 +485,7 @@ test.serial("does not save overlay status when build successful", async (t) => {
       }),
       parseRepositoryNwo("github/codeql-action"),
       createFeatures([Feature.OverlayAnalysisStatusSave]),
+      "success",
       getRunnerLogger(true),
     );
 
@@ -517,6 +523,7 @@ test.serial(
         }),
         parseRepositoryNwo("github/codeql-action"),
         createFeatures([]),
+        "success",
         getRunnerLogger(true),
       );
 
@@ -528,6 +535,94 @@ test.serial(
   },
 );
 
+/**
+ * Runs `uploadFailureInfo` for an overlay-base job that did not complete successfully, for a job
+ * that the Actions runtime environment reports as cancelled.
+ */
+async function testCancelledOverlayJob({
+  jobStatus = "cancelled",
+  codeQlReportedError = false,
+}: {
+  jobStatus?: string;
+  codeQlReportedError?: boolean;
+} = {}) {
+  return await util.withTmpDir(async (tmpDir) => {
+    setupActionsVars(tmpDir, tmpDir);
+    delete process.env[EnvVar.ANALYZE_DID_COMPLETE_SUCCESSFULLY];
+    if (codeQlReportedError) {
+      process.env[EnvVar.JOB_STATUS] = JobStatus.FailureStatus;
+    } else {
+      delete process.env[EnvVar.JOB_STATUS];
+    }
+
+    sinon.stub(util, "checkDiskUsage").resolves({
+      numAvailableBytes: 100 * NUM_BYTES_PER_GIB,
+      numTotalBytes: 200 * NUM_BYTES_PER_GIB,
+    });
+
+    const saveOverlayStatusStub = sinon
+      .stub(overlayStatus, "saveOverlayStatus")
+      .resolves(true);
+
+    await initActionPostHelper.uploadFailureInfo(
+      sinon.spy(),
+      sinon.spy(),
+      codeql.createStubCodeQL({}),
+      createTestConfig({
+        debugMode: false,
+        languages: ["javascript"],
+        overlayDatabaseMode: OverlayDatabaseMode.OverlayBase,
+      }),
+      parseRepositoryNwo("github/codeql-action"),
+      createFeatures([Feature.OverlayAnalysisStatusSave]),
+      jobStatus,
+      getRunnerLogger(true),
+    );
+
+    return { saveOverlayStatusStub };
+  });
+}
+
+test.serial(
+  "does not save overlay status when the job was cancelled",
+  async (t) => {
+    const { saveOverlayStatusStub } = await testCancelledOverlayJob();
+
+    t.true(
+      saveOverlayStatusStub.notCalled,
+      "a cancellation tells us nothing about whether the analysis would have succeeded",
+    );
+  },
+);
+
+test.serial(
+  "saves overlay status when the job failed rather than being cancelled",
+  async (t) => {
+    const { saveOverlayStatusStub } = await testCancelledOverlayJob({
+      jobStatus: "failure",
+    });
+
+    t.true(
+      saveOverlayStatusStub.calledOnce,
+      "only cancellations are treated as unrelated to the analysis",
+    );
+  },
+);
+
+test.serial(
+  "saves overlay status when a CodeQL Action reported an error before the run was cancelled",
+  async (t) => {
+    const { saveOverlayStatusStub } = await testCancelledOverlayJob({
+      codeQlReportedError: true,
+    });
+
+    t.true(
+      saveOverlayStatusStub.calledOnce,
+      "the analysis genuinely failed, even though the run was later cancelled",
+    );
+  },
+);
+
 function createTestWorkflow(
   steps: workflow.WorkflowJobStep[],
 ): workflow.Workflow {

src/init-action-post-helper.test.ts

@@ -316,6 +316,7 @@ export async function tryUploadSarifIfRunFailed(
  * @param config The CodeQL Action configuration.
  * @param repositoryNwo The name and owner of the repository.
  * @param features Information about enabled features.
+ * @param jobStatus The status of the job, as reported by the Actions runtime environment.
  * @param logger The logger to use.
  * @returns The results of uploading the SARIF file for the failure.
  */
@@ -331,9 +332,10 @@ export async function uploadFailureInfo(
   config: Config,
   repositoryNwo: RepositoryNwo,
   features: FeatureEnablement,
+  jobStatus: string | undefined,
   logger: Logger,
 ): Promise<UploadFailedSarifResult> {
-  await recordOverlayStatus(codeql, config, features, logger);
+  await recordOverlayStatus(codeql, config, features, jobStatus, logger);
 
   const uploadFailedSarifResult = await tryUploadSarifIfRunFailed(
     config,
@@ -412,6 +414,21 @@ export async function uploadFailureInfo(
   return uploadFailedSarifResult;
 }
 
+/**
+ * Whether one of the CodeQL Actions reported an error for this job, which means the analysis
+ * genuinely failed.
+ *
+ * Note that the converse does not hold: an Action that is terminated abruptly, or that fails before
+ * it can gather telemetry, does not get to report anything.
+ */
+function didCodeQlReportError(): boolean {
+  const jobStatus = process.env[EnvVar.JOB_STATUS];
+  return (
+    jobStatus === JobStatus.FailureStatus ||
+    jobStatus === JobStatus.ConfigErrorStatus
+  );
+}
+
 /**
  * If overlay base database creation was attempted but the analysis did not complete
  * successfully, save the failure status to the Actions cache so that subsequent runs
@@ -421,6 +438,7 @@ async function recordOverlayStatus(
   codeql: CodeQL,
   config: Config,
   features: FeatureEnablement,
+  jobStatus: string | undefined,
   logger: Logger,
 ) {
   if (
@@ -431,6 +449,20 @@ async function recordOverlayStatus(
     return;
   }
 
+  // A cancelled run tells us nothing about whether the analysis would have succeeded, so recording
+  // a failure would disable overlay analysis needlessly. Note that we still record a failure if one
+  // of our own Actions reported an error before the run was cancelled.
+  if (
+    jobStatus?.trim().toLowerCase() === "cancelled" &&
+    !didCodeQlReportError()
+  ) {
+    logger.info(
+      "Not recording an improved incremental analysis failure for this job because the workflow " +
+        "run was cancelled.",
+    );
+    return;
+  }
+
   const checkRunIdInput = actionsUtil.getOptionalInput("check-run-id");
   const checkRunId =
     checkRunIdInput !== undefined ? parseInt(checkRunIdInput, 10) : undefined;

src/init-action-post-helper.ts

@@ -8,6 +8,7 @@ import * as core from "@actions/core";
 
 import {
   restoreInputs,
+  getOptionalInput,
   getTemporaryDirectory,
   printDebugLogs,
 } from "./actions-util";
@@ -55,6 +56,11 @@ async function run(startedAt: Date) {
     | undefined;
   let dependencyCachingUsage: DependencyCachingUsageReport | undefined;
   try {
+    // Read the job status before restoring inputs, since it is provided by the Actions runtime
+    // environment for this step and would otherwise be overwritten by the value that the `init`
+    // Action saw, which is always a success.
+    const jobStatus = getOptionalInput("job-status");
+
     // Restore inputs from `init` Action.
     restoreInputs();
 
@@ -84,6 +90,7 @@ async function run(startedAt: Date) {
         config,
         repositoryNwo,
         features,
+        jobStatus,
         logger,
       );