diff --git a/package-lock.json b/package-lock.json index fbc09d12..27495733 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1908,9 +1908,9 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "20.19.15", - "resolved": "https://registry.npmjs.org/@types/node/-/node-20.19.15.tgz", - "integrity": "sha512-W3bqcbLsRdFDVcmAM5l6oLlcl67vjevn8j1FPZ4nx+K5jNoWCh+FC/btxFoBPnvQlrHHDwfjp1kjIEDfwJ0Mog==", + "version": "20.19.17", + "resolved": "https://registry.npmjs.org/@types/node/-/node-20.19.17.tgz", + "integrity": "sha512-gfehUI8N1z92kygssiuWvLiwcbOB3IRktR6hTDgJlXMYh5OvkPSRmgfoBUmfZt+vhwJtX7v1Yw4KvvAf7c5QKQ==", "license": "MIT", "dependencies": { "undici-types": "~6.21.0" @@ -1936,13 +1936,6 @@ "devOptional": true, "license": "MIT" }, - "node_modules/@types/tmp": { - "version": "0.2.6", - "resolved": "https://registry.npmjs.org/@types/tmp/-/tmp-0.2.6.tgz", - "integrity": "sha512-chhaNf2oKHlRkDGt+tiKE2Z5aJ6qalm7Z9rlLdBwmOiAAf09YQvvoLXjWK4HWPF1xU/fqvMgfNfpVoBscA/tKA==", - "dev": true, - "license": "MIT" - }, "node_modules/@types/unzipper": { "version": "0.10.11", "resolved": "https://registry.npmjs.org/@types/unzipper/-/unzipper-0.10.11.tgz", @@ -2853,9 +2846,9 @@ "license": "MIT" }, "node_modules/baseline-browser-mapping": { - "version": "2.8.4", - "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.8.4.tgz", - "integrity": "sha512-L+YvJwGAgwJBV1p6ffpSTa2KRc69EeeYGYjRVWKs0GKrK+LON0GC0gV+rKSNtALEDvMDqkvCFq9r1r94/Gjwxw==", + "version": "2.8.6", + "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.8.6.tgz", + "integrity": "sha512-wrH5NNqren/QMtKUEEJf7z86YjfqW/2uw3IL3/xpqZUC95SSVIFXYQeeGjL6FT/X68IROu6RMehZQS5foy2BXw==", "license": "Apache-2.0", "bin": { "baseline-browser-mapping": "dist/cli.js" @@ -3463,9 +3456,9 @@ "license": "MIT" }, "node_modules/electron-to-chromium": { - "version": "1.5.218", - "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.218.tgz", - "integrity": "sha512-uwwdN0TUHs8u6iRgN8vKeWZMRll4gBkz+QMqdS7DDe49uiK68/UX92lFb61oiFPrpYZNeZIqa4bA7O6Aiasnzg==", + "version": "1.5.222", + "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.222.tgz", + "integrity": "sha512-gA7psSwSwQRE60CEoLz6JBCQPIxNeuzB2nL8vE03GK/OHxlvykbLyeiumQy1iH5C2f3YbRAZpGCMT12a/9ih9w==", "license": "ISC" }, "node_modules/emittery": { @@ -7950,15 +7943,6 @@ "integrity": "sha512-N+8UisAXDGk8PFXP4HAzVR9nbfmVJ3zYLAWiTIoqC5v5isinhr+r5uaO8+7r3BMfuNIufIsA7RdpVgacC2cSpw==", "license": "MIT" }, - "node_modules/tmp": { - "version": "0.2.5", - "resolved": "https://registry.npmjs.org/tmp/-/tmp-0.2.5.tgz", - "integrity": "sha512-voyz6MApa1rQGUxT3E+BK7/ROe8itEx7vD8/HEvt4xwXucvQ5G5oeEiHkmHZJuBO21RpOf+YYm9MOivj709jow==", - "license": "MIT", - "engines": { - "node": ">=14.14" - } - }, "node_modules/tmpl": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/tmpl/-/tmpl-1.0.5.tgz", @@ -7991,9 +7975,9 @@ } }, "node_modules/ts-jest": { - "version": "29.4.2", - "resolved": "https://registry.npmjs.org/ts-jest/-/ts-jest-29.4.2.tgz", - "integrity": "sha512-pBNOkn4HtuLpNrXTMVRC9b642CBaDnKqWXny4OzuoULT9S7Kf8MMlaRe2veKax12rjf5WcpMBhVPbQurlWGNxA==", + "version": "29.4.3", + "resolved": "https://registry.npmjs.org/ts-jest/-/ts-jest-29.4.3.tgz", + "integrity": "sha512-KTWbK2Wot8VXargsLoxhSoEQ9OyMdzQXQoUDeIulWu2Tf7gghuBHeg+agZqVLdTOHhQHVKAaeuctBDRkhWE7hg==", "dev": true, "license": "MIT", "dependencies": { @@ -8774,7 +8758,6 @@ "csv-stringify": "^6.6.0", "js-yaml": "^4.1.0", "semver": "^7.7.2", - "tmp": "^0.2.5", "xmlbuilder": "^15.1.1" }, "devDependencies": { @@ -8783,7 +8766,6 @@ "@types/js-yaml": "^4.0.9", "@types/sarif": "^2.1.7", "@types/semver": "^7.7.1", - "@types/tmp": "^0.2.6", "cross-env": "^10.0.0", "eslint": "^9.35.0", "jest": "^30.1.3", diff --git a/packages/code-analyzer-core/package.json b/packages/code-analyzer-core/package.json index 25962027..db57158b 100644 --- a/packages/code-analyzer-core/package.json +++ b/packages/code-analyzer-core/package.json @@ -21,8 +21,7 @@ "csv-stringify": "^6.6.0", "js-yaml": "^4.1.0", "semver": "^7.7.2", - "xmlbuilder": "^15.1.1", - "tmp": "^0.2.5" + "xmlbuilder": "^15.1.1" }, "devDependencies": { "@eslint/js": "^9.35.0", @@ -30,7 +29,6 @@ "@types/jest": "^30.0.0", "@types/sarif": "^2.1.7", "@types/semver": "^7.7.1", - "@types/tmp": "^0.2.6", "cross-env": "^10.0.0", "eslint": "^9.35.0", "jest": "^30.1.3", diff --git a/packages/code-analyzer-core/src/code-analyzer.ts b/packages/code-analyzer-core/src/code-analyzer.ts index 007c95bf..10421f91 100644 --- a/packages/code-analyzer-core/src/code-analyzer.ts +++ b/packages/code-analyzer-core/src/code-analyzer.ts @@ -25,7 +25,9 @@ import {Clock, RealClock} from '@salesforce/code-analyzer-engine-api/utils'; import {EventEmitter} from "node:events"; import {CodeAnalyzerConfig, ConfigDescription, EngineOverrides, FIELDS, RuleOverride} from "./config"; import { - EngineProgressAggregator, RuntimeTempFolder, + EngineProgressAggregator, + FileSystem, + RealFileSystem, RuntimeUniqueIdGenerator, TempFolder, toAbsolutePath, @@ -99,8 +101,8 @@ const MINIMUM_SUPPORTED_NODE = 20; */ export class CodeAnalyzer { private readonly config: CodeAnalyzerConfig; + private readonly tempFolder: TempFolder; private clock: Clock = new RealClock(); - private tempFolder: TempFolder = new RuntimeTempFolder(); private uniqueIdGenerator: UniqueIdGenerator = new RuntimeUniqueIdGenerator(); private readonly eventEmitter: EventEmitter = new EventEmitter(); private readonly engines: Map = new Map(); @@ -110,9 +112,15 @@ export class CodeAnalyzer { private readonly rulesCache: Map = new Map(); private readonly engineRuleDiscoveryProgressAggregator: EngineProgressAggregator = new EngineProgressAggregator(); - constructor(config: CodeAnalyzerConfig, version: string = process.version) { - this.validateEnvironment(version); + constructor(config: CodeAnalyzerConfig, fileSystem: FileSystem = new RealFileSystem(), nodeVersion: string = process.version) { + this.validateEnvironment(nodeVersion); this.config = config; + this.tempFolder = new TempFolder(fileSystem); + /* istanbul ignore next */ + process.addListener('exit', async () => { + // Note that on node exit there is no more event loop, so removal must take place synchronously + this.tempFolder.removeSyncIfNotKept(); + }); } private validateEnvironment(version: string): void { @@ -131,9 +139,6 @@ export class CodeAnalyzer { _setUniqueIdGenerator(uniqueIdGenerator: UniqueIdGenerator): void { this.uniqueIdGenerator = uniqueIdGenerator; } - _setTempFolder(tempFolder: TempFolder): void { - this.tempFolder = tempFolder; - } /** * Convenience method to return the same CodeAnalyzerConfig instance that was provided to the constructor @@ -306,21 +311,37 @@ export class CodeAnalyzer { // called a second time before the first call to run hasn't finished. This can occur if someone builds // up a bunch of RunResults promises and then does a Promise.all on them. Otherwise, the progress events may // override each other. - const runWorkingFolderName: string = `code-analyzer-run-${this.clock.formatToDateTimeString()}`; this.emitLogEvent(LogLevel.Debug, getMessage('RunningWithWorkspace', JSON.stringify({ filesAndFolders: runOptions.workspace.getRawFilesAndFolders(), targets: runOptions.workspace.getRawTargets() }))); - const runPromises: Promise[] = ruleSelection.getEngineNames().map( - async (engineName) => this.runEngineAndValidateResults(engineName, ruleSelection, { + const engApiWorkspace: engApi.Workspace = toEngApiWorkspace(runOptions.workspace); + const runWorkingFolderName: string = `run-${this.clock.formatToDateTimeString()}`; + await this.tempFolder.makeSubfolder(runWorkingFolderName); + + const runPromises: Promise[] = ruleSelection.getEngineNames().map(async (engineName) => { + const workingFolder: string = await this.tempFolder.makeSubfolder(runWorkingFolderName, engineName); + const engineRunOptions: engApi.RunOptions = { logFolder: this.config.getLogFolder(), - workingFolder: await this.tempFolder.createSubfolder(runWorkingFolderName, engineName), - workspace: toEngApiWorkspace(runOptions.workspace) - })); + workingFolder: workingFolder, + workspace: engApiWorkspace + }; + const errorCallback: () => void = () => { + if (!this.tempFolder.isKept(runWorkingFolderName, engineName)) { + this.emitLogEvent(LogLevel.Debug, getMessage('EngineWorkingFolderKept', engineName, workingFolder)); + this.tempFolder.markToBeKept(runWorkingFolderName, engineName); + } + }; + const results: EngineRunResults = await this.runEngineAndValidateResults(engineName, ruleSelection, engineRunOptions, errorCallback); + await this.tempFolder.removeIfNotKept(runWorkingFolderName, engineName); + return results; + }); const engineRunResultsList: EngineRunResults[] = await Promise.all(runPromises); + await this.tempFolder.removeIfNotKept(runWorkingFolderName); + const runResults: RunResultsImpl = new RunResultsImpl(this.clock); for (const engineRunResults of engineRunResultsList) { runResults.addEngineRunResults(engineRunResults); @@ -344,32 +365,62 @@ export class CodeAnalyzer { private async getAllRules(workspace?: Workspace): Promise { const cacheKey: string = workspace ? workspace.getWorkspaceId() : process.cwd(); - const describeWorkingFolderName: string = `code-analyzer-describe-${this.clock.formatToDateTimeString()}`; if (!this.rulesCache.has(cacheKey)) { this.engineRuleDiscoveryProgressAggregator.reset(this.getEngineNames()); const engApiWorkspace: engApi.Workspace | undefined = workspace ? toEngApiWorkspace(workspace) : undefined; - const rulePromises: Promise[] = this.getEngineNames().map(async (engineName) => - this.getAllRulesFor(engineName, { + const rulesWorkingFolderName: string = `rules-${this.clock.formatToDateTimeString()}`; + + await this.tempFolder.makeSubfolder(rulesWorkingFolderName); + + const rulePromises: Promise[] = this.getEngineNames().map(async (engineName) => { + const workingFolder: string = await this.tempFolder.makeSubfolder(rulesWorkingFolderName, engineName); + const describeOptions: engApi.DescribeOptions = { workspace: engApiWorkspace, - workingFolder: await this.tempFolder.createSubfolder(describeWorkingFolderName, engineName), + workingFolder: workingFolder, logFolder: this.config.getLogFolder() - })); + }; + const errorCallback: () => void = () => { + if (!this.tempFolder.isKept(rulesWorkingFolderName, engineName)) { + this.emitLogEvent(LogLevel.Debug, getMessage('EngineWorkingFolderKept', engineName, workingFolder)); + this.tempFolder.markToBeKept(rulesWorkingFolderName, engineName); + } + }; + const rules: RuleImpl[] = await this.getAllRulesFor(engineName, describeOptions, errorCallback); + await this.tempFolder.removeIfNotKept(rulesWorkingFolderName, engineName); + return rules; + }); + this.rulesCache.set(cacheKey, (await Promise.all(rulePromises)).flat()); + + await this.tempFolder.removeIfNotKept(rulesWorkingFolderName); } return this.rulesCache.get(cacheKey)!; } - private async getAllRulesFor(engineName: string, describeOptions: engApi.DescribeOptions): Promise { + private async getAllRulesFor(engineName: string, describeOptions: engApi.DescribeOptions, errorCallback: () => void): Promise { this.emitLogEvent(LogLevel.Debug, getMessage('GatheringRulesFromEngine', engineName)); + const invokeErrorCallbackIfErrorIsLoggedFcn = (event: engApi.LogEvent) => { + if (event.logLevel === engApi.LogLevel.Error) { + errorCallback(); + } + }; + + const engine: engApi.Engine = this.getEngine(engineName); + engine.onEvent(engApi.EventType.LogEvent, invokeErrorCallbackIfErrorIsLoggedFcn); + let ruleDescriptions: engApi.RuleDescription[] = []; try { - ruleDescriptions = await this.getEngine(engineName).describeRules(describeOptions); + ruleDescriptions = await engine.describeRules(describeOptions); } catch (err) { + errorCallback(); this.uninstantiableEnginesMap.set(engineName, err as Error); this.emitLogEvent(LogLevel.Error, getMessage('PluginErrorWhenGettingRules', engineName, (err as Error).message + '\n\n' + getMessage('InstructionsToIgnoreErrorAndDisableEngine', engineName))); return []; + } finally { + engine.removeEventListener(engApi.EventType.LogEvent, invokeErrorCallbackIfErrorIsLoggedFcn); } + this.emitLogEvent(LogLevel.Debug, getMessage('FinishedGatheringRulesFromEngine', ruleDescriptions.length, engineName)); validateRuleDescriptions(ruleDescriptions, engineName); @@ -385,20 +436,29 @@ export class CodeAnalyzer { this.emitEvent({type: EventType.RuleSelectionProgressEvent, timestamp: this.clock.now(), percentComplete: aggregatedPerc}); } - private async runEngineAndValidateResults(engineName: string, ruleSelection: RuleSelection, engineRunOptions: engApi.RunOptions): Promise { + private async runEngineAndValidateResults(engineName: string, ruleSelection: RuleSelection, engineRunOptions: engApi.RunOptions, errorCallback: () => void): Promise { this.emitEvent({ type: EventType.EngineRunProgressEvent, timestamp: this.clock.now(), engineName: engineName, percentComplete: 0 }); - const rulesToRun: string[] = ruleSelection.getRulesFor(engineName).map(r => r.getName()); + this.emitLogEvent(LogLevel.Debug, getMessage('RunningEngineWithRules', engineName, JSON.stringify(rulesToRun))); + const invokeErrorCallbackIfErrorIsLoggedFcn = (event: engApi.LogEvent) => { + if (event.logLevel === engApi.LogLevel.Error) { + errorCallback(); + } + }; const engine: engApi.Engine = this.getEngine(engineName); + engine.onEvent(engApi.EventType.LogEvent, invokeErrorCallbackIfErrorIsLoggedFcn); let apiEngineRunResults: engApi.EngineRunResults; try { apiEngineRunResults = await engine.runRules(rulesToRun, engineRunOptions); } catch (error) { + errorCallback(); return new UnexpectedErrorEngineRunResults(engineName, await engine.getEngineVersion(), error as Error); + } finally { + engine.removeEventListener(engApi.EventType.LogEvent, invokeErrorCallbackIfErrorIsLoggedFcn); } validateEngineRunResults(engineName, apiEngineRunResults, ruleSelection); diff --git a/packages/code-analyzer-core/src/messages.ts b/packages/code-analyzer-core/src/messages.ts index 097687a8..47089666 100644 --- a/packages/code-analyzer-core/src/messages.ts +++ b/packages/code-analyzer-core/src/messages.ts @@ -193,6 +193,8 @@ const MESSAGE_CATALOG : MessageCatalog = { EngineReturnedViolationWithCodeLocationWithEndColumnBeforeStartColumnOnSameLine: `Engine failure. The engine '%s' returned a violation for rule '%s' that contains a code location with the endLine equal to the startLine and the endColumn %d before the startColumn %d.`, + EngineWorkingFolderKept: + `Since the engine '%s' emitted an error, the following temporary working folder will not be removed: %s` } /** diff --git a/packages/code-analyzer-core/src/utils.ts b/packages/code-analyzer-core/src/utils.ts index 71c905dd..3ad3e0bf 100644 --- a/packages/code-analyzer-core/src/utils.ts +++ b/packages/code-analyzer-core/src/utils.ts @@ -1,15 +1,11 @@ -import * as tmp from 'tmp'; -import * as path from "node:path"; -import crypto from "node:crypto"; +import * as crypto from "node:crypto"; import * as fs from "node:fs"; -import {promisify} from "node:util"; +import * as os from "node:os"; +import * as path from "node:path"; // THIS FILE CONTAINS UTILITIES WHICH ARE USED INTERNALLY ONLY. // None of the following exported interfaces and functions should be exported from the index file. -tmp.setGracefulCleanup(); -const tmpDirAsync = promisify((options: tmp.DirOptions, cb: tmp.DirCallback) => tmp.dir(options, cb)); - export function toAbsolutePath(fileOrFolder: string): string { // Convert slashes to platform specific slashes and then convert to absolute path @@ -34,25 +30,82 @@ export class RuntimeUniqueIdGenerator implements UniqueIdGenerator { } } -export interface TempFolder { - getPath(): Promise; - createSubfolder(...subfolderPathSegs: string[]): Promise; +export interface FileSystem { + mkdir(absPath: fs.PathLike, options?: fs.MakeDirectoryOptions): Promise + + mkdtemp(prefix: string): Promise + + rm(absPath: fs.PathLike, options?: fs.RmOptions): Promise + + rmSync(absPath: fs.PathLike, options?: fs.RmOptions): void +} + +export class RealFileSystem implements FileSystem { + mkdir(absPath: fs.PathLike, options?: fs.MakeDirectoryOptions): Promise { + return fs.promises.mkdir(absPath, options); + } + + mkdtemp(prefix: string): Promise { + return fs.promises.mkdtemp(prefix); + } + + rm(absPath: fs.PathLike, options?: fs.RmOptions): Promise { + return fs.promises.rm(absPath, options); + } + + rmSync(absPath: fs.PathLike, options?: fs.RmOptions): void { + return fs.rmSync(absPath, options); + } } -export class RuntimeTempFolder implements TempFolder { +export class TempFolder { + private readonly fileSystem: FileSystem; + private readonly rootFolderPrefix: string; private rootFolder?: string; + private relPathsToKeep: Set = new Set(); - async getPath(): Promise { + constructor(fileSystem: FileSystem = new RealFileSystem(), rootFolderPrefix: string = path.join(os.tmpdir(), 'code-analyzer-')) { + this.fileSystem = fileSystem; + this.rootFolderPrefix = rootFolderPrefix; + } + + async getPath(...subfolderPathSegments: string[]): Promise { if (!this.rootFolder) { - this.rootFolder = await tmpDirAsync({keep: false, unsafeCleanup: true}); + this.rootFolder = await this.fileSystem.mkdtemp(this.rootFolderPrefix); + } + return path.join(this.rootFolder,...subfolderPathSegments); + } + + async makeSubfolder(firstSubfolderPathSegment: string, ...otherSubfolderPathSegments: string[]): Promise { + const absSubfolderPath: string = await this.getPath(firstSubfolderPathSegment, ...otherSubfolderPathSegments); + await this.fileSystem.mkdir(absSubfolderPath, {recursive: true}); + return absSubfolderPath; + } + + markToBeKept(...subfolderPathSegments: string[]): void { + this.relPathsToKeep.add(path.join(...subfolderPathSegments)); + if (subfolderPathSegments.length !== 0) { + this.markToBeKept(...subfolderPathSegments.slice(0, -1)); } - return this.rootFolder; } - async createSubfolder(...subFolderPathSegs: string[]): Promise { - const absPathToSubFolder: string = path.join(await this.getPath(), ...subFolderPathSegs); - await fs.promises.mkdir(absPathToSubFolder, {recursive: true}); - return absPathToSubFolder; + isKept(...subfolderPathSegments: string[]): boolean { + return this.relPathsToKeep.has(path.join(...subfolderPathSegments)); + } + + async removeIfNotKept(...subfolderPathSegments: string[]): Promise { + if (!this.isKept(...subfolderPathSegments)) { + const absPath: string = await this.getPath(...subfolderPathSegments); + await this.fileSystem.rm(absPath, {recursive: true, force: true}); + } + } + + // Note this sync version exists since, we must have a sync version for final removal of the temp folder if done + // with process.on('exit',...) because on exit there is no event loop. + removeSyncIfNotKept(): void { + if (!this.isKept() && this.rootFolder) { + this.fileSystem.rmSync(this.rootFolder, {recursive: true, force: true}); + } } } @@ -130,4 +183,4 @@ export function deepEquals(value1: unknown, value2: unknown): boolean { // For all other types (number, string, boolean, etc.), use strict equality return false; -} \ No newline at end of file +} diff --git a/packages/code-analyzer-core/test/add-engines.test.ts b/packages/code-analyzer-core/test/add-engines.test.ts index bdb2a202..9d6dd2c3 100644 --- a/packages/code-analyzer-core/test/add-engines.test.ts +++ b/packages/code-analyzer-core/test/add-engines.test.ts @@ -1,7 +1,7 @@ import {CodeAnalyzer, CodeAnalyzerConfig, ConfigDescription, ConfigFieldDescription, EventType, LogEvent, LogLevel} from "../src"; import * as stubs from "./stubs"; import {getMessage} from "../src/messages"; -import {changeWorkingDirectoryToPackageRoot} from "./test-helpers"; +import {changeWorkingDirectoryToPackageRoot, FakeFileSystem, FixedUniqueIdGenerator} from "./test-helpers"; import path from "node:path"; import {StubEngine1, StubEngine2, StubEngine3, ThrowingPlugin2} from "./stubs"; import * as engApi from "@salesforce/code-analyzer-engine-api"; @@ -13,11 +13,20 @@ const DEFAULT_CONFIG_ROOT: string = process.cwd(); const TEST_DATA_DIR: string= path.resolve(__dirname, 'test-data'); describe("Tests for adding engines to Code Analyzer", () => { + let fileSystem: FakeFileSystem; let codeAnalyzer: CodeAnalyzer; let logEvents: LogEvent[]; + function createCodeAnalyzer(config: CodeAnalyzerConfig = CodeAnalyzerConfig.withDefaults()): CodeAnalyzer { + fileSystem = new FakeFileSystem(); + codeAnalyzer = new CodeAnalyzer(config, fileSystem); + codeAnalyzer._setUniqueIdGenerator(new FixedUniqueIdGenerator()); + return codeAnalyzer; + } + beforeEach(() => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.withDefaults()); + fileSystem = new FakeFileSystem(); + codeAnalyzer = createCodeAnalyzer(); logEvents = []; codeAnalyzer.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event)); }); @@ -51,7 +60,7 @@ describe("Tests for adding engines to Code Analyzer", () => { }); it('When adding engine plugin using non-default config then engines are correctly added with engine specific configurations', async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-02.Yml'))); + codeAnalyzer = createCodeAnalyzer(CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-02.Yml'))); const stubEnginePlugin: stubs.StubEnginePlugin = new stubs.StubEnginePlugin(); await codeAnalyzer.addEnginePlugin(stubEnginePlugin); @@ -136,7 +145,7 @@ describe("Tests for adding engines to Code Analyzer", () => { expect(codeAnalyzer.getEngineNames().sort()).toEqual([]); }) - it('When calling dynamicallyAddEnginePlugin on a module that has a createEnginePlugin function, then it is used to add create the plugin and then added', async () => { + it('When calling dynamicallyAddEnginePlugin on a module that has a createEnginePlugin function, then it is used to create the plugin and then add it', async () => { const pluginModulePath: string = require.resolve('./stubs'); await codeAnalyzer.dynamicallyAddEnginePlugin(pluginModulePath); expect(codeAnalyzer.getEngineNames().sort()).toEqual(["stubEngine1", "stubEngine2", "stubEngine3"]); @@ -160,7 +169,7 @@ describe("Tests for adding engines to Code Analyzer", () => { }); it('When an engine is disabled, then addEnginePlugin does not add that particular engine and gives debug log', async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-04.yml'))); + codeAnalyzer = createCodeAnalyzer(CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-04.yml'))); codeAnalyzer.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event)); const sampleTimestamp: Date = new Date(2024, 7, 30, 11, 14, 34, 567); codeAnalyzer._setClock(new FixedClock(sampleTimestamp)); @@ -210,7 +219,7 @@ describe("Tests for adding engines to Code Analyzer", () => { }); it('If an engine configuration value is overridden with a null value, then getEngineConfigDescription should be treated as if it has not been overridden', async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromObject({ + codeAnalyzer = createCodeAnalyzer(CodeAnalyzerConfig.fromObject({ engines: { stubEngine1: { misc_value: null @@ -240,7 +249,7 @@ describe("Tests for adding engines to Code Analyzer", () => { }); it('When engine is disabled, we can still access its unresolved user provided properties', async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-04.yml'))); + codeAnalyzer = createCodeAnalyzer(CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-04.yml'))); await codeAnalyzer.addEnginePlugin(new stubs.StubEnginePlugin()); expect(codeAnalyzer.getEngineConfig('stubEngine1')).toEqual({ disable_enginE: true, @@ -249,7 +258,7 @@ describe("Tests for adding engines to Code Analyzer", () => { }); it('When engine is disabled, we can still access its config description', async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-04.yml'))); + codeAnalyzer = createCodeAnalyzer(CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-04.yml'))); await codeAnalyzer.addEnginePlugin(new stubs.StubEnginePlugin()); expect(codeAnalyzer.getEngineConfigDescription('stubEngine1')).toEqual({ overview: "OverviewForStub1", diff --git a/packages/code-analyzer-core/test/code-analyzer.test.ts b/packages/code-analyzer-core/test/code-analyzer.test.ts index d7d030c7..58f124ce 100644 --- a/packages/code-analyzer-core/test/code-analyzer.test.ts +++ b/packages/code-analyzer-core/test/code-analyzer.test.ts @@ -19,13 +19,13 @@ import { } from "../src"; import * as stubs from "./stubs"; import {getMessage} from "../src/messages"; -import path from "node:path"; -import {changeWorkingDirectoryToPackageRoot, FixedUniqueIdGenerator, SimulatedTempFolder} from "./test-helpers"; +import * as os from "node:os"; +import * as path from "node:path"; +import {changeWorkingDirectoryToPackageRoot, FakeFileSystem, FixedUniqueIdGenerator} from "./test-helpers"; import * as engApi from "@salesforce/code-analyzer-engine-api" -import {FixedClock} from "@salesforce/code-analyzer-engine-api/utils" +import {Clock, FixedClock} from "@salesforce/code-analyzer-engine-api/utils" import {UnexpectedEngineErrorRule} from "../src/rules"; import {UndefinedCodeLocation} from "../src/results"; -import {StubWorkspace} from "./stubs"; changeWorkingDirectoryToPackageRoot(); @@ -37,7 +37,7 @@ describe("Tests for CodeAnalyzer constructor", () => { {version: 'v4.0.0'} // 4 is less than 20, but 4 is greater than 2. This is a classic trap for SemVer comparisons. ])("When supplied with a Node Version prior to v20, construction fails. Case: $version", ({version}) => { // Expect the construction to fail with an error message that mentions v20, the minimum compatible version. - expect(() => new CodeAnalyzer(CodeAnalyzerConfig.withDefaults(), version)).toThrow('v20'); + expect(() => new CodeAnalyzer(CodeAnalyzerConfig.withDefaults(), new FakeFileSystem(), version)).toThrow('v20'); }); it.each([ @@ -45,7 +45,7 @@ describe("Tests for CodeAnalyzer constructor", () => { {version: 'v21.0.0'}, {version: 'v100.0.0'} // 100 is greater than 20, but 1 is less than 2. This is a classic trap for SemVer comparisons. ])('When supplied with a Node Version of v20 or later, construction succeeds. Case: $version"', ({version}) => { - expect(new CodeAnalyzer(CodeAnalyzerConfig.withDefaults(), version)).toBeInstanceOf(CodeAnalyzer); + expect(new CodeAnalyzer(CodeAnalyzerConfig.withDefaults(), new FakeFileSystem(), version)).toBeInstanceOf(CodeAnalyzer); }); it("Constructor prepends the currently-running Node's parent folder to the PATH", () => { @@ -54,7 +54,7 @@ describe("Tests for CodeAnalyzer constructor", () => { const nodeParentDir: string = path.dirname(process.execPath); // Instantiate a Code Analyzer. - new CodeAnalyzer(CodeAnalyzerConfig.withDefaults()); + new CodeAnalyzer(CodeAnalyzerConfig.withDefaults(), new FakeFileSystem()); // Verify that the PATH was changed. expect(process.env.PATH).toEqual(`${nodeParentDir}${path.delimiter}${initialPath}`); @@ -66,13 +66,13 @@ describe("Tests for getConfig method", () => { CodeAnalyzerConfig.withDefaults(), CodeAnalyzerConfig.fromObject({log_folder: __dirname}) ])("When getConfig is called, it returns the exact CodeAnalyzerConfig that was passed to the constructor", (config: CodeAnalyzerConfig) => { - const codeAnalyzer: CodeAnalyzer = new CodeAnalyzer(config); + const codeAnalyzer: CodeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); expect(codeAnalyzer.getConfig()).toEqual(config); }); }); describe("Tests for the createWorkspace method", () => { - const codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.withDefaults()); + const codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.withDefaults(), new FakeFileSystem()); it('When creating multiple workspaces, then they each get a unique id', async () => { const workspace1: Workspace = await codeAnalyzer.createWorkspace([SAMPLE_WORKSPACE_FOLDER]); @@ -181,8 +181,9 @@ describe("Tests for the createWorkspace method", () => { describe("Tests for the run method of CodeAnalyzer", () => { let sampleRunOptions: RunOptions; + let fileSystem: FakeFileSystem; let sampleTimestamp: Date; - let simulatedTempFolder: SimulatedTempFolder; + let clock: Clock; let codeAnalyzer: CodeAnalyzer; let stubEngine1: stubs.StubEngine1; let stubEngine2: stubs.StubEngine2; @@ -190,13 +191,18 @@ describe("Tests for the run method of CodeAnalyzer", () => { const expectedStubEngine1RuleNames: string[] = ['stub1RuleA', 'stub1RuleB', 'stub1RuleC']; const expectedStubEngine2RuleNames: string[] = ['stub2RuleA', 'stub2RuleC']; - beforeEach(async () => { + function createCodeAnalyzer(config: CodeAnalyzerConfig = CodeAnalyzerConfig.withDefaults()) : CodeAnalyzer { + fileSystem = new FakeFileSystem(); + codeAnalyzer = new CodeAnalyzer(config, fileSystem); sampleTimestamp = new Date(); - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.withDefaults()); - codeAnalyzer._setClock(new FixedClock(sampleTimestamp)); - simulatedTempFolder = new SimulatedTempFolder(); - codeAnalyzer._setTempFolder(simulatedTempFolder); + clock = new FixedClock(sampleTimestamp); + codeAnalyzer._setClock(clock); codeAnalyzer._setUniqueIdGenerator(new FixedUniqueIdGenerator()); + return codeAnalyzer; + } + + beforeEach(async () => { + codeAnalyzer = createCodeAnalyzer(); sampleRunOptions = {workspace: await codeAnalyzer.createWorkspace([__dirname])}; const stubPlugin: stubs.StubEnginePlugin = new stubs.StubEnginePlugin(); await codeAnalyzer.addEnginePlugin(stubPlugin); @@ -211,16 +217,11 @@ describe("Tests for the run method of CodeAnalyzer", () => { path.join(SAMPLE_WORKSPACE_FOLDER, 'someFile.cls') ]), }); - - const tempSubfolders: Set = simulatedTempFolder.getCreatedSubfolders(); - - const stubEngine1WorkingFolder: string|undefined = [...tempSubfolders.keys()].find(f => { - return f.includes('code-analyzer-run') && f.includes('stubEngine1'); - }); - expect(stubEngine1WorkingFolder).toBeDefined(); + const expectedWorkingFolder1: string = path.join(os.tmpdir(), 'code-analyzer-0', + 'run-' + clock.formatToDateTimeString(), 'stubEngine1'); const expectedStub1EngineRunOptions: engApi.RunOptions = { logFolder: codeAnalyzer.getConfig().getLogFolder(), - workingFolder: stubEngine1WorkingFolder!, + workingFolder: expectedWorkingFolder1, workspace: new engApi.Workspace("FixedId", [SAMPLE_WORKSPACE_FOLDER], [ path.join(SAMPLE_WORKSPACE_FOLDER, 'someFile.cls')]) }; @@ -228,13 +229,11 @@ describe("Tests for the run method of CodeAnalyzer", () => { expect(stubEngine1.runRulesCallHistory[0].ruleNames).toEqual(expectedStubEngine1RuleNames); expectEquivalentRunOptions(stubEngine1.runRulesCallHistory[0].runOptions, expectedStub1EngineRunOptions); - const stubEngine2WorkingFolder: string|undefined = [...tempSubfolders.keys()].find(f => { - return f.includes('code-analyzer-run') && f.includes('stubEngine2'); - }); - expect(stubEngine2WorkingFolder).toBeDefined(); + const expectedWorkingFolder2: string = path.join(os.tmpdir(), 'code-analyzer-0', + 'run-' + clock.formatToDateTimeString(), 'stubEngine2'); const expectedStub2EngineRunOptions: engApi.RunOptions = { logFolder: codeAnalyzer.getConfig().getLogFolder(), - workingFolder: stubEngine2WorkingFolder!, + workingFolder: expectedWorkingFolder2, workspace: new engApi.Workspace("FixedId", [SAMPLE_WORKSPACE_FOLDER], [ path.join(SAMPLE_WORKSPACE_FOLDER, 'someFile.cls')]) }; @@ -244,22 +243,19 @@ describe("Tests for the run method of CodeAnalyzer", () => { }); it("When the workspace provided is one that is not constructed from CodeAnalyzer's createWorkspace method, then it should still work", async () => { - const dummyWorkspace: Workspace = new StubWorkspace(); + const dummyWorkspace: Workspace = new stubs.StubWorkspace(); await codeAnalyzer.run(selection, { workspace: dummyWorkspace }); - const tempSubfolders: Set = simulatedTempFolder.getCreatedSubfolders(); - const stubEngine1WorkingFolder: string|undefined = [...tempSubfolders.keys()].find(f => { - return f.includes('code-analyzer-run') && f.includes('stubEngine1'); - }); - expect(stubEngine1WorkingFolder).toBeDefined(); + const expectedWorkingFolder: string = path.join(os.tmpdir(), 'code-analyzer-0', + 'run-' + clock.formatToDateTimeString(), 'stubEngine1'); expect(stubEngine1.runRulesCallHistory).toEqual([{ ruleNames: expectedStubEngine1RuleNames, runOptions: { logFolder: codeAnalyzer.getConfig().getLogFolder(), - workingFolder: stubEngine1WorkingFolder, + workingFolder: expectedWorkingFolder, workspace: new engApi.Workspace(dummyWorkspace.getWorkspaceId(), dummyWorkspace.getRawFilesAndFolders(), dummyWorkspace.getRawTargets()) } @@ -270,14 +266,11 @@ describe("Tests for the run method of CodeAnalyzer", () => { selection = await codeAnalyzer.selectRules(['stubEngine1:Recommended']); await codeAnalyzer.run(selection, sampleRunOptions); - const tempSubfolders: Set = simulatedTempFolder.getCreatedSubfolders(); - const stubEngine1WorkingFolder: string|undefined = [...tempSubfolders.keys()].find(f => { - return f.includes('code-analyzer-run') && f.includes('stubEngine1'); - }); - expect(stubEngine1WorkingFolder).toBeDefined(); + const expectedWorkingFolder: string = path.join(os.tmpdir(), 'code-analyzer-0', + 'run-' + clock.formatToDateTimeString(), 'stubEngine1'); const expectedEngineRunOptions: engApi.RunOptions = { logFolder: codeAnalyzer.getConfig().getLogFolder(), - workingFolder: stubEngine1WorkingFolder!, + workingFolder: expectedWorkingFolder, workspace: new engApi.Workspace("FixedId", [__dirname]) }; expect(stubEngine1.runRulesCallHistory).toHaveLength(1); @@ -566,7 +559,7 @@ describe("Tests for the run method of CodeAnalyzer", () => { }); it("When an engine throws an exception when running, then a result is returned with a Critical violation of type UnexpectedError", async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.withDefaults()); + codeAnalyzer = createCodeAnalyzer(); await codeAnalyzer.addEnginePlugin(new stubs.ThrowingEnginePlugin()); selection = await codeAnalyzer.selectRules([]); const overallResults: RunResults = await codeAnalyzer.run(selection, sampleRunOptions); @@ -600,7 +593,7 @@ describe("Tests for the run method of CodeAnalyzer", () => { {plugin: new stubs.ThrowingPlugin4() as engApi.EnginePluginV1, msg: 'SomeErrorFromCreateEngine', case: 'error in #createEngine'}, {plugin: new stubs.ThrowingEnginePlugin2() as engApi.EnginePluginV1, msg: 'SomeErrorFromDescribeRules', case: 'error in #describeRules'} ])(`When an engine could not be instantiated, running rules produces a Critical violation of type UninstantiableEngineError. Case: $case`, async ({plugin, msg}) => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.withDefaults()); + codeAnalyzer = createCodeAnalyzer(); await codeAnalyzer.addEnginePlugin(plugin); selection = await codeAnalyzer.selectRules([]); const overallResults: RunResults = await codeAnalyzer.run(selection, sampleRunOptions); @@ -660,7 +653,7 @@ describe("Tests for the run method of CodeAnalyzer", () => { }); it('When running, any core level events that are greater than the user defined level should not be emitted', async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromObject({ + codeAnalyzer = createCodeAnalyzer(CodeAnalyzerConfig.fromObject({ log_level: LogLevel.Warn })); const stubPlugin: stubs.StubEnginePlugin = new stubs.StubEnginePlugin(); @@ -759,11 +752,9 @@ describe("Tests for the run method of CodeAnalyzer", () => { it("When running engines, then engine-specific log events are wired up and emitted fully from the engines when using fine level debugging", async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromObject({ + codeAnalyzer = createCodeAnalyzer(CodeAnalyzerConfig.fromObject({ log_level: LogLevel.Fine })); - codeAnalyzer._setClock(new FixedClock(sampleTimestamp)); - codeAnalyzer._setUniqueIdGenerator(new FixedUniqueIdGenerator()); const stubPlugin: stubs.StubEnginePlugin = new stubs.StubEnginePlugin(); await codeAnalyzer.addEnginePlugin(stubPlugin); @@ -796,7 +787,7 @@ describe("Tests for the run method of CodeAnalyzer", () => { }); it("When running engines, then engine-specific log events do not get emitted if they are greater than the user defined log level", async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromObject({ + codeAnalyzer = createCodeAnalyzer(CodeAnalyzerConfig.fromObject({ log_level: LogLevel.Error })); const stubPlugin: stubs.StubEnginePlugin = new stubs.StubEnginePlugin(); @@ -840,6 +831,107 @@ describe("Tests for the run method of CodeAnalyzer", () => { } }); }); + + it("When running rules, if no engine errors, then we fully remove the run working folder and its associated engine run folders", async () => { + await codeAnalyzer.run(selection, sampleRunOptions); + + const expectedRunWorkingFolderRoot: string = path.join(os.tmpdir(),'code-analyzer-0','run-' + clock.formatToDateTimeString()); + const expectedRunWorkingFolderForStubEngine1: string = path.join(expectedRunWorkingFolderRoot, 'stubEngine1'); + const expectedRunWorkingFolderForStubEngine2: string = path.join(expectedRunWorkingFolderRoot, 'stubEngine2'); + const expectedRunWorkingFolderForStubEngine3: string = path.join(expectedRunWorkingFolderRoot, 'stubEngine3'); + + // First confirm that the root folder and all 3 engines run working folders were created + const createdFolders: string[] = fileSystem.mkdirCallHistory.map(args => args.absPath.toString()); + expect(createdFolders).toContain(expectedRunWorkingFolderRoot); + expect(createdFolders).toContain(expectedRunWorkingFolderForStubEngine1); + expect(createdFolders).toContain(expectedRunWorkingFolderForStubEngine2); + expect(createdFolders).toContain(expectedRunWorkingFolderForStubEngine3); + + // Confirm that the root folder and all 3 engines run working folders were removed (because none of them errored during run) + const removedFolders: string[] = fileSystem.rmCallHistory.map(args => args.absPath.toString()); + expect(removedFolders).toContain(expectedRunWorkingFolderRoot); + expect(removedFolders).toContain(expectedRunWorkingFolderForStubEngine1); + expect(removedFolders).toContain(expectedRunWorkingFolderForStubEngine2); + expect(removedFolders).toContain(expectedRunWorkingFolderForStubEngine3); + + // Verify end result + expect(fileSystem.files).not.toContain(expectedRunWorkingFolderRoot); + expect(fileSystem.files).not.toContain(expectedRunWorkingFolderForStubEngine1); + expect(fileSystem.files).not.toContain(expectedRunWorkingFolderForStubEngine2); + expect(fileSystem.files).not.toContain(expectedRunWorkingFolderForStubEngine3); + }); + + it("When running rules, if an engine issues an error, then we preserve that run working folder and issue a log pointing to it", async () => { + codeAnalyzer = createCodeAnalyzer(); + const logEvents: LogEvent[] = []; + codeAnalyzer.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event)); + await codeAnalyzer.addEnginePlugin(new stubs.FlexibleEnginePlugin([new stubs.EngineWithRunMethodThatIssuesErrorLog()])); + selection = await codeAnalyzer.selectRules(['all']); + + await codeAnalyzer.run(selection, sampleRunOptions); + + const expectedRunWorkingFolderRoot: string = path.join(os.tmpdir(),'code-analyzer-0','run-' + clock.formatToDateTimeString()); + const expectedRunWorkingFolderForEngine: string = path.join(expectedRunWorkingFolderRoot, 'engineWithRunMethodThatIssuesErrorLog'); + + // First confirm that the root folder and the engine working folder were created + const createdFolders: string[] = fileSystem.mkdirCallHistory.map(args => args.absPath.toString()); + expect(createdFolders).toContain(expectedRunWorkingFolderRoot); + expect(createdFolders).toContain(expectedRunWorkingFolderForEngine); + + // Confirm that the root folder and the engine working folder was kept + const removedFolders: string[] = fileSystem.rmCallHistory.map(args => args.absPath.toString()); + expect(removedFolders).not.toContain(expectedRunWorkingFolderRoot); + expect(removedFolders).not.toContain(expectedRunWorkingFolderForEngine); + + // Verify end result + expect(fileSystem.files).toContain(expectedRunWorkingFolderRoot); + expect(fileSystem.files).toContain(expectedRunWorkingFolderForEngine); + + // Verify log lines + const relevantLogMsgs: string[] = logEvents.filter(e => e.logLevel === LogLevel.Debug && + e.message.includes('the following temporary working folder will not be removed')).map(e => e.message); + expect(relevantLogMsgs.filter(m => m.endsWith(expectedRunWorkingFolderForEngine))).toHaveLength(1); + }); + + it("When running rules, if an engine throws an exception, then we preserve that run working folder and issue a log pointing to it", async () => { + codeAnalyzer = createCodeAnalyzer(); + const logEvents: LogEvent[] = []; + codeAnalyzer.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event)); + await codeAnalyzer.addEnginePlugin(new stubs.FlexibleEnginePlugin([ + new stubs.ThrowingEngine({}), + new stubs.StubEngine2({}) + ])); + selection = await codeAnalyzer.selectRules(['all']); + + await codeAnalyzer.run(selection, sampleRunOptions); + + const expectedRunWorkingFolderRoot: string = path.join(os.tmpdir(),'code-analyzer-0','run-' + clock.formatToDateTimeString()); + const expectedRunWorkingFolderForThrowingEngine: string = path.join(expectedRunWorkingFolderRoot, 'throwingEngine'); + const expectedRunWorkingFolderForStubEngine2: string = path.join(expectedRunWorkingFolderRoot, 'stubEngine2'); + + // First confirm that the root folder and both engine working folders were created + const createdFolders: string[] = fileSystem.mkdirCallHistory.map(args => args.absPath.toString()); + expect(createdFolders).toContain(expectedRunWorkingFolderRoot); + expect(createdFolders).toContain(expectedRunWorkingFolderForThrowingEngine); + expect(createdFolders).toContain(expectedRunWorkingFolderForStubEngine2); + + // Confirm that the root folder and the throwing working folder were kept but the sub2 engine working folder was removed + const removedFolders: string[] = fileSystem.rmCallHistory.map(args => args.absPath.toString()); + expect(removedFolders).not.toContain(expectedRunWorkingFolderRoot); + expect(removedFolders).not.toContain(expectedRunWorkingFolderForThrowingEngine); + expect(removedFolders).toContain(expectedRunWorkingFolderForStubEngine2); + + // Verify end result + expect(fileSystem.files).toContain(expectedRunWorkingFolderRoot); + expect(fileSystem.files).toContain(expectedRunWorkingFolderForThrowingEngine); + expect(fileSystem.files).not.toContain(expectedRunWorkingFolderForStubEngine2); + + // Verify log lines + const relevantLogMsgs: string[] = logEvents.filter(e => e.logLevel === LogLevel.Debug && + e.message.includes('the following temporary working folder will not be removed')).map(e => e.message); + expect(relevantLogMsgs.filter(m => m.endsWith(expectedRunWorkingFolderForThrowingEngine))).toHaveLength(1); + expect(relevantLogMsgs.filter(m => m.endsWith(expectedRunWorkingFolderForStubEngine2))).toHaveLength(0); + }); }); function getAllSeverityLevels(): SeverityLevel[] { diff --git a/packages/code-analyzer-core/test/rule-selection.test.ts b/packages/code-analyzer-core/test/rule-selection.test.ts index c661a7cc..af66adb0 100644 --- a/packages/code-analyzer-core/test/rule-selection.test.ts +++ b/packages/code-analyzer-core/test/rule-selection.test.ts @@ -13,36 +13,40 @@ import { SeverityLevel } from "../src"; import * as engApi from "@salesforce/code-analyzer-engine-api" -import {FixedClock} from "@salesforce/code-analyzer-engine-api/utils"; -import {RepeatedRuleNameEnginePlugin, StubEnginePlugin} from "./stubs"; +import {Clock, FixedClock} from "@salesforce/code-analyzer-engine-api/utils"; import path from "node:path"; -import {changeWorkingDirectoryToPackageRoot, FixedUniqueIdGenerator, SimulatedTempFolder} from "./test-helpers"; +import {changeWorkingDirectoryToPackageRoot, FakeFileSystem, FixedUniqueIdGenerator} from "./test-helpers"; import {getMessage} from "../src/messages"; import * as stubs from "./stubs"; +import os from "node:os"; changeWorkingDirectoryToPackageRoot(); describe('Tests for selecting rules', () => { + let fileSystem: FakeFileSystem; let codeAnalyzer: CodeAnalyzer; - let plugin: StubEnginePlugin; + let plugin: stubs.StubEnginePlugin; let sampleTimestamp: Date; - let fixedClock: FixedClock; - let simulatedTempFolder: SimulatedTempFolder; + let clock: Clock; - async function setupCodeAnalyzer(codeAnalyzer: CodeAnalyzer) : Promise { - plugin = new StubEnginePlugin(); - await codeAnalyzer.addEnginePlugin(plugin); + function createCodeAnalyzer(config: CodeAnalyzerConfig = CodeAnalyzerConfig.withDefaults()): CodeAnalyzer { + fileSystem = new FakeFileSystem(); + codeAnalyzer = new CodeAnalyzer(config, fileSystem); + sampleTimestamp = new Date(); + clock = new FixedClock(sampleTimestamp); + codeAnalyzer._setClock(clock); codeAnalyzer._setUniqueIdGenerator(new FixedUniqueIdGenerator()); + return codeAnalyzer; + } + + async function setupCodeAnalyzerWithStubPlugin(config: CodeAnalyzerConfig = CodeAnalyzerConfig.withDefaults()): Promise { + codeAnalyzer = createCodeAnalyzer(config); + plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); } beforeEach(async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.withDefaults()); - await setupCodeAnalyzer(codeAnalyzer); - sampleTimestamp = new Date(); - fixedClock = new FixedClock(sampleTimestamp); - codeAnalyzer._setClock(fixedClock); - simulatedTempFolder = new SimulatedTempFolder(); - codeAnalyzer._setTempFolder(simulatedTempFolder); + await setupCodeAnalyzerWithStubPlugin(); }) it('When no rule selectors are provided then the Recommended tag is used', async () => { @@ -188,8 +192,7 @@ describe('Tests for selecting rules', () => { }); it('When config contains rule overrides for the selected rules, then the rule selection contains these overrides', async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromFile(path.resolve(__dirname, "test-data", "sample-config-01.yaml"))); - await setupCodeAnalyzer(codeAnalyzer); + await setupCodeAnalyzerWithStubPlugin(CodeAnalyzerConfig.fromFile(path.resolve(__dirname, "test-data", "sample-config-01.yaml"))); const selection: RuleSelection = await codeAnalyzer.selectRules([]); @@ -212,8 +215,7 @@ describe('Tests for selecting rules', () => { }); it('When config contains rule overrides, then we can select based on the new tags', async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromFile(path.resolve(__dirname, "test-data", "sample-config-01.yaml"))); - await setupCodeAnalyzer(codeAnalyzer); + await setupCodeAnalyzerWithStubPlugin(CodeAnalyzerConfig.fromFile(path.resolve(__dirname, "test-data", "sample-config-01.yaml"))); const selection: RuleSelection = await codeAnalyzer.selectRules(['SomeNewTag']); expect(ruleNamesFor(selection, 'stubEngine1')).toEqual([]); @@ -230,8 +232,7 @@ describe('Tests for selecting rules', () => { }); it('When config contains severity overrides, then we can select based on the severity values', async () => { - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromFile(path.resolve(__dirname, "test-data", "sample-config-01.yaml"))); - await setupCodeAnalyzer(codeAnalyzer); + await setupCodeAnalyzerWithStubPlugin(CodeAnalyzerConfig.fromFile(path.resolve(__dirname, "test-data", "sample-config-01.yaml"))); const selection: RuleSelection = await codeAnalyzer.selectRules(['5']); expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(['stub1RuleD']); @@ -240,7 +241,7 @@ describe('Tests for selecting rules', () => { it('When an engine fails to return its rules, an error is logged and empty results are returned', async () => { // ====== TEST SETUP ====== - codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.withDefaults()); + codeAnalyzer = createCodeAnalyzer(); await codeAnalyzer.addEnginePlugin(new stubs.ThrowingEnginePlugin2()); const logEvents: LogEvent[] = []; codeAnalyzer.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event)); @@ -266,7 +267,7 @@ describe('Tests for selecting rules', () => { }); it('When an engine returns multiple rules with the same name, then error', async () => { - await codeAnalyzer.addEnginePlugin(new RepeatedRuleNameEnginePlugin()); + await codeAnalyzer.addEnginePlugin(new stubs.RepeatedRuleNameEnginePlugin()); await expect(codeAnalyzer.selectRules([])).rejects.toThrow( getMessage('EngineReturnedMultipleRulesWithSameName', 'repeatedRuleNameEngine', 'repeatedRule')); }); @@ -274,27 +275,21 @@ describe('Tests for selecting rules', () => { it('When selectRules is not provided with SelectOptions, then workspace should be undefined for all engines', async () => { await codeAnalyzer.selectRules(['all']); - const tempSubfolders: Set = simulatedTempFolder.getCreatedSubfolders(); - - const stubEngine1WorkingFolder: string|undefined = [...tempSubfolders.keys()].find(f => { - return f.includes('code-analyzer-describe') && f.includes('stubEngine1'); - }); - expect(stubEngine1WorkingFolder).toBeDefined(); + const expectedWorkingFolder1: string = path.join(os.tmpdir(), 'code-analyzer-0', + 'rules-' + clock.formatToDateTimeString(), 'stubEngine1'); const expectedStub1DescribeOptions: engApi.DescribeOptions = { logFolder: codeAnalyzer.getConfig().getLogFolder(), - workingFolder: stubEngine1WorkingFolder!, + workingFolder: expectedWorkingFolder1, workspace: undefined }; const stubEngine1: stubs.StubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; expect(stubEngine1.describeRulesCallHistory).toEqual([{describeOptions: expectedStub1DescribeOptions}]); - const stubEngine2WorkingFolder: string|undefined = [...tempSubfolders.keys()].find(f => { - return f.includes('code-analyzer-describe') && f.includes('stubEngine2'); - }); - expect(stubEngine2WorkingFolder).toBeDefined(); + const expectedWorkingFolder2: string = path.join(os.tmpdir(), 'code-analyzer-0', + 'rules-' + clock.formatToDateTimeString(), 'stubEngine2'); const expectedStub2DescribeOptions: engApi.DescribeOptions = { logFolder: codeAnalyzer.getConfig().getLogFolder(), - workingFolder: stubEngine2WorkingFolder!, + workingFolder: expectedWorkingFolder2, workspace: undefined }; const stubEngine2: stubs.StubEngine2 = plugin.getCreatedEngine('stubEngine2') as stubs.StubEngine2; @@ -442,6 +437,99 @@ describe('Tests for selecting rules', () => { } }); }); + + it("When selecting rules, and one or more engines emit an error log, then their working folders are kept (and logged), but others are still removed", async () => { + const logEvents: LogEvent[] = []; + codeAnalyzer.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event)); + + await codeAnalyzer.selectRules(['all']); + + const expectedRulesWorkingFolderRoot: string = path.join(os.tmpdir(),'code-analyzer-0','rules-' + clock.formatToDateTimeString()); + const expectedRulesWorkingFolderForStubEngine1: string = path.join(expectedRulesWorkingFolderRoot, 'stubEngine1'); + const expectedRulesWorkingFolderForStubEngine2: string = path.join(expectedRulesWorkingFolderRoot, 'stubEngine2'); + const expectedRulesWorkingFolderForStubEngine3: string = path.join(expectedRulesWorkingFolderRoot, 'stubEngine3'); + + // First confirm that the root folder and all 3 engine rule working folders were created + const createdFolders: string[] = fileSystem.mkdirCallHistory.map(args => args.absPath.toString()); + expect(createdFolders).toContain(expectedRulesWorkingFolderRoot); + expect(createdFolders).toContain(expectedRulesWorkingFolderForStubEngine1); + expect(createdFolders).toContain(expectedRulesWorkingFolderForStubEngine2); + expect(createdFolders).toContain(expectedRulesWorkingFolderForStubEngine3); + + // Next confirm that the root folder and 2 of the engine working folders were kept while 1 was removed (because all issued errors except for stubEngine1) + const removedFolders: string[] = fileSystem.rmCallHistory.map(args => args.absPath.toString()); + expect(removedFolders).not.toContain(expectedRulesWorkingFolderRoot); + expect(removedFolders).toContain(expectedRulesWorkingFolderForStubEngine1); + expect(removedFolders).not.toContain(expectedRulesWorkingFolderForStubEngine2); + expect(removedFolders).not.toContain(expectedRulesWorkingFolderForStubEngine3); + + // Verify end result + expect(fileSystem.files).toContain(expectedRulesWorkingFolderRoot); + expect(fileSystem.files).not.toContain(expectedRulesWorkingFolderForStubEngine1); + expect(fileSystem.files).toContain(expectedRulesWorkingFolderForStubEngine2); + expect(fileSystem.files).toContain(expectedRulesWorkingFolderForStubEngine3); + + // Verify log lines + const relevantLogMsgs: string[] = logEvents.filter(e => e.logLevel === LogLevel.Debug && + e.message.includes('the following temporary working folder will not be removed')).map(e => e.message); + expect(relevantLogMsgs.filter(m => m.endsWith(expectedRulesWorkingFolderForStubEngine1))).toHaveLength(0); + expect(relevantLogMsgs.filter(m => m.endsWith(expectedRulesWorkingFolderForStubEngine2))).toHaveLength(1); + expect(relevantLogMsgs.filter(m => m.endsWith(expectedRulesWorkingFolderForStubEngine2))).toHaveLength(1); + }); + + it("When selecting rules, if no engine errors, then we fully remove the rules working folder", async () => { + codeAnalyzer = createCodeAnalyzer(); + await codeAnalyzer.addEnginePlugin(new stubs.EmptyTagEnginePlugin()); + + await codeAnalyzer.selectRules(['all']); + + const expectedRulesWorkingFolderRoot: string = path.join(os.tmpdir(),'code-analyzer-0','rules-' + clock.formatToDateTimeString()); + const expectedRulesWorkingFolderForEngine: string = path.join(expectedRulesWorkingFolderRoot, 'emptyTags'); + + // First confirm that the root folder and the engine folder were created + const createdFolders: string[] = fileSystem.mkdirCallHistory.map(args => args.absPath.toString()); + expect(createdFolders).toContain(expectedRulesWorkingFolderRoot); + expect(createdFolders).toContain(expectedRulesWorkingFolderForEngine); + + // Confirm folders were removed + const removedFolders: string[] = fileSystem.rmCallHistory.map(args => args.absPath.toString()); + expect(removedFolders).toContain(expectedRulesWorkingFolderRoot); + expect(removedFolders).toContain(expectedRulesWorkingFolderForEngine); + + // Verify end result + expect(fileSystem.files).not.toContain(expectedRulesWorkingFolderRoot); + expect(fileSystem.files).not.toContain(expectedRulesWorkingFolderForEngine); + }); + + it("When selecting rules, and an engine emit throws an exception, then we keep its rules working folder and log it", async () => { + codeAnalyzer = createCodeAnalyzer(); + const logEvents: LogEvent[] = []; + codeAnalyzer.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event)); + await codeAnalyzer.addEnginePlugin(new stubs.ThrowingEnginePlugin2()); + + await codeAnalyzer.selectRules(['all']); + + const expectedRulesWorkingFolderRoot: string = path.join(os.tmpdir(),'code-analyzer-0','rules-' + clock.formatToDateTimeString()); + const expectedRulesWorkingFolderForEngine: string = path.join(expectedRulesWorkingFolderRoot, 'someEngine'); + + // First confirm that the root folder and the engine folder were created + const createdFolders: string[] = fileSystem.mkdirCallHistory.map(args => args.absPath.toString()); + expect(createdFolders).toContain(expectedRulesWorkingFolderRoot); + expect(createdFolders).toContain(expectedRulesWorkingFolderForEngine); + + // Confirm nothing was removed + const removedFolders: string[] = fileSystem.rmCallHistory.map(args => args.absPath.toString()); + expect(removedFolders).toHaveLength(0); + + // Verify end result + expect(fileSystem.files).toContain(expectedRulesWorkingFolderRoot); + expect(fileSystem.files).toContain(expectedRulesWorkingFolderForEngine); + + // Verify log lines + const relevantLogMsgs: string[] = logEvents.filter(e => e.logLevel === LogLevel.Debug && + e.message.includes('the following temporary working folder will not be removed')).map(e => e.message); + expect(relevantLogMsgs.filter(m => m.endsWith(expectedRulesWorkingFolderForEngine))).toHaveLength(1); + }); }); diff --git a/packages/code-analyzer-core/test/stubs.ts b/packages/code-analyzer-core/test/stubs.ts index 1ffbc75e..cf5d3cde 100644 --- a/packages/code-analyzer-core/test/stubs.ts +++ b/packages/code-analyzer-core/test/stubs.ts @@ -1,4 +1,5 @@ import * as engApi from "@salesforce/code-analyzer-engine-api" +import {LogLevel} from "@salesforce/code-analyzer-engine-api" import {Workspace} from "../src"; import path from "node:path"; @@ -625,7 +626,7 @@ export class ThrowingEnginePlugin2 extends engApi.EnginePluginV1 { /** * ThrowingEngine - An engine that throws an error when ran */ -class ThrowingEngine extends StubEngine1 { +export class ThrowingEngine extends StubEngine1 { constructor(config: engApi.ConfigObject) { super(config); } @@ -713,3 +714,50 @@ class RepeatedRuleNameEngine extends engApi.Engine { return { violations: [] }; } } + + +export class FlexibleEnginePlugin extends engApi.EnginePluginV1 { + private readonly engines: engApi.Engine[]; + constructor(engines: engApi.Engine[]) { + super(); + this.engines = engines; + } + + getAvailableEngineNames(): string[] { + return this.engines.map(e => e.getName()); + } + + createEngine(engineName: string, _resolvedConfig: engApi.ConfigObject): Promise { + const engine: engApi.Engine | undefined = this.engines.find(e => e.getName() === engineName); + if (engine) { + return Promise.resolve(engine); + } + throw new Error(`No engine with name '${engineName}' found.`); + } +} + +export class EngineWithRunMethodThatIssuesErrorLog extends engApi.Engine { + getName(): string { + return 'engineWithRunMethodThatIssuesErrorLog'; + } + + describeRules(_describeOptions: engApi.DescribeOptions): Promise { + return Promise.resolve([{ + name: 'someRule', + severityLevel: 3, + tags: [], + description: 'someDescription', + resourceUrls: [] + }]); + } + + runRules(_ruleNames: string[], _runOptions: engApi.RunOptions): Promise { + this.emitLogEvent(LogLevel.Error, 'Some Error Log'); + return Promise.resolve({violations: []}); + } + + getEngineVersion(): Promise { + return Promise.resolve('1.0.0'); + } + +} diff --git a/packages/code-analyzer-core/test/test-helpers.ts b/packages/code-analyzer-core/test/test-helpers.ts index 57f54e05..c17cf380 100644 --- a/packages/code-analyzer-core/test/test-helpers.ts +++ b/packages/code-analyzer-core/test/test-helpers.ts @@ -1,6 +1,7 @@ import process from "node:process"; import path from "node:path"; -import {UniqueIdGenerator, TempFolder} from "../src/utils"; +import {UniqueIdGenerator, FileSystem} from "../src/utils"; +import * as fs from "fs"; export function changeWorkingDirectoryToPackageRoot() { let original_working_directory: string; @@ -11,7 +12,7 @@ export function changeWorkingDirectoryToPackageRoot() { // package root directory is instead of the test directory since some IDEs (like IntelliJ) fail to collect // code coverage correctly unless this package root directory is used. original_working_directory = process.cwd(); - process.chdir(path.resolve(__dirname,'..')); + process.chdir(path.resolve(__dirname, '..')); }); afterAll(() => { process.chdir(original_working_directory); @@ -28,24 +29,45 @@ export class FixedUniqueIdGenerator implements UniqueIdGenerator { } } -export class SimulatedTempFolder implements TempFolder { - private readonly simulatedRoot: string = 'simulatedRoot'; - private subfolderSet: Set = new Set(); +export class FakeFileSystem implements FileSystem { + private counter: number = 0; + files: Set = new Set(); - getPath(): Promise { - return Promise.resolve(this.simulatedRoot); + mkdirCallHistory: {absPath: fs.PathLike, options?: fs.MakeDirectoryOptions}[] = []; + mkdir(absPath: fs.PathLike, options?: fs.MakeDirectoryOptions): Promise { + this.mkdirCallHistory.push({absPath, options}); + this.files.add(absPath.toString()); + return Promise.resolve(absPath.toString()); } - createSubfolder(...subFolderPathSegs: string[]): Promise { - const joinedPath: string = path.join(this.simulatedRoot, ...subFolderPathSegs); - if (this.subfolderSet.has(joinedPath)) { - throw new Error(`Attempted to create path ${joinedPath} twice`); - } - this.subfolderSet.add(joinedPath); - return Promise.resolve(joinedPath); + mkdtempCallHistory: {prefix: string}[] = []; + mkdtemp(prefix: string): Promise { + this.mkdtempCallHistory.push({prefix}); + const tempDirPath: string = `${prefix}${this.counter++}`; + this.files.add(tempDirPath); + return Promise.resolve(tempDirPath); + } + + rmCallHistory: {absPath: fs.PathLike, options?: fs.RmOptions}[] = []; + rm(absPath: fs.PathLike, options?: fs.RmOptions): Promise { + this.rmCallHistory.push({absPath, options}); + this.rmImpl(absPath, options); + return Promise.resolve(); } - getCreatedSubfolders(): Set { - return this.subfolderSet; + rmSyncCallHistory: {absPath: fs.PathLike, options?: fs.RmOptions}[] = []; + rmSync(absPath: fs.PathLike, options?: fs.RmOptions): void { + this.rmSyncCallHistory.push({absPath, options}); + this.rmImpl(absPath, options); + } + + // Implementation shared with both rm and rmSync + private rmImpl(absPath: fs.PathLike, options?: fs.RmOptions): void { + this.files.delete(absPath.toString()); + for (const entry of this.files) { + if (options?.recursive && entry.startsWith(absPath + path.sep)) { + this.files.delete(entry); + } + } } }