Refactor output-caching functions to accept file path dependency

This makes it easier to test. Credit to @mbg.

Author: mario-campos

lib/entry-points.js

@@ -1,16 +1,13 @@
 import * as fs from "fs";
+import path from "path";
 
 import test from "ava";
 
-import { EnvVar } from "../environment";
 import { getRunnerLogger } from "../logging";
-import { getTestEnv, setupTests } from "../testing-utils";
+import { setupTests } from "../testing-utils";
 import * as util from "../util";
 
-import {
-  getCachedCodeQlVersion,
-  getCommandCacheFilePath,
-} from "./output-cache";
+import { getCachedCodeQlVersion } from "./output-cache";
 
 setupTests(test);
 
@@ -20,19 +17,22 @@ test.serial(
   "getCachedCodeQlVersion reuses a version persisted by an earlier step",
   async (t) => {
     await util.withTmpDir(async (tmpDir: string) => {
-      const env = getTestEnv({ [EnvVar.TEMP]: tmpDir });
-      const cacheFile = getCommandCacheFilePath(env);
+      const cacheFilePath = path.join(tmpDir, "cache.json");
+
       fs.writeFileSync(
-        cacheFile,
+        cacheFilePath,
         JSON.stringify({
           cmd: "/path/to/codeql",
           entries: { version: { version: "2.20.0" } },
         }),
         "utf8",
       );
-      t.deepEqual(getCachedCodeQlVersion(logger, env, "/path/to/codeql"), {
-        version: "2.20.0",
-      });
+      t.deepEqual(
+        getCachedCodeQlVersion(logger, cacheFilePath, "/path/to/codeql"),
+        {
+          version: "2.20.0",
+        },
+      );
     });
   },
 );
@@ -41,17 +41,19 @@ test.serial(
   "getCachedCodeQlVersion ignores a persisted version from a different CLI",
   async (t) => {
     await util.withTmpDir(async (tmpDir: string) => {
-      const env = getTestEnv({ [EnvVar.TEMP]: tmpDir });
-      const cacheFile = getCommandCacheFilePath(env);
+      const cacheFilePath = path.join(tmpDir, "cache.json");
       fs.writeFileSync(
-        cacheFile,
+        cacheFilePath,
         JSON.stringify({
           cmd: "/path/to/other-codeql",
           entries: { version: { version: "2.20.0" } },
         }),
         "utf8",
       );
-      t.is(getCachedCodeQlVersion(logger, env, "/path/to/codeql"), undefined);
+      t.is(
+        getCachedCodeQlVersion(logger, cacheFilePath, "/path/to/codeql"),
+        undefined,
+      );
     });
   },
 );
@@ -60,10 +62,12 @@ test.serial(
   "getCachedCodeQlVersion ignores a malformed persisted value",
   async (t) => {
     await util.withTmpDir(async (tmpDir: string) => {
-      const env = getTestEnv({ [EnvVar.TEMP]: tmpDir });
-      const cacheFile = getCommandCacheFilePath(env);
-      fs.writeFileSync(cacheFile, "not valid json", "utf8");
-      t.is(getCachedCodeQlVersion(logger, env, "/path/to/codeql"), undefined);
+      const cacheFilePath = path.join(tmpDir, "cache.json");
+      fs.writeFileSync(cacheFilePath, "not valid json", "utf8");
+      t.is(
+        getCachedCodeQlVersion(logger, cacheFilePath, "/path/to/codeql"),
+        undefined,
+      );
     });
   },
 );
