Merge pull request #4005 from github/mbg/test-env/improvements

Improve `TestEnv` for cleaner tests

Author: mbg

src/config/file.test.ts

@@ -13,7 +13,7 @@ test("getConfigFileInput returns undefined by default", async (t) => {
   await callee(getConfigFileInput)
     .withArgs({})
     .withFeatures([Feature.ConfigFileRepositoryProperty])
-    .passes(async (fn) => t.is(await fn(), undefined));
+    .passes(t.is, undefined);
 });
 
 const repositoryProperties = {
@@ -22,75 +22,48 @@ const repositoryProperties = {
 
 test("getConfigFileInput returns input value", async (t) => {
   const testInput = "/some/path";
-  const target = callee(getConfigFileInput).withFeatures([
-    Feature.ConfigFileRepositoryProperty,
-  ]);
-
-  const actionsEnv = target.getState().actions;
-  sinon
-    .stub(actionsEnv, "getOptionalInput")
-    .withArgs("config-file")
-    .returns(testInput);
 
   // Even though both an input and repository property are configured,
   // we prefer the direct input to the Action.
-  const targetWithArgs = target
-    .withActions(actionsEnv)
-    .withArgs(repositoryProperties);
-  await targetWithArgs.passes(async (fn) => t.is(await fn(), testInput));
-
-  // Check for the expected log message.
-  t.true(
-    targetWithArgs
-      .getLogger()
-      .hasMessage("Using configuration file input from workflow"),
-  );
+  await callee(getConfigFileInput)
+    .withFeatures([Feature.ConfigFileRepositoryProperty])
+    .withActions((actionsEnv) => {
+      sinon
+        .stub(actionsEnv, "getOptionalInput")
+        .withArgs("config-file")
+        .returns(testInput);
+    })
+    .withArgs(repositoryProperties)
+    .logs(t, "Using configuration file input from workflow")
+    .passes(t.is, testInput);
 });
 
 test("getConfigFileInput returns repository property value", async (t) => {
   // Since there is no direct input, we should use the repository property.
-  const target = callee(getConfigFileInput)
+  await callee(getConfigFileInput)
     .withFeatures([Feature.ConfigFileRepositoryProperty])
-    .withArgs(repositoryProperties);
-
-  await target.passes(async (fn) =>
-    t.is(await fn(), repositoryProperties[RepositoryPropertyName.CONFIG_FILE]),
-  );
-
-  // Check for the expected log message.
-  t.true(
-    target
-      .getLogger()
-      .hasMessage("Using configuration file input from repository property"),
-  );
+    .withArgs(repositoryProperties)
+    .logs(t, "Using configuration file input from repository property")
+    .passes(t.is, repositoryProperties[RepositoryPropertyName.CONFIG_FILE]);
 });
 
 test("getConfigFileInput ignores empty repository property value", async (t) => {
   // Since the repository property value is an empty/whitespace string, we should ignore it.
   await callee(getConfigFileInput)
     .withFeatures([Feature.ConfigFileRepositoryProperty])
     .withArgs({ [RepositoryPropertyName.CONFIG_FILE]: "   " })
-    .passes(async (fn) => t.is(await fn(), undefined));
+    .passes(t.is, undefined);
 });
 
 test("getConfigFileInput ignores repository property value when FF is off", async (t) => {
   // Since the FF is off, we should ignore the repository property value.
-  const target = callee(getConfigFileInput)
+  await callee(getConfigFileInput)
     .withFeatures([])
-    .withArgs(repositoryProperties);
-
-  await target.passes(async (fn) => t.is(await fn(), undefined));
-
-  t.false(
-    target
-      .getLogger()
-      .hasMessage("Using configuration file input from repository property"),
-  );
-  t.true(
-    target
-      .getLogger()
-      .hasMessage(
-        "Ignoring configuration file input from repository property, because the corresponding feature flag is disabled.",
-      ),
-  );
+    .withArgs(repositoryProperties)
+    .notLogs(t, "Using configuration file input from repository property")
+    .logs(
+      t,
+      "Ignoring configuration file input from repository property, because the corresponding feature flag is disabled.",
+    )
+    .passes(t.is, undefined);
 });

src/config/remote-file.test.ts

@@ -4,7 +4,7 @@ import sinon from "sinon";
 import { ActionsEnvVars } from "../environment";
 import * as errors from "../error-messages";
 import { Feature } from "../feature-flags";
-import { callee, getTestEnv } from "../testing-utils";
+import { callee } from "../testing-utils";
 import { ConfigurationError } from "../util";
 
 import {
@@ -50,7 +50,7 @@ test("parseRemoteFileAddress accepts full remote addresses", async (t) => {
   for (const oldFormatInput of oldFormatInputs) {
     await target
       .withArgs(oldFormatInput.input)
-      .passes(async (fn) => t.deepEqual(await fn(), oldFormatInput.expected));
+      .passes(t.deepEqual, oldFormatInput.expected);
   }
 
   // New format.
@@ -78,28 +78,23 @@ test("parseRemoteFileAddress accepts full remote addresses", async (t) => {
     // Should fail when the FF is not enabled.
     await targetWithArgs
       .withFeatures([])
-      .passes(async (fn) =>
-        t.throwsAsync(fn, { instanceOf: ConfigurationError }),
-      );
+      .throws(t, { instanceOf: ConfigurationError });
 
     // And pass when the FF is enabled.
     await targetWithArgs
       .withFeatures([Feature.NewRemoteFileAddresses])
-      .passes(async (fn) => t.deepEqual(await fn(), newFormatInput.expected));
+      .passes(t.deepEqual, newFormatInput.expected);
   }
 });
 
 test("parseRemoteFileAddress accepts remote address without an owner", async (t) => {
-  const target = callee(parseRemoteFileAddress);
-
-  const env = target.getState().env;
   const owner = "test-owner";
-  const getRequired = sinon.stub(env, "getRequired");
-  getRequired
-    .withArgs(ActionsEnvVars.GITHUB_REPOSITORY)
-    .returns(`${owner}/current-repo`);
-
-  const targetWithEnv = target.withEnv(env);
+  const target = callee(parseRemoteFileAddress).withEnv((env) => {
+    const getRequired = sinon.stub(env, "getRequired");
+    getRequired
+      .withArgs(ActionsEnvVars.GITHUB_REPOSITORY)
+      .returns(`${owner}/current-repo`);
+  });
 
   const testCases: ParseRemoteFileAddressTest[] = [
     {
@@ -141,33 +136,33 @@ test("parseRemoteFileAddress accepts remote address without an owner", async (t)
   ];
 
   for (const testCase of testCases) {
-    const targetWithArgs = targetWithEnv.withArgs(testCase.input);
+    const targetWithArgs = target.withArgs(testCase.input);
 
     // Should fail when the FF is not enabled.
     await targetWithArgs
       .withFeatures([])
-      .passes(async (fn) =>
-        t.throwsAsync(fn, { instanceOf: ConfigurationError }),
-      );
+      .throws(t, { instanceOf: ConfigurationError });
 
     // And pass when the FF is enabled.
     await targetWithArgs
       .withFeatures([Feature.NewRemoteFileAddresses])
-      .passes(async (fn) => t.deepEqual(await fn(), testCase.expected));
+      .passes(t.deepEqual, testCase.expected);
   }
 });
 
 test("parseRemoteFileAddress throws for invalid `GITHUB_REPOSITORY`", async (t) => {
-  const target = callee(parseRemoteFileAddress).withArgs("repo@ref");
-
-  const env = target.getState().env;
-  const getRequired = sinon.stub(env, "getRequired");
+  const getRequired: sinon.SinonStub = sinon.stub();
   getRequired.withArgs(ActionsEnvVars.GITHUB_REPOSITORY).returns(`not-valid`);
 
+  const target = callee(parseRemoteFileAddress)
+    .withArgs("repo@ref")
+    .withEnv((env) => {
+      sinon.define(env, "getRequired", getRequired);
+    });
+
   await target
-    .withEnv(env)
     .withFeatures([Feature.NewRemoteFileAddresses])
-    .passes(async (fn) => t.throwsAsync(fn, { instanceOf: Error }));
+    .throws(t, { instanceOf: Error });
 
   t.assert(getRequired.calledOnceWith(ActionsEnvVars.GITHUB_REPOSITORY));
 });
@@ -202,43 +197,38 @@ test("parseRemoteFileAddress accepts remote address without a path", async (t) =
     // Should fail when the FF is not enabled.
     await targetWithArgs
       .withFeatures([])
-      .passes(async (fn) =>
-        t.throwsAsync(fn, { instanceOf: ConfigurationError }),
-      );
+      .throws(t, { instanceOf: ConfigurationError });
 
     // And pass when the FF is enabled.
     await targetWithArgs
       .withFeatures([Feature.NewRemoteFileAddresses])
-      .passes(async (fn) => t.deepEqual(await fn(), testCase.expected));
+      .passes(t.deepEqual, testCase.expected);
   }
 });
 
 test("parseRemoteFileAddress accepts remote address without a ref", async (t) => {
   const target = callee(parseRemoteFileAddress).withArgs("owner/repo:path");
 
   // Should only accept the input if the FF is enabled.
-  await target.withFeatures([]).passes(t.throwsAsync);
+  await target.withFeatures([]).throws(t);
   await target
     .withFeatures([Feature.NewRemoteFileAddresses])
-    .passes(async (fn) =>
-      t.deepEqual(await fn(), {
-        owner: "owner",
-        repo: "repo",
-        path: "path",
-        ref: DEFAULT_CONFIG_FILE_REF,
-      } satisfies RemoteFileAddress),
-    );
+    .passes(t.deepEqual, {
+      owner: "owner",
+      repo: "repo",
+      path: "path",
+      ref: DEFAULT_CONFIG_FILE_REF,
+    } satisfies RemoteFileAddress);
 });
 
 test("parseRemoteFileAddress rejects invalid values", async (t) => {
-  const env = getTestEnv();
   const owner = "owner";
-  const getRequired = sinon.stub(env, "getRequired");
-  getRequired
-    .withArgs(ActionsEnvVars.GITHUB_REPOSITORY)
-    .returns(`${owner}/current-repo`);
-
-  const target = callee(parseRemoteFileAddress).withEnv(env);
+  const target = callee(parseRemoteFileAddress).withEnv((env) => {
+    const getRequired = sinon.stub(env, "getRequired");
+    getRequired
+      .withArgs(ActionsEnvVars.GITHUB_REPOSITORY)
+      .returns(`${owner}/current-repo`);
+  });
 
   const testInputs = [
     "  ",
@@ -262,21 +252,17 @@ test("parseRemoteFileAddress rejects invalid values", async (t) => {
     const targetWithArgs = target.withArgs(testInput);
 
     // Should throw both when the new format is and isn't accepted.
-    await targetWithArgs.withFeatures([]).passes(async (fn) =>
-      t.throwsAsync(fn, {
-        instanceOf: ConfigurationError,
-        message: errors.getConfigFileRepoOldFormatInvalidMessage(testInput),
-      }),
-    );
+    await targetWithArgs.withFeatures([]).throws(t, {
+      instanceOf: ConfigurationError,
+      message: errors.getConfigFileRepoOldFormatInvalidMessage(testInput),
+    });
     await targetWithArgs
       .withFeatures([Feature.NewRemoteFileAddresses])
-      .passes(async (fn) =>
-        t.throwsAsync(fn, {
-          // When the new format is accepted, there are some more specific
-          // errors in some cases. It is sufficient for us to check that
-          // an exception is thrown.
-          instanceOf: ConfigurationError,
-        }),
-      );
+      .throws(t, {
+        // When the new format is accepted, there are some more specific
+        // errors in some cases. It is sufficient for us to check that
+        // an exception is thrown.
+        instanceOf: ConfigurationError,
+      });
   }
 });

src/environment.ts

@@ -193,10 +193,12 @@ export enum ActionsEnvVars {
   GITHUB_SERVER_URL = "GITHUB_SERVER_URL",
   GITHUB_SHA = "GITHUB_SHA",
   GITHUB_WORKFLOW = "GITHUB_WORKFLOW",
+  GITHUB_WORKSPACE = "GITHUB_WORKSPACE",
   RUNNER_ENVIRONMENT = "RUNNER_ENVIRONMENT",
   RUNNER_NAME = "RUNNER_NAME",
   RUNNER_OS = "RUNNER_OS",
   RUNNER_TEMP = "RUNNER_TEMP",
+  RUNNER_TOOL_CACHE = "RUNNER_TOOL_CACHE",
 }
 
 /** A type representing all known environment variables. */