Address review

2026-04-16 11:43:18 +00:00 · 2025-08-07 15:43:56 +02:00
parent 1cfc0c2621
commit bb56324516
12 changed files with 155 additions and 147 deletions
--- a/src/actions-util.ts
+++ b/src/actions-util.ts
@@ -264,7 +264,7 @@ export class CommandInvocationError extends Error {
    public args: string[],
    public exitCode: number | undefined,
    public stderr: string,
-    public stdout: string,
+    public stdout: string = "",
  ) {
    const prettyCommand = prettyPrintInvocation(cmd, args);
    const lastLine = ensureEndsInPeriod(
--- a/src/cli-errors.test.ts
+++ b/src/cli-errors.test.ts
@@ -1,4 +1,5 @@
 import test from "ava";
+import * as sinon from "sinon";

 import { CommandInvocationError } from "./actions-util";
 import {
@@ -11,19 +12,8 @@ import { ConfigurationError } from "./util";

 setupTests(test);

-// Helper function to create CommandInvocationError for testing
-function createCommandInvocationError(
-  cmd: string,
-  args: string[],
-  exitCode: number | undefined,
-  stderr: string,
-  stdout: string = "",
-): CommandInvocationError {
-  return new CommandInvocationError(cmd, args, exitCode, stderr, stdout);
-}
-
 test("CliError constructor with fatal errors", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "finalize"],
    32,
@@ -50,7 +40,7 @@ test("CliError constructor with fatal errors", (t) => {
 });

 test("CliError constructor with single fatal error", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "create"],
    1,
@@ -65,7 +55,7 @@ test("CliError constructor with single fatal error", (t) => {
 });

 test("CliError constructor with autobuild errors", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "create"],
    1,
@@ -88,7 +78,7 @@ test("CliError constructor with truncated autobuild errors", (t) => {
    { length: 12 },
    (_, i) => `[autobuild] [ERROR] Error ${i + 1}`,
  ).join("\n");
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "create"],
    1,
@@ -106,7 +96,7 @@ test("CliError constructor with truncated autobuild errors", (t) => {
 });

 test("CliError constructor with generic error", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["version"],
    1,
@@ -129,28 +119,22 @@ test("CliError constructor with generic error", (t) => {
 });

 test("CliError constructor with empty stderr", (t) => {
-  const commandError = createCommandInvocationError(
-    "codeql",
-    ["version"],
-    1,
-    "",
-  );
+  const commandError = new CommandInvocationError("codeql", ["version"], 1, "");

  const cliError = new CliError(commandError);

  t.true(cliError.message.includes("last log line was: n/a"));
 });

-test("wrapCliConfigurationError - unsupported platform", (t) => {
-  const originalPlatform = process.platform;
-  const originalArch = process.arch;
-
-  try {
-    // Mock unsupported platform/arch
-    Object.defineProperty(process, "platform", { value: "unsupported" });
-    Object.defineProperty(process, "arch", { value: "unsupported" });
-
-    const commandError = createCommandInvocationError(
+for (const [platform, arch] of [
+  ["weird_plat", "x64"],
+  ["linux", "arm64"],
+  ["win32", "arm64"],
+]) {
+  test(`wrapCliConfigurationError - ${platform}/${arch} unsupported`, (t) => {
+    sinon.stub(process, "platform").value(platform);
+    sinon.stub(process, "arch").value(arch);
+    const commandError = new CommandInvocationError(
      "codeql",
      ["version"],
      1,
@@ -166,16 +150,12 @@ test("wrapCliConfigurationError - unsupported platform", (t) => {
        "CodeQL CLI does not support the platform/architecture combination",
      ),
    );
-    t.true(wrappedError.message.includes("unsupported/unsupported"));
-  } finally {
-    // Restore original values
-    Object.defineProperty(process, "platform", { value: originalPlatform });
-    Object.defineProperty(process, "arch", { value: originalArch });
-  }
-});
+    t.true(wrappedError.message.includes(`${platform}/${arch}`));
+  });
+}

 test("wrapCliConfigurationError - supported platform", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["version"],
    1,
@@ -190,7 +170,7 @@ test("wrapCliConfigurationError - supported platform", (t) => {
 });

 test("wrapCliConfigurationError - autobuild error", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "create"],
    1,
@@ -209,7 +189,7 @@ test("wrapCliConfigurationError - autobuild error", (t) => {
 });

 test("wrapCliConfigurationError - init called twice", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "create"],
    1,
@@ -228,7 +208,7 @@ test("wrapCliConfigurationError - init called twice", (t) => {
 });

 test("wrapCliConfigurationError - no source code seen by exit code", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "finalize"],
    32,
@@ -242,7 +222,7 @@ test("wrapCliConfigurationError - no source code seen by exit code", (t) => {
 });

 test("wrapCliConfigurationError - no source code seen by message", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "finalize"],
    1,
@@ -256,7 +236,7 @@ test("wrapCliConfigurationError - no source code seen by message", (t) => {
 });

 test("wrapCliConfigurationError - out of memory error with additional message", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "analyze"],
    1,
@@ -275,7 +255,7 @@ test("wrapCliConfigurationError - out of memory error with additional message",
 });

 test("wrapCliConfigurationError - gradle build failed", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "create"],
    1,
@@ -289,7 +269,7 @@ test("wrapCliConfigurationError - gradle build failed", (t) => {
 });

 test("wrapCliConfigurationError - maven build failed", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "create"],
    1,
@@ -303,7 +283,7 @@ test("wrapCliConfigurationError - maven build failed", (t) => {
 });

 test("wrapCliConfigurationError - swift build failed", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "create"],
    1,
@@ -317,7 +297,7 @@ test("wrapCliConfigurationError - swift build failed", (t) => {
 });

 test("wrapCliConfigurationError - pack cannot be found", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["pack", "install"],
    1,
@@ -331,7 +311,7 @@ test("wrapCliConfigurationError - pack cannot be found", (t) => {
 });

 test("wrapCliConfigurationError - pack missing auth", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["pack", "download"],
    1,
@@ -345,7 +325,7 @@ test("wrapCliConfigurationError - pack missing auth", (t) => {
 });

 test("wrapCliConfigurationError - invalid config file", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["database", "create"],
    1,
@@ -359,7 +339,7 @@ test("wrapCliConfigurationError - invalid config file", (t) => {
 });

 test("wrapCliConfigurationError - incompatible CLI version", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["version"],
    1,
@@ -373,7 +353,7 @@ test("wrapCliConfigurationError - incompatible CLI version", (t) => {
 });

 test("wrapCliConfigurationError - unknown error remains unchanged", (t) => {
-  const commandError = createCommandInvocationError(
+  const commandError = new CommandInvocationError(
    "codeql",
    ["version"],
    1,
@@ -404,13 +384,13 @@ test("all CLI config error categories have valid configurations", (t) => {
      case CliConfigErrorCategory.NoSourceCodeSeen:
        // This category matches by exit code
        testError = new CliError(
-          createCommandInvocationError("codeql", [], 32, "some error"),
+          new CommandInvocationError("codeql", [], 32, "some error"),
        );
        break;
      default:
        // For other categories, we'll test with a generic message that should not match
        testError = new CliError(
-          createCommandInvocationError("codeql", [], 1, "generic error"),
+          new CommandInvocationError("codeql", [], 1, "generic error"),
        );
        break;
    }
--- a/src/cli-errors.ts
+++ b/src/cli-errors.ts
@@ -6,6 +6,13 @@ import {
 import { DocUrl } from "./doc-url";
 import { ConfigurationError } from "./util";

+const SUPPORTED_PLATFORMS = [
+  ["linux", "x64"],
+  ["win32", "x64"],
+  ["darwin", "x64"],
+  ["darwin", "arm64"],
+];
+
 /**
 * An error from a CodeQL CLI invocation, with associated exit code, stderr, etc.
 */
@@ -318,31 +325,24 @@ function getCliConfigCategoryIfExists(

 /**
 * Check if we are running on an unsupported platform/architecture combination.
- * If so, return a ConfigurationError with a message that explains that, mentioning
- * the underlying `cliError`. Otherwise, reutrn `undefined`.
 */
-function getUnsupportedPlatformError(
-  cliError: CliError,
-): ConfigurationError | undefined {
-  if (
-    ![
-      ["linux", "x64"],
-      ["win32", "x64"],
-      ["darwin", "x64"],
-      ["darwin", "arm64"],
-    ].some(
-      ([platform, arch]) =>
-        platform === process.platform && arch === process.arch,
-    )
-  ) {
-    return new ConfigurationError(
-      "The CodeQL CLI does not support the platform/architecture combination of " +
-        `${process.platform}/${process.arch} ` +
-        "(see https://codeql.github.com/docs/codeql-overview/system-requirements). " +
-        `The underlying error was: ${cliError.message}`,
-    );
-  }
-  return undefined;
+function isUnsupportedPlatform(): boolean {
+  return !SUPPORTED_PLATFORMS.some(
+    ([platform, arch]) =>
+      platform === process.platform && arch === process.arch,
+  );
+}
+
+/**
+ * Transform a CLI error into a ConfigurationError for an unsupported platform.
+ */
+function getUnsupportedPlatformError(cliError: CliError): ConfigurationError {
+  return new ConfigurationError(
+    "The CodeQL CLI does not support the platform/architecture combination of " +
+      `${process.platform}/${process.arch} ` +
+      `(see ${DocUrl.SYSTEM_REQUIREMENTS}). ` +
+      `The underlying error was: ${cliError.message}`,
+  );
 }

 /**
@@ -351,9 +351,8 @@ function getUnsupportedPlatformError(
 * simply returns the original error.
 */
 export function wrapCliConfigurationError(cliError: CliError): Error {
-  const unsupportedPlatformError = getUnsupportedPlatformError(cliError);
-  if (unsupportedPlatformError !== undefined) {
-    return unsupportedPlatformError;
+  if (isUnsupportedPlatform()) {
+    return getUnsupportedPlatformError(cliError);
  }

  const cliConfigErrorCategory = getCliConfigCategoryIfExists(cliError);
--- a/src/doc-url.ts
+++ b/src/doc-url.ts
@@ -10,4 +10,5 @@ export enum DocUrl {
  SPECIFY_BUILD_STEPS_MANUALLY = "https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages#about-specifying-build-steps-manually",
  TRACK_CODE_SCANNING_ALERTS_ACROSS_RUNS = "https://docs.github.com/en/enterprise-cloud@latest/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#providing-data-to-track-code-scanning-alerts-across-runs",
  CODEQL_BUILD_MODES = "https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages#codeql-build-modes",
+  SYSTEM_REQUIREMENTS = "https://codeql.github.com/docs/codeql-overview/system-requirements/",
 }