Add better error messages when determining merge-base

Avoid printing scary error messages to console when the current
directory is not a git repo. Instead provide a better reason for the git
failure and continue on.

lib/actions-util.js

@@ -75,6 +75,7 @@ const getCommitOid = async function (checkoutPath, ref = "HEAD") {
     // the merge commit, which must mean that git is available.
     // Even if this does go wrong, it's not a huge problem for the alerts to
     // reported on the merge commit.
+    let output = "";
     try {
         let commitOid = "";
         await new toolrunner.ToolRunner(await safeWhich.safeWhich("git"), ["rev-parse", ref], {
@@ -84,7 +85,7 @@ const getCommitOid = async function (checkoutPath, ref = "HEAD") {
                     commitOid += data.toString();
                 },
                 stderr: (data) => {
-                    process.stderr.write(data);
+                    output += data.toString();
                 },
             },
             cwd: checkoutPath,
@@ -92,9 +93,12 @@ const getCommitOid = async function (checkoutPath, ref = "HEAD") {
         return commitOid.trim();
     }
     catch (e) {
-        core.info("Could not determine current commit SHA using git. Continuing with data from user input or environment.");
+        if (output.includes("not a git repository")) {
-        core.debug(`Reason: ${e.message}`);
+            core.info("Could not determine current commit SHA using git. Continuing with data from user input or environment. The checkout path provided to the action does not appear to be a git repository.");
-        core.debug(e.stack || "NO STACK");
+        }
+        else {
+            core.info(`Could not determine current commit SHA using git. Continuing with data from user input or environment. Failed to call git to determine merge base. Continuing with data from environment: ${output}`);
+        }
         return (0, exports.getOptionalInput)("sha") || (0, util_1.getRequiredEnvParam)("GITHUB_SHA");
     }
 };
@@ -103,12 +107,13 @@ exports.getCommitOid = getCommitOid;
  * If the action was triggered by a pull request, determine the commit sha of the merge base.
  * Returns undefined if run by other triggers or the merge base cannot be determined.
  */
-const determineMergeBaseCommitOid = async function () {
+const determineMergeBaseCommitOid = async function (checkoutPathOverride) {
     if (getWorkflowEventName() !== "pull_request") {
         return undefined;
     }
     const mergeSha = (0, util_1.getRequiredEnvParam)("GITHUB_SHA");
-    const checkoutPath = (0, exports.getOptionalInput)("checkout_path");
+    const checkoutPath = checkoutPathOverride ?? (0, exports.getOptionalInput)("checkout_path");
+    let output = "";
     try {
         let commitOid = "";
         let baseOid = "";
@@ -130,7 +135,7 @@ const determineMergeBaseCommitOid = async function () {
                     }
                 },
                 stderr: (data) => {
-                    process.stderr.write(data);
+                    output += data.toString();
                 },
             },
             cwd: checkoutPath,
@@ -144,8 +149,12 @@ const determineMergeBaseCommitOid = async function () {
         return undefined;
     }
     catch (e) {
-        core.info(`Failed to call git to determine merge base. Continuing with data from environment: ${e}`);
+        if (output.includes("not a git repository")) {
-        core.info(e.stack || "NO STACK");
+            core.info("The checkout path provided to the action does not appear to be a git repository. Continuing with data from environment.");
+        }
+        else {
+            core.info(`Failed to call git to determine merge base. Continuing with data from environment: ${output}`);
+        }
         return undefined;
     }
 };

lib/actions-util.test.js

@@ -28,6 +28,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
 Object.defineProperty(exports, "__esModule", { value: true });
 const fs = __importStar(require("fs"));
 const path = __importStar(require("path"));
+const core = __importStar(require("@actions/core"));
 const ava_1 = __importDefault(require("ava"));
 const sinon = __importStar(require("sinon"));
 const actionsUtil = __importStar(require("./actions-util"));
@@ -213,4 +214,32 @@ const util_1 = require("./util");
         getAdditionalInputStub.restore();
     });
 });
+(0, ava_1.default)("determineMergeBaseCommitOid non-pullrequest", async (t) => {
+    const stub = sinon.stub(core, "info");
+    process.env["GITHUB_EVENT_NAME"] = "hucairz";
+    process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
+    const result = await actionsUtil.determineMergeBaseCommitOid(__dirname);
+    t.deepEqual(result, undefined);
+    t.deepEqual(0, stub.callCount);
+    stub.restore();
+});
+(0, ava_1.default)("determineMergeBaseCommitOid no error", async (t) => {
+    const stub = sinon.stub(core, "info");
+    process.env["GITHUB_EVENT_NAME"] = "pull_request";
+    process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
+    await actionsUtil.determineMergeBaseCommitOid(path.join(__dirname, "../.."));
+    t.deepEqual(1, stub.callCount);
+    t.assert(stub.firstCall.args[0].startsWith("The checkout path provided to the action does not appear to be a git repository."));
+    stub.restore();
+});
+(0, ava_1.default)("determineMergeBaseCommitOid other error", async (t) => {
+    const stub = sinon.stub(core, "info");
+    process.env["GITHUB_EVENT_NAME"] = "pull_request";
+    process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
+    const result = await actionsUtil.determineMergeBaseCommitOid(path.join(__dirname, "../../i-dont-exist"));
+    t.deepEqual(result, undefined);
+    t.deepEqual(1, stub.callCount);
+    t.assert(stub.firstCall.args[0].startsWith("Failed to call git to determine merge base."));
+    stub.restore();
+});
 //# sourceMappingURL=actions-util.test.js.map