@@ -72,8 +76,7 @@ test.serial(
   "getCachedCodeQlVersion ignores a persisted value with the wrong structure",
   async (t) => {
     await util.withTmpDir(async (tmpDir: string) => {
-      const env = getTestEnv({ [EnvVar.TEMP]: tmpDir });
-      const cacheFile = getCommandCacheFilePath(env);
+      const cacheFilePath = path.join(tmpDir, "cache.json");
       const testValues = [
         { cmd: "/path/to/codeql" },
         { entries: { version: { version: "2.20.0" } } },
@@ -96,9 +99,9 @@ test.serial(
       ].map((v) => JSON.stringify(v));
 
       for (const value of testValues) {
-        fs.writeFileSync(cacheFile, value, "utf8");
+        fs.writeFileSync(cacheFilePath, value, "utf8");
         t.is(
-          getCachedCodeQlVersion(logger, env, "/path/to/codeql"),
+          getCachedCodeQlVersion(logger, cacheFilePath, "/path/to/codeql"),
           undefined,
           value,
         );
@@ -109,9 +112,12 @@ test.serial(
 
 test.serial("getCachedCodeQlVersion ignores non-existent file", async (t) => {
   await util.withTmpDir(async (tmpDir: string) => {
-    const env = getTestEnv({ [EnvVar.TEMP]: tmpDir });
+    const cacheFilePath = path.join(tmpDir, "cache.json");
     t.notThrows(() => {
-      t.is(getCachedCodeQlVersion(logger, env, "/path/to/codeql"), undefined);
+      t.is(
+        getCachedCodeQlVersion(logger, cacheFilePath, "/path/to/codeql"),
+        undefined,
+      );
     });
   });
 });

src/cli/output-cache.test.ts

@@ -57,12 +57,12 @@ export function getCommandCacheFilePath(env: Env): string {
 
 /**
  * Caches the CodeQL CLI version both in-memory and on disk.
- * @param env The environment variables to use.
+ * @param cacheFilePath The path to the cache file.
  * @param cmd The path to the CodeQL CLI.
  * @param version The version information to cache.
  */
 export function cacheCodeQlVersion(
-  env: Env,
+  cacheFilePath: string,
   cmd: string,
   version: VersionInfo,
 ): void {
@@ -78,22 +78,18 @@ export function cacheCodeQlVersion(
   // processes, can reuse it rather than invoking `codeql version` again. We
   // record the CLI path so that a different step using a different CodeQL bundle
   // doesn't pick up a stale version.
-  fs.writeFileSync(
-    getCommandCacheFilePath(env),
-    JSON.stringify(outputCache),
-    "utf8",
-  );
+  fs.writeFileSync(cacheFilePath, JSON.stringify(outputCache), "utf8");
 }
 
 /**
  * Returns the cached CodeQL CLI version, if any.
  * @param logger The logger to use for logging messages.
- * @param env The environment variables to use.
+ * @param cacheFilePath The path to the cache file.
  * @param cmd The path to the CodeQL CLI.
  */
 export function getCachedCodeQlVersion(
   logger: Logger,
-  env: Env,
+  cacheFilePath: string,
   cmd?: string,
 ): undefined | VersionInfo {
   if (cachedCodeQlVersion !== undefined) {
@@ -104,11 +100,9 @@ export function getCachedCodeQlVersion(
   // invokes `codeql version` instead.
   let serialized: string;
   try {
-    serialized = fs.readFileSync(getCommandCacheFilePath(env), "utf8");
+    serialized = fs.readFileSync(cacheFilePath, "utf8");
   } catch (e) {
-    logger.debug(
-      `Cannot read CLI-cache file ${getCommandCacheFilePath(env)}: ${e}`,
-    );
+    logger.debug(`Cannot read CLI-cache file ${cacheFilePath}: ${e}`);
     return undefined;
   }
   let persisted: unknown;

src/cli/output-cache.ts

@@ -491,7 +491,12 @@ async function getCodeQLForCmd(
       return cmd;
     },
     async getVersion() {
-      let result = outputCache.getCachedCodeQlVersion(logger, getEnv(), cmd);
+      const cacheFilePath = outputCache.getCommandCacheFilePath(getEnv());
+      let result = outputCache.getCachedCodeQlVersion(
+        logger,
+        cacheFilePath,
+        cmd,
+      );
       if (result === undefined) {
         result = await runCliJson<VersionInfo>(
           cmd,
@@ -500,7 +505,7 @@ async function getCodeQLForCmd(
             noStreamStdout: true,
           },
         );
-        outputCache.cacheCodeQlVersion(getEnv(), cmd, result);
+        outputCache.cacheCodeQlVersion(cacheFilePath, cmd, result);
       }
       return result;
     },

src/codeql.ts

@@ -14,7 +14,10 @@ import {
   isSelfHostedRunner,
 } from "./actions-util";
 import { getAnalysisKey, getApiClient } from "./api-client";
-import { getCachedCodeQlVersion } from "./cli/output-cache";
+import {
+  getCachedCodeQlVersion,
+  getCommandCacheFilePath,
+} from "./cli/output-cache";
 import type { Config } from "./config/action-config";
 import type { ComputedInput, InputName } from "./config/inputs";
 import { parseRegistriesWithoutCredentials } from "./config/pack-registries";
@@ -376,7 +379,10 @@ export async function createStatusReportBase(
       core.exportVariable(EnvVar.WORKFLOW_STARTED_AT, workflowStartedAt);
     }
     const runnerOs = getRequiredEnvParam("RUNNER_OS");
-    const codeQlCliVersion = getCachedCodeQlVersion(logger, getEnv());
+    const codeQlCliVersion = getCachedCodeQlVersion(
+      logger,
+      getCommandCacheFilePath(getEnv()),
+    );
     const actionRef = process.env["GITHUB_ACTION_REF"] || "";
     const testingEnvironment = getTestingEnvironment();
     // re-export the testing environment variable so that it is available to subsequent steps,