From cb48d08dce145954a6cd546baa7e79d5b4c0aa4c Mon Sep 17 00:00:00 2001 From: Shane McLaughlin Date: Tue, 19 Sep 2023 12:15:49 -0500 Subject: [PATCH] fix: properly render `uncovered lines` column (#950) * feat: improve code coverage percentage outputs and add yellow color * fix: use red for values lower than 75% * fix: not covered lines aren't json * refactor: share table printing function (dry) * test: coverage line formatting --------- Co-authored-by: Allan Oricil --- src/coverageUtils.ts | 33 +- src/formatters/codeCoverageTable.ts | 42 ++ src/formatters/deployResultFormatter.ts | 23 +- .../mdapi/mdDeployResultFormatter.ts | 25 +- test/coverageUtils.test.ts | 426 ++++++++++++------ 5 files changed, 359 insertions(+), 190 deletions(-) create mode 100644 src/formatters/codeCoverageTable.ts diff --git a/src/coverageUtils.ts b/src/coverageUtils.ts index 7bcf7f829..e220dd090 100644 --- a/src/coverageUtils.ts +++ b/src/coverageUtils.ts @@ -39,27 +39,32 @@ function mapTestResults(testResults: Failures[] | Successes[]): ApexTestResultDa }); } -export function prepCoverageForDisplay(codeCoverage: CodeCoverage[]): CodeCoverage[] { +export function prepCoverageForDisplay(codeCoverage: CodeCoverage[]): Array { const coverage = codeCoverage.sort((a, b) => (a.name.toUpperCase() > b.name.toUpperCase() ? 1 : -1)); - coverage.forEach((cov: CodeCoverage & { lineNotCovered: string }) => { - const numLocationsNum = parseInt(cov.numLocations, 10); - const numLocationsNotCovered = parseInt(cov.numLocationsNotCovered, 10); - const color = numLocationsNotCovered > 0 ? chalk.red : chalk.green; - - const coverageDecimal = parseFloat(((numLocationsNum - numLocationsNotCovered) / numLocationsNum).toFixed(2)); - const pctCovered = numLocationsNum > 0 ? coverageDecimal * 100 : 100; - cov.numLocations = color(`${pctCovered}%`); - - cov.lineNotCovered = cov.locationsNotCovered + return coverage.map((cov) => ({ + ...cov, + numLocations: stylePercentage(calculateCoveragePercent(cov)), + lineNotCovered: cov.locationsNotCovered ? ensureArray(cov.locationsNotCovered) .map((location) => location.line) .join(',') - : ''; - }); - return coverage; + : '', + })); } +const stylePercentage = (pct: number): string => { + const color = pct < 75 ? chalk.red : pct >= 90 ? chalk.green : chalk.yellow; + return color(`${pct}%`); +}; + +const calculateCoveragePercent = (cov: CodeCoverage): number => { + const numLocationsNum = parseInt(cov.numLocations, 10); + const numLocationsNotCovered = parseInt(cov.numLocationsNotCovered, 10); + const coverageDecimal = parseFloat(((numLocationsNum - numLocationsNotCovered) / numLocationsNum).toFixed(2)); + return numLocationsNum > 0 ? coverageDecimal * 100 : 100; +}; + function generateCoveredLines(cov: CodeCoverage): [number[], number[]] { const numCovered = parseInt(cov.numLocations, 10); const numUncovered = parseInt(cov.numLocationsNotCovered, 10); diff --git a/src/formatters/codeCoverageTable.ts b/src/formatters/codeCoverageTable.ts new file mode 100644 index 000000000..6b277be68 --- /dev/null +++ b/src/formatters/codeCoverageTable.ts @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2023, salesforce.com, inc. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ +import { ensureArray } from '@salesforce/kit'; +import { Ux } from '@salesforce/sf-plugins-core'; +import { CodeCoverage } from '@salesforce/source-deploy-retrieve'; +import chalk = require('chalk'); +import { prepCoverageForDisplay } from '../coverageUtils'; + +/** + * prints a table of formatted code coverage results if there are any + * This is a no-op if tests didn't run or there is no coverage + * + * @param coverageFromMdapiResult + * @param ux + */ +export const maybePrintCodeCoverageTable = (coverageFromMdapiResult: CodeCoverage | CodeCoverage[], ux: Ux): void => { + const codeCoverage = ensureArray(coverageFromMdapiResult); + + if (codeCoverage.length) { + const coverage = prepCoverageForDisplay(codeCoverage); + + ux.log(''); + ux.styledHeader(chalk.blue('Apex Code Coverage')); + + ux.table( + coverage.map((entry) => ({ + name: entry.name, + numLocations: entry.numLocations, + lineNotCovered: entry.lineNotCovered, + })), + { + name: { header: 'Name' }, + numLocations: { header: '% Covered' }, + lineNotCovered: { header: 'Uncovered Lines' }, + } + ); + } +}; diff --git a/src/formatters/deployResultFormatter.ts b/src/formatters/deployResultFormatter.ts index c4d9b1230..4ce2f7226 100644 --- a/src/formatters/deployResultFormatter.ts +++ b/src/formatters/deployResultFormatter.ts @@ -22,9 +22,9 @@ import { Successes, } from '@salesforce/source-deploy-retrieve'; import { Ux } from '@salesforce/sf-plugins-core'; -import { prepCoverageForDisplay } from '../coverageUtils'; import { ResultFormatter, ResultFormatterOptions } from './resultFormatter'; import { MdDeployResult } from './mdapi/mdDeployResultFormatter'; +import { maybePrintCodeCoverageTable } from './codeCoverageTable'; Messages.importMessagesDirectory(__dirname); const messages = Messages.loadMessages('@salesforce/plugin-source', 'deploy'); @@ -304,26 +304,7 @@ export class DeployResultFormatter extends ResultFormatter { } ); } - const codeCoverage = ensureArray(this.result?.response?.details?.runTestResult?.codeCoverage); - - if (codeCoverage.length) { - const coverage = prepCoverageForDisplay(codeCoverage); - - this.ux.log(''); - this.ux.styledHeader(chalk.blue('Apex Code Coverage')); - this.ux.table( - coverage.map((cov) => ({ - name: cov.name, - numLocations: cov.numLocations, - lineNotCovered: cov.locationsNotCovered, - })), - { - name: { header: 'Name' }, - numLocations: { header: '% Covered' }, - lineNotCovered: { header: 'Uncovered Lines' }, - } - ); - } + maybePrintCodeCoverageTable(this.result.response.details?.runTestResult?.codeCoverage, this.ux); } protected verboseTestTime(): void { diff --git a/src/formatters/mdapi/mdDeployResultFormatter.ts b/src/formatters/mdapi/mdDeployResultFormatter.ts index 92a6d2afe..455ca466a 100644 --- a/src/formatters/mdapi/mdDeployResultFormatter.ts +++ b/src/formatters/mdapi/mdDeployResultFormatter.ts @@ -19,7 +19,7 @@ import { import { ensureArray } from '@salesforce/kit'; import { Ux } from '@salesforce/sf-plugins-core'; import { CoverageResultsFileInfo, ResultFormatter, ResultFormatterOptions } from '../resultFormatter'; -import { prepCoverageForDisplay } from '../../coverageUtils'; +import { maybePrintCodeCoverageTable } from '../codeCoverageTable'; Messages.importMessagesDirectory(__dirname); const messages = Messages.loadMessages('@salesforce/plugin-source', 'md.deploy'); @@ -212,28 +212,7 @@ export class MdDeployResultFormatter extends ResultFormatter { } ); } - const codeCoverage = ensureArray(this.result?.response?.details?.runTestResult?.codeCoverage); - - if (codeCoverage.length) { - const coverage = prepCoverageForDisplay(codeCoverage); - - this.ux.log(''); - this.ux.styledHeader(chalk.blue('Apex Code Coverage')); - - // TODO: unsure about locationsNotCovered vs lineNotCovered - this.ux.table( - coverage.map((entry) => ({ - name: entry.name, - numLocations: entry.numLocations, - lineNotCovered: entry.locationsNotCovered, - })), - { - name: { header: 'Name' }, - numLocations: { header: '% Covered' }, - lineNotCovered: { header: 'Uncovered Lines' }, - } - ); - } + maybePrintCodeCoverageTable(this.result.response.details?.runTestResult?.codeCoverage, this.ux); } protected verboseTestTime(): void { diff --git a/test/coverageUtils.test.ts b/test/coverageUtils.test.ts index 4ae501e79..74690e088 100644 --- a/test/coverageUtils.test.ts +++ b/test/coverageUtils.test.ts @@ -8,182 +8,296 @@ import { expect } from 'chai'; import { MockTestOrgData, TestContext } from '@salesforce/core/lib/testSetup'; import { AuthInfo, Connection } from '@salesforce/core'; import { createSandbox, SinonSandbox } from 'sinon'; -import { transformCoverageToApexCoverage } from '../src/coverageUtils'; +import * as chalk from 'chalk'; +import { transformCoverageToApexCoverage, prepCoverageForDisplay } from '../src/coverageUtils'; import { transformDeployTestsResultsToTestResult } from '../src/coverageUtils'; -const sampleRunTestResult = { - codeCoverage: [ - { - id: '01p19000002uDLAAA2', - locationsNotCovered: { - column: '0', - line: '12', - numExecutions: '0', - time: '-1.0', - }, - name: 'PagedResult', - namespace: { - $: { - 'xsi:nil': 'true', +// methods are mutating the object instead of returning new ones +function getSampleTestResult() { + return { + codeCoverage: [ + { + id: '01p19000002uDLAAA2', + locationsNotCovered: { + column: '0', + line: '12', + numExecutions: '0', + time: '-1.0', }, + name: 'PagedResult', + namespace: { + $: { + 'xsi:nil': 'true', + }, + }, + numLocations: '4', + numLocationsNotCovered: '1', + type: 'Class', }, - numLocations: '4', - numLocationsNotCovered: '1', - type: 'Class', - }, - { - id: '01p19000002uDLBAA2', - locationsNotCovered: [ - { + { + id: '01p19000002uDLBAA2', + locationsNotCovered: [ + { + column: '0', + line: '26', + numExecutions: '0', + time: '-1.0', + }, + { + column: '0', + line: '31', + numExecutions: '0', + time: '-1.0', + }, + { + column: '0', + line: '78', + numExecutions: '0', + time: '-1.0', + }, + ], + name: 'PropertyController', + namespace: { + $: { + 'xsi:nil': 'true', + }, + }, + numLocations: '44', + numLocationsNotCovered: '3', + type: 'Class', + }, + { + id: '01p19000002uDLCAA2', + name: 'SampleDataController', + namespace: { + $: { + 'xsi:nil': 'true', + }, + }, + numLocations: '34', + numLocationsNotCovered: '0', + type: 'Class', + }, + { + id: '01p19000002uDL8AAM', + name: 'GeocodingService', + namespace: { + $: { + 'xsi:nil': 'true', + }, + }, + numLocations: '36', + numLocationsNotCovered: '0', + type: 'Class', + }, + { + id: '01p19000002uDLAAAN', + locationsNotCovered: { column: '0', - line: '26', + line: '12', numExecutions: '0', time: '-1.0', }, - { + name: 'A', + namespace: { + $: { + 'xsi:nil': 'true', + }, + }, + numLocations: '100', + numLocationsNotCovered: '100', + type: 'Class', + }, + { + id: '01p19000002uDLAAAN', + locationsNotCovered: { column: '0', - line: '31', + line: '12', numExecutions: '0', time: '-1.0', }, - { + name: 'B', + namespace: { + $: { + 'xsi:nil': 'true', + }, + }, + numLocations: '100', + numLocationsNotCovered: '26', + type: 'Class', + }, + { + id: '01p19000002uDLAABN', + locationsNotCovered: { column: '0', - line: '78', + line: '12', numExecutions: '0', time: '-1.0', }, - ], - name: 'PropertyController', - namespace: { - $: { - 'xsi:nil': 'true', + name: 'C', + namespace: { + $: { + 'xsi:nil': 'true', + }, }, + numLocations: '100', + numLocationsNotCovered: '25', + type: 'Class', }, - numLocations: '44', - numLocationsNotCovered: '3', - type: 'Class', - }, - { - id: '01p19000002uDLCAA2', - name: 'SampleDataController', - namespace: { - $: { - 'xsi:nil': 'true', + { + id: '01p19000002uDLAABN', + locationsNotCovered: { + column: '0', + line: '12', + numExecutions: '0', + time: '-1.0', + }, + name: 'D', + namespace: { + $: { + 'xsi:nil': 'true', + }, }, + numLocations: '100', + numLocationsNotCovered: '11', + type: 'Class', }, - numLocations: '34', - numLocationsNotCovered: '0', - type: 'Class', - }, - { - id: '01p19000002uDL8AAM', - name: 'GeocodingService', - namespace: { - $: { - 'xsi:nil': 'true', + { + id: '01p19000002uDLAABN', + locationsNotCovered: { + column: '0', + line: '12', + numExecutions: '0', + time: '-1.0', + }, + name: 'E', + namespace: { + $: { + 'xsi:nil': 'true', + }, }, + numLocations: '100', + numLocationsNotCovered: '10', + type: 'Class', }, - numLocations: '36', - numLocationsNotCovered: '0', - type: 'Class', - }, - ], - failures: { - id: '01p19000002uDLDAA2', - message: 'System.QueryException: Insufficient permissions: secure query included inaccessible field', - methodName: 'testGetPagedPropertyList', - name: 'TestPropertyController', - namespace: { - $: { - 'xsi:nil': 'true', + { + id: '01p19000002uDLAACN', + locationsNotCovered: { + column: '0', + line: '12', + numExecutions: '0', + time: '-1.0', + }, + name: 'F', + namespace: { + $: { + 'xsi:nil': 'true', + }, + }, + numLocations: '100', + numLocationsNotCovered: '0', + type: 'Class', }, - }, - packageName: 'TestPropertyController', - stackTrace: - 'Class.PropertyController.getPagedPropertyList: line 52, column 1\nClass.TestPropertyController.testGetPagedPropertyList: line 22, column 1', - time: '604.0', - type: 'Class', - }, - numFailures: '1', - numTestsRun: '7', - successes: [ - { - id: '01p19000002uDL9AAM', - methodName: 'blankAddress', - name: 'GeocodingServiceTest', + ], + failures: { + id: '01p19000002uDLDAA2', + message: 'System.QueryException: Insufficient permissions: secure query included inaccessible field', + methodName: 'testGetPagedPropertyList', + name: 'TestPropertyController', namespace: { $: { 'xsi:nil': 'true', }, }, - time: '26.0', + packageName: 'TestPropertyController', + stackTrace: + 'Class.PropertyController.getPagedPropertyList: line 52, column 1\nClass.TestPropertyController.testGetPagedPropertyList: line 22, column 1', + time: '604.0', + type: 'Class', }, - { - id: '01p19000002uDL9AAM', - methodName: 'errorResponse', - name: 'GeocodingServiceTest', - namespace: { - $: { - 'xsi:nil': 'true', + numFailures: '1', + numTestsRun: '7', + successes: [ + { + id: '01p19000002uDL9AAM', + methodName: 'blankAddress', + name: 'GeocodingServiceTest', + namespace: { + $: { + 'xsi:nil': 'true', + }, }, + time: '26.0', }, - time: '77.0', - }, - { - id: '01p19000002uDL9AAM', - methodName: 'successResponse', - name: 'GeocodingServiceTest', - namespace: { - $: { - 'xsi:nil': 'true', + { + id: '01p19000002uDL9AAM', + methodName: 'errorResponse', + name: 'GeocodingServiceTest', + namespace: { + $: { + 'xsi:nil': 'true', + }, }, + time: '77.0', }, - time: '63.0', - }, - { - id: '01p19000002uDLDAA2', - methodName: 'testGetPicturesNoResults', - name: 'TestPropertyController', - namespace: { - $: { - 'xsi:nil': 'true', + { + id: '01p19000002uDL9AAM', + methodName: 'successResponse', + name: 'GeocodingServiceTest', + namespace: { + $: { + 'xsi:nil': 'true', + }, }, + time: '63.0', }, - time: '691.0', - }, - { - id: '01p19000002uDLDAA2', - methodName: 'testGetPicturesWithResults', - name: 'TestPropertyController', - namespace: { - $: { - 'xsi:nil': 'true', + { + id: '01p19000002uDLDAA2', + methodName: 'testGetPicturesNoResults', + name: 'TestPropertyController', + namespace: { + $: { + 'xsi:nil': 'true', + }, }, + time: '691.0', }, - time: '1873.0', - }, - { - id: '01p19000002uDLEAA2', - methodName: 'importSampleData', - name: 'TestSampleDataController', - namespace: { - $: { - 'xsi:nil': 'true', + { + id: '01p19000002uDLDAA2', + methodName: 'testGetPicturesWithResults', + name: 'TestPropertyController', + namespace: { + $: { + 'xsi:nil': 'true', + }, }, + time: '1873.0', }, - time: '1535.0', - }, - ], - totalTime: '4952.0', -}; + { + id: '01p19000002uDLEAA2', + methodName: 'importSampleData', + name: 'TestSampleDataController', + namespace: { + $: { + 'xsi:nil': 'true', + }, + }, + time: '1535.0', + }, + ], + totalTime: '4952.0', + }; +} describe('transform md RunTestResult', () => { const $$ = new TestContext(); let mockConnection: Connection; const testData = new MockTestOrgData(); + let sampleTestResult = getSampleTestResult(); let sandboxStub: SinonSandbox; beforeEach(async () => { + sampleTestResult = getSampleTestResult(); sandboxStub = createSandbox(); $$.setConfigStubContents('StateAggregator', { @@ -206,9 +320,10 @@ describe('transform md RunTestResult', () => { afterEach(() => { sandboxStub.restore(); }); + it('should transform md coverage to apex coverage format', () => { - const apexCoverage = transformCoverageToApexCoverage(sampleRunTestResult.codeCoverage); - expect(apexCoverage.records).to.have.length(4); + const apexCoverage = transformCoverageToApexCoverage(sampleTestResult.codeCoverage); + expect(apexCoverage.records).to.have.length(10); expect(apexCoverage.records[0].ApexClassOrTrigger.Name).to.equal('PagedResult'); expect(apexCoverage.records[1].ApexClassOrTrigger.Name).to.equal('PropertyController'); expect(apexCoverage.records[2].ApexClassOrTrigger.Name).to.equal('SampleDataController'); @@ -219,8 +334,55 @@ describe('transform md RunTestResult', () => { expect(apexCoverage.records[2].Coverage.uncoveredLines).to.have.lengthOf(apexCoverage.records[2].NumLinesUncovered); expect(apexCoverage.records[2].Coverage.coveredLines).to.have.lengthOf(apexCoverage.records[2].NumLinesCovered); }); + it('should transform md test results to apex test results format', () => { - const apexTestResults = transformDeployTestsResultsToTestResult(mockConnection, sampleRunTestResult); + const apexTestResults = transformDeployTestsResultsToTestResult(mockConnection, sampleTestResult); expect(apexTestResults).to.be.ok; }); + + it('should display code coverage classes ordered alphabetically', () => { + const codeCoverage = prepCoverageForDisplay(sampleTestResult.codeCoverage); + + expect(codeCoverage[0].name).to.equal('A'); + expect(codeCoverage[1].name).to.equal('B'); + expect(codeCoverage[2].name).to.equal('C'); + expect(codeCoverage[3].name).to.equal('D'); + expect(codeCoverage[4].name).to.equal('E'); + expect(codeCoverage[5].name).to.equal('F'); + expect(codeCoverage[6].name).to.equal('GeocodingService'); + expect(codeCoverage[7].name).to.equal('PagedResult'); + expect(codeCoverage[8].name).to.equal('PropertyController'); + expect(codeCoverage[9].name).to.equal('SampleDataController'); + }); + + it('should display code coverage percentage with red color when its value is < 75%', () => { + const codeCoverage = prepCoverageForDisplay(sampleTestResult.codeCoverage); + expect(codeCoverage[0].numLocations).to.equal(chalk.red('0%')); + expect(codeCoverage[1].numLocations).to.equal(chalk.red('74%')); + }); + + it('should display code coverage percentage with yellow color when its value is >= 75% and < 90%', () => { + const codeCoverage = prepCoverageForDisplay(sampleTestResult.codeCoverage); + expect(codeCoverage[2].numLocations).to.equal(chalk.yellow('75%')); + expect(codeCoverage[3].numLocations).to.equal(chalk.yellow('89%')); + }); + + it('should display code coverage percentage with green color when its value is >= 90%', () => { + const codeCoverage = prepCoverageForDisplay(sampleTestResult.codeCoverage); + expect(codeCoverage[4].numLocations).to.equal(chalk.green('90%')); + expect(codeCoverage[5].numLocations).to.equal(chalk.green('100%')); + }); + + it('lineNotCovered is empty string when there is no data', () => { + const codeCoverage = prepCoverageForDisplay(sampleTestResult.codeCoverage); + expect(codeCoverage.find((c) => c.name === 'SampleDataController').lineNotCovered).equal(''); + }); + it('lineNotCovered is single number for one item', () => { + const codeCoverage = prepCoverageForDisplay(sampleTestResult.codeCoverage); + expect(codeCoverage.find((c) => c.name === 'PagedResult').lineNotCovered).equal('12'); + }); + it('lineNotCovered is comma separated list for multiple items', () => { + const codeCoverage = prepCoverageForDisplay(sampleTestResult.codeCoverage); + expect(codeCoverage.find((c) => c.name === 'PropertyController').lineNotCovered).equal('26,31,78'); + }); });