From bc2758628366e28b37219aec5ba108edbe46d9a0 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Wed, 13 Jul 2022 16:09:51 -0500 Subject: [PATCH 1/2] @W-11397711@: Added banner requesting that people take our survey. --- messages/common.js | 3 +++ src/commands/scanner/rule/add.ts | 3 ++- src/commands/scanner/rule/describe.ts | 2 ++ src/commands/scanner/rule/list.ts | 2 ++ src/commands/scanner/rule/remove.ts | 2 ++ src/lib/ScannerRunCommand.ts | 2 ++ test/commands/scanner/rule/list.test.ts | 7 ++++--- test/commands/scanner/run.severity.test.ts | 14 ++++++++------ test/commands/scanner/run.test.ts | 4 ++-- 9 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 messages/common.js diff --git a/messages/common.js b/messages/common.js new file mode 100644 index 000000000..46571ef4f --- /dev/null +++ b/messages/common.js @@ -0,0 +1,3 @@ +module.exports = { + FEEDBACK_SURVEY_BANNER: `What do you think of Salesforce Code Analyzer? Take our feedback survey at https://research.net/r/SalesforceCA.` +}; diff --git a/src/commands/scanner/rule/add.ts b/src/commands/scanner/rule/add.ts index 54e76c231..b4e34515b 100644 --- a/src/commands/scanner/rule/add.ts +++ b/src/commands/scanner/rule/add.ts @@ -13,7 +13,7 @@ Messages.importMessagesDirectory(__dirname); // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core, // or any library that is using the messages framework can also be loaded this way. const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'add'); - +const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common'); export default class Add extends SfdxCommand { @@ -41,6 +41,7 @@ export default class Add extends SfdxCommand { public async run(): Promise { this.validateFlags(); + this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); const language = this.flags.language as string; const paths = this.resolvePaths(); diff --git a/src/commands/scanner/rule/describe.ts b/src/commands/scanner/rule/describe.ts index 1e2557305..c5dd74f6a 100644 --- a/src/commands/scanner/rule/describe.ts +++ b/src/commands/scanner/rule/describe.ts @@ -12,6 +12,7 @@ Messages.importMessagesDirectory(__dirname); // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core, // or any library that is using the messages framework can also be loaded this way. const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'describe'); +const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common'); type DescribeStyledRule = Rule & { enabled: boolean; @@ -41,6 +42,7 @@ export default class Describe extends ScannerCommand { }; public async run(): Promise { + this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); const ruleFilters = this.buildRuleFilters(); // It's possible for this line to throw an error, but that's fine because the error will be an SfdxError that we can // allow to boil over. diff --git a/src/commands/scanner/rule/list.ts b/src/commands/scanner/rule/list.ts index ffe4ddc84..ba5406632 100644 --- a/src/commands/scanner/rule/list.ts +++ b/src/commands/scanner/rule/list.ts @@ -13,6 +13,7 @@ Messages.importMessagesDirectory(__dirname); // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core, // or any library that is using the messages framework can also be loaded this way. const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'list'); +const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common'); const columns = [messages.getMessage('columnNames.name'), messages.getMessage('columnNames.languages'), messages.getMessage('columnNames.categories'), @@ -64,6 +65,7 @@ export default class List extends ScannerCommand { }; public async run(): Promise { + this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); const ruleFilters = this.buildRuleFilters(); // It's possible for this line to throw an error, but that's fine because the error will be an SfdxError that we can // allow to boil over. diff --git a/src/commands/scanner/rule/remove.ts b/src/commands/scanner/rule/remove.ts index b87f712ec..f121782de 100644 --- a/src/commands/scanner/rule/remove.ts +++ b/src/commands/scanner/rule/remove.ts @@ -15,6 +15,7 @@ Messages.importMessagesDirectory(__dirname); // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core, // or any library that is using the messages framework can also be loaded this way. const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'remove'); +const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common'); export default class Remove extends ScannerCommand { // These determine what's displayed when the --help/-h flag is supplied. @@ -44,6 +45,7 @@ export default class Remove extends ScannerCommand { }; public async run(): Promise { + this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); // Step 1: Validate our input. this.validateFlags(); diff --git a/src/lib/ScannerRunCommand.ts b/src/lib/ScannerRunCommand.ts index 4e4fb69e3..6cc092fee 100644 --- a/src/lib/ScannerRunCommand.ts +++ b/src/lib/ScannerRunCommand.ts @@ -14,6 +14,7 @@ Messages.importMessagesDirectory(__dirname); // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core, // or any library that is using the messages framework can also be loaded this way. const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'run'); +const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common'); // This code is used for internal errors. export const INTERNAL_ERROR_CODE = 1; @@ -22,6 +23,7 @@ export abstract class ScannerRunCommand extends ScannerCommand { public async run(): Promise { // First, do any validations that can't be handled with out-of-the-box stuff. await this.validateFlags(); + this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); // If severity-threshold is used, that implicitly normalizes the severity. const normalizeSeverity: boolean = (this.flags['normalize-severity'] || this.flags['severity-threshold']) as boolean; diff --git a/test/commands/scanner/rule/list.test.ts b/test/commands/scanner/rule/list.test.ts index 3bfc3ef50..7330f892d 100644 --- a/test/commands/scanner/rule/list.test.ts +++ b/test/commands/scanner/rule/list.test.ts @@ -54,11 +54,12 @@ describe('scanner:rule:list', () => { .it('All rules for enabled engines are returned', async ctx => { const totalRuleCount = await getRulesFilteredByCategoryCount(false); - // Split the output table by newline and throw out the first two rows, since they just contain header information. That + // Split the output table by newline and throw out the first three rows, since they just contain header information. That // should leave us with the actual data. const rows = ctx.stdout.trim().split('\n'); rows.shift(); rows.shift(); + rows.shift(); expect(rows).to.have.lengthOf(totalRuleCount, 'All rules should have been returned'); }); @@ -305,9 +306,9 @@ describe('scanner:rule:list', () => { setupCommandTest .command(['scanner:rule:list', '--category', 'Beebleborp']) .it('Without --json flag, an empty table is printed', ctx => { - // Split the result by newline, and make sure there are two rows. + // Split the result by newline, and make sure there are three rows. const rows = ctx.stdout.trim().split('\n'); - expect(rows).to.have.lengthOf(2, 'Only the header rows should have been printed'); + expect(rows).to.have.lengthOf(3, 'Only the header rows should have been printed'); }); setupCommandTest diff --git a/test/commands/scanner/run.severity.test.ts b/test/commands/scanner/run.severity.test.ts index 6c9384dc4..cddfaf993 100644 --- a/test/commands/scanner/run.severity.test.ts +++ b/test/commands/scanner/run.severity.test.ts @@ -31,8 +31,8 @@ describe('scanner:run', function () { '--severity-threshold', '1' ]) .it('When no violations are found equal to or greater than flag value, no error is thrown', ctx => { - - const output = JSON.parse(ctx.stdout); + // The first line is a header. Strip that away, and parse the rest as JSON. + const output = JSON.parse(ctx.stdout.split('\n')[1]); // check that test file still has severities of 3 for (let i=0; i { - - const output = JSON.parse(ctx.stdout); + // The first line is a header. Strip that away, and parse the rest as JSON. + const output = JSON.parse(ctx.stdout.split('\n')[1]); // check that test file still has severities of 3 for (let i=0; i { - const output = JSON.parse(ctx.stdout); + // The first line is a header. Strip that away, and parse the rest as JSON. + const output = JSON.parse(ctx.stdout.split('\n')[1]); for (let i=0; i { - const output = JSON.parse(ctx.stdout); + // The first line is a header. Strip that away, and parse the rest as JSON. + const output = JSON.parse(ctx.stdout.split('\n')[1]); for (let i=0; i { - // Split the output by newline characters and throw away the first entry, so we're left with just the rows. - validateCsvOutput(ctx.stdout, false); + // Throw away the first line of output, since it's a banner. + validateCsvOutput(ctx.stdout.slice(ctx.stdout.indexOf('\n')), false); }); setupCommandTest From ddeb2e5ec955b9ba98e52de3f2d35f06ffd48c65 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Thu, 14 Jul 2022 13:40:48 -0500 Subject: [PATCH 2/2] @W-11397711@: Integrated feedback from code review. --- test/TestUtils.ts | 9 ++- test/commands/scanner/rule/list.test.ts | 15 +++-- test/commands/scanner/run.severity.test.ts | 74 +++++++++++----------- test/commands/scanner/run.test.ts | 6 +- 4 files changed, 56 insertions(+), 48 deletions(-) diff --git a/test/TestUtils.ts b/test/TestUtils.ts index b50b3770d..06da598c4 100644 --- a/test/TestUtils.ts +++ b/test/TestUtils.ts @@ -1,6 +1,7 @@ import fs = require('fs'); import path = require('path'); -import { test } from '@salesforce/command/lib/test'; +import { test, expect } from '@salesforce/command/lib/test'; +import * as CommonMessages from '../messages/common.js'; import * as TestOverrides from './test-related-lib/TestOverrides'; import Sinon = require('sinon'); import LocalCatalog from '../src/lib/services/LocalCatalog'; @@ -15,6 +16,12 @@ export function prettyPrint(obj: any): string { return JSON.stringify(obj, null, 2); } +export function stripExtraneousOutput(stdout: string): string { + const splitOutput = stdout.split('\n'); + expect(splitOutput[0]).to.equal('=== ' + CommonMessages.FEEDBACK_SURVEY_BANNER); + return splitOutput.slice(1).join('\n'); +} + export function stubCatalogFixture(): void { // Make sure all catalogs exist where they're supposed to. if (!fs.existsSync(CATALOG_FIXTURE_PATH)) { diff --git a/test/commands/scanner/rule/list.test.ts b/test/commands/scanner/rule/list.test.ts index 7330f892d..973782dfd 100644 --- a/test/commands/scanner/rule/list.test.ts +++ b/test/commands/scanner/rule/list.test.ts @@ -1,5 +1,5 @@ import {expect} from '@salesforce/command/lib/test'; -import {setupCommandTest} from '../../../TestUtils'; +import {setupCommandTest, stripExtraneousOutput} from '../../../TestUtils'; import {Rule} from '../../../../src/types'; import {CATALOG_FILE, ENGINE} from '../../../../src/Constants'; import fs = require('fs'); @@ -54,10 +54,10 @@ describe('scanner:rule:list', () => { .it('All rules for enabled engines are returned', async ctx => { const totalRuleCount = await getRulesFilteredByCategoryCount(false); - // Split the output table by newline and throw out the first three rows, since they just contain header information. That + // Split the output table by newline and throw out the first two rows, since they just contain header information. That // should leave us with the actual data. - const rows = ctx.stdout.trim().split('\n'); - rows.shift(); + const output = stripExtraneousOutput(ctx.stdout); + const rows = output.trim().split('\n'); rows.shift(); rows.shift(); expect(rows).to.have.lengthOf(totalRuleCount, 'All rules should have been returned'); @@ -306,9 +306,10 @@ describe('scanner:rule:list', () => { setupCommandTest .command(['scanner:rule:list', '--category', 'Beebleborp']) .it('Without --json flag, an empty table is printed', ctx => { - // Split the result by newline, and make sure there are three rows. - const rows = ctx.stdout.trim().split('\n'); - expect(rows).to.have.lengthOf(3, 'Only the header rows should have been printed'); + // Split the result by newline, and make sure there are two rows. + const output = stripExtraneousOutput(ctx.stdout); + const rows = output.trim().split('\n'); + expect(rows).to.have.lengthOf(2, 'Only the header rows should have been printed'); }); setupCommandTest diff --git a/test/commands/scanner/run.severity.test.ts b/test/commands/scanner/run.severity.test.ts index cddfaf993..4e411acff 100644 --- a/test/commands/scanner/run.severity.test.ts +++ b/test/commands/scanner/run.severity.test.ts @@ -1,5 +1,5 @@ import {expect} from '@salesforce/command/lib/test'; -import {setupCommandTest} from '../../TestUtils'; +import {setupCommandTest, stripExtraneousOutput} from '../../TestUtils'; import {Messages} from '@salesforce/core'; import path = require('path'); @@ -31,12 +31,12 @@ describe('scanner:run', function () { '--severity-threshold', '1' ]) .it('When no violations are found equal to or greater than flag value, no error is thrown', ctx => { - // The first line is a header. Strip that away, and parse the rest as JSON. - const output = JSON.parse(ctx.stdout.split('\n')[1]); + const output = stripExtraneousOutput(ctx.stdout); + const outputJson = JSON.parse(output); // check that test file still has severities of 3 - for (let i=0; i { - // The first line is a header. Strip that away, and parse the rest as JSON. - const output = JSON.parse(ctx.stdout.split('\n')[1]); + const output = stripExtraneousOutput(ctx.stdout); + const outputJson = JSON.parse(output); // check that test file still has severities of 3 - for (let i=0; i { - // The first line is a header. Strip that away, and parse the rest as JSON. - const output = JSON.parse(ctx.stdout.split('\n')[1]); - - for (let i=0; i { - // The first line is a header. Strip that away, and parse the rest as JSON. - const output = JSON.parse(ctx.stdout.split('\n')[1]); + const output = stripExtraneousOutput(ctx.stdout); + const outputJson = JSON.parse(output); - for (let i=0; i { - // Throw away the first line of output, since it's a banner. - validateCsvOutput(ctx.stdout.slice(ctx.stdout.indexOf('\n')), false); + const output = stripExtraneousOutput(ctx.stdout); + validateCsvOutput(output, false); }); setupCommandTest