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
12 changes: 12 additions & 0 deletions messages/action-summary-viewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,15 @@ Additional log information written to:
# config-action.outfile-location

Configuration written to:

# rules-action.found-no-rules

Found 0 rules.

# rules-action.rules-total

Found %d rule(s) from %d engine(s):

# rules-action.rules-item

%d %s rule(s) found.
2 changes: 1 addition & 1 deletion messages/progress-event-listener.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Selecting rules
Eligible engines: %s; Completion: %d%; Elapsed time: %ds

# selection-spinner.finished-status
done. Selected rules from %s.
done.

# execution-spinner.action
Executing rules
Expand Down
2 changes: 2 additions & 0 deletions src/commands/code-analyzer/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {View} from '../../Constants';
import {CodeAnalyzerConfigFactoryImpl} from '../../lib/factories/CodeAnalyzerConfigFactory';
import {EnginePluginsFactoryImpl} from '../../lib/factories/EnginePluginsFactory';
import {RuleDetailDisplayer, RuleTableDisplayer} from '../../lib/viewers/RuleViewer';
import {RulesActionSummaryViewer} from '../../lib/viewers/ActionSummaryViewer';
import {RulesAction, RulesDependencies} from '../../lib/actions/RulesAction';
import {BundleName, getMessage, getMessages} from '../../lib/messages';
import {Displayable, UxDisplay} from '../../lib/Display';
Expand Down Expand Up @@ -67,6 +68,7 @@ export default class RulesCommand extends SfCommand<void> implements Displayable
pluginsFactory: new EnginePluginsFactoryImpl(),
logEventListeners: [new LogEventDisplayer(uxDisplay)],
progressListeners: [new RuleSelectionProgressSpinner(uxDisplay)],
actionSummaryViewer: new RulesActionSummaryViewer(uxDisplay),
viewer: view === View.TABLE ? new RuleTableDisplayer(uxDisplay) : new RuleDetailDisplayer(uxDisplay)
};
}
Expand Down
3 changes: 3 additions & 0 deletions src/lib/actions/RulesAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import {ProgressEventListener} from '../listeners/ProgressEventListener';
import {LogFileWriter} from '../writers/LogWriter';
import {LogEventListener, LogEventLogger} from '../listeners/LogEventListener';
import {RuleViewer} from '../viewers/RuleViewer';
import {RulesActionSummaryViewer} from '../viewers/ActionSummaryViewer';

export type RulesDependencies = {
configFactory: CodeAnalyzerConfigFactory;
pluginsFactory: EnginePluginsFactory;
logEventListeners: LogEventListener[];
progressListeners: ProgressEventListener[];
actionSummaryViewer: RulesActionSummaryViewer,
viewer: RuleViewer;
}

Expand Down Expand Up @@ -58,6 +60,7 @@ export class RulesAction {
const rules: Rule[] = core.getEngineNames().flatMap(name => ruleSelection.getRulesFor(name));

this.dependencies.viewer.view(rules);
this.dependencies.actionSummaryViewer.view(ruleSelection, logWriter.getLogDestination());
}

public static createAction(dependencies: RulesDependencies): RulesAction {
Expand Down
3 changes: 1 addition & 2 deletions src/lib/listeners/ProgressEventListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,7 @@ export class RuleSelectionProgressSpinner extends ProgressSpinner implements Pro
}

protected createFinishedSpinnerStatus(): string {
return getMessage(BundleName.ProgressEventListener, 'selection-spinner.finished-status',
[[...this.engineNames.keys()].join(', ')]);
return getMessage(BundleName.ProgressEventListener, 'selection-spinner.finished-status');
}
}

Expand Down
33 changes: 33 additions & 0 deletions src/lib/viewers/ActionSummaryViewer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Display} from '../Display';
import {RuleSelection} from '@salesforce/code-analyzer-core';
import {toStyledHeader, indent} from '../utils/StylingUtil';
import {BundleName, getMessage} from '../messages';

Expand Down Expand Up @@ -45,3 +46,35 @@ export class ConfigActionSummaryViewer extends AbstractActionSummaryViewer {
this.display.displayLog(indent(outfile));
}
}

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

public view(ruleSelection: RuleSelection, logFile: string): void {
// Start with separator to cleanly break from anything that's already been logged.
this.displayLineSeparator();
this.displaySummaryHeader();
this.displayLineSeparator();

if (ruleSelection.getCount() === 0) {
this.display.displayLog(getMessage(BundleName.ActionSummaryViewer, 'rules-action.found-no-rules'));
} else {
this.displayRuleSelection(ruleSelection);
}
this.displayLineSeparator();

this.displayLogFileInfo(logFile);
}

