Merge pull request #3166 from github/mbg/upload-sarif/add-tests

Add tests for `upload-sarif`

.github/workflows/__upload-quality-sarif.yml

@@ -89,7 +89,7 @@ jobs:
           ref: refs/heads/main
           sha: 5e235361806c361d4d3f8859e3c897658025a9a2
       - name: Check output from `upload-sarif` step
-        if: fromJSON(steps.upload-sarif.outputs.sarif-ids)[0].analysis != 'code-quality'
+        if: '!(fromJSON(steps.upload-sarif.outputs.sarif-ids).code-quality)'
         run: exit 1
     env:
       CODEQL_ACTION_TEST_MODE: true

lib/upload-sarif-action.js

@@ -84816,7 +84816,6 @@ var require_sarif_schema_2_1_0 = __commonJS({
 });
 
 // src/upload-sarif-action.ts
-var fs15 = __toESM(require("fs"));
 var core13 = __toESM(require_core());
 
 // src/actions-util.ts
@@ -93416,7 +93415,8 @@ function filterAlertsByDiffRange(logger, sarif) {
   return sarif;
 }
 
-// src/upload-sarif-action.ts
+// src/upload-sarif.ts
+var fs15 = __toESM(require("fs"));
 async function findAndUpload(logger, features, sarifPath, pathStats, checkoutPath, analysis, category) {
   let sarifFiles;
   if (pathStats.isDirectory()) {
@@ -93441,6 +93441,40 @@ async function findAndUpload(logger, features, sarifPath, pathStats, checkoutPat
   }
   return void 0;
 }
+async function uploadSarif(logger, features, checkoutPath, sarifPath, category) {
+  const pathStats = fs15.lstatSync(sarifPath, { throwIfNoEntry: false });
+  if (pathStats === void 0) {
+    throw new ConfigurationError(`Path does not exist: ${sarifPath}.`);
+  }
+  const uploadResults = {};
+  const uploadResult = await findAndUpload(
+    logger,
+    features,
+    sarifPath,
+    pathStats,
+    checkoutPath,
+    CodeScanning,
+    category
+  );
+  if (uploadResult !== void 0) {
+    uploadResults["code-scanning" /* CodeScanning */] = uploadResult;
+  }
+  const qualityUploadResult = await findAndUpload(
+    logger,
+    features,
+    sarifPath,
+    pathStats,
+    checkoutPath,
+    CodeQuality,
+    fixCodeQualityCategory(logger, category)
+  );
+  if (qualityUploadResult !== void 0) {
+    uploadResults["code-quality" /* CodeQuality */] = qualityUploadResult;
+  }
+  return uploadResults;
+}
+
+// src/upload-sarif-action.ts
 async function sendSuccessStatusReport(startedAt, uploadStats, logger) {
   const statusReportBase = await createStatusReportBase(
     "upload-sarif" /* UploadSarif */,
@@ -93487,57 +93521,37 @@ async function run() {
     const sarifPath = getRequiredInput("sarif_file");
     const checkoutPath = getRequiredInput("checkout_path");
     const category = getOptionalInput("category");
-    const pathStats = fs15.lstatSync(sarifPath, { throwIfNoEntry: false });
-    if (pathStats === void 0) {
-      throw new ConfigurationError(`Path does not exist: ${sarifPath}.`);
-    }
-    const sarifIds = [];
-    const uploadResult = await findAndUpload(
+    const uploadResults = await uploadSarif(
       logger,
       features,
-      sarifPath,
-      pathStats,
       checkoutPath,
-      CodeScanning,
+      sarifPath,
       category
     );
-    if (uploadResult !== void 0) {
-      core13.setOutput("sarif-id", uploadResult.sarifID);
-      sarifIds.push({
-        analysis: "code-scanning" /* CodeScanning */,
-        id: uploadResult.sarifID
-      });
+    if (Object.keys(uploadResults).length === 0) {
+      throw new ConfigurationError(
+        `No SARIF files found to upload in "${sarifPath}".`
+      );
     }
-    const qualityUploadResult = await findAndUpload(
-      logger,
-      features,
-      sarifPath,
-      pathStats,
-      checkoutPath,
-      CodeQuality,
-      fixCodeQualityCategory(logger, category)
-    );
-    if (qualityUploadResult !== void 0) {
-      sarifIds.push({
-        analysis: "code-quality" /* CodeQuality */,
-        id: qualityUploadResult.sarifID
-      });
+    const codeScanningResult = uploadResults["code-scanning" /* CodeScanning */];
+    if (codeScanningResult !== void 0) {
+      core13.setOutput("sarif-id", codeScanningResult.sarifID);
     }
-    core13.setOutput("sarif-ids", JSON.stringify(sarifIds));
+    core13.setOutput("sarif-ids", JSON.stringify(uploadResults));
     if (isInTestMode()) {
       core13.debug("In test mode. Waiting for processing is disabled.");
     } else if (getRequiredInput("wait-for-processing") === "true") {
-      if (uploadResult !== void 0) {
+      if (codeScanningResult !== void 0) {
         await waitForProcessing(
           getRepositoryNwo(),
-          uploadResult.sarifID,
+          codeScanningResult.sarifID,
           logger
         );
       }
     }
     await sendSuccessStatusReport(
       startedAt,
-      uploadResult?.statusReport || {},
+      codeScanningResult?.statusReport || {},
       logger
     );
   } catch (unwrappedError) {

pr-checks/checks/upload-quality-sarif.yml

@@ -22,5 +22,5 @@ steps:
       ref: 'refs/heads/main'
       sha: '5e235361806c361d4d3f8859e3c897658025a9a2'
   - name: "Check output from `upload-sarif` step"
-    if: fromJSON(steps.upload-sarif.outputs.sarif-ids)[0].analysis != 'code-quality'
+    if: '!(fromJSON(steps.upload-sarif.outputs.sarif-ids).code-quality)'
     run: exit 1

src/upload-sarif-action.ts

@@ -1,5 +1,3 @@
-import * as fs from "fs";
-
 import * as core from "@actions/core";
 
 import * as actionsUtil from "./actions-util";
@@ -18,6 +16,7 @@ import {
   isThirdPartyAnalysis,
 } from "./status-report";
 import * as upload_lib from "./upload-lib";
+import { uploadSarif } from "./upload-sarif";
 import {
   ConfigurationError,
   checkActionVersion,
@@ -32,60 +31,6 @@ interface UploadSarifStatusReport
   extends StatusReportBase,
     upload_lib.UploadStatusReport {}
 
-/**
- * Searches for SARIF files for the given `analysis` in the given `sarifPath`.
- * If any are found, then they are uploaded to the appropriate endpoint for the given `analysis`.
- *
- * @param logger The logger to use.
- * @param features Information about FFs.
- * @param sarifPath The path to a SARIF file or directory containing SARIF files.
- * @param pathStats Information about `sarifPath`.
- * @param checkoutPath The checkout path.
- * @param analysis The configuration of the analysis we should upload SARIF files for.
- * @param category The SARIF category to use for the upload.
- * @returns The result of uploading the SARIF file(s) or `undefined` if there are none.
- */
-async function findAndUpload(
-  logger: Logger,
-  features: Features,
-  sarifPath: string,
-  pathStats: fs.Stats,
-  checkoutPath: string,
-  analysis: analyses.AnalysisConfig,
-  category?: string,
-): Promise<upload_lib.UploadResult | undefined> {
-  let sarifFiles: string[] | undefined;
-
-  if (pathStats.isDirectory()) {
-    sarifFiles = upload_lib.findSarifFilesInDir(
-      sarifPath,
-      analysis.sarifPredicate,
-    );
-  } else if (
-    pathStats.isFile() &&
-    (analysis.sarifPredicate(sarifPath) ||
-      (analysis.kind === analyses.AnalysisKind.CodeScanning &&
-        !analyses.CodeQuality.sarifPredicate(sarifPath)))
-  ) {
-    sarifFiles = [sarifPath];
-  } else {
-    return undefined;
-  }
-
-  if (sarifFiles.length !== 0) {
-    return await upload_lib.uploadSpecifiedFiles(
-      sarifFiles,
-      checkoutPath,
-      category,
-      features,
-      logger,
-      analysis,
-    );
-  }
-
-  return undefined;
-}
-
 async function sendSuccessStatusReport(
   startedAt: Date,
   uploadStats: upload_lib.UploadStatusReport,
@@ -144,56 +89,37 @@ async function run() {
     const sarifPath = actionsUtil.getRequiredInput("sarif_file");
     const checkoutPath = actionsUtil.getRequiredInput("checkout_path");
     const category = actionsUtil.getOptionalInput("category");
-    const pathStats = fs.lstatSync(sarifPath, { throwIfNoEntry: false });
 
-    if (pathStats === undefined) {
-      throw new ConfigurationError(`Path does not exist: ${sarifPath}.`);
-    }
-
-    const sarifIds: Array<{ analysis: string; id: string }> = [];
-    const uploadResult = await findAndUpload(
+    const uploadResults = await uploadSarif(
       logger,
       features,
-      sarifPath,
-      pathStats,
       checkoutPath,
-      analyses.CodeScanning,
+      sarifPath,
       category,
     );
-    if (uploadResult !== undefined) {
-      core.setOutput("sarif-id", uploadResult.sarifID);
-      sarifIds.push({
-        analysis: analyses.AnalysisKind.CodeScanning,
-        id: uploadResult.sarifID,
-      });
+
+    // Fail if we didn't upload anything.
+    if (Object.keys(uploadResults).length === 0) {
+      throw new ConfigurationError(
+        `No SARIF files found to upload in "${sarifPath}".`,
+      );
     }
 
-    // If there are `.quality.sarif` files in `sarifPath`, then upload those to the code quality service.
-    const qualityUploadResult = await findAndUpload(
-      logger,
-      features,
-      sarifPath,
-      pathStats,
-      checkoutPath,
-      analyses.CodeQuality,
-      actionsUtil.fixCodeQualityCategory(logger, category),
-    );
-    if (qualityUploadResult !== undefined) {
-      sarifIds.push({
-        analysis: analyses.AnalysisKind.CodeQuality,
-        id: qualityUploadResult.sarifID,
-      });
+    const codeScanningResult =
+      uploadResults[analyses.AnalysisKind.CodeScanning];
+    if (codeScanningResult !== undefined) {
+      core.setOutput("sarif-id", codeScanningResult.sarifID);
     }
-    core.setOutput("sarif-ids", JSON.stringify(sarifIds));
+    core.setOutput("sarif-ids", JSON.stringify(uploadResults));
 
     // We don't upload results in test mode, so don't wait for processing
     if (isInTestMode()) {
       core.debug("In test mode. Waiting for processing is disabled.");
     } else if (actionsUtil.getRequiredInput("wait-for-processing") === "true") {
-      if (uploadResult !== undefined) {
+      if (codeScanningResult !== undefined) {
         await upload_lib.waitForProcessing(
           getRepositoryNwo(),
-          uploadResult.sarifID,
+          codeScanningResult.sarifID,
           logger,
         );
       }
@@ -202,7 +128,7 @@ async function run() {
     }
     await sendSuccessStatusReport(
       startedAt,
-      uploadResult?.statusReport || {},
+      codeScanningResult?.statusReport || {},
       logger,
     );
   } catch (unwrappedError) {

src/upload-sarif.test.ts

@@ -0,0 +1,262 @@
+import * as fs from "fs";
+import * as path from "path";
+
+import test, { ExecutionContext } from "ava";
+import * as sinon from "sinon";
+
+import {
+  AnalysisConfig,
+  AnalysisKind,
+  CodeQuality,
+  CodeScanning,
+} from "./analyses";
+import { getRunnerLogger } from "./logging";
+import { createFeatures, setupTests } from "./testing-utils";
+import { UploadResult } from "./upload-lib";
+import * as uploadLib from "./upload-lib";
+import { findAndUpload, uploadSarif } from "./upload-sarif";
+import * as util from "./util";
+
+setupTests(test);
+
+const findAndUploadMacro = test.macro({
+  exec: async (
+    t: ExecutionContext<unknown>,
+    sarifFiles: string[],
+    analysis: AnalysisConfig,
+    sarifPath: (tempDir: string) => string = (tempDir) => tempDir,
+    expectedResult: UploadResult | undefined,
+  ) => {
+    await util.withTmpDir(async (tempDir) => {
+      sinon.stub(uploadLib, "uploadSpecifiedFiles").resolves(expectedResult);
+      const logger = getRunnerLogger(true);
+      const features = createFeatures([]);
+
+      for (const sarifFile of sarifFiles) {
+        fs.writeFileSync(path.join(tempDir, sarifFile), "");
+      }
+
+      const stats = fs.statSync(sarifPath(tempDir));
+      const actual = await findAndUpload(
+        logger,
+        features,
+        sarifPath(tempDir),
+        stats,
+        "",
+        analysis,
+      );
+
+      t.deepEqual(actual, expectedResult);
+    });
+  },
+  title: (providedTitle = "") => `findAndUpload - ${providedTitle}`,
+});
+
+test(
+  "no matching files",
+  findAndUploadMacro,
+  ["test.json"],
+  CodeScanning,
+  undefined,
+  undefined,
+);
+
+test(
+  "matching files for Code Scanning with directory path",
+  findAndUploadMacro,
+  ["test.sarif"],
+  CodeScanning,
+  undefined,
+  {
+    statusReport: {},
+    sarifID: "some-id",
+  },
+);
+
+test(
+  "matching files for Code Scanning with file path",
+  findAndUploadMacro,
+  ["test.sarif"],
+  CodeScanning,
+  (tempDir) => path.join(tempDir, "test.sarif"),
+  {
+    statusReport: {},
+    sarifID: "some-id",
+  },
+);
+
+interface UploadSarifExpectedResult {
+  uploadResult?: UploadResult;
+  expectedFiles?: string[];
+}
+
+const uploadSarifMacro = test.macro({
+  exec: async (
+    t: ExecutionContext<unknown>,
+    sarifFiles: string[],
+    sarifPath: (tempDir: string) => string = (tempDir) => tempDir,
+    expectedResult: Partial<Record<AnalysisKind, UploadSarifExpectedResult>>,
+  ) => {
+    await util.withTmpDir(async (tempDir) => {
+      const logger = getRunnerLogger(true);
+      const testPath = sarifPath(tempDir);
+      const features = createFeatures([]);
+
+      const toFullPath = (filename: string) => path.join(tempDir, filename);
+
+      const uploadSpecifiedFiles = sinon.stub(
+        uploadLib,
+        "uploadSpecifiedFiles",
+      );
+
+      for (const analysisKind of Object.values(AnalysisKind)) {
+        uploadSpecifiedFiles
+          .withArgs(
+            sinon.match.any,
+            sinon.match.any,
+            sinon.match.any,
+            features,
+            logger,
+            analysisKind === AnalysisKind.CodeScanning
+              ? CodeScanning
+              : CodeQuality,
+          )
+          .resolves(expectedResult[analysisKind as AnalysisKind]?.uploadResult);
+      }
+
+      const fullSarifPaths = sarifFiles.map(toFullPath);
+      for (const sarifFile of fullSarifPaths) {
+        fs.writeFileSync(sarifFile, "");
+      }
+
+      const actual = await uploadSarif(logger, features, "", testPath);
+
+      for (const analysisKind of Object.values(AnalysisKind)) {
+        const analysisKindResult = expectedResult[analysisKind];
+        if (analysisKindResult) {
+          // We are expecting a result for this analysis kind, check that we have it.
+          t.deepEqual(actual[analysisKind], analysisKindResult.uploadResult);
+          // Additionally, check that the mocked `uploadSpecifiedFiles` was called with only the file paths
+          // that we expected it to be called with.
+          t.assert(
+            uploadSpecifiedFiles.calledWith(
+              analysisKindResult.expectedFiles?.map(toFullPath) ??
+                fullSarifPaths,
+              sinon.match.any,
+              sinon.match.any,
+              features,
+              logger,
+              analysisKind === AnalysisKind.CodeScanning
+                ? CodeScanning
+                : CodeQuality,
+            ),
+          );
+        } else {
+          // Otherwise, we are not expecting a result for this analysis kind. However, note that `undefined`
+          // is also returned by our mocked `uploadSpecifiedFiles` when there is no expected result for this
+          // analysis kind.
+          t.is(actual[analysisKind], undefined);
+          // Therefore, we also check that the mocked `uploadSpecifiedFiles` was not called for this analysis kind.
+          t.assert(
+            !uploadSpecifiedFiles.calledWith(
+              sinon.match.any,
+              sinon.match.any,
+              sinon.match.any,
+              features,
+              logger,
+              analysisKind === AnalysisKind.CodeScanning
+                ? CodeScanning
+                : CodeQuality,
+            ),
+            `uploadSpecifiedFiles was called for ${analysisKind}, but should not have been.`,
+          );
+        }
+      }
+    });
+  },
+  title: (providedTitle = "") => `uploadSarif - ${providedTitle}`,
+});
+
+test(
+  "SARIF file",
+  uploadSarifMacro,
+  ["test.sarif"],
+  (tempDir) => path.join(tempDir, "test.sarif"),
+  {
+    "code-scanning": {
+      uploadResult: {
+        statusReport: {},
+        sarifID: "code-scanning-sarif",
+      },
+    },
+  },
+);
+
+test(
+  "JSON file",
+  uploadSarifMacro,
+  ["test.json"],
+  (tempDir) => path.join(tempDir, "test.json"),
+  {
+    "code-scanning": {
+      uploadResult: {
+        statusReport: {},
+        sarifID: "code-scanning-sarif",
+      },
+    },
+  },
+);
+
+test(
+  "Code Scanning files",
+  uploadSarifMacro,
+  ["test.json", "test.sarif"],
+  undefined,
+  {
+    "code-scanning": {
+      uploadResult: {
+        statusReport: {},
+        sarifID: "code-scanning-sarif",
+      },
+      expectedFiles: ["test.sarif"],
+    },
+  },
+);
+
+test(
+  "Code Quality file",
+  uploadSarifMacro,
+  ["test.quality.sarif"],
+  (tempDir) => path.join(tempDir, "test.quality.sarif"),
+  {
+    "code-quality": {
+      uploadResult: {
+        statusReport: {},
+        sarifID: "code-quality-sarif",
+      },
+    },
+  },
+);
+
+test(
+  "Mixed files",
+  uploadSarifMacro,
+  ["test.sarif", "test.quality.sarif"],
+  undefined,
+  {
+    "code-scanning": {
+      uploadResult: {
+        statusReport: {},
+        sarifID: "code-scanning-sarif",
+      },
+      expectedFiles: ["test.sarif"],
+    },
+    "code-quality": {
+      uploadResult: {
+        statusReport: {},
+        sarifID: "code-quality-sarif",
+      },
+      expectedFiles: ["test.quality.sarif"],
+    },
+  },
+);

src/upload-sarif.ts

@@ -0,0 +1,122 @@
+import * as fs from "fs";
+
+import * as actionsUtil from "./actions-util";
+import * as analyses from "./analyses";
+import { FeatureEnablement } from "./feature-flags";
+import { Logger } from "./logging";
+import * as upload_lib from "./upload-lib";
+import { ConfigurationError } from "./util";
+
+/**
+ * Searches for SARIF files for the given `analysis` in the given `sarifPath`.
+ * If any are found, then they are uploaded to the appropriate endpoint for the given `analysis`.
+ *
+ * @param logger The logger to use.
+ * @param features Information about FFs.
+ * @param sarifPath The path to a SARIF file or directory containing SARIF files.
+ * @param pathStats Information about `sarifPath`.
+ * @param checkoutPath The checkout path.
+ * @param analysis The configuration of the analysis we should upload SARIF files for.
+ * @param category The SARIF category to use for the upload.
+ * @returns The result of uploading the SARIF file(s) or `undefined` if there are none.
+ */
+export async function findAndUpload(
+  logger: Logger,
+  features: FeatureEnablement,
+  sarifPath: string,
+  pathStats: fs.Stats,
+  checkoutPath: string,
+  analysis: analyses.AnalysisConfig,
+  category?: string,
+): Promise<upload_lib.UploadResult | undefined> {
+  let sarifFiles: string[] | undefined;
+
+  if (pathStats.isDirectory()) {
+    sarifFiles = upload_lib.findSarifFilesInDir(
+      sarifPath,
+      analysis.sarifPredicate,
+    );
+  } else if (
+    pathStats.isFile() &&
+    (analysis.sarifPredicate(sarifPath) ||
+      (analysis.kind === analyses.AnalysisKind.CodeScanning &&
+        !analyses.CodeQuality.sarifPredicate(sarifPath)))
+  ) {
+    sarifFiles = [sarifPath];
+  } else {
+    return undefined;
+  }
+
+  if (sarifFiles.length !== 0) {
+    return await upload_lib.uploadSpecifiedFiles(
+      sarifFiles,
+      checkoutPath,
+      category,
+      features,
+      logger,
+      analysis,
+    );
+  }
+
+  return undefined;
+}
+
+// Maps analysis kinds to SARIF IDs.
+export type UploadSarifResults = Partial<
+  Record<analyses.AnalysisKind, upload_lib.UploadResult>
+>;
+
+/**
+ * Finds SARIF files in `sarifPath` and uploads them to the appropriate services.
+ *
+ * @param logger The logger to use.
+ * @param features Information about enabled features.
+ * @param checkoutPath The path where the repository was checked out at.
+ * @param sarifPath The path to the file or directory to upload.
+ * @param category The analysis category.
+ *
+ * @returns A partial mapping from analysis kinds to the upload results.
+ */
+export async function uploadSarif(
+  logger: Logger,
+  features: FeatureEnablement,
+  checkoutPath: string,
+  sarifPath: string,
+  category?: string,
+): Promise<UploadSarifResults> {
+  const pathStats = fs.lstatSync(sarifPath, { throwIfNoEntry: false });
+
+  if (pathStats === undefined) {
+    throw new ConfigurationError(`Path does not exist: ${sarifPath}.`);
+  }
+
+  const uploadResults: UploadSarifResults = {};
+  const uploadResult = await findAndUpload(
+    logger,
+    features,
+    sarifPath,
+    pathStats,
+    checkoutPath,
+    analyses.CodeScanning,
+    category,
+  );
+  if (uploadResult !== undefined) {
+    uploadResults[analyses.AnalysisKind.CodeScanning] = uploadResult;
+  }
+
+  // If there are `.quality.sarif` files in `sarifPath`, then upload those to the code quality service.
+  const qualityUploadResult = await findAndUpload(
+    logger,
+    features,
+    sarifPath,
+    pathStats,
+    checkoutPath,
+    analyses.CodeQuality,
+    actionsUtil.fixCodeQualityCategory(logger, category),
+  );
+  if (qualityUploadResult !== undefined) {
+    uploadResults[analyses.AnalysisKind.CodeQuality] = qualityUploadResult;
+  }
+
+  return uploadResults;
+}