Merge pull request #1666 from forcedotcom/d/W-17052953

jfeingold35 · web-flow · commit cebd1e93fb34 · 2024-11-01T10:59:39.000-05:00
CHANGE @W-17052953@ Polished output for rules command.
diff --git a/messages/action-summary-viewer.md b/messages/action-summary-viewer.md
@@ -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.
diff --git a/messages/progress-event-listener.md b/messages/progress-event-listener.md
@@ -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
diff --git a/src/commands/code-analyzer/rules.ts b/src/commands/code-analyzer/rules.ts
@@ -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';
@@ -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)
 		};
 	}
diff --git a/src/lib/actions/RulesAction.ts b/src/lib/actions/RulesAction.ts
@@ -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;
 }
 
@@ -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 {
diff --git a/src/lib/listeners/ProgressEventListener.ts b/src/lib/listeners/ProgressEventListener.ts
@@ -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');
 	}
 }
 
diff --git a/src/lib/viewers/ActionSummaryViewer.ts b/src/lib/viewers/ActionSummaryViewer.ts
@@ -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';
 
@@ -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])));
+		}
+	}
+
+
+}
diff --git a/test/fixtures/comparison-files/lib/actions/RulesAction.test.ts/action-summaries/no-rules.txt.goldfile b/test/fixtures/comparison-files/lib/actions/RulesAction.test.ts/action-summaries/no-rules.txt.goldfile
@@ -0,0 +1,6 @@
+
+=== Summary
+
+Found 0 rules.
+
+Additional log information written to:
diff --git a/test/fixtures/comparison-files/lib/actions/RulesAction.test.ts/action-summaries/some-rules.txt.goldfile b/test/fixtures/comparison-files/lib/actions/RulesAction.test.ts/action-summaries/some-rules.txt.goldfile
@@ -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:
diff --git a/test/lib/actions/RulesAction.test.ts b/test/lib/actions/RulesAction.test.ts
@@ -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;
@@ -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);
@@ -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);
@@ -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'];
@@ -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);
@@ -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);
@@ -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
diff --git a/test/lib/listeners/ProgressEventListener.test.ts b/test/lib/listeners/ProgressEventListener.test.ts
@@ -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 () => {
@@ -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 () => {
@@ -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.');
 		});
 	});
 

Original file line number	Diff line number	Diff line change
`@@ -138,8 +138,7 @@ export class RuleSelectionProgressSpinner extends ProgressSpinner implements Pro`
`138`	`138`	`}`
`139`	`139`
`140`	`140`	`protected createFinishedSpinnerStatus(): string {`
`141`		`- return getMessage(BundleName.ProgressEventListener, 'selection-spinner.finished-status',`
`142`		`- [[...this.engineNames.keys()].join(', ')]);`
	`141`	`+ return getMessage(BundleName.ProgressEventListener, 'selection-spinner.finished-status');`
`143`	`142`	`}`
`144`	`143`	`}`
`145`	`144`