Patch Status should compare to merge-base instead of HEAD

[sfgeorge]

A common false-positive I see from Codecov is as follows

1. I create a pull request which Codecov reports green as improving code coverage by ~ 1%. ✅
2. Separately, a colleague's PR which improves coverage by ~ 4% is merged into the integration branch. ✅
3. I trigger a new build on my PR - and now Codecov falsely reports that the Patch Status for the change in coverage introduced by my PR would purportedly decrease coverage by ~3%. ❌

The problem: The base of comparison being used is the latest-and-greatest of the integration branch, even though I branched off from an older commit of the integration branch.

One work-around is for me to simply rebase my feature branch atop the integration branch, and rebuild to get an accurate delta.

But it would be ideal if Codecov did a comparison against the merge-base for the PR. In git terms that would be git merge-base integration-branch feature-branch.

[stevepeak]

Can you diagram this out? Because from my understand this is not an expected behavior.

The base commit used for comparison would only change if the pull request is rebased, not upon new commits to the base branch.

Please see https://docs.codecov.io/docs/comparing-commits for more details.

[marc-hb]

The base commit used for comparison would only change if the pull request is rebased, not upon new commits to the base branch.

@sfgeorge and I wish. This is totally not happening at for instance https://github.com/zephyrproject-rtos/zephyr/pulls. Unless the submitter constantly rebases, even for a fixing a typo, the codecov report for is:open requests is useless because polluted by a daily stream of newer and completely unrelated commits.

In other words all reports compare PR with master instead of comparing PR with PRbase as they should

master PRbase . . . . . master=wrong!
pull      \ . . PR

Here's a specific example at https://github.com/zephyrproject-rtos/zephyr/pull/13608

Merging #13608 into master will decrease coverage by 0.03% => obviously WRONG, look at the one-line change ... Powered by Codecov. Last update 992f29a...b40d6d0. Read the comment docs.

b40d6d0 is the PR and 992f29a is newer by 112 commits so 992f29a is clearly not the PR base!

(Not sure about is:closed requests, they seem OK. I don't care much about is:closed in this project)

[marc-hb]

One may suggest: "just rebase all the time". Sure, it's a mitigation. However:

• it's racy, so just a mitigation. There will still be broken codecov reports, just less frequently.
• github doesn't seem able to filter out rebase noise. So a reviewer who has thoroughly reviewed version 5 of the submission and wants to focus on the difference between version 5 and 6 is spammed by unrelated changes from unrelated commits if candidate version 6 is rebased.

[marc-hb]

For zephyr the "solution" was unfortunately to disable/remove the codecov spam.

[zFernand0]

Are there any updates on this issue? We've had unexpected behavior for quite some time now and we captured it here: https://github.com/zowe/vscode-extension-for-zowe/pull/296#issuecomment-552892394

[earonesty]

Is there a workaround where we can specify the merge base to codecov? This is getting to the point where it's a showstopper on more complex/bigger projects.

[juanvillegas]

Any updates on this? It's practically useless for us because the PR has to rebase (or worse, merge) the base before triggering the build, and that creates a really bad practice among our team.

[marc-h38]

Some hopefully relevant discussions and pointers:

• https://docs.github.com/en/rest/reference/pulls#list-commits-on-a-pull-request Maybe there is some github action providing this list without any coding?

• The triple dots for git log is more or less equivalent to the double dots for git diff and vice versa: https://github.com/travis-ci/travis-ci/issues/4596#issuecomment-614397803

• Shallow cloning is even more incompatible with merge base than you might think: https://github.com/thesofproject/linux/issues/2556

[juanvillegas]

@marc-h38 I'm having a hard time trying to understand how those links are relevant to this discussion. Are you trying to infer that there is work to be done here to get to the desired behavior and those pointers should serve as reference?

[eivindjahren]

So for anyone dealing with this issue in terms of github actions: Jobs run with on: [push] will run the latest commit in the PR but jobs run with on: [pull_request] will merge first and then run. If you are using the codecov uploader action then it will upload with the SHA being the pre-merged commit, but it does not change the base. So the base it compares against might be an outdated commit in the main branch while your PR was merged into an up to date main branch when the report was created.