force:source:deploy --coverageformatters cobertura | Incorrect class filename in Cobertura XML reports

[davidbrowaeys]

Summary

When running force:source:deploy with --coverageformatters cobertura, it generate a cobertura xml file with the coverage results. However the filename path is invalid which means ci/cd tooling can't associate the coverage with the actual file.

Steps To Reproduce:

Just run a source:deploy with test level set as RunLocalTest or RunSpecifiedTests and make sure coverageformatters is set as cobertura.

sfdx force:source:deploy -g -w 999 --testlevel RunSpecifiedTests  -p force-app/main/default/classes/SampleClass.cls  -r "SampleClass_Test" --junit --resultsdir tests --coverageformatters cobertura --checkonly --verbose

Expected result

Cobertura xml with valid filename should be generated.

<?xml version="1.0" ?>
<!DOCTYPE coverage SYSTEM "http://cobertura.sourceforge.net/xml/coverage-04.dtd">
<coverage lines-valid="1" lines-covered="1" line-rate="1" branches-valid="0" branches-covered="0" branch-rate="1" timestamp="1664334267360" complexity="0" version="0.1">
  <sources>
    <source>.</source>
  </sources>
  <packages>
    <package name="main" line-rate="1" branch-rate="1">
      <classes>
        <class name="SampleClass" filename="force-app/main/default/classes/SampleClass.cls" line-rate="1" branch-rate="1">
          <methods>
          </methods>
          <lines>
            <line number="1" hits="1" branch="false"/>
          </lines>
        </class>
      </classes>
    </package>
  </packages>
</coverage>

Actual result

Filename path is completely incorrect

<?xml version="1.0" ?>
<!DOCTYPE coverage SYSTEM "http://cobertura.sourceforge.net/xml/coverage-04.dtd">
<coverage lines-valid="1" lines-covered="1" line-rate="1" branches-valid="0" branches-covered="0" branch-rate="1" timestamp="1664334267360" complexity="0" version="0.1">
  <sources>
    <source>.</source>
  </sources>
  <packages>
    <package name="main" line-rate="1" branch-rate="1">
      <classes>
        <class name="SampleClass" filename="no-map/SampleClass" line-rate="1" branch-rate="1">
          <methods>
          </methods>
          <lines>
            <line number="1" hits="1" branch="false"/>
          </lines>
        </class>
      </classes>
    </package>
  </packages>
</coverage>

System Information

• Which shell/terminal are you using? bash

• If you are using sfdx sfdx-cli/7.168.0 darwin-x64 node-v14.15.4

[github-actions[bot]]

Thank you for filing this issue. We appreciate your feedback and will review the issue as soon as possible. Remember, however, that GitHub isn't a mechanism for receiving support under any agreement or SLA. If you require immediate assistance, contact Salesforce Customer Support.

[szymon-halik]

I am experiencing a similar issue with clover output of coverage results. The path attribute always begins with no-map no matter what --resultsdir flag is - initially I thought that we should place the results in the directory containing tested classes for example force-app/main/default/classes but it's not the case - the issue persists.

@mshanemc do you have any insights how to get the proper file path in the coverage results files?

[davidbrowaeys]

@szymon-halik So after posting this above, I've actually spent quite a bit of time trying to work it out myself. Because from a sfdx perspective the running test class is happening on the server side not from your local, so the system has no idea of your class path.

What most CI/CD tooling will ask you is to change the no-map/SampleClass with simply the relative path of the file location in your project. Here is an example:

I've created a simple command in my own plugin, DXB. It find the no-map/SampleClass in the xml file, then get the name of the class and try to find all the class files from local project repo.

Usage: sfdx dxb:apex:coverage:cleanup -f tests/coverage/cobertura.xml

Once you publish that as a a report artefacts in Azure, it looks like this:

[AllanOricil]

This happens with coverage.json as well

[AllanOricil]

I created a simple script that fixes all of these 'no-map' paths in the coverage.jsonreport, but I could only do it based on the classes versioned in a giving sfdx project. And this is not a good fix because the coverage output, when deploying to Production while also running all tests, will contain classes that aren't versioned in the current repository.

update: I can update my script to remove coverage entries for classes that aren't versioned in the current project before uploading to, for instance, Codecov.

[mshanemc]