private displayRuleSelection(ruleSelection: RuleSelection): void {
this.display.displayLog(getMessage(BundleName.ActionSummaryViewer, 'rules-action.rules-total', [ruleSelection.getCount(), ruleSelection.getEngineNames().length]));
for (const engineName of ruleSelection.getEngineNames()) {
const ruleCountForEngine: number = ruleSelection.getRulesFor(engineName).length;
this.display.displayLog(indent(getMessage(BundleName.ActionSummaryViewer, 'rules-action.rules-item', [ruleCountForEngine, engineName])));
}
}


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

=== Summary

Found 0 rules.

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

=== Summary

Found 5 rule(s) from 2 engine(s):
3 stubEngine1 rule(s) found.
2 stubEngine2 rule(s) found.

Additional log information written to:
55 changes: 55 additions & 0 deletions test/lib/actions/RulesAction.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import * as path from 'node:path';
import * as fsp from 'node:fs/promises';
import ansis from 'ansis';
import {RulesAction, RulesDependencies, RulesInput} from '../../../src/lib/actions/RulesAction';
import {RulesActionSummaryViewer} from '../../../src/lib/viewers/ActionSummaryViewer';
import {StubDefaultConfigFactory} from '../../stubs/StubCodeAnalyzerConfigFactories';
import * as StubEnginePluginFactories from '../../stubs/StubEnginePluginsFactories';
import {SpyRuleViewer} from '../../stubs/SpyRuleViewer';
import {DisplayEventType, SpyDisplay} from '../../stubs/SpyDisplay';

const PATH_TO_GOLDFILES = path.join(__dirname, '..', '..', 'fixtures', 'comparison-files', 'lib', 'actions', 'RulesAction.test.ts');

