Contributors mailing list archives
Re: 100 % coverage not enough?by "Ronald Portier" <firstname.lastname@example.org> - 11/09/2023 10:37:11
Rebasing the branch, as suggegested by Jairo solved the problem. So let us keep that in mind with future occurences of this issue.
Denis, I understand why you do not want to remove the check, but I do not see how a PR that only concerns one module, could influence other modules. The decrease must be an artifact of comparing a branch based on an older version of the main branch, that had less coverage overall, with the current version of the main branch. However merging such a PR will not undo the changes that have occurred in the main branch since creating the PR/feature branch. So the proper measure on whether a PR increases or decreases overall coverage would be to compare the coverage percentage in only the patch (the changed code) with the overall coverage of the project (the main branch).
Kind regards, Ronald
The answer is in the details link.
@jairo I don't agree about removing that check.
The 'patch' coverage report concerns exclusively the code you introduce with your PR.The 'project' coverage report concerns the already existing code outside your PR. So, a decrease there means that your PR influences other modules (not executing a super in an inherited method, ...)
Even if coverage is not mandatory in OCA, it gives indicators on technical tests quality.
My two cents
On Mon, Sep 11, 2023 at 10:11 AM jairo <email@example.com> wrote:
El vie, 08-09-2023 a las 18:57 +0000, Ronald Portier escribió:Supposedly the PR would decrease the overall coverage percentage. How??
The 1st thing you need to know is that hose checks are not required.
Now, this is how your PR looks:
That ✅ is telling the reviewer that the PR has a 100% of coverage. That's the important part.
The ❌ however is the weirdo. Sometimes it happens, so I systematically ignore that check. IMHO it could be removed. My guess is that the base branch evolved and got more coverage, and that if you rebased it would always increase. But maybe it's just a bug somewhere.