We made this feature to provide the high-level coverage info. The metadata deploy result doesn't contain the information we'd need to do this correctly (it's less detailed than the results of apex test execution).

https://developer.salesforce.com/docs/atlas.en-us.api_meta.meta/api_meta/meta_deployresult.htm#codecoverageresult

We've asked that mdapi deployResult change to look like the Apex test results, and aren't going to solve this in an even more hacky way until that work is finished.

If anyone wants to give it a go, the code is here: https://github.com/salesforcecli/plugin-source/blob/9f48ef68712448532ca11323723bb307ee087c96/src/formatters/resultFormatter.ts#L124

[AllanOricil]

@mshanemc I was able to fix my issue with that simple script I mentioned above. But today I noticed another bug which is related to the way the coverage.json report is created.

When a salesforce org has a class and a trigger with the same name, for instance COM_Lead.cls and COM_Lead.trigger, only a single entry is added to coverage.json as no-map/COM_Lead, which we can't determine if it the coverage for the trigger or for the class. To solve this issue, the keys in the coverage.json must include the file extension.

Because we can't determine if that entry is the coverage for the class or for the trigger, we can't fix its path.

I just opened a new issue in this repo: https://github.com/forcedotcom/cli/issues/1813

[AllanOricil]

@mshanemc fixing line 136 would also fix this issue

https://github.com/forcedotcom/salesforcedx-apex/blob/main/src/reporters/coverageReporter.ts#L126-L138

Don't know yet why pathsToFiles.find(file => fileRegEx.test(file)) isn't working. There is probably a bug here, in this method called findFullPathToClass

https://github.com/forcedotcom/salesforcedx-apex/blob/f8ae861e6abb2a40af22cc2c0011c2ec7d7027c4/src/reporters/coverageReporter.ts#L124

[davidbrowaeys]

@AllanOricil @mshanemc actually was quite easy and fun to analyse.

If you replace no-map by root folder as '.' at line 188 in lib/deployCommand.js then it works very well.

The apex-node module handle that already actually.

Example:

However it might include undesired classes/triggers that are not specified as packageDirectory in sfdx-project.json. You would need to update the @salesforce/apex-node findFullPathToClass method to only include packageDirectories path.

[peternhale]

@davidbrowaeys the paths expressed in the details for cobertura and all other report formatters was intentional due to the lack of complete line coverage data when running tests with coverage via metadata deploys (force:source:deploy or force:mdapi:deploy).

When the detail data are available we will revisit the coverage reporters.

To get complete detail coverage reports, you can use the force:apex:test:run/report commands.

[AllanOricil]

@cromwellryan I believe this can also be solved. no-map is currently hardcoded. I can also take care of it.

https://github.com/salesforcecli/plugin-source/blob/747c7e67fc15d6fcbdad1d2ceceb955300c83931/src/deployCommand.ts#L184-L194

[davidbrowaeys]

@AllanOricil that's what I tried to point out. I think there is a simple solution we can apply quite quickly. Thank you.

[AllanOricil]

@davidbrowaeys does your solution work on a monorepo with more than one package? What about classes that aren't versioned in the repository, but are part of the code coverage report? For instance, this happens when running all tests in a sandbox. These two problems have to be addressed in the final solution. In order to fix my issue with the wrong paths, I created a Github Action that fixes all code coverage entry paths based on the files that are versioned in a particular repository. I can move my code into sfdx if @cromwellryan don't mind.

[davidbrowaeys]

Great questions. @AllanOricil

Multi packages will work yes if you use simply project root (as '.'). However, if you have some classes in a random folder and which are not specified as sfdx packageDirectories it will also include it(i.e.: backup/MyOldClass.cls folder or so).

If you use force-app (or even if you find the default from packageDirectories) then yeah it's assuming it's a single package. But that's just the way @salesforce/apex-node function is working.

For your second point, if file is not in the repo, it seems that @salesforce/apex-node findFullPathToClass ignore the code coverage result completely in the output file which is maybe not great. See below where UtilsTest is a test class for Utils.cls, it doesn't appear in the file.

Something would need to change in here in my opinion https://github.com/forcedotcom/salesforcedx-apex/blob/f8ae861e6abb2a40af22cc2c0011c2ec7d7027c4/src/reporters/coverageReporter.ts#L185

[AllanOricil]

sfdx force:apex:test:run|report has the same problem. File extension and path are both missing.

[jfaderanga]

Linking the open PR for the partial fix (file extension): PR-309.

Also sharing the xslt template I've written to transform the cobertura xml to Sonar Cloud generic coverage for others who's having the same issue.

The template has conditions to define the file path if you have multiple package directories. The below only suffix .cls extension as it so complicated to do a matching based on file name as discussed on above comments.

<?xml version="1.0" encoding="UTF-8"?>
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">

    <xsl:output method="xml" version="1.0" encoding="UTF-8" indent="yes"/>

    <xsl:template match="/">
        <coverage version="1">
            <xsl:apply-templates select="//class"/>
        </coverage>
    </xsl:template>

    <xsl:template match="class">
        <file>
            <xsl:attribute name="path">
                <xsl:choose>
                    <xsl:when test="starts-with(@name, 'PREFIX1_')">
                        <xsl:text>artefacts/components/Prefix 1/main/default/classes/</xsl:text>
                    </xsl:when>
                    <xsl:when test="starts-with(@name, 'PREFIX2_')">
                        <xsl:text>artefacts/components/Prefix 2/main/default/classes/</xsl:text>
                    </xsl:when>
                    <xsl:otherwise>
                        <xsl:text>artefacts/components/force-app/main/default/classes/</xsl:text>
                    </xsl:otherwise>
                </xsl:choose>

                <xsl:value-of select="@name"/>
                <xsl:text>.cls</xsl:text>
            </xsl:attribute>

            <xsl:attribute name="name">
                <xsl:value-of select="@name"/>
            </xsl:attribute>

            <xsl:apply-templates select="lines/line"/>
        </file>
    </xsl:template>

    <xsl:template match="line">
        <lineToCover lineNumber="{@number}" covered="{@hits > 0}"/>
    </xsl:template>

</xsl:stylesheet>

Just use xsltproc to transform the cobertura xml.

sudo apt-get update
sudo apt-get install -y xsltproc

xsltproc -o "$output" "$template" "$input"

Hoping the PR to get approved and merge soon. That'll make the above at least a acceptable workaround. Thanks @AllanOricil @davidbrowaeys @peternhale @mshanemc @szymon-halik for looking into the issue.