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

fix: add file extension to code coverage entries #309

Conversation

AllanOricil
Copy link

@AllanOricil AllanOricil commented Dec 4, 2022

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?

forcedotcom/cli#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
Copy link
Collaborator

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

randi274 commented Dec 5, 2022

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!

@klewis-sfdc klewis-sfdc self-assigned this Dec 8, 2022
@klewis-sfdc klewis-sfdc removed their assignment Dec 16, 2022
@AllanOricil AllanOricil reopened this Jul 11, 2023
@AllanOricil AllanOricil changed the title bugfix - Add file extension to code coverage entries fix: add file extension to code coverage entries Nov 13, 2023
@AllanOricil
Copy link
Author

AllanOricil commented Nov 13, 2023

could someone review this, please?

SmartSelect_20231113_154145_Brave.jpg

@peternhale
Copy link
Collaborator

peternhale commented Nov 13, 2023

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

🚀

@peternhale
Copy link
Collaborator

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

Thanks @peternhale if you guys need any help I will be here 👍
Have a good weekend

@peternhale peternhale closed this Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants