From 53a7be5ceb77157f3cd40d9bce8889adf55578f8 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Mon, 11 Sep 2023 14:30:22 -0500 Subject: [PATCH] FIX (CodeAnalyzer): @W-14096244@: Restored behavior for violationless output on console. --- messages/RunOutputProcessor.js | 1 + src/lib/util/RunOutputProcessor.ts | 20 +++++++++-- test/commands/scanner/run.severity.test.ts | 4 +-- test/commands/scanner/run.test.ts | 39 ++++++++-------------- test/lib/util/RunOutputProcessor.test.ts | 27 +++++---------- 5 files changed, 43 insertions(+), 48 deletions(-) diff --git a/messages/RunOutputProcessor.js b/messages/RunOutputProcessor.js index 1e6949496..7340a2432 100644 --- a/messages/RunOutputProcessor.js +++ b/messages/RunOutputProcessor.js @@ -1,6 +1,7 @@ module.exports = { "output": { "engineSummaryTemplate": "Executed %s, found %s violation(s) across %s file(s).", + "noViolationsDetected": "Executed engines: %s. No rule violations found.", "sevThresholdSummary": "Rule violations of severity %s or more severe were detected.", "writtenToConsole": "Rule violations were logged to the console.", "writtenToOutFile": "Rule violations were written to %s." diff --git a/src/lib/util/RunOutputProcessor.ts b/src/lib/util/RunOutputProcessor.ts index 43c25b351..c7b76099a 100644 --- a/src/lib/util/RunOutputProcessor.ts +++ b/src/lib/util/RunOutputProcessor.ts @@ -28,9 +28,25 @@ export class RunOutputProcessor { public processRunOutput(rrr: RecombinedRuleResults): AnyJson { - const {minSev, results} = rrr; + const {minSev, results, summaryMap} = rrr; + + const hasViolations = [...summaryMap.values()].some(summary => summary.violationCount !== 0); + + // If there are neither violations nor an outfile, then we want to avoid writing empty + // results to the console. + // NOTE: If there's an outfile, we skip this part. This is because we still want to generate + // an empty outfile + if (!this.opts.outfile && !hasViolations) { + // Build a message indicating which engines were run... + const msg = messages.getMessage('output.noViolationsDetected', [[...summaryMap.keys()].join(', ')]); + // ...log it to the console... + this.ux.log(msg); + // ...and return it for use with the --json flag. + return msg; + } - // With the list of violations, we'll build an array of message parts, and then log them all at the end. + // If we have violations (or an outfile but no violations), we'll build an array of + // message parts, and then log them all at the end. let msgComponents: string[] = []; // We need a summary of the information we were provided (blank/empty if no violations). msgComponents = [...msgComponents, ...this.buildRunSummaryMsgParts(rrr)]; diff --git a/test/commands/scanner/run.severity.test.ts b/test/commands/scanner/run.severity.test.ts index 4e9682f45..d5d7de7be 100644 --- a/test/commands/scanner/run.severity.test.ts +++ b/test/commands/scanner/run.severity.test.ts @@ -20,9 +20,7 @@ describe('scanner:run', function () { '--severity-threshold', '3' ]) .it('When no violations are found, no error is thrown', ctx => { - const output = JSON.parse(ctx.stdout); - // There should be no violations (nor any exceptions). - expect(output.length).to.equal(0, 'Should be no violations from one engine'); + expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js'])); expect(ctx.stderr).to.not.contain(processorMessages.getMessage('output.sevThresholdSummary', ['3']), 'Error should not be present'); }); diff --git a/test/commands/scanner/run.test.ts b/test/commands/scanner/run.test.ts index 3fdad9dd5..0239ba4b3 100644 --- a/test/commands/scanner/run.test.ts +++ b/test/commands/scanner/run.test.ts @@ -63,8 +63,8 @@ describe('scanner:run', function () { '--ruleset', 'ApexUnit', '--format', 'xml' ]) - .it('When the file contains no violations, XML with no violations is logged to the console', ctx => { - validateNoViolationsXmlOutput(ctx.stdout); + .it('When the file contains no violations, a message is logged to the console', ctx => { + expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js'])); }); }); @@ -180,7 +180,7 @@ describe('scanner:run', function () { fs.unlinkSync('testout.xml'); } }) - .it('The violations are written to the file as an XML', ctx => { + .it('Violations are written to file as XML', ctx => { // Verify that the file we wanted was actually created. expect(fs.existsSync('testout.xml')).to.equal(true, 'The command should have created the expected output file'); const fileContents = fs.readFileSync('testout.xml').toString(); @@ -199,7 +199,7 @@ describe('scanner:run', function () { fs.unlinkSync('testout.xml'); } }) - .it('An XML file with no violations is created', ctx => { + .it('Absence of violations yields empty XML file', ctx => { // Verify that an empty XML file was actually created. expect(fs.existsSync('testout.xml')).to.equal(true, 'The command should have created an empty output file'); const fileContents = fs.readFileSync('testout.xml').toString(); @@ -289,8 +289,8 @@ describe('scanner:run', function () { '--ruleset', 'ApexUnit', '--format', 'csv' ]) - .it('When no violations are detected, empty CSV is printed to the console', ctx => { - validateNoViolationsCsvOutput(ctx.stdout, false); + .it('When no violations are detected, a message is logged to the console', ctx => { + expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js'])); }); setupCommandTest @@ -379,8 +379,8 @@ describe('scanner:run', function () { '--ruleset', 'ApexUnit', '--format', 'html' ]) - .it('When no violations are detected, HTML with no violations is logged to the console', ctx => { - validateNoViolationsHtmlOutput(ctx.stdout); + .it('When no violations are detected, a message is logged to the console', ctx => { + expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js'])); }); setupCommandTest @@ -462,8 +462,8 @@ describe('scanner:run', function () { '--ruleset', 'ApexUnit', '--format', 'json' ]) - .it('When no violations are detected, JSON with no violations is logged to the console', ctx => { - validateNoViolationsJsonOutput(ctx.stdout); + .it('When no violations are detected, a message is logged to the console', ctx => { + expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js'])); }); setupCommandTest @@ -604,7 +604,7 @@ describe('scanner:run', function () { .it('--json flag wraps message about no violations occuring', ctx => { const output = JSON.parse(ctx.stdout); expect(output.status).to.equal(0, 'Should have finished properly'); - expect(output.result.length).to.equal(0, 'When no violations are present, JSON result should be empty array.') + expect(output.result).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js'])); }); }); @@ -730,13 +730,8 @@ describe('scanner:run', function () { '--format', 'csv' ]) .it('The baseConfig enables the usage of default Javascript Types', ctx => { - // If there's a summary, then it'll be separated from the CSV by an empty line. Throw it away. - const [csv, _] = ctx.stdout.trim().split(/\n\r?\n/); - - // Confirm there are no violations. - // Since it's a CSV, the rows themselves are separated by newline characters. - // The header should not have any newline characters after it. There should be no violation rows. - expect(csv.indexOf('\n')).to.equal(-1, "Should be no violations detected"); + // There should be no violations. + expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, eslint, retire-js'])); }); // TODO: THIS TEST WAS IMPLEMENTED FOR W-7791882. THE FIX FOR THAT BUG WAS SUB-OPTIMAL, AND WE NEED TO CHANGE IT IN 3.0. @@ -763,13 +758,7 @@ describe('scanner:run', function () { '--env', '{"qunit": true}' ]) .it('Providing qunit in the --env override should resolve errors about that framework', ctx => { - // If there's a summary, then it'll be separated from the CSV by an empty line. Throw it away. - const [csv, _] = ctx.stdout.trim().split(/\n\r?\n/); - - // Confirm there are no violations. - // Since it's a CSV, the rows themselves are separated by newline characters. - // The header should not have any newline characters after it. There should be no violation rows. - expect(csv.indexOf('\n')).to.equal(-1, "Should be no violations detected"); + expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, eslint, retire-js'])); }); }); diff --git a/test/lib/util/RunOutputProcessor.test.ts b/test/lib/util/RunOutputProcessor.test.ts index 54e961b0c..4265aa40d 100644 --- a/test/lib/util/RunOutputProcessor.test.ts +++ b/test/lib/util/RunOutputProcessor.test.ts @@ -62,16 +62,6 @@ const FAKE_TABLE_OUTPUT = { ] }; -const EMPTY_TABLE_OUTPUT = { - "columns": [ - "Location", - "Description", - "Category", - "URL" - ], - "rows": [] -}; - const FAKE_CSV_OUTPUT = `"Problem","File","Severity","Line","Column","Rule","Description","URL","Category","Engine" "1","/Users/jfeingold/ts-sample-project/src/file-with-problems.ts","2","3","7","no-unused-vars","'UNUSED' is assigned a value but never used.","https://eslint.org/docs/rules/no-unused-vars","Variables","eslint-typescript" "2","/Users/jfeingold/ts-sample-project/src/file-with-problems.ts","2","3","7","@typescript-eslint/no-unused-vars","'UNUSED' is assigned a value but never used.","https://github.com/typescript-eslint/typescript-eslint/blob/v2.33.0/packages/eslint-plugin/docs/rules/no-unused-vars.md","Variables","eslint-typescript" @@ -146,18 +136,19 @@ describe('RunOutputProcessor', () => { }; const rop = new RunOutputProcessor(opts, testUx); const summaryMap: Map = new Map(); - const fakeTableResults: RecombinedRuleResults = {minSev: 1, results: EMPTY_TABLE_OUTPUT, summaryMap: summaryMap} + summaryMap.set('pmd', {fileCount: 0, violationCount: 0}); + summaryMap.set('eslint', {fileCount: 0, violationCount: 0}); + const fakeRes: RecombinedRuleResults = {minSev: 0, summaryMap, results: ''}; // THIS IS THE PART BEING TESTED. - const output: AnyJson = rop.processRunOutput(fakeTableResults); + const output: AnyJson = rop.processRunOutput(fakeRes); - const expectedTableSummary = `${processorMessages.getMessage('output.writtenToConsole')}`; - // We expect that an empty table should be logged to the console and that the output should be an empty array + // We expect that the message logged to the console and the message returned should both be the default + const expectedMsg = processorMessages.getMessage('output.noViolationsDetected', ['pmd, eslint']); Sinon.assert.callCount(logSpy, 1); - Sinon.assert.callCount(tableSpy, 1); - Sinon.assert.calledWith(logSpy, expectedTableSummary); - // TODO is there a better way to check for empty array? output is an empty array, but typed as AnyJson. - expect(output.toString()).to.equal('', 'Should have returned empty results'); + Sinon.assert.callCount(tableSpy, 0); + Sinon.assert.calledWith(logSpy, expectedMsg); + expect(output).to.equal(expectedMsg, 'Should have returned expected message'); }); describe('Test Case: Table', () => {