forcedotcom / cli

Salesforce CLI
https://developer.salesforce.com/docs/atlas.en-us.sfdx_cli_reference.meta/sfdx_cli_reference/
BSD 3-Clause "New" or "Revised" License
476 stars 77 forks source link

Deployment Validation with coverage-formatters writes to nested coverage/ folder #2816

Open codefriar opened 3 months ago

codefriar commented 3 months ago

Summary

We use Github actions. One of our steps is to do a trial deployment to validate and do code coverage calculations. Specifically we use this command:

sf project deploy start --verbose --dry-run -x package/package.xml --target-org stage --ignore-conflicts --wait 45 --test-level RunSpecifiedTests --tests ${{ steps.related-tests.outputs.result }} --coverage-formatters json --coverage-formatters cobertura

This command executes normally. Here's the log output from that command:

... (skipping deployment and testing lines of 'In Progress'

Test Success [1]
✓ CaseRepoTest.unitTestConstructorPositive

Apex Code Coverage
 Name         % Covered Uncovered Lines 
 ──────────── ───────── ─────────────── 
 CaseRepo 100%                      

Test Results Summary
Passing: 1
Failing: 0
Total: 1
Time: 70
Code Coverage formats, [json, cobertura], written to coverage/
Dry-run complete.

This results in the cobertura file being written to coverage/coverage/cobertura.xml' notcoverage/cobertura.xml` as I would expect, given the output line saying "Code Coverage formats, [json, cobertura], written to coverage/"

Steps To Reproduce

I'm unable to create a full repo, for security reasons. However, the operant bits of the workflow file are as follows:

      - name: Generate Deployment Delta
        run: |
          sf sgd source delta -t="HEAD" -f="HEAD~1" -o "." --loglevel=debug
      - name: Deploy to Stage and run Tests (Check Only)
        if: ${{ steps.related-tests.outputs.result != null }}
        run: |
          echo "Running related tests: ${{ steps.related-tests.outputs.result }}"
          sf project deploy start --verbose --dry-run -x package/package.xml --target-org stage --ignore-conflicts --wait 45 --test-level RunSpecifiedTests --tests ${{ steps.related-tests.outputs.result }} --coverage-formatters json --coverage-formatters cobertura
      - name: Deploy to Stage without running Tests (Check Only)
        if: ${{ steps.related-tests.outputs.result == null }}
        run: |
          echo "Running related tests: ${{ steps.related-tests.outputs.result }}"
          sf project deploy start --verbose --dry-run -x package/package.xml --target-org stage --ignore-conflicts --wait 45 --coverage-formatters json --coverage-formatters cobertura
      - name: "Check Cobertura file was created"
        id: does_cobetura_exist
        shell: bash
        run: |
          file=$(pwd)/coverage/coverage/cobertura.xml
          if [ -f $file ]; then
            echo "coverage/coverage/cobertura.xml exists"
            echo "FILE_EXISTS=true" >> $GITHUB_OUTPUT
          else
            echo "$(pwd)/coverage/coverage/cobertura.xml does not exist"
            echo "FILE_EXISTS=true" >> $GITHUB_OUTPUT
          fi

Expected result

cobertura.xml should be found in the WorkingDir's coverage/ folder.

Actual result

cobertura.xml (and it's sister Json file) are written to coverage/coverage/ folder.

System Information

Github Actions using the official SF cli docker image:

    container:
      image: salesforce/cli:latest-full

Additional information

image

github-actions[bot] commented 3 months ago

Hello @codefriar :wave: It looks like you didn't include the full Salesforce CLI version information in your issue. Please provide the output of version --verbose --json for the CLI you're using (sf or sfdx).

A few more things to check:

Thank you!

github-actions[bot] commented 3 months ago

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.

codefriar commented 3 months ago
{
  "architecture": "linux-x64",
  "cliVersion": "@salesforce/cli/2.35.6",
  "nodeVersion": "node-v18.20.1",
  "osVersion": "Linux 6.5.0-1017-azure",
  "rootPath": "/usr/local/lib/nodejs/lib/node_modules/@salesforce/cli",
  "shell": "bash",
  "pluginVersions": [
    "@oclif/plugin-autocomplete 3.0.[13](https://github.com/department-of-veterans-affairs/va-teams/actions/runs/8635526082/job/23673436239#step:14:14) (core)",
    "@oclif/plugin-commands 3.2.2 (core)",
    "@oclif/plugin-help 6.0.20 (core)",
    "@oclif/plugin-not-found 3.1.1 (core)",
    "@oclif/plugin-plugins 5.0.1 (core)",
    "@oclif/plugin-search 1.0.20 (core)",
    "@oclif/plugin-update 4.2.2 (core)",
    "@oclif/plugin-version 2.0.16 (core)",
    "@oclif/plugin-warn-if-update-available 3.0.[15](https://github.com/department-of-veterans-affairs/va-teams/actions/runs/8635526082/job/23673436239#step:14:16) (core)",
    "@oclif/plugin-which 3.1.7 (core)",
    "@salesforce/cli 2.35.6 (core)",
    "apex 3.1.0 (core)",
    "auth 3.5.0 (core)",
    "data 3.2.2 (core)",
    "deploy-retrieve 3.4.0 (core)",
    "info 3.1.0 (core)",
    "limits 3.2.0 (core)",
    "marketplace 1.1.0 (core)",
    "org 3.6.0 (core)",
    "packaging 2.2.0 (core)",
    "schema 3.2.0 (core)",
    "settings 2.1.0 (core)",
    "sobject 1.2.0 (core)",
    "source 3.2.0 (core)",
    "telemetry 3.1.[17](https://github.com/department-of-veterans-affairs/va-teams/actions/runs/8635526082/job/23673436239#step:14:18) (core)",
    "templates 56.1.0 (core)",
    "trust 3.4.0 (core)",
    "user 3.4.0 (core)",
    "sfdx-git-delta 5.34.0 (user)"
  ]
}

You should change your info-required-bot to accept a docker image tag as well.

WillieRuemmele commented 3 months ago

hey @codefriar - nice catch - we'll get this fixed

codefriar commented 3 months ago

@WillieRuemmele - at the risk of being told off here,

it'd be nice if i could specify the folder / filename ;)

WillieRuemmele commented 3 months ago

@codefriar more than just --output-dir?

update on the PR, but this seems to be edging towards a breaking change, or at least breaking an existing bug that everyone's got accustomed to... how about making the message correct, e.g.

Code Coverage formats, [cobertura], written to /coverage/coverage
codefriar commented 3 months ago

@WillieRuemmele Apologies. I didn't realize --output-dir was an option. It's not listed here: https://developer.salesforce.com/docs/atlas.en-us.sfdx_cli_reference.meta/sfdx_cli_reference/cli_reference_project_commands_unified.htm#cli_reference_project_deploy_start_unified

Nor is it listed in the command output: image Perhaps that's a docs bug? I'm currently testing adding --output-dir to my ci setup.

I understand this is likely a breaking change. is there any other solution than just updating the output of where it's stored?

WillieRuemmele commented 3 months ago

sorry, I mislead you, it's the --results-dir flag

codefriar commented 3 months ago

ah, then it's my bad for not having caught --results-dir I kept looking for an output option.

mcarvin8 commented 2 months ago

@codefriar - I'm not affiliated with Salesforce, but this is consistent with what I've seen developing a plugin to convert the code coverage output for SonarQube and what I've listed in that plugin's readme (not to advertise it or suggest you use it - https://github.com/mcarvin8/apex-code-coverage-transformer).

The --results-dir flag is the one you use to specify a different directory besides the default one (which I believe defaults to the deploy ID). But this still creates an additional coverage sub-folder no matter what results-dir you specify.

I usually append this to my deploy command (my plugin converts the JSON output over the cobertura output but I think the directory logic works the same) - --coverage-formatters json --results-dir coverage.

This still creates the coverage file in ../coverage/coverage/coverage.json

I've tested json-summary and that gets added to the same folder path, just a different file name.

I think it would be nice if this extra sub-folder created by the CLI was removed since I've definitely had failed pipelines when passing the coverage file path to my plugin.

mathpaquette commented 3 weeks ago

@codefriar - I'm not affiliated with Salesforce, but this is consistent with what I've seen developing a plugin to convert the code coverage output for SonarQube and what I've listed in that plugin's readme (not to advertise it or suggest you use it - https://github.com/mcarvin8/apex-code-coverage-transformer).

The --results-dir flag is the one you use to specify a different directory besides the default one (which I believe defaults to the deploy ID). But this still creates an additional coverage sub-folder no matter what results-dir you specify.

I usually append this to my deploy command (my plugin converts the JSON output over the cobertura output but I think the directory logic works the same) - --coverage-formatters json --results-dir coverage.

This still creates the coverage file in ../coverage/coverage/coverage.json

I've tested json-summary and that gets added to the same folder path, just a different file name.

I think it would be nice if this extra sub-folder created by the CLI was removed since I've definitely had failed pipelines when passing the coverage file path to my plugin.

@mcarvin8 is there any ongoing work by Salesforce to support sf run test format (test-result-codecoverage.json) while doing sf project deploy ? Did you create an issue to request that ?

mcarvin8 commented 1 week ago

@mathpaquette - I'm not aware of any ongoing work by Salesforce to have the same coverage JSON formats for both deployments and test run. That would be ideal. But I haven't created an issue to request that, but let me dig first to see if anyone else has submitted a request for that.

I did submit a case before I went on travel to have Salesforce fix the deployment command coverage reports since the deployment coverage reports inaccurately report covered lines in Apex classes/triggers, but I think that was closed while I was away. This bug doesn't affect code coverage reports generated by sf apex run/get test, so there definitely seems to be more discrepancies between how the code base generates coverage reports for deployments and tests outside of the different formats.

In terms of this original issue with how deployments write to deeply nested coverage folders, this behavior isn't the same with the tests commands. The test command --output-dir dumps them in the desired folder, where the deployment command adds an additional sub-folder.

sf project deploy [start/validate] --coverage-formatters json --results-dir "coverage"
sf apex run test --code-coverage --result-format json --output-dir "coverage"

IMO, it would be nice for both commands to have the same flag names for coverage format and results directory, and use the same format for the coverage reports as much as possible.