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
15 changes: 15 additions & 0 deletions messages/action-summary-viewer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# common.summary-header

Summary

# common.logfile-location

Additional log information written to:
Comment on lines +1 to +7
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied from run-summary-viewer.md. Named common.blah to allow for porting the existing RunSummaryViewer implementation to this new style.


# config-action.no-outfiles

No output file was specified.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Analogous to run-summary-viewer.md's message "No results files were specified."


# config-action.outfile-location
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Analogous to run-summary-viewer.md's message "Results written to:".


Configuration written to:
3 changes: 3 additions & 0 deletions messages/config-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ Empty object used because rule selection returned no rules

# template.yaml.no-rules-selected
Remove this empty object {} when you are ready to specify your first rule override

# template.common.end-of-config
END OF CODE ANALYZER CONFIGURATION
2 changes: 2 additions & 0 deletions src/commands/code-analyzer/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {Flags, SfCommand} from '@salesforce/sf-plugins-core';
import {ConfigAction, ConfigDependencies} from '../../lib/actions/ConfigAction';
import {ConfigFileWriter} from '../../lib/writers/ConfigWriter';
import {ConfigStyledYamlViewer} from '../../lib/viewers/ConfigViewer';
import {ConfigActionSummaryViewer} from '../../lib/viewers/ActionSummaryViewer';
import {CodeAnalyzerConfigFactoryImpl} from '../../lib/factories/CodeAnalyzerConfigFactory';
import {EnginePluginsFactoryImpl} from '../../lib/factories/EnginePluginsFactory';
import {BundleName, getMessage, getMessages} from '../../lib/messages';
Expand Down Expand Up @@ -70,6 +71,7 @@ export default class ConfigCommand extends SfCommand<void> implements Displayabl
logEventListeners: [new LogEventDisplayer(uxDisplay)],
progressEventListeners: [new RuleSelectionProgressSpinner(uxDisplay)],
modelGenerator: modelGeneratorFunction,
actionSummaryViewer: new ConfigActionSummaryViewer(uxDisplay),
viewer: new ConfigStyledYamlViewer(uxDisplay)
};
if (outputFile) {
Expand Down
11 changes: 9 additions & 2 deletions src/lib/actions/ConfigAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {ConfigViewer} from '../viewers/ConfigViewer';
import {createWorkspace} from '../utils/WorkspaceUtil';
import {LogEventListener, LogEventLogger} from '../listeners/LogEventListener';
import {ProgressEventListener} from '../listeners/ProgressEventListener';
import {ConfigActionSummaryViewer} from '../viewers/ActionSummaryViewer';
import {ConfigModel, ConfigModelGeneratorFunction, ConfigContext} from '../models/ConfigModel';

export type ConfigDependencies = {
Expand All @@ -16,11 +17,13 @@ export type ConfigDependencies = {
logEventListeners: LogEventListener[];
progressEventListeners: ProgressEventListener[];
writer?: ConfigWriter;
actionSummaryViewer: ConfigActionSummaryViewer;
viewer: ConfigViewer;
};

export type ConfigInput = {
'config-file'?: string;
'output-file'?: string;
'rule-selector': string[];
workspace?: string[];
};
Expand All @@ -38,7 +41,8 @@ export class ConfigAction {
const defaultConfig: CodeAnalyzerConfig = CodeAnalyzerConfig.withDefaults();

// We always add a Logger Listener to the appropriate listeners list, because we should always be logging.
const logEventLogger: LogEventLogger = new LogEventLogger(await LogFileWriter.fromConfig(userConfig));
const logFileWriter: LogFileWriter = await LogFileWriter.fromConfig(userConfig);
const logEventLogger: LogEventLogger = new LogEventLogger(logFileWriter);
this.dependencies.logEventListeners.push(logEventLogger);

// The User's config produces one Core.
Expand Down Expand Up @@ -118,7 +122,10 @@ export class ConfigAction {
const configModel: ConfigModel = this.dependencies.modelGenerator(relevantEngines, userConfigContext, defaultConfigContext);

this.dependencies.viewer.view(configModel);
await this.dependencies.writer?.write(configModel);
const fileWritten: boolean = this.dependencies.writer
? await this.dependencies.writer.write(configModel)
: false;
this.dependencies.actionSummaryViewer.view(logFileWriter.getLogDestination(), fileWritten ? input['output-file'] : undefined);
return Promise.resolve();
}

Expand Down
1 change: 1 addition & 0 deletions src/lib/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {Tokens} from '@salesforce/core/lib/messages';
Messages.importMessagesDirectory(__dirname);

export enum BundleName {
ActionSummaryViewer = 'action-summary-viewer',
ConfigCommand = 'config-command',
ConfigModel = 'config-model',
ConfigWriter = 'config-writer',
Expand Down
4 changes: 3 additions & 1 deletion src/lib/models/ConfigModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ abstract class YamlFormatter {
this.toYamlRuleOverrides() + '\n' +
'\n' +
this.toYamlComment(topLevelDescription.fieldDescriptions!.engines) + '\n' +
this.toYamlEngineOverrides() + '\n';
this.toYamlEngineOverrides() + '\n' +
'\n' +
this.toYamlSectionHeadingComment(getMessage(BundleName.ConfigModel, 'template.common.end-of-config')) + '\n';
Comment on lines +117 to +119
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this end of config message might end up in the file as well. I was hoping it would just be for the terminal output only.

}

private toYamlRuleOverrides(): string {
Expand Down
49 changes: 49 additions & 0 deletions src/lib/viewers/ActionSummaryViewer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import {Display} from '../Display';
import {toStyledHeader, indent} from '../utils/StylingUtil';
import {BundleName, getMessage} from '../messages';

abstract class AbstractActionSummaryViewer {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's only one child class right now, but the parent class will be helpful for porting the Run Summary Viewer to the new style.

protected readonly display: Display;

protected constructor(display: Display) {
this.display = display;
}

protected displaySummaryHeader(): void {
this.display.displayLog(toStyledHeader(getMessage(BundleName.ActionSummaryViewer, 'common.summary-header')));
}

protected displayLineSeparator(): void {
this.display.displayLog("");
}

protected displayLogFileInfo(logFile: string): void {
this.display.displayLog(getMessage(BundleName.ActionSummaryViewer, 'common.logfile-location'));
this.display.displayLog(indent(logFile));
}
}

export class ConfigActionSummaryViewer extends AbstractActionSummaryViewer {
public constructor(display: Display) {
super(display);
}

public view(logFile: string, outfile?: string): void {
this.displaySummaryHeader();
this.displayLineSeparator();

if (outfile) {
this.displayOutfile(outfile);
} else {
this.display.displayLog(getMessage(BundleName.ActionSummaryViewer, 'config-action.no-outfiles'));
}
this.displayLineSeparator();

this.displayLogFileInfo(logFile);
}

private displayOutfile(outfile: string): void {
this.display.displayLog(getMessage(BundleName.ActionSummaryViewer, 'config-action.outfile-location'));
this.display.displayLog(indent(outfile));
}
}
7 changes: 5 additions & 2 deletions src/lib/writers/ConfigWriter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {Display} from '../Display';
import {exists} from '../utils/FileUtil';

export interface ConfigWriter {
write(model: ConfigModel): Promise<void>;
write(model: ConfigModel): Promise<boolean>;
}

export class ConfigFileWriter implements ConfigWriter {
Expand All @@ -20,10 +20,13 @@ export class ConfigFileWriter implements ConfigWriter {
this.display = display;
}

public async write(model: ConfigModel): Promise<void> {
public async write(model: ConfigModel): Promise<boolean> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this method isn't guaranteed to actually write anything, it now returns a boolean to indicate whether it did.

// Only write to the file if it doesn't already exist, or if the user confirms that they want to overwrite it.
if (!(await exists(this.file)) || await this.display.confirm(getMessage(BundleName.ConfigWriter, 'prompt.overwrite-existing-file', [this.file]))) {
fs.writeFileSync(this.file, model.toFormattedOutput(this.format));
return true;
} else {
return false;
}
}

Expand Down
3 changes: 3 additions & 0 deletions test/commands/code-analyzer/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ describe('`code-analyzer config` tests', () => {
expect(createActionSpy).toHaveBeenCalled();
expect(fromFileSpy).toHaveBeenCalled();
expect(receivedFile).toEqual(inputValue);
expect(receivedActionInput).toHaveProperty('output-file', inputValue);
});

it('Can be referenced by its shortname, -f', async () => {
Expand All @@ -186,6 +187,7 @@ describe('`code-analyzer config` tests', () => {
expect(createActionSpy).toHaveBeenCalled();
expect(fromFileSpy).toHaveBeenCalled();
expect(receivedFile).toEqual(inputValue);
expect(receivedActionInput).toHaveProperty('output-file', inputValue);
});

it('Cannot be supplied multiple times', async () => {
Expand All @@ -201,6 +203,7 @@ describe('`code-analyzer config` tests', () => {
expect(executeSpy).toHaveBeenCalled();
expect(createActionSpy).toHaveBeenCalled();
expect(fromFileSpy).not.toHaveBeenCalled();
expect(receivedActionInput['output-file']).toBeUndefined();
});

});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
=== Summary

No output file was specified.

Additional log information written to:
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
=== Summary

Configuration written to:
out-config.yml

Additional log information written to:
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# ======================================================================
# END OF CODE ANALYZER CONFIGURATION
# ======================================================================
89 changes: 83 additions & 6 deletions test/lib/actions/ConfigAction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {CodeAnalyzerConfigFactory} from "../../../src/lib/factories/CodeAnalyzer
import {EnginePluginsFactory} from '../../../src/lib/factories/EnginePluginsFactory';
import {ConfigAction, ConfigDependencies, ConfigInput} from '../../../src/lib/actions/ConfigAction';
import {AnnotatedConfigModel} from '../../../src/lib/models/ConfigModel';
import {ConfigStyledYamlViewer} from '../../../lib/lib/viewers/ConfigViewer';
import {ConfigStyledYamlViewer} from '../../../src/lib/viewers/ConfigViewer';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Import path was wrong here.

import {ConfigActionSummaryViewer} from '../../../src/lib/viewers/ActionSummaryViewer';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Testing through ConfigAction using the actual implementation, instead of using an Interface/Stub setup.


import {SpyConfigWriter} from '../../stubs/SpyConfigWriter';
import {DisplayEventType, SpyDisplay} from '../../stubs/SpyDisplay';
Expand All @@ -36,17 +37,21 @@ describe('ConfigAction tests', () => {
viewer: new ConfigStyledYamlViewer(spyDisplay),
configFactory: new DefaultStubCodeAnalyzerConfigFactory(),
modelGenerator: AnnotatedConfigModel.fromSelection,
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
pluginsFactory: new StubEnginePluginFactory()
};
});

it('Top-level overview-comment is correct', async () => {
it.each([
{position: 'start'},
{position: 'end'}
])('Top-level $position comment is correct', async ({position}) => {
// ==== TESTED BEHAVIOR ====
// Just select all rules for this test, since we don't care about the rules here.
const output = await runActionAndGetDisplayedConfig(dependencies, ['all']);

// ==== ASSERTIONS ====
const goldFileContents = await readGoldFile(path.join(PATH_TO_COMPARISON_DIR, 'header-comments', 'top-level.yml.goldfile'));
const goldFileContents = await readGoldFile(path.join(PATH_TO_COMPARISON_DIR, 'header-comments', `top-level-${position}.yml.goldfile`));
expect(output).toContain(goldFileContents);
});

Expand Down Expand Up @@ -159,12 +164,16 @@ describe('ConfigAction tests', () => {
viewer: new ConfigStyledYamlViewer(spyDisplay),
configFactory: stubConfigFactory,
modelGenerator: AnnotatedConfigModel.fromSelection,
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
pluginsFactory: new StubEnginePluginFactory()
};
});


it('Top-level overview-comment is correct', async () => {
it.each([
{position: 'start'},
{position: 'end'}
])('Top-level $position comment is correct', async ({position}) => {
// ==== SETUP ====
// Set the dummy config properties to null; it's fine for this test.
stubConfigFactory.setDummyConfigRoot('null');
Expand All @@ -175,7 +184,7 @@ describe('ConfigAction tests', () => {
const output = await runActionAndGetDisplayedConfig(dependencies, ['all']);

// ==== ASSERTIONS ====
const goldFileContents = await readGoldFile(path.join(PATH_TO_COMPARISON_DIR, 'header-comments', 'top-level.yml.goldfile'));
const goldFileContents = await readGoldFile(path.join(PATH_TO_COMPARISON_DIR, 'header-comments', `top-level-${position}.yml.goldfile`));
expect(output).toContain(goldFileContents);
});

Expand Down Expand Up @@ -389,6 +398,7 @@ describe('ConfigAction tests', () => {
viewer: new ConfigStyledYamlViewer(spyDisplay),
configFactory: new DefaultStubCodeAnalyzerConfigFactory(),
modelGenerator: AnnotatedConfigModel.fromSelection,
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
pluginsFactory: new StubEnginePluginFactory()
};
});
Expand All @@ -406,6 +416,74 @@ describe('ConfigAction tests', () => {
expect(spyWriter.getCallHistory()).toHaveLength(1);
});
});

describe('Summary generation', () => {
beforeEach(() => {
spyDisplay = new SpyDisplay();
dependencies = {
logEventListeners: [],
progressEventListeners: [],
viewer: new ConfigStyledYamlViewer(spyDisplay),
configFactory: new DefaultStubCodeAnalyzerConfigFactory(),
modelGenerator: AnnotatedConfigModel.fromSelection,
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
pluginsFactory: new StubEnginePluginFactory()
}
});

it('When an Outfile is created, it is mentioned by the Summarizer', async () => {
// ==== SETUP ====
// Assign a Writer to the dependencies.
dependencies.writer = new SpyConfigWriter(true);

// ==== TESTED BEHAVIOR ====
// Invoke the action, specifying an outfile.
const action = ConfigAction.createAction(dependencies);
const input: ConfigInput = {
'rule-selector': ['all'],
'output-file': 'out-config.yml'
};
await action.execute(input);

// ==== ASSERTIONS ====
const displayEvents = spyDisplay.getDisplayEvents();
const displayedLogEvents = ansis.strip(displayEvents
.filter(e => e.type === DisplayEventType.LOG)
.map(e => e.data)
.join('\n'));

const goldfileContents: string = await readGoldFile(path.join(PATH_TO_COMPARISON_DIR, 'action-summaries', 'outfile-created.txt.goldfile'));
expect(displayedLogEvents).toContain(goldfileContents);
});

it.each([
{case: 'an Outfile is specified but not written', writer: new SpyConfigWriter(false), outfile: 'out-config.yml'},
{case: 'an Outfile is not specified at all', writer: undefined, outfile: undefined}
])('When $case, the Summarizer mentions no outfile', async ({writer, outfile}) => {
// ==== SETUP ====
// Add the specified Writer (or lack-of-Writer) to the dependencies.
dependencies.writer = writer;

// ==== TESTED BEHAVIOR ====
// Invoke the action, specifying an outfile (or lack of one).
const action = ConfigAction.createAction(dependencies);
const input: ConfigInput = {
'rule-selector': ['all'],
'output-file': outfile
};
await action.execute(input);

// ==== ASSERTIONS ====
const displayEvents = spyDisplay.getDisplayEvents();
const displayedLogEvents = ansis.strip(displayEvents
.filter(e => e.type === DisplayEventType.LOG)
.map(e => e.data)
.join('\n'));

const goldfileContents: string = await readGoldFile(path.join(PATH_TO_COMPARISON_DIR, 'action-summaries', 'no-outfile-created.txt.goldfile'));
expect(displayedLogEvents).toContain(goldfileContents);
});
})
// ====== HELPER FUNCTIONS ======

async function readGoldFile(goldFilePath: string): Promise<string> {
Expand All @@ -425,7 +503,6 @@ describe('ConfigAction tests', () => {

// ==== OUTPUT PROCESSING ====
const displayEvents = spyDisplay.getDisplayEvents();
expect(displayEvents).toHaveLength(1);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Array length is now variable, so we're not checking for an exact length anymore.

expect(displayEvents[0].type).toEqual(DisplayEventType.LOG);
return ansis.strip(displayEvents[0].data);
}
Expand Down
6 changes: 4 additions & 2 deletions test/lib/writers/ConfigWriter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ describe('ConfigWriter implementations', () => {

const stubbedConfig = new StubConfigModel();

await configFileWriter.write(stubbedConfig);
const result: boolean = await configFileWriter.write(stubbedConfig);

expect(result).toEqual(true);
expect(spyDisplay.getDisplayEvents()).toHaveLength(0);

expect(writeFileSpy).toHaveBeenCalled();
Expand All @@ -52,10 +53,11 @@ describe('ConfigWriter implementations', () => {

const stubbedConfig = new StubConfigModel();

await configFileWriter.write(stubbedConfig);
const result: boolean = await configFileWriter.write(stubbedConfig);

const displayEvents: DisplayEvent[] = spyDisplay.getDisplayEvents();
expect(displayEvents).toHaveLength(1);
expect(result).toEqual(confirmation);
// The user should be prompted to confirm override.
expect(displayEvents[0].type).toEqual(DisplayEventType.CONFIRM);
expect(displayEvents[0].data).toContain('overwrite');
Expand Down
Loading