Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/code-analyzer-core/src/code-analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export class CodeAnalyzer {
constructor(config: CodeAnalyzerConfig, fileSystem: FileSystem = new RealFileSystem(), nodeVersion: string = process.version) {
this.validateEnvironment(nodeVersion);
this.config = config;
this.tempFolder = new TempFolder(fileSystem);
this.tempFolder = new TempFolder(fileSystem, this.config.getRootWorkingFolder());
/* istanbul ignore next */
process.addListener('exit', async () => {
// Note that on node exit there is no more event loop, so removal must take place synchronously
Expand Down
15 changes: 14 additions & 1 deletion packages/code-analyzer-core/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const FIELDS = {
CONFIG_ROOT: 'config_root',
LOG_FOLDER: 'log_folder',
LOG_LEVEL: 'log_level',
ROOT_WORKING_FOLDER: 'root_working_folder', // Hidden
CUSTOM_ENGINE_PLUGIN_MODULES: 'custom_engine_plugin_modules', // Hidden
PRESERVE_ALL_WORKING_FOLDERS: 'preserve_all_working_folders', // Hidden
RULES: 'rules',
Expand Down Expand Up @@ -42,6 +43,7 @@ type TopLevelConfig = {
log_level: LogLevel
rules: Record<string, RuleOverrides>
engines: Record<string, EngineOverrides>
root_working_folder: string, // INTERNAL USE ONLY
preserve_all_working_folders: boolean // INTERNAL USE ONLY
custom_engine_plugin_modules: string[] // INTERNAL USE ONLY
}
Expand All @@ -53,6 +55,7 @@ export const DEFAULT_CONFIG: TopLevelConfig = {
log_level: LogLevel.Debug,
rules: {},
engines: {},
root_working_folder: os.tmpdir(), // INTERNAL USE ONLY
preserve_all_working_folders: false, // INTERNAL USE ONLY
custom_engine_plugin_modules: [], // INTERNAL USE ONLY
};
Expand Down Expand Up @@ -139,7 +142,7 @@ export class CodeAnalyzerConfig {
configRoot = !rawConfig.config_root ? (configRoot ?? process.cwd()) :
validateAbsoluteFolder(rawConfig.config_root, FIELDS.CONFIG_ROOT);
const configExtractor: engApi.ConfigValueExtractor = new engApi.ConfigValueExtractor(rawConfig, '', configRoot);
configExtractor.addKeysThatBypassValidation([FIELDS.CUSTOM_ENGINE_PLUGIN_MODULES, FIELDS.PRESERVE_ALL_WORKING_FOLDERS]); // Hidden fields bypass validation
configExtractor.addKeysThatBypassValidation([FIELDS.CUSTOM_ENGINE_PLUGIN_MODULES, FIELDS.PRESERVE_ALL_WORKING_FOLDERS, FIELDS.ROOT_WORKING_FOLDER]); // Hidden fields bypass validation
configExtractor.validateContainsOnlySpecifiedKeys([FIELDS.CONFIG_ROOT, FIELDS.LOG_FOLDER, FIELDS.LOG_LEVEL ,FIELDS.RULES, FIELDS.ENGINES]);
const config: TopLevelConfig = {
config_root: configRoot,
Expand All @@ -148,6 +151,7 @@ export class CodeAnalyzerConfig {
custom_engine_plugin_modules: configExtractor.extractArray(FIELDS.CUSTOM_ENGINE_PLUGIN_MODULES,
engApi.ValueValidator.validateString,
DEFAULT_CONFIG.custom_engine_plugin_modules)!,
root_working_folder: !rawConfig.root_working_folder ? os.tmpdir() : validateAbsoluteFolder(rawConfig.root_working_folder, FIELDS.ROOT_WORKING_FOLDER),
preserve_all_working_folders: configExtractor.extractBoolean(FIELDS.PRESERVE_ALL_WORKING_FOLDERS, DEFAULT_CONFIG.preserve_all_working_folders)!,
rules: extractRulesValue(configExtractor),
engines: extractEnginesValue(configExtractor)
Expand Down Expand Up @@ -239,6 +243,15 @@ export class CodeAnalyzerConfig {
return this.config.preserve_all_working_folders;
}


/**
* Returns the absolute path to a folder that will serve as the root for all temporary working folders associated with
* this execution.
*/
public getRootWorkingFolder(): string {
return this.config.root_working_folder;
}

/**
* Returns a {@link RuleOverrides} instance containing the user specified overrides for all rules associated with the specified engine
* @param engineName name of the engine
Expand Down
4 changes: 2 additions & 2 deletions packages/code-analyzer-core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ export class TempFolder {
private rootFolder?: string;
private relPathsToKeep: Set<string> = new Set();

constructor(fileSystem: FileSystem = new RealFileSystem(), rootFolderPrefix: string = path.join(os.tmpdir(), 'code-analyzer-')) {
constructor(fileSystem: FileSystem = new RealFileSystem(), rootFolderPath: string = os.tmpdir()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice how you have this say rootFolderPath - which is another indication that we are dealing with a rootWorkingFolder instead of a workingFoldersRoot. Just an observation.

this.fileSystem = fileSystem;
this.rootFolderPrefix = rootFolderPrefix;
this.rootFolderPrefix = path.join(rootFolderPath, 'code-analyzer-');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you removed option to supply the prefix. I think this is fine since I was on the fence of whether to do so before. Until we reuse the TempFolder for other purposes - this seems fine.

}

async getPath(...subfolderPathSegments: string[]): Promise<string> {
Expand Down
32 changes: 32 additions & 0 deletions packages/code-analyzer-core/test/code-analyzer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,38 @@ describe("Tests for the run method of CodeAnalyzer", () => {
expect(fileSystem.files).not.toContain(expectedRunWorkingFolderForStubEngine3);
});

it("When running rules, the root_working_folder config property designates where working folders are created", async () => {
await setupCodeAnalyzerWithStubs(CodeAnalyzerConfig.fromObject({
root_working_folder: path.resolve(__dirname, 'test-data')
}));
await codeAnalyzer.run(selection, sampleRunOptions);

const expectedRunWorkingFolderRoot: string = path.resolve(__dirname, 'test-data','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 the top-level preserve_all_working_folders flag is true, all run working folders are preserved and a log is issued", async () => {
await setupCodeAnalyzerWithStubs(CodeAnalyzerConfig.fromObject({
Expand Down
23 changes: 23 additions & 0 deletions packages/code-analyzer-core/test/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe("Tests for creating and accessing configuration values", () => {
expect(conf.getLogLevel()).toEqual(LogLevel.Debug);
expect(conf.getCustomEnginePluginModules()).toEqual([]);
expect(conf.getPreserveAllWorkingFolders()).toEqual(false);
expect(conf.getRootWorkingFolder()).toEqual(os.tmpdir());
expect(conf.getRuleOverridesFor("stubEngine1")).toEqual({});
expect(conf.getEngineOverridesFor("stubEngine1")).toEqual({});
expect(conf.getRuleOverridesFor("stubEngine2")).toEqual({});
Expand Down Expand Up @@ -83,6 +84,7 @@ describe("Tests for creating and accessing configuration values", () => {
expect(conf.getLogFolder()).toEqual(os.tmpdir());
expect(conf.getCustomEnginePluginModules()).toEqual(['dummy_plugin_module_path']);
expect(conf.getPreserveAllWorkingFolders()).toEqual(true);
expect(conf.getRootWorkingFolder()).toEqual(os.tmpdir());
expect(conf.getRuleOverridesFor('stubEngine1')).toEqual({});
expect(conf.getRuleOverridesFor('stubEngine2')).toEqual({
stub2RuleC: {
Expand Down Expand Up @@ -306,6 +308,27 @@ describe("Tests for creating and accessing configuration values", () => {
getMessageFromCatalog(SHARED_MESSAGE_CATALOG, 'ConfigValueMustBeOfType','preserve_all_working_folders', 'boolean', 'string'));
})

it("When supplied root_working_folder is a valid absolute path, then we use it", () => {
const workingFoldersRootValue: string = path.join(TEST_DATA_DIR, 'sampleWorkspace');
const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromObject({root_working_folder: workingFoldersRootValue});
expect(conf.getRootWorkingFolder()).toEqual(workingFoldersRootValue);
});

it("When supplied root_working_folder does not exist, then we error", () => {
expect(() => CodeAnalyzerConfig.fromObject({root_working_folder: path.resolve('doesNotExist')})).toThrow(
getMessageFromCatalog(SHARED_MESSAGE_CATALOG, 'ConfigPathValueDoesNotExist', 'root_working_folder', path.resolve('doesNotExist')));
});

it("When supplied root_working_folder is a file instead of a folder, then we error", () => {
expect(() => CodeAnalyzerConfig.fromObject({root_working_folder: path.resolve('package.json')})).toThrow(
getMessageFromCatalog(SHARED_MESSAGE_CATALOG, 'ConfigFolderValueMustNotBeFile', 'root_working_folder', path.resolve('package.json')));
});

it("When supplied root_working_folder is a relative folder, then we error", () => {
expect(() => CodeAnalyzerConfig.fromObject({root_working_folder: 'test/test-data'})).toThrow(
getMessage('ConfigPathValueMustBeAbsolute', 'root_working_folder', 'test/test-data', path.resolve('test', 'test-data')));
});

it("When supplied config_root path is a valid absolute path, then we use it", () => {
const configRootValue: string = path.join(TEST_DATA_DIR, 'sampleWorkspace');
const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromObject({config_root: configRootValue});
Expand Down
43 changes: 43 additions & 0 deletions packages/code-analyzer-core/test/rule-selection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,49 @@ describe('Tests for selecting rules', () => {
expect(relevantLogMsgs.filter(m => m.endsWith(expectedRulesWorkingFolderForStubEngine2))).toHaveLength(1);
});

it("When selecting rules, the root_working_folder config property designates where the working folders are created", async () => {
await setupCodeAnalyzerWithStubPlugin(CodeAnalyzerConfig.fromObject({
root_working_folder: path.resolve(__dirname, 'test-data')
}));

const logEvents: LogEvent[] = [];
codeAnalyzer.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event));

await codeAnalyzer.selectRules(['all']);

const expectedRulesWorkingFolderRoot: string = path.resolve(__dirname, 'test-data', '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 preserve_all_working_folders is true, then all working folders are kept regardless of failures and the root is logged", async () => {
await setupCodeAnalyzerWithStubPlugin(CodeAnalyzerConfig.fromObject({
preserve_all_working_folders: true
Expand Down