refactor: pass checkoutPath as param and fix docs for relative path semantics

Author: sam-robson

lib/analyze-action.js

@@ -4,7 +4,6 @@ import * as path from "path";
 import test from "ava";
 import * as sinon from "sinon";
 
-import * as actionsUtil from "./actions-util";
 import { CodeQuality, CodeScanning, RiskAssessment } from "./analyses";
 import {
   runQueries,
@@ -162,17 +161,16 @@ test("addSarifExtension", (t) => {
 });
 
 test("diffRangeExtensionPackContents", (t) => {
-  sinon
-    .stub(actionsUtil, "getRequiredInput")
-    .withArgs("checkout_path")
-    .returns("/checkout/path");
-  const output = diffRangeExtensionPackContents([
-    {
-      path: "main.js",
-      startLine: 10,
-      endLine: 20,
-    },
-  ]);
+  const output = diffRangeExtensionPackContents(
+    [
+      {
+        path: "main.js",
+        startLine: 10,
+        endLine: 20,
+      },
+    ],
+    "/checkout/path",
+  );
 
   const expected = fs.readFileSync(
     `${__dirname}/../src/testdata/pr-diff-range.yml`,

src/analyze.test.ts

@@ -265,6 +265,7 @@ export async function setupDiffInformedQueryRun(
 
 export function diffRangeExtensionPackContents(
   ranges: DiffThunkRange[],
+  checkoutPath: string,
 ): string {
   const header = `
 extensions:
@@ -281,7 +282,7 @@ extensions:
       // uses forward slashes as the path separator, so on Windows we need to
       // replace any backslashes with forward slashes.
       const filename = path
-        .join(getRequiredInput("checkout_path"), range.path)
+        .join(checkoutPath, range.path)
         .replaceAll(path.sep, "/");
 
       // Using yaml.dump() with `forceQuotes: true` ensures that all special
@@ -350,7 +351,10 @@ dataExtensions:
 `,
   );
 
-  const extensionContents = diffRangeExtensionPackContents(ranges);
+  const extensionContents = diffRangeExtensionPackContents(
+    ranges,
+    getRequiredInput("checkout_path"),
+  );
   const extensionFilePath = path.join(diffRangeDir, "pr-diff-range.yml");
   fs.writeFileSync(extensionFilePath, extensionContents);
   logger.debug(

src/analyze.ts

@@ -71,6 +71,7 @@ export async function getDiffInformedAnalysisBranches(
 }
 
 export interface DiffThunkRange {
+  /** Relative path from the repository root, using forward slashes as separators. */
   path: string;
   startLine: number;
   endLine: number;
@@ -112,8 +113,9 @@ export function readDiffRangesJsonFile(
  *
  * @param branches The base and head branches of the pull request.
  * @param logger
- * @returns An array of tuples, where each tuple contains the absolute path of a
- * file, the start line and the end line (both 1-based and inclusive) of an
+ * @returns An array of tuples, where each tuple contains the relative path of a
+ * file (relative to the repository root, as returned by the GitHub compare API),
+ * the start line and the end line (both 1-based and inclusive) of an
  * added or modified range in that file. Returns `undefined` if the action was
  * not triggered by a pull request or if there was an error.
  */

src/diff-informed-analysis-utils.ts

@@ -1090,21 +1090,24 @@ function runFilterAlertsByDiffRange(
   return uploadLib.filterAlertsByDiffRange(getRunnerLogger(true), input);
 }
 
-test("filterAlertsByDiffRange filters out alerts outside diff-range", (t) => {
-  const input = sarif.readSarifFile(
-    `${__dirname}/../src/testdata/valid-sarif.sarif`,
-  );
-  const actualOutput = runFilterAlertsByDiffRange(input, [
-    {
-      path: "main.js",
-      startLine: 1,
-      endLine: 3,
-    },
-  ]);
+test.serial(
+  "filterAlertsByDiffRange filters out alerts outside diff-range",
+  (t) => {
+    const input = sarif.readSarifFile(
+      `${__dirname}/../src/testdata/valid-sarif.sarif`,
+    );
+    const actualOutput = runFilterAlertsByDiffRange(input, [
+      {
+        path: "main.js",
+        startLine: 1,
+        endLine: 3,
+      },
+    ]);
 
-  const expectedOutput = sarif.readSarifFile(
-    `${__dirname}/../src/testdata/valid-sarif-diff-filtered.sarif`,
-  );
+    const expectedOutput = sarif.readSarifFile(
+      `${__dirname}/../src/testdata/valid-sarif-diff-filtered.sarif`,
+    );
 
-  t.deepEqual(actualOutput, expectedOutput);
-});
+    t.deepEqual(actualOutput, expectedOutput);
+  },
+);