describe('RulesAction tests', () => {
let viewer: SpyRuleViewer;
Expand All @@ -12,11 +18,14 @@ describe('RulesAction tests', () => {
})

it('Submitting the all-selector returns all rules', async () => {
const spyDisplay: SpyDisplay = new SpyDisplay();
const actionSummaryViewer: RulesActionSummaryViewer = new RulesActionSummaryViewer(spyDisplay);
const dependencies: RulesDependencies = {
configFactory: new StubDefaultConfigFactory(),
pluginsFactory: new StubEnginePluginFactories.StubEnginePluginsFactory_withFunctionalStubEngine(),
logEventListeners: [],
progressListeners: [],
actionSummaryViewer,
viewer
};
const action = RulesAction.createAction(dependencies);
Expand All @@ -41,11 +50,14 @@ describe('RulesAction tests', () => {
});

it('Submitting a filtering selector returns only matching rules', async () => {
const spyDisplay: SpyDisplay = new SpyDisplay();
const actionSummaryViewer: RulesActionSummaryViewer = new RulesActionSummaryViewer(spyDisplay);
const dependencies: RulesDependencies = {
configFactory: new StubDefaultConfigFactory(),
pluginsFactory: new StubEnginePluginFactories.StubEnginePluginsFactory_withFunctionalStubEngine(),
logEventListeners: [],
progressListeners: [],
actionSummaryViewer,
viewer
};
const action = RulesAction.createAction(dependencies);
Expand All @@ -64,12 +76,15 @@ describe('RulesAction tests', () => {
});

it('Engines with target-dependent rules return the right rules', async () => {
const spyDisplay: SpyDisplay = new SpyDisplay();
const actionSummaryViewer: RulesActionSummaryViewer = new RulesActionSummaryViewer(spyDisplay);
const dependencies: RulesDependencies = {
configFactory: new StubDefaultConfigFactory(),
// The engine we're using here will synthesize one rule per target.
pluginsFactory: new StubEnginePluginFactories.StubEnginePluginsFactory_withTargetDependentStubEngine(),
logEventListeners: [],
progressListeners: [],
actionSummaryViewer,
viewer
};
const targetedFilesAndFolders = ['package.json', 'src', 'README.md'];
Expand Down Expand Up @@ -98,11 +113,14 @@ describe('RulesAction tests', () => {
* test will help us do that.
*/
it('When no engines are registered, empty results are displayed', async () => {
const spyDisplay: SpyDisplay = new SpyDisplay();
const actionSummaryViewer: RulesActionSummaryViewer = new RulesActionSummaryViewer(spyDisplay);
const dependencies: RulesDependencies = {
configFactory: new StubDefaultConfigFactory(),
pluginsFactory: new StubEnginePluginFactories.StubEnginePluginsFactory_withNoPlugins(),
logEventListeners: [],
progressListeners: [],
actionSummaryViewer,
viewer
};
const action = RulesAction.createAction(dependencies);
Expand All @@ -118,11 +136,14 @@ describe('RulesAction tests', () => {
});

it('Throws an error when an engine throws an error', async () => {
const spyDisplay: SpyDisplay = new SpyDisplay();
const actionSummaryViewer: RulesActionSummaryViewer = new RulesActionSummaryViewer(spyDisplay);
const dependencies: RulesDependencies = {
configFactory: new StubDefaultConfigFactory(),
pluginsFactory: new StubEnginePluginFactories.StubEnginePluginsFactory_withThrowingStubPlugin(),
logEventListeners: [],
progressListeners: [],
actionSummaryViewer,
viewer
};
const action = RulesAction.createAction(dependencies);
Expand All @@ -133,6 +154,40 @@ describe('RulesAction tests', () => {

await expect(executionPromise).rejects.toThrow('SomeErrorFromGetAvailableEngineNames');
});

describe('Summary generation', () => {
it.each([
{quantifier: 'no', expectation: 'Summary indicates absence of rules', selector: 'NonsensicalTag', goldfile: 'no-rules.txt.goldfile'},
{quantifier: 'some', expectation: 'Summary provides breakdown by engine', selector: 'Recommended', goldfile: 'some-rules.txt.goldfile'}
])('When $quantifier rules are returned, $expectation', async ({selector, goldfile}) => {
const goldfilePath: string = path.join(PATH_TO_GOLDFILES, 'action-summaries', goldfile);
const spyDisplay: SpyDisplay = new SpyDisplay();
const actionSummaryViewer: RulesActionSummaryViewer = new RulesActionSummaryViewer(spyDisplay);
const dependencies: RulesDependencies = {
configFactory: new StubDefaultConfigFactory(),
pluginsFactory: new StubEnginePluginFactories.StubEnginePluginsFactory_withFunctionalStubEngine(),
logEventListeners: [],
progressListeners: [],
actionSummaryViewer,
viewer
};
const action = RulesAction.createAction(dependencies);
const input: RulesInput = {
'rule-selector': [selector]
};

await action.execute(input);

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 fsp.readFile(goldfilePath, 'utf-8');
expect(displayedLogEvents).toContain(goldfileContents);
});
});
});

// TODO: Whenever we decide to document the custom_engine_plugin_modules flag in our configuration file, then we'll want
Expand Down
6 changes: 3 additions & 3 deletions test/lib/listeners/ProgressEventListener.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe('ProgressEventListener implementations', () => {
expect(percentagesInOrder).toEqual([0, 25, 50, 100]);
const endEvent = displayEvents[displayEvents.length - 1];
expect(endEvent).toHaveProperty('type', DisplayEventType.SPINNER_STOP);
expect(endEvent.data).toContain(`done. Selected rules from stubEngine1, stubEngine2.`);
expect(endEvent.data).toEqual(`done.`);
});

it('Properly aggregates percentages across multiple Cores', async () => {
Expand Down Expand Up @@ -123,7 +123,7 @@ describe('ProgressEventListener implementations', () => {
expect(percentagesInOrder).toEqual([0, 12, 25, 50, 62, 75, 100]);
const endEvent = displayEvents[displayEvents.length - 1];
expect(endEvent).toHaveProperty('type', DisplayEventType.SPINNER_STOP);
expect(endEvent.data).toContain('done. Selected rules from stubEngine1, stubEngine2.');
expect(endEvent.data).toEqual('done.');
});

it('Properly interleaves progress updates with ticking', async () => {
Expand Down Expand Up @@ -159,7 +159,7 @@ describe('ProgressEventListener implementations', () => {
expect(percentagesInOrder).toEqual([0, 20, 40, 50, 60, 80, 100]);
const endEvent = displayEvents[displayEvents.length - 1];
expect(endEvent).toHaveProperty('type', DisplayEventType.SPINNER_STOP);
expect(endEvent.data).toContain('done. Selected rules from timeableEngine1, timeableEngine2.');
expect(endEvent.data).toEqual('done.');
});
});

Expand Down