From 7db741781411ad09c941715a10158428704cb31e Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Mon, 12 Oct 2026 12:02:33 +0100 Subject: [PATCH] Experiment with stubbing fs using sinon --- lib/entry-points.js | 58 +++++++++++++++++------------------------ src/diagnostics.test.ts | 44 +++++-------------------------- src/diagnostics.ts | 34 ++++++++---------------- 3 files changed, 42 insertions(+), 94 deletions(-) diff --git a/lib/entry-points.js b/lib/entry-points.js index 43c0ffa44..ee7ea0056 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -166088,7 +166088,6 @@ var import_perf_hooks6 = require("perf_hooks"); var core14 = __toESM(require_core()); // src/action-common.ts -var fs5 = __toESM(require("fs")); var os4 = __toESM(require("os")); var core9 = __toESM(require_core()); @@ -171750,7 +171749,6 @@ async function runInActions(action) { platform: process.platform, arch: process.arch, osRelease: os4.release(), - fs: fs5, logger: logger2, env, actions: actionsEnv @@ -171772,7 +171770,7 @@ async function runInActions(action) { } // src/feature-flags.ts -var fs7 = __toESM(require("fs")); +var fs6 = __toESM(require("fs")); var path6 = __toESM(require("path")); var semver4 = __toESM(require_semver2()); @@ -171781,7 +171779,7 @@ var bundleVersion = "codeql-bundle-v2.27.2"; var cliVersion = "2.27.2"; // src/overlay/index.ts -var fs6 = __toESM(require("fs")); +var fs5 = __toESM(require("fs")); var path5 = __toESM(require("path")); var CODEQL_OVERLAY_MINIMUM_VERSION = "2.23.8"; var CODEQL_OVERLAY_MINIMUM_VERSION_CPP = "2.25.0"; @@ -171795,12 +171793,12 @@ async function writeBaseDatabaseOidsFile(config, sourceRoot) { const gitFileOids = await getFileOidsUnderPath(sourceRoot); const gitFileOidsJson = JSON.stringify(gitFileOids); const baseDatabaseOidsFilePath = getBaseDatabaseOidsFilePath(config); - await fs6.promises.writeFile(baseDatabaseOidsFilePath, gitFileOidsJson); + await fs5.promises.writeFile(baseDatabaseOidsFilePath, gitFileOidsJson); } async function readBaseDatabaseOidsFile(config, logger2) { const baseDatabaseOidsFilePath = getBaseDatabaseOidsFilePath(config); try { - const contents = await fs6.promises.readFile( + const contents = await fs5.promises.readFile( baseDatabaseOidsFilePath, "utf-8" ); @@ -171829,7 +171827,7 @@ async function writeOverlayChangesFile(config, sourceRoot, logger2) { logger2.debug( `Writing overlay changed files to ${overlayChangesFile}: ${changedFilesJson}` ); - await fs6.promises.writeFile(overlayChangesFile, changedFilesJson); + await fs5.promises.writeFile(overlayChangesFile, changedFilesJson); return overlayChangesFile; } function computeChangedFiles(baseFileOids, overlayFileOids) { @@ -171848,7 +171846,7 @@ function computeChangedFiles(baseFileOids, overlayFileOids) { } async function getDiffRangeFilePaths(sourceRoot, logger2) { const jsonFilePath = getDiffRangesJsonFilePath(); - if (!fs6.existsSync(jsonFilePath)) { + if (!fs5.existsSync(jsonFilePath)) { logger2.debug( `No diff ranges JSON file found at ${jsonFilePath}; skipping.` ); @@ -171856,7 +171854,7 @@ async function getDiffRangeFilePaths(sourceRoot, logger2) { } let contents; try { - contents = await fs6.promises.readFile(jsonFilePath, "utf8"); + contents = await fs5.promises.readFile(jsonFilePath, "utf8"); } catch (e) { logger2.warning( `Failed to read diff ranges JSON file at ${jsonFilePath}: ${e}` @@ -172425,12 +172423,12 @@ var GitHubFeatureFlags = class { } async readLocalFlags() { try { - if (fs7.existsSync(this.featureFlagsFile)) { + if (fs6.existsSync(this.featureFlagsFile)) { this.logger.debug( `Loading feature flags from ${this.featureFlagsFile}` ); return JSON.parse( - fs7.readFileSync(this.featureFlagsFile, "utf8") + fs6.readFileSync(this.featureFlagsFile, "utf8") ); } } catch (e) { @@ -172443,7 +172441,7 @@ var GitHubFeatureFlags = class { async writeLocalFlags(flags) { try { this.logger.debug(`Writing feature flags to ${this.featureFlagsFile}`); - fs7.writeFileSync(this.featureFlagsFile, JSON.stringify(flags)); + fs6.writeFileSync(this.featureFlagsFile, JSON.stringify(flags)); } catch (e) { this.logger.warning( `Error writing cached feature flags file ${this.featureFlagsFile}: ${e}.` @@ -172979,7 +172977,7 @@ var jsonschema = __toESM(require_lib2()); var semver5 = __toESM(require_semver2()); // src/diagnostics.ts -var nodefs = __toESM(require("fs")); +var fs7 = __toESM(require("fs")); var import_path2 = __toESM(require("path")); var temporaryDiagnostics = []; var diagnosticCounter = 0; @@ -173000,24 +172998,20 @@ function getDiagnosticsPath(basePath, language) { } return basePath; } -function addDiagnostic(config, language, diagnostic, logger2 = getActionsLogger(), fs33 = nodefs) { +function addDiagnostic(config, language, diagnostic, logger2 = getActionsLogger()) { const databasePath = language ? getCodeQLDatabasePath(config, language) : config.dbLocation; - if (fs33.existsSync(databasePath)) { - writeDiagnostic({ logger: logger2, fs: fs33 }, config, language, diagnostic); + if (fs7.existsSync(databasePath)) { + writeDiagnostic({ logger: logger2 }, config, language, diagnostic); } else { logger2.debug( `Writing a diagnostic for ${language}, but the database at ${databasePath} does not exist yet.` ); const tempDiagnosticsPath = getTempDiagnosticPath(config.tempDir, language); - const diagnosticPath = writeDiagnosticFile( - fs33, - tempDiagnosticsPath, - diagnostic - ); + const diagnosticPath = writeDiagnosticFile(tempDiagnosticsPath, diagnostic); temporaryDiagnostics.push({ path: diagnosticPath, language }); } } -function addNoLanguageDiagnostic(config, diagnostic, fs33 = nodefs) { +function addNoLanguageDiagnostic(config, diagnostic) { if (config !== void 0) { addDiagnostic( config, @@ -173028,16 +173022,12 @@ function addNoLanguageDiagnostic(config, diagnostic, fs33 = nodefs) { ); } else { const tempDiagnosticsPath = getTempDiagnosticPath(getTemporaryDirectory()); - const diagnosticPath = writeDiagnosticFile( - fs33, - tempDiagnosticsPath, - diagnostic - ); + const diagnosticPath = writeDiagnosticFile(tempDiagnosticsPath, diagnostic); temporaryDiagnostics.push({ path: diagnosticPath }); } } -function writeDiagnosticFile(fs33, diagnosticsPath, diagnostic) { - fs33.mkdirSync(diagnosticsPath, { recursive: true }); +function writeDiagnosticFile(diagnosticsPath, diagnostic) { + fs7.mkdirSync(diagnosticsPath, { recursive: true }); const uniqueSuffix = (diagnosticCounter++).toString(); const sanitizedTimestamp = diagnostic.timestamp.replace( /[^a-zA-Z0-9.-]/g, @@ -173047,7 +173037,7 @@ function writeDiagnosticFile(fs33, diagnosticsPath, diagnostic) { diagnosticsPath, `codeql-action-${sanitizedTimestamp}-${uniqueSuffix}.json` ); - fs33.writeFileSync(jsonPath, JSON.stringify(diagnostic)); + fs7.writeFileSync(jsonPath, JSON.stringify(diagnostic)); return jsonPath; } function getDatabaseDiagnosticsPath(config, language) { @@ -173056,7 +173046,7 @@ function getDatabaseDiagnosticsPath(config, language) { function writeDiagnostic(action, config, language, diagnostic) { const diagnosticsPath = getDatabaseDiagnosticsPath(config, language); try { - writeDiagnosticFile(action.fs, diagnosticsPath, diagnostic); + writeDiagnosticFile(diagnosticsPath, diagnostic); } catch (err) { action.logger.warning( `Unable to write diagnostic message to database: ${err}` @@ -173071,7 +173061,7 @@ function logTemporaryDiagnostics(action) { `${num} diagnostic(s) could not be written to the database and will not appear on the Tool Status Page.` ); for (const temporary of temporaryDiagnostics) { - action.logger.debug(action.fs.readFileSync(temporary.path, "utf-8")); + action.logger.debug(fs7.readFileSync(temporary.path, "utf-8")); } } } @@ -173089,8 +173079,8 @@ function flushDiagnostics(action, config) { const filename = import_path2.default.basename(temporary.path); const destination = import_path2.default.join(directory, filename); try { - action.fs.mkdirSync(directory, { recursive: true }); - action.fs.renameSync(temporary.path, destination); + fs7.mkdirSync(directory, { recursive: true }); + fs7.renameSync(temporary.path, destination); } catch (err) { action.logger.warning( `Failed to rename '${temporary.path}' to '${destination}': ${getErrorMessage(err)}` diff --git a/src/diagnostics.test.ts b/src/diagnostics.test.ts index 7d898defc..29e2ffdc5 100644 --- a/src/diagnostics.test.ts +++ b/src/diagnostics.test.ts @@ -1,11 +1,11 @@ import * as fs from "fs"; +import fsModule from "fs"; import path from "path"; import test from "ava"; import * as sinon from "sinon"; import * as diagnostics from "./diagnostics"; -import type { FileSystem } from "./fs"; import { BuiltInLanguage } from "./languages"; import { checkExpectedLogMessages, @@ -17,37 +17,6 @@ import { setupTests(test); -type TestFS = FileSystem< - "existsSync" | "mkdirSync" | "writeFileSync" | "readFileSync" | "renameSync" ->; - -function makeStubFS() { - // The functions in `fs` cannot be stubbed. Create a new, minimal object - // that we can stub. - const stubbedFS: TestFS = { - existsSync: fs.existsSync, - mkdirSync: fs.mkdirSync, - writeFileSync: fs.writeFileSync, - readFileSync: fs.readFileSync, - renameSync: fs.renameSync, - }; - - const existsSync = sinon.stub(stubbedFS, "existsSync"); - const mkdirSync = sinon.stub(stubbedFS, "mkdirSync"); - const writeFileSync = sinon.stub(stubbedFS, "writeFileSync"); - const readFileSync = sinon.stub(stubbedFS, "readFileSync"); - const renameSync = sinon.stub(stubbedFS, "renameSync"); - - return { - stubbedFS, - existsSync, - mkdirSync, - writeFileSync, - readFileSync, - renameSync, - }; -} - const id = "codeql-action/test-diagnostic"; const name = "Test title"; @@ -61,14 +30,16 @@ test("makeDiagnostic - adds expected properties", (t) => { test("addDiagnostic writes temporary diagnostics and flushDiagnostics moves them", (t) => { const logger = new RecordingLogger(); - const { stubbedFS, existsSync, mkdirSync, writeFileSync, renameSync } = - makeStubFS(); let databasePath: fs.PathLike | undefined; - existsSync.callsFake((p) => { + + const existsSync = sinon.stub(fsModule, "existsSync").callsFake((p) => { databasePath = p; return false; }); + const mkdirSync = sinon.stub(fsModule, "mkdirSync").callThrough(); + const renameSync = sinon.stub(fsModule, "renameSync").callThrough(); + const writeFileSync = sinon.stub(fsModule, "writeFileSync").callThrough(); const diagnostic = diagnostics.makeDiagnostic(id, name, {}); t.notThrows(() => { @@ -77,10 +48,9 @@ test("addDiagnostic writes temporary diagnostics and flushDiagnostics moves them BuiltInLanguage.actions, diagnostic, logger, - stubbedFS as FileSystem, ); diagnostics.flushDiagnostics( - initAllState({ logger, fs: stubbedFS as FileSystem }), + initAllState({ logger }), createTestConfig({ tempDir: path.resolve("/temp/"), dbLocation: path.resolve("/test/database/"), diff --git a/src/diagnostics.ts b/src/diagnostics.ts index a29ef25ec..18996ca0b 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -1,10 +1,9 @@ -import * as nodefs from "fs"; +import * as fs from "fs"; import path from "path"; import type { ActionState } from "./action-common"; import { getTemporaryDirectory } from "./actions-util"; import type { Config } from "./config-utils"; -import type { FileSystem } from "./fs"; import { Language } from "./languages"; import { getActionsLogger, type Logger } from "./logging"; import { getCodeQLDatabasePath, getErrorMessage } from "./util"; @@ -161,7 +160,6 @@ export function addDiagnostic( language: Language, diagnostic: DiagnosticMessage, logger: Logger = getActionsLogger(), - fs: FileSystem = nodefs, ) { const databasePath = language ? getCodeQLDatabasePath(config, language) @@ -170,7 +168,7 @@ export function addDiagnostic( // Check that the database exists before writing to it. If the database does not yet exist, // store the diagnostic in memory and write it later. if (fs.existsSync(databasePath)) { - writeDiagnostic({ logger, fs }, config, language, diagnostic); + writeDiagnostic({ logger }, config, language, diagnostic); } else { logger.debug( `Writing a diagnostic for ${language}, but the database at ${databasePath} does not exist yet.`, @@ -178,11 +176,7 @@ export function addDiagnostic( // Write the diagnostic to a temporary location. const tempDiagnosticsPath = getTempDiagnosticPath(config.tempDir, language); - const diagnosticPath = writeDiagnosticFile( - fs, - tempDiagnosticsPath, - diagnostic, - ); + const diagnosticPath = writeDiagnosticFile(tempDiagnosticsPath, diagnostic); // Track the temporary file. temporaryDiagnostics.push({ path: diagnosticPath, language }); @@ -193,7 +187,6 @@ export function addDiagnostic( export function addNoLanguageDiagnostic( config: Config | undefined, diagnostic: DiagnosticMessage, - fs: FileSystem = nodefs, ) { if (config !== undefined) { addDiagnostic( @@ -205,11 +198,7 @@ export function addNoLanguageDiagnostic( ); } else { const tempDiagnosticsPath = getTempDiagnosticPath(getTemporaryDirectory()); - const diagnosticPath = writeDiagnosticFile( - fs, - tempDiagnosticsPath, - diagnostic, - ); + const diagnosticPath = writeDiagnosticFile(tempDiagnosticsPath, diagnostic); // Track the temporary file. temporaryDiagnostics.push({ path: diagnosticPath }); @@ -220,7 +209,6 @@ export function addNoLanguageDiagnostic( * Writes {@link diagnostic} to a file in {@link diagnosticsPath}. */ function writeDiagnosticFile( - fs: FileSystem<"mkdirSync" | "writeFileSync">, diagnosticsPath: string, diagnostic: DiagnosticMessage, ): string { @@ -262,7 +250,7 @@ export function getDatabaseDiagnosticsPath( * @param diagnostic The diagnostic message to add to the database. */ function writeDiagnostic( - action: ActionState<["Logger", "FS"]>, + action: ActionState<["Logger"]>, config: Config, language: Language | undefined, diagnostic: DiagnosticMessage, @@ -270,7 +258,7 @@ function writeDiagnostic( const diagnosticsPath = getDatabaseDiagnosticsPath(config, language); try { - writeDiagnosticFile(action.fs, diagnosticsPath, diagnostic); + writeDiagnosticFile(diagnosticsPath, diagnostic); } catch (err) { action.logger.warning( `Unable to write diagnostic message to database: ${err}`, @@ -280,7 +268,7 @@ function writeDiagnostic( } /** Report if there are temporary diagnostics and write them to the log. */ -export function logTemporaryDiagnostics(action: ActionState<["Logger", "FS"]>) { +export function logTemporaryDiagnostics(action: ActionState<["Logger"]>) { const num = temporaryDiagnostics.length; if (num > 0) { @@ -289,14 +277,14 @@ export function logTemporaryDiagnostics(action: ActionState<["Logger", "FS"]>) { ); for (const temporary of temporaryDiagnostics) { - action.logger.debug(action.fs.readFileSync(temporary.path, "utf-8")); + action.logger.debug(fs.readFileSync(temporary.path, "utf-8")); } } } /** Relocates all temporary diagnostics to the respective databases. */ export function flushDiagnostics( - action: ActionState<["Logger", "FS"]>, + action: ActionState<["Logger"]>, config: Config, ) { const diagnosticsCount = temporaryDiagnostics.length; @@ -317,8 +305,8 @@ export function flushDiagnostics( const destination = path.join(directory, filename); try { - action.fs.mkdirSync(directory, { recursive: true }); - action.fs.renameSync(temporary.path, destination); + fs.mkdirSync(directory, { recursive: true }); + fs.renameSync(temporary.path, destination); } catch (err) { action.logger.warning( `Failed to rename '${temporary.path}' to '${destination}': ${getErrorMessage(err)}`,