search

apex-enterprise-patterns / fflib-apex-common

Common Apex Library supporting Apex Enterprise Patterns and much more!
BSD 3-Clause "New" or "Revised" License
901 stars 513 forks source link

Code Coverage #239

Closed AllanOricil closed 1 year ago

AllanOricil commented 4 years ago

I tried to deploy the fflib-apex-common classes and mocks but the code coverage was not enough. image

AllanOricil commented 4 years ago

I was able to deploy it when I create a class for the fflib_QUalified MEthodAndArgValues. But the other two classes still need coverage to be deployed.

image

cpfrommer commented 4 years ago

I saw the same coverage issue with the fflib_SObjectUnitOfWork and fflib_SObjectSelector classes. I saw some comments relating to #212 about how the additions can't really be unit tested, so that might explain the low coverage on fflib_SObjectUnitOfWork. Not sure about fflib_SObjectSelector. It doesn't look like the virtual getOrderBy() method is covered at all, and it looks like it has grown since the last time fflib_SObjectSelectorTest was changed.

cpfrommer commented 4 years ago

So I think between the addition of the registerPublish* methods added in pull request #2 (which currently have no coverage) and the new untestable stuff in #212 the coverage on fflib_SObjectUnitOfWork slipped below 75%.

I might try to fix this myself and submit a PR at some point...

evanmcd commented 4 years ago

@cpfrommer Hey, I don't suppose you ever got to writing additional tests for this, did you? I'm hitting the same issue now.

Thanks.

cpfrommer commented 4 years ago

@evanmcd no, unfortunately I haven't gotten to that :(

evanmcd commented 4 years ago

OK. Thx for the quick update. I put in a PR for a missing test class on the mocks library earlier today. Maybe I'll be able to get to this at some point as well.

MorganMarchese commented 3 years ago

8/23/2021 - this is still an issue blocking "Run Specified Tests" deployments.

ImJohnMDaniel commented 3 years ago

@MorganMarchese -- which classes are you seeing insufficient when you run specified tests??

MorganMarchese commented 3 years ago

@ImJohnMDaniel

fflib_SObjectSelector and fflib_SObjectUnitOfWork both have under 75% coverage. Since I need fflib for an upcoming prod deployment, I've been working on my own personal coverage improvements since my comment yesterday - but if I recall correctly, the original count was something like...

fflib_SObjectSelector: 48% fflib_SObjectUnitofWork: 68%

Edit: this is with the latest version of all files from Git, as I deployed the most recent version yesterday to make sure everything was up to date with the repo.

ImJohnMDaniel commented 3 years ago

OK. This is somewhat of a known issue. It doesn't affect most folks because they either deploy the entire framework or the entire app and run all of the unit tests instead these specific ones. The other option that is getting more traction with folks is make a private clone of this repo, add your own second generation unlocked package info to the sfdx-project.json and deploy the framework as an unlocked package. That actually shifts when the unit tests are run from "deployment time" back to "package version creation time."

What is driving the need to specify these particular unit tests during the deployment?

MorganMarchese commented 3 years ago

@ImJohnMDaniel unfortunately what is driving the need is a bad org. I recently adopted an org where you cannot run/pass all test classes during a production deployment due to 5-7 years of mismanaged consultant code being rammed into production.

I've been brought on to resolve some core architectural/process challenges within the org, but as of today due to the number of failing test classes (hundreds) the only way I can deploy anything is to build strong unit tests for all new/changed classes which independently cover the code at 75%+. We're working our way through the tests, but it's a long, tedious process of learning expected outcomes and updating tests appropriately. Most of the tests don't even have assertions or they are riddled with purposeless try/catch blocks.

One of my upcoming improvements is the introduction of the Selector layer pattern from FFLib, but I can't deploy FFLib until I get that coverage above 75% for each class. I managed to get UnitOfWork up to 82% by adding custom coverage for some things that can't be covered by standard object unit tests (like registering with External Ids, and Event Publishing methods), and working on getting above 75% with SObjectSelector today. So far I'm at 62%.

I haven't worked much with unlocked packages yet. By doing it this way, are you saying that Salesforce doesn't enforce 75% coverage on each class during package creation time?

AllanOricil commented 3 years ago

@ImJohnMDaniel im my case it was a company policy and architect's agreement to not allow libraries that don't have enough code coverage to get into prod. I think this is going to be a common scenario that would imped developers to try it out. So, if it is really not possible to get the required code coverage, add a section on the readme where you explain why it has less than 75% coverage so architects are aware of it before deploying.

PeterLin888 commented 3 years ago

@MorganMarchese, I agreed with @ImJohnMDaniel that Salesforce DX Unlocked Package (SFDX-UP) is one of the good options for installing/implementing FFLIB.

Based on my own experience, I believe that SFDX-UP might be one of some good options for your use case above.

As my understanding, when we create a SFDX-UP version (excluding --orgdependent SFDX-UP ), Salesforce DX will run the code coverage (option --codecoverage) and store the code coverage in the "version" (see examples below). And we can only promote-release SFDX-UP version as "Released" when the code coverage is greater than 75%. Only "Released" version is allowed to install into Salesforce production org. When we install a "Released" SFDX-UP version, Salesforce production org will NOT need to check the code coverage because as above any "Released" SFDX-UP version's code coverage is greater than 75% (see examples below: Code Coverage Met true).

I hope this helps.

