forcedotcom / salesforcedx-apex

Salesforce Apex Node Library
BSD 3-Clause "New" or "Revised" License
19 stars 25 forks source link

fix: add file extension to code coverage entries #309

Closed AllanOricil closed 10 months ago

AllanOricil commented 1 year ago

What does this PR do?

Add file extension to code coverage entries. With this solution, the Toolling API team won't need to change the api to include the file extension when querying theApexCodeCoverageAggregate object.

What issues does this PR fix or reference?

https://github.com/forcedotcom/cli/issues/1813

BEFORE

because the file extension was not present, the code coverage for classes could be overwritten by the codecoverage of triggers, when both had the same name. Moreover, all code coverage entries were treated as coverage for classes, disregarding the existence of triggers at all.

{
   "no-map/Lead":{
      "fnMap":{},
      "branchMap":{},
      "path":"no-map/Lead"
      ...
   }
}

AFTER

Now both code coverage for apex class and triggers exist, and one can't overwrite the other.

{
   "no-map/Lead.cls":{
      "fnMap":{},
      "branchMap":{},
      "path":"no-map/Lead.cls"
      ...
   },
   "no-map/Lead.trigger":{
      "fnMap":{},
      "branchMap":{},
      "path":"no-map/Lead.trigger"
      ...
   }
}

I attached an output when running these changes in a sandbox that has lots of apex classes and triggers

coverage.zip

In this PR Im also enabling developers to quickly run mocha tests using a vscode extension called Mocha Test Explorer.

image

without this change Mocha Test Explorer feaures won't work and developers will have to run tests using a terminal

image

image

peternhale commented 1 year ago

@AllanOricil This PR will require approval of the repo owners. @randi274 when you have a moment, could we get this PR moving forward. Thanks.

randi274 commented 1 year ago

Thanks @peternhale - we're reviewing a long outstanding list of external contributor PRs and I'll make sure this one gets added to the list for review!

AllanOricil commented 1 year ago

could someone review this, please?

SmartSelect_20231113_154145_Brave.jpg

peternhale commented 1 year ago

@AllanOricil I moved this PR into a new branch so I can get tests run on it. Thanks for the contribution. I will run this by the team so it gets all of the needed approvals

AllanOricil commented 11 months ago

πŸš€

peternhale commented 10 months ago

@AllanOricil I know you have been waiting for some time to get this PR merged, as it fulfills your need to have class and trigger coverage for tests run via metadata deploy. From a coverage reporting perspective, discriminating class from trigger is the right thing to do, so we appreciate the effort.

However, this solution only answers part of the coverage reporting problem, as it focuses on coverage reported by metadata deploy. The other half of the coverage reporting solution is contained code that directly runs apex tests and reports on those results. From an architecture perspective, there should only be one coverage reporting solution, so that both metadata deploy and apex test run can use the same coverage reporting solution.

I need to speak with product management about how a single coverage reporting framework would look like to consumers and then execute that vision.

I will be closing this PR. I have captured you work on branch phale/oricli-add_apex_trigger_coverage as I am sure your work will factor into a single coverage reporting solution.

cc @AnanyaJha

AllanOricil commented 10 months ago

Thanks @peternhale if you guys need any help I will be here πŸ‘ Have a good weekend