src/actions-util.test.ts

@@ -1,6 +1,8 @@
 import * as fs from "fs";
 import * as path from "path";
 
+import * as core from "@actions/core";
+import * as toolRunner from "@actions/exec/lib/toolrunner";
 import test from "ava";
 import * as sinon from "sinon";
 
@@ -267,3 +269,50 @@ test("isAnalyzingDefaultBranch()", async (t) => {
     getAdditionalInputStub.restore();
   });
 });
+
+test("determineMergeBaseCommitOid non-pullrequest", async (t) => {
+  const stub = sinon.stub(core, "info");
+
+  process.env["GITHUB_EVENT_NAME"] = "hucairz";
+  process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
+  const result = await actionsUtil.determineMergeBaseCommitOid(__dirname);
+  t.deepEqual(result, undefined);
+  t.deepEqual(0, stub.callCount);
+
+  stub.restore();
+});
+
+test("determineMergeBaseCommitOid no error", async (t) => {
+  const stub = sinon.stub(core, "info");
+
+  process.env["GITHUB_EVENT_NAME"] = "pull_request";
+  process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
+  await actionsUtil.determineMergeBaseCommitOid(path.join(__dirname, "../.."));
+  t.deepEqual(1, stub.callCount);
+  t.assert(
+    stub.firstCall.args[0].startsWith(
+      "The checkout path provided to the action does not appear to be a git repository.",
+    ),
+  );
+
+  stub.restore();
+});
+
+test("determineMergeBaseCommitOid other error", async (t) => {
+  const stub = sinon.stub(core, "info");
+
+  process.env["GITHUB_EVENT_NAME"] = "pull_request";
+  process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
+  const result = await actionsUtil.determineMergeBaseCommitOid(
+    path.join(__dirname, "../../i-dont-exist"),
+  );
+  t.deepEqual(result, undefined);
+  t.deepEqual(1, stub.callCount);
+  t.assert(
+    stub.firstCall.args[0].startsWith(
+      "Failed to call git to determine merge base.",
+    ),
+  );
+
+  stub.restore();
+});

src/actions-util.ts

@@ -63,6 +63,7 @@ export const getCommitOid = async function (
   // the merge commit, which must mean that git is available.
   // Even if this does go wrong, it's not a huge problem for the alerts to
   // reported on the merge commit.
+  let output = "";
   try {
     let commitOid = "";
     await new toolrunner.ToolRunner(
@@ -75,7 +76,7 @@ export const getCommitOid = async function (
             commitOid += data.toString();
           },
           stderr: (data) => {
-            process.stderr.write(data);
+            output += data.toString();
           },
         },
         cwd: checkoutPath,
@@ -83,11 +84,16 @@ export const getCommitOid = async function (
     ).exec();
     return commitOid.trim();
   } catch (e) {
-    core.info(
+    if (output.includes("not a git repository")) {
-      "Could not determine current commit SHA using git. Continuing with data from user input or environment.",
+      core.info(
-    );
+        "Could not determine current commit SHA using git. Continuing with data from user input or environment. The checkout path provided to the action does not appear to be a git repository.",
-    core.debug(`Reason: ${(e as Error).message}`);
+      );
-    core.debug((e as Error).stack || "NO STACK");
+    } else {
+      core.info(
+        `Could not determine current commit SHA using git. Continuing with data from user input or environment. Failed to call git to determine merge base. Continuing with data from environment: ${output}`,
+      );
+    }
+
     return getOptionalInput("sha") || getRequiredEnvParam("GITHUB_SHA");
   }
 };
@@ -96,15 +102,17 @@ export const getCommitOid = async function (
  * If the action was triggered by a pull request, determine the commit sha of the merge base.
  * Returns undefined if run by other triggers or the merge base cannot be determined.
  */
-export const determineMergeBaseCommitOid = async function (): Promise<
+export const determineMergeBaseCommitOid = async function (
-  string | undefined
+  checkoutPathOverride?: string,
-> {
+): Promise<string | undefined> {
   if (getWorkflowEventName() !== "pull_request") {
     return undefined;
   }
 
   const mergeSha = getRequiredEnvParam("GITHUB_SHA");
-  const checkoutPath = getOptionalInput("checkout_path");
+  const checkoutPath =
+    checkoutPathOverride ?? getOptionalInput("checkout_path");
+  let output = "";
 
   try {
     let commitOid = "";
@@ -129,7 +137,7 @@ export const determineMergeBaseCommitOid = async function (): Promise<
             }
           },
           stderr: (data) => {
-            process.stderr.write(data);
+            output += data.toString();
           },
         },
         cwd: checkoutPath,
@@ -146,10 +154,15 @@ export const determineMergeBaseCommitOid = async function (): Promise<
     }
     return undefined;
   } catch (e) {
-    core.info(
+    if (output.includes("not a git repository")) {
-      `Failed to call git to determine merge base. Continuing with data from environment: ${e}`,
+      core.info(
-    );
+        "The checkout path provided to the action does not appear to be a git repository. Continuing with data from environment.",
-    core.info((e as Error).stack || "NO STACK");
+      );
+    } else {
+      core.info(
+        `Failed to call git to determine merge base. Continuing with data from environment: ${output}`,
+      );
+    }
     return undefined;
   }
 };