fix: remove covered line adjustment in deploy coverage

mcarvin8 · Jun 14, 2024 · bc6c5b2 · bc6c5b2
1 parent cc289a2
commit bc6c5b2
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 222 deletions.
diff --git a/README.md b/README.md
@@ -25,8 +25,6 @@ sf apex get test --test-run-id <test run id> --code-coverage --result-format jso
 
 The code coverage JSONs created by the Salesforce CLI aren't accepted by SonarQube automatically for git-based Salesforce repositories and needs to be converted using this plugin.
 
-**Disclaimer**: Due to existing bugs with how the Salesforce CLI reports covered lines during deployments (see [5511](https://github.com/forcedotcom/salesforcedx-vscode/issues/5511) and [1568](https://github.com/forcedotcom/cli/issues/1568)), to add support for covered lines in this plugin for deployment coverage files, I had to add a function to re-number out-of-range covered lines the CLI may report (ex: line 100 in a 98-line Apex Class is reported back as covered by the Salesforce CLI deploy command). Salesforce's coverage result may also include extra lines as covered (ex: 120 lines are included in the coverage report for a 100 line file), so the coverage percentage may vary based on how many lines the API returns in the coverage report. Once Salesforce fixes the API to correctly return covered lines in the deploy command, this function will be removed.
-
 ## Install
 
 ```bash

diff --git a/src/helpers/getTotalLines.ts b/src/helpers/getTotalLines.ts
diff --git a/src/helpers/setCoveredLines.ts b/src/helpers/setCoveredLines.ts
diff --git a/src/helpers/transformDeployCoverageReport.ts b/src/helpers/transformDeployCoverageReport.ts
@@ -6,7 +6,6 @@ import { create } from 'xmlbuilder2';
 import { DeployCoverageData, CoverageObject, FileObject } from './types.js';
 import { getPackageDirectories } from './getPackageDirectories.js';
 import { findFilePath } from './findFilePath.js';
-import { setCoveredLines } from './setCoveredLines.js';
 import { normalizePathToUnix } from './normalizePathToUnix.js';
 
 export async function transformDeployCoverageReport(
@@ -26,23 +25,21 @@ export async function transformDeployCoverageReport(
       warnings.push(`The file name ${formattedFileName} was not found in any package directory.`);
       continue;
     }
-    const uncoveredLines = Object.keys(fileInfo.s)
-      .filter((lineNumber) => fileInfo.s[lineNumber] === 0)
-      .map(Number);
-    const coveredLines = Object.keys(fileInfo.s)
-      .filter((lineNumber) => fileInfo.s[lineNumber] === 1)
-      .map(Number);
-
     const fileObj: FileObject = {
       '@path': normalizePathToUnix(relativeFilePath),
-      lineToCover: uncoveredLines.map((lineNumber: number) => ({
-        '@lineNumber': lineNumber,
-        '@covered': 'false',
-      })),
+      lineToCover: [],
     };
 
-    // this function is only needed until Salesforce fixes the API to correctly return covered lines
-    await setCoveredLines(coveredLines, uncoveredLines, repoRoot, relativeFilePath, fileObj);
+    for (const lineNumberString in fileInfo.s) {
+      if (!Object.hasOwn(fileInfo.s, lineNumberString)) continue;
+
+      const covered = fileInfo.s[lineNumberString] === 1 ? 'true' : 'false';
+      fileObj.lineToCover.push({
+        '@lineNumber': Number(lineNumberString),
+        '@covered': covered,
+      });
+    }
+
     filesProcessed++;
     coverageObj.coverage.file.push(fileObj);
   }

diff --git a/test/baselines/classes/AccountProfile.cls b/test/baselines/classes/AccountProfile.cls
diff --git a/test/baselines/triggers/AccountTrigger.trigger b/test/baselines/triggers/AccountTrigger.trigger
diff --git a/test/commands/transformer/unit.test.ts b/test/commands/transformer/unit.test.ts
@@ -1,6 +1,6 @@
 'use strict';
 
-import { copyFile, readFile, writeFile, rm, mkdir } from 'node:fs/promises';
+import { readFile, writeFile, rm, mkdir } from 'node:fs/promises';
 import { strictEqual } from 'node:assert';
 import { resolve } from 'node:path';
 
@@ -12,8 +12,10 @@ import TransformerTransform from '../../../src/commands/apex-code-coverage/trans
 describe('main', () => {
   const $$ = new TestContext();
   let sfCommandStubs: ReturnType<typeof stubSfCommandUx>;
-  const baselineClassPath = resolve('test/baselines/classes/AccountProfile.cls');
-  const baselineTriggerPath = resolve('test/baselines/triggers/AccountTrigger.trigger');
+  const mockClassContent = '// Test Apex Class';
+  const mockTriggerContent = '// Test Apex Trigger';
+  const baselineClassPath = resolve('force-app/main/default/classes/AccountProfile.cls');
+  const baselineTriggerPath = resolve('packaged/triggers/AccountTrigger.trigger');
   const deployCoverageNoExts = resolve('test/deploy_coverage_no_file_exts.json');
   const deployCoverageWithExts = resolve('test/deploy_coverage_with_file_exts.json');
   const testCoverage = resolve('test/test_coverage.json');
@@ -34,8 +36,8 @@ describe('main', () => {
   before(async () => {
     await mkdir('force-app/main/default/classes', { recursive: true });
     await mkdir('packaged/triggers', { recursive: true });
-    await copyFile(baselineClassPath, 'force-app/main/default/classes/AccountProfile.cls');
-    await copyFile(baselineTriggerPath, 'packaged/triggers/AccountTrigger.trigger');
+    await writeFile(baselineClassPath, mockClassContent);
+    await writeFile(baselineTriggerPath, mockTriggerContent);
     await writeFile(sfdxConfigFile, configJsonString);
   });
 
@@ -48,8 +50,8 @@ describe('main', () => {
   });
 
   after(async () => {
-    await rm('force-app/main/default/classes/AccountProfile.cls');
-    await rm('packaged/triggers/AccountTrigger.trigger');
+    await rm(baselineClassPath);
+    await rm(baselineTriggerPath);
     await rm('force-app', { recursive: true });
     await rm('packaged', { recursive: true });
     await rm(testXmlPath1);

diff --git a/test/coverage_baseline.xml b/test/coverage_baseline.xml
@@ -3,46 +3,46 @@
   <file path="packaged/triggers/AccountTrigger.trigger">
     <lineToCover lineNumber="52" covered="false"/>
     <lineToCover lineNumber="53" covered="false"/>
+    <lineToCover lineNumber="54" covered="true"/>
+    <lineToCover lineNumber="55" covered="true"/>
+    <lineToCover lineNumber="56" covered="true"/>
+    <lineToCover lineNumber="57" covered="true"/>
+    <lineToCover lineNumber="58" covered="true"/>
     <lineToCover lineNumber="59" covered="false"/>
     <lineToCover lineNumber="60" covered="false"/>
-    <lineToCover lineNumber="1" covered="true"/>
-    <lineToCover lineNumber="2" covered="true"/>
-    <lineToCover lineNumber="3" covered="true"/>
-    <lineToCover lineNumber="4" covered="true"/>
-    <lineToCover lineNumber="5" covered="true"/>
-    <lineToCover lineNumber="6" covered="true"/>
-    <lineToCover lineNumber="7" covered="true"/>
-    <lineToCover lineNumber="8" covered="true"/>
-    <lineToCover lineNumber="9" covered="true"/>
-    <lineToCover lineNumber="10" covered="true"/>
-    <lineToCover lineNumber="11" covered="true"/>
-    <lineToCover lineNumber="12" covered="true"/>
-    <lineToCover lineNumber="13" covered="true"/>
-    <lineToCover lineNumber="14" covered="true"/>
-    <lineToCover lineNumber="15" covered="true"/>
-    <lineToCover lineNumber="16" covered="true"/>
-    <lineToCover lineNumber="17" covered="true"/>
-    <lineToCover lineNumber="18" covered="true"/>
-    <lineToCover lineNumber="19" covered="true"/>
-    <lineToCover lineNumber="20" covered="true"/>
-    <lineToCover lineNumber="21" covered="true"/>
-    <lineToCover lineNumber="22" covered="true"/>
-    <lineToCover lineNumber="23" covered="true"/>
-    <lineToCover lineNumber="24" covered="true"/>
-    <lineToCover lineNumber="25" covered="true"/>
-    <lineToCover lineNumber="26" covered="true"/>
-    <lineToCover lineNumber="27" covered="true"/>
+    <lineToCover lineNumber="61" covered="true"/>
+    <lineToCover lineNumber="62" covered="true"/>
+    <lineToCover lineNumber="63" covered="true"/>
+    <lineToCover lineNumber="64" covered="true"/>
+    <lineToCover lineNumber="65" covered="true"/>
+    <lineToCover lineNumber="66" covered="true"/>
+    <lineToCover lineNumber="67" covered="true"/>
+    <lineToCover lineNumber="68" covered="true"/>
+    <lineToCover lineNumber="69" covered="true"/>
+    <lineToCover lineNumber="70" covered="true"/>
+    <lineToCover lineNumber="71" covered="true"/>
+    <lineToCover lineNumber="72" covered="true"/>
+    <lineToCover lineNumber="73" covered="true"/>
+    <lineToCover lineNumber="74" covered="true"/>
+    <lineToCover lineNumber="75" covered="true"/>
+    <lineToCover lineNumber="76" covered="true"/>
+    <lineToCover lineNumber="77" covered="true"/>
+    <lineToCover lineNumber="78" covered="true"/>
+    <lineToCover lineNumber="79" covered="true"/>
+    <lineToCover lineNumber="80" covered="true"/>
+    <lineToCover lineNumber="81" covered="true"/>
+    <lineToCover lineNumber="82" covered="true"/>
   </file>
   <file path="force-app/main/default/classes/AccountProfile.cls">
     <lineToCover lineNumber="52" covered="false"/>
     <lineToCover lineNumber="53" covered="false"/>
-    <lineToCover lineNumber="59" covered="false"/>
-    <lineToCover lineNumber="60" covered="false"/>
     <lineToCover lineNumber="54" covered="true"/>
     <lineToCover lineNumber="55" covered="true"/>
     <lineToCover lineNumber="56" covered="true"/>
     <lineToCover lineNumber="57" covered="true"/>
     <lineToCover lineNumber="58" covered="true"/>
+    <lineToCover lineNumber="59" covered="false"/>
+    <lineToCover lineNumber="60" covered="false"/>
     <lineToCover lineNumber="61" covered="true"/>
     <lineToCover lineNumber="62" covered="true"/>
     <lineToCover lineNumber="63" covered="true"/>
@@ -55,15 +55,15 @@
     <lineToCover lineNumber="70" covered="true"/>
     <lineToCover lineNumber="71" covered="true"/>
     <lineToCover lineNumber="72" covered="true"/>
-    <lineToCover lineNumber="1" covered="true"/>
-    <lineToCover lineNumber="2" covered="true"/>
-    <lineToCover lineNumber="3" covered="true"/>
-    <lineToCover lineNumber="4" covered="true"/>
-    <lineToCover lineNumber="5" covered="true"/>
-    <lineToCover lineNumber="6" covered="true"/>
-    <lineToCover lineNumber="7" covered="true"/>
-    <lineToCover lineNumber="8" covered="true"/>
-    <lineToCover lineNumber="9" covered="true"/>
-    <lineToCover lineNumber="10" covered="true"/>
+    <lineToCover lineNumber="73" covered="true"/>
+    <lineToCover lineNumber="74" covered="true"/>
+    <lineToCover lineNumber="75" covered="true"/>
+    <lineToCover lineNumber="76" covered="true"/>
+    <lineToCover lineNumber="77" covered="true"/>
+    <lineToCover lineNumber="78" covered="true"/>
+    <lineToCover lineNumber="79" covered="true"/>
+    <lineToCover lineNumber="80" covered="true"/>
+    <lineToCover lineNumber="81" covered="true"/>
+    <lineToCover lineNumber="82" covered="true"/>
   </file>
 </coverage>