From 26804552e4e731f906550537f732e8dc17c10d0d Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 5 Nov 2025 17:23:22 +0000 Subject: [PATCH] Use `undefined` instead of `NoMatchingFilesError` Add tests for `makePatternCheck` and `checkHashPatterns` --- lib/analyze-action.js | 26 +++------- lib/init-action.js | 26 +++------- src/dependency-caching.test.ts | 93 +++++++++++++++++++++++++++++++++- src/dependency-caching.ts | 61 ++++++++++------------ 4 files changed, 135 insertions(+), 71 deletions(-) diff --git a/lib/analyze-action.js b/lib/analyze-action.js index a95d9b12f..6238973f8 100644 --- a/lib/analyze-action.js +++ b/lib/analyze-action.js @@ -91053,12 +91053,6 @@ var os3 = __toESM(require("os")); var import_path = require("path"); var actionsCache3 = __toESM(require_cache3()); var glob = __toESM(require_glob2()); -var NoMatchingFilesError = class extends Error { - constructor(msg) { - super(msg); - this.name = "NoMatchingFilesError"; - } -}; var CODEQL_DEPENDENCY_CACHE_PREFIX = "codeql-dependencies"; var CODEQL_DEPENDENCY_CACHE_VERSION = 1; function getJavaTempDependencyDir() { @@ -91077,7 +91071,7 @@ function getJavaDependencyDirs() { async function makePatternCheck(patterns) { const globber = await makeGlobber(patterns); if ((await globber.glob()).length === 0) { - throw new NoMatchingFilesError(); + return void 0; } return patterns; } @@ -91099,7 +91093,7 @@ async function getCsharpHashPatterns(codeql, features) { "**/nuget.config" ]); } - throw new NoMatchingFilesError(); + return void 0; } var defaultCacheConfigs = { java: { @@ -91129,17 +91123,13 @@ async function makeGlobber(patterns) { return glob.create(patterns.join("\n")); } async function checkHashPatterns(codeql, features, language, cacheConfig, logger) { - try { - return cacheConfig.getHashPatterns(codeql, features); - } catch (err) { - if (err instanceof NoMatchingFilesError) { - logger.info( - `Skipping download of dependency cache for ${language} as we cannot calculate a hash for the cache key.` - ); - return void 0; - } - throw err; + const patterns = await cacheConfig.getHashPatterns(codeql, features); + if (patterns === void 0) { + logger.info( + `Skipping download of dependency cache for ${language} as we cannot calculate a hash for the cache key.` + ); } + return patterns; } async function uploadDependencyCaches(codeql, features, config, logger) { const status = []; diff --git a/lib/init-action.js b/lib/init-action.js index 0bb4376ea..7a5a844fe 100644 --- a/lib/init-action.js +++ b/lib/init-action.js @@ -87240,12 +87240,6 @@ var os2 = __toESM(require("os")); var import_path = require("path"); var actionsCache3 = __toESM(require_cache3()); var glob = __toESM(require_glob2()); -var NoMatchingFilesError = class extends Error { - constructor(msg) { - super(msg); - this.name = "NoMatchingFilesError"; - } -}; var CODEQL_DEPENDENCY_CACHE_PREFIX = "codeql-dependencies"; var CODEQL_DEPENDENCY_CACHE_VERSION = 1; function getJavaTempDependencyDir() { @@ -87264,7 +87258,7 @@ function getJavaDependencyDirs() { async function makePatternCheck(patterns) { const globber = await makeGlobber(patterns); if ((await globber.glob()).length === 0) { - throw new NoMatchingFilesError(); + return void 0; } return patterns; } @@ -87286,7 +87280,7 @@ async function getCsharpHashPatterns(codeql, features) { "**/nuget.config" ]); } - throw new NoMatchingFilesError(); + return void 0; } var defaultCacheConfigs = { java: { @@ -87316,17 +87310,13 @@ async function makeGlobber(patterns) { return glob.create(patterns.join("\n")); } async function checkHashPatterns(codeql, features, language, cacheConfig, logger) { - try { - return cacheConfig.getHashPatterns(codeql, features); - } catch (err) { - if (err instanceof NoMatchingFilesError) { - logger.info( - `Skipping download of dependency cache for ${language} as we cannot calculate a hash for the cache key.` - ); - return void 0; - } - throw err; + const patterns = await cacheConfig.getHashPatterns(codeql, features); + if (patterns === void 0) { + logger.info( + `Skipping download of dependency cache for ${language} as we cannot calculate a hash for the cache key.` + ); } + return patterns; } async function downloadDependencyCaches(codeql, features, languages, logger) { const status = []; diff --git a/src/dependency-caching.test.ts b/src/dependency-caching.test.ts index 7cf174c03..87501c995 100644 --- a/src/dependency-caching.test.ts +++ b/src/dependency-caching.test.ts @@ -1,15 +1,104 @@ +import * as fs from "fs"; +import path from "path"; + import test from "ava"; + // import * as sinon from "sinon"; import { cacheKeyHashLength } from "./caching-utils"; import { createStubCodeQL } from "./codeql"; -import { getFeaturePrefix } from "./dependency-caching"; +import { + CacheConfig, + checkHashPatterns, + getFeaturePrefix, + makePatternCheck, +} from "./dependency-caching"; import { Feature } from "./feature-flags"; import { KnownLanguage } from "./languages"; -import { setupTests, createFeatures } from "./testing-utils"; +import { + setupTests, + createFeatures, + getRecordingLogger, + checkExpectedLogMessages, + LoggedMessage, +} from "./testing-utils"; +import { withTmpDir } from "./util"; setupTests(test); +function makeAbsolutePatterns(tmpDir: string, patterns: string[]): string[] { + return patterns.map((pattern) => path.join(tmpDir, pattern)); +} + +test("makePatternCheck - returns undefined if no patterns match", async (t) => { + await withTmpDir(async (tmpDir) => { + fs.writeFileSync(path.join(tmpDir, "test.java"), ""); + const result = await makePatternCheck( + makeAbsolutePatterns(tmpDir, ["**/*.cs"]), + ); + t.is(result, undefined); + }); +}); + +test("makePatternCheck - returns all patterns if any pattern matches", async (t) => { + await withTmpDir(async (tmpDir) => { + fs.writeFileSync(path.join(tmpDir, "test.java"), ""); + const patterns = makeAbsolutePatterns(tmpDir, ["**/*.cs", "**/*.java"]); + const result = await makePatternCheck(patterns); + t.deepEqual(result, patterns); + }); +}); + +test("checkHashPatterns - logs when no patterns match", async (t) => { + const codeql = createStubCodeQL({}); + const features = createFeatures([]); + const messages: LoggedMessage[] = []; + const config: CacheConfig = { + getDependencyPaths: () => [], + getHashPatterns: async () => undefined, + }; + + const result = await checkHashPatterns( + codeql, + features, + KnownLanguage.csharp, + config, + getRecordingLogger(messages), + ); + + t.is(result, undefined); + checkExpectedLogMessages(t, messages, [ + "Skipping download of dependency cache", + ]); +}); + +test("checkHashPatterns - returns patterns when patterns match", async (t) => { + await withTmpDir(async (tmpDir) => { + const codeql = createStubCodeQL({}); + const features = createFeatures([]); + const messages: LoggedMessage[] = []; + const patterns = makeAbsolutePatterns(tmpDir, ["**/*.cs", "**/*.java"]); + + fs.writeFileSync(path.join(tmpDir, "test.java"), ""); + + const config: CacheConfig = { + getDependencyPaths: () => [], + getHashPatterns: async () => makePatternCheck(patterns), + }; + + const result = await checkHashPatterns( + codeql, + features, + KnownLanguage.csharp, + config, + getRecordingLogger(messages), + ); + + t.deepEqual(result, patterns); + t.deepEqual(messages, []); + }); +}); + test("getFeaturePrefix - returns empty string if no features are enabled", async (t) => { const codeql = createStubCodeQL({}); const features = createFeatures([]); diff --git a/src/dependency-caching.ts b/src/dependency-caching.ts index 6ee36a3bb..8bc5ec1ed 100644 --- a/src/dependency-caching.ts +++ b/src/dependency-caching.ts @@ -15,29 +15,24 @@ import { KnownLanguage, Language } from "./languages"; import { Logger } from "./logging"; import { getErrorMessage, getRequiredEnvParam } from "./util"; -class NoMatchingFilesError extends Error { - constructor(msg?: string) { - super(msg); - - this.name = "NoMatchingFilesError"; - } -} - /** * Caching configuration for a particular language. */ -interface CacheConfig { +export interface CacheConfig { /** Gets the paths of directories on the runner that should be included in the cache. */ getDependencyPaths: () => string[]; /** * Gets an array of glob patterns for the paths of files whose contents affect which dependencies are used - * by a project. This function also checks whether there are any matching files and throws - * a `NoMatchingFilesError` error if no files match. + * by a project. This function also checks whether there are any matching files and returns + * `undefined` if no files match. * * The glob patterns are intended to be used for cache keys, where we find all files which match these * patterns, calculate a hash for their contents, and use that hash as part of the cache key. */ - getHashPatterns: (codeql: CodeQL, features: Features) => Promise; + getHashPatterns: ( + codeql: CodeQL, + features: FeatureEnablement, + ) => Promise; } const CODEQL_DEPENDENCY_CACHE_PREFIX = "codeql-dependencies"; @@ -73,16 +68,18 @@ export function getJavaDependencyDirs(): string[] { /** * Checks that there are files which match `patterns`. If there are matching files for any of the patterns, - * this function returns all `patterns`. Otherwise, a `NoMatchingFilesError` is thrown. + * this function returns all `patterns`. Otherwise, `undefined` is returned. * * @param patterns The glob patterns to find matching files for. - * @returns The array of glob patterns if there are matching files. + * @returns The array of glob patterns if there are matching files, or `undefined` otherwise. */ -async function makePatternCheck(patterns: string[]): Promise { +export async function makePatternCheck( + patterns: string[], +): Promise { const globber = await makeGlobber(patterns); if ((await globber.glob()).length === 0) { - throw new NoMatchingFilesError(); + return undefined; } return patterns; @@ -98,8 +95,8 @@ async function makePatternCheck(patterns: string[]): Promise { */ async function getCsharpHashPatterns( codeql: CodeQL, - features: Features, -): Promise { + features: FeatureEnablement, +): Promise { // These files contain accurate information about dependencies, including the exact versions // that the relevant package manager has determined for the project. Using these gives us // stable hashes unless the dependencies change. @@ -133,7 +130,7 @@ async function getCsharpHashPatterns( // If we get to this point, the `basePatterns` didn't find any files, // and `Feature.CsharpNewCacheKey` is either not enabled or we didn't // find any files using those patterns either. - throw new NoMatchingFilesError(); + return undefined; } /** @@ -192,8 +189,8 @@ export interface DependencyCacheRestoreStatus { export type DependencyCacheRestoreStatusReport = DependencyCacheRestoreStatus[]; /** - * A wrapper around `cacheConfig.getHashPatterns` which catches `NoMatchingFilesError` errors, - * and logs that there are no files to calculate a hash for the cache key from. + * A wrapper around `cacheConfig.getHashPatterns` which logs when there are no files to calculate + * a hash for the cache key from. * * @param codeql The CodeQL instance to use. * @param features Information about which FFs are enabled. @@ -202,24 +199,22 @@ export type DependencyCacheRestoreStatusReport = DependencyCacheRestoreStatus[]; * @param logger The logger to write the log message to if there is an error. * @returns An array of glob patterns to use for hashing files, or `undefined` if there are no matching files. */ -async function checkHashPatterns( +export async function checkHashPatterns( codeql: CodeQL, - features: Features, + features: FeatureEnablement, language: Language, cacheConfig: CacheConfig, logger: Logger, ): Promise { - try { - return cacheConfig.getHashPatterns(codeql, features); - } catch (err) { - if (err instanceof NoMatchingFilesError) { - logger.info( - `Skipping download of dependency cache for ${language} as we cannot calculate a hash for the cache key.`, - ); - return undefined; - } - throw err; + const patterns = await cacheConfig.getHashPatterns(codeql, features); + + if (patterns === undefined) { + logger.info( + `Skipping download of dependency cache for ${language} as we cannot calculate a hash for the cache key.`, + ); } + + return patterns; } /**