The followings are some examples of the code coverage of the two package versions.

apex-common

Code Coverage 84%

Released                        true
Validation Skipped              false
Ancestor                        N/A
Ancestor Version                N/A
Code Coverage                   84%
Code Coverage Met               true
Org-Dependent Unlocked Package  No
Release Version                 52.0
Build Duration in Seconds       120
Managed Metadata Removed        N/A

apex-mocks

Code Coverage 99%

Released                        true
Validation Skipped              false
Ancestor                        N/A
Ancestor Version                N/A
Code Coverage                   99%
Code Coverage Met               true
Org-Dependent Unlocked Package  No
Release Version                 52.0
Build Duration in Seconds       100
Managed Metadata Removed        N/A
MorganMarchese commented 3 years ago

Is there any reason (other than that nobody has done it yet) that the main fflib-apex-common hasn't been packaged/offered as an unlocked package?

PeterLin888 commented 3 years ago

@MorganMarchese, Each SFDX-UP version is managed and (100% controlled) by a specific Salesforce Dev Hub Org (regular production org, Developer Edition org or Salesforce Partner Business Org, etc.)

If I were to share with you my SFDX-UP version, you probably don't want to use it to install into any of your production orgs, because I can change that SFDX-UP version any minute (so the next changed version may only work in my production org but NOT work in any other production orgs, e.g. one of your production orgs) which is out of your control.

So, I suggest that each production org (Dev Hub Org) should manage its own SFDX-UP Packages and versions with 100% control over the changes.

fransf-wtax commented 2 years ago

Hi all, running into the same issues: insufficient code coverage for fflib_SObjectSelector and fflib_SObjectUnitOfWork. I've already forked the project so the quickest way out for me is probably just to add some tests that at least work in my orgs and that achieve the required coverage (or to go the unlocked package route suggested). But it does feel like a waste for every user of this great library to reinvent the wheel here.

daveespo commented 2 years ago

1GP managed packages are no better, really. First off, it would need to go through security review which is only available to Partners -- we're just a bunch of guys maintaining a project with no business entity behind it.

Second, there is no support on the platform for installing multiple packages in one install via AppExchange install -- so if anyone wanted to integrate the 1GP into their ISV product, it would preclude it from being installed via the 'Get it now' feature. No ISV would limit themselves in such a way so they'd likely vendor the fflib code (like they do today) .. but that brings us to point three:

Third, we'd need to expose every class and method as global so it could be accessed outside of the 1GP package. That would basically freeze the interface and limit future refactoring options dramatically. Again, not something we're going to do.

In short, there is sufficient test coverage in this project if you deploy it in its entirety. Trying to do piecemeal deployments isn't possible and it's covered in earlier comments on this thread (notably https://github.com/apex-enterprise-patterns/fflib-apex-common/issues/239#issuecomment-904646020 and the ones after it)

fransf-wtax commented 2 years ago

Thanks for that explanation @daveespo :)

I've managed to get the coverage to over 75% on my fork for the two classes where it was lacking. Is it worth the effort to submit a pull request? They're probably not the best tests, I just looked at which lines were not covered and were not related to multi-currency, and wrote the smallest unit test I could to get them covered. But it might help others in the same position as me.

MorganMarchese commented 2 years ago

1GP managed packages are no better, really. First off, it would need to go through security review which is only available to Partners -- we're just a bunch of guys maintaining a project with no business entity behind it.

Second, there is no support on the platform for installing multiple packages in one install via AppExchange install -- so if anyone wanted to integrate the 1GP into their ISV product, it would preclude it from being installed via the 'Get it now' feature. No ISV would limit themselves in such a way so they'd likely vendor the fflib code (like they do today) .. but that brings us to point three:

Third, we'd need to expose every class and method as global so it could be accessed outside of the 1GP package. That would basically freeze the interface and limit future refactoring options dramatically. Again, not something we're going to do.

In short, there is sufficient test coverage in this project if you deploy it in its entirety. Trying to do piecemeal deployments isn't possible and it's covered in earlier comments on this thread (notably #239 (comment) and the ones after it)

To be clear @daveespo , my use case is not to do "piecemeal" deployments of small portions of fflib. The real challenge is the inability to install it into an org which has even a single failing test class in production. In the org I joined last year, we are forced to do "run specified tests" deployments at present.

Taking aside the obvious underpinning issue which is that our tests shouldn't be failing in production (which I'm working on), the fflib library cannot be deployed to orgs in this state because when you use "Run Specified Tests" Salesforce requires that EACH class being deployed has a minimum of 75% coverage, rather than the collective whole being > 75%.

This makes it an increasingly difficult challenge for people to adopt this library as a mechanism for assisting in fixing bad orgs. Thankfully I was able to overcome this by taking the earlier advice of creating two 2GP's for ApexMocks and Common,

@fransf-wtax it was my experience that coverage couldn't be "generically" increased above 75%, which is why these classes lack coverage. If you've somehow managed to create non-org dependent test classes that give adequate coverage for these, they'd probably welcome the contribution.

fransf-wtax commented 2 years ago

I just used the standard SObjects they were already testing with -- Opportunity and Product2 -- so as far as I can tell the tests are not org-dependent. I'll isolate my changes into a single commit and submit a pull request. Let's see what happens :-)

fransf-wtax commented 2 years ago

https://github.com/apex-enterprise-patterns/fflib-apex-common/pull/366