Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
davidbrowaeys opened this issue Sep 28, 2022 · 18 comments
Labels
investigating We're actively investigating this issue

Comments

@davidbrowaeys
Copy link

davidbrowaeys commented Sep 28, 2022

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

@davidbrowaeys davidbrowaeys added the investigating We're actively investigating this issue label Sep 28, 2022
@github-actions
Copy link

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
Copy link

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
Copy link
Author

@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:
image

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:
image

@AllanOricil
Copy link

AllanOricil commented Nov 15, 2022

This happens with coverage.json as well

image

@AllanOricil
Copy link

AllanOricil commented Nov 15, 2022

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
Copy link
Contributor

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
Copy link

AllanOricil commented Nov 17, 2022

@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.

image

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:
#1813

@AllanOricil
Copy link

@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
Copy link
Author

@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.

image

The apex-node module handle that already actually.

Example:
image

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.
image

davidbrowaeys added a commit to davidbrowaeys/plugin-source that referenced this issue Nov 22, 2022
This is in regards to issue forcedotcom/cli#1725. 

The fix is to simply update no-map by project root directory to force apex-node to find all cls and trigger in working directory. While the fix will work for now, I believe it is still incomplete as it doesn't consider packageDirectories filtering from sfdx-project.json.
Copy link
Contributor

@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
Copy link

AllanOricil commented Dec 4, 2022

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

image

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

@davidbrowaeys
Copy link
Author

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

@davidbrowaeys
Copy link
Author

davidbrowaeys commented Dec 5, 2022

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

image

@AllanOricil
Copy link

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

@jfaderanga
Copy link

jfaderanga commented Nov 29, 2023

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.

@mcarvin8
Copy link

Coming into this a little late, but this seems to be one of the few issues I've seen with coverage reports

  • file paths incorrect for a Salesforce DX repo (force-app)
  • deployment coverage inaccurately reports "covered" lines

I've made a plugin for SonarQube which works around both - https://github.com/mcarvin8/apex-code-coverage-transformer

I could add cobertura format support to my plugin.

There are few open bugs regarding the inaccuracies in the deployment report "covered" lines. My plugin has a renumbering function to work around this.

@jfaderanga
Copy link

Kudos to @mcarvin8 ^^ for the plugin. I deprecated the powershell workaround I wrote and use his plugin instead, it's an awesome plugin.

@mcarvin8
Copy link

mcarvin8 commented Dec 16, 2024

BTW - I'm working on cobertura format support in my plugin via mcarvin8/apex-code-coverage-transformer#65

Input file format still must be the normal JSON format from the Salesforce CLI. I know it creates cobertura format already. But to add support to my existing plugin, it seemed simpler to just stick with 1 input and have 2 possible output formats now.

Output formats will be sonar (default version) and cobertura.

Just verifying some things with Cobertura format and working on test classes. Keep an eye when that PR is accepted.

This will account for:

  • Adding correct file paths per your Salesforce DX repo
  • Fixing the covered lines the Salesforce CLI deploy command will inaccurately report
    • No covered line adjustment is needed for the Salesforce CLI test command reports

EDIT: RELEASED via apex-code-coverage-transformer@2.3.0. please open PRs on that repo if there's something wrong with cobertura report as created by that plugin. After several tests, I've confirmed this generates similar looking cobertura reports that the Salesforce CLI will create, but this time with correct file-paths and adjusted covered lines if using the deploy command reports. Only weird delta I see currently is the empty methods tag is different in my report (<method/>) over how Salesforce CLI creates it (<method></method>). I can look into adjusting this, but I'm not sure if that will break anything since it's empty. I don't want to use allowEmptyTags on xmlbuilder2 since that affects all of the <lines> as well. I confirmed via GitLab CI/CD's coverage reports that this plugin's cobertura reports are accepted and displayed on the Apex files in a MR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigating We're actively investigating this issue
Projects
None yet
Development

No branches or pull requests

7 participants