Improved Patch Coverage Validation and Reporting

[vlad-ko]

Summary:
Enhance Codecov's reporting capabilities to ensure customers can verify that patch coverage for a given pull request (PR) is calculated correctly. This feature will provide clear visibility into whether all files present in the Git diff for a PR have 1) received a coverage report and 2) been successfully processed.

Problem Statement:
Many of our customers primarily focus on patch coverage rather than the overall monorepo coverage. Currently, it is challenging to validate whether patch coverage is being calculated accurately, because the customer doesn't know if they missed an upload due to internal sharding of test suites. There is no straightforward way to confirm if all files in the Git diff for a PR are accounted for in the coverage report and whether the report has been processed correctly. This lack of clarity can lead to mistrust in the coverage metrics or missed opportunities to address gaps in test coverage.

Proposed Solution:
Introduce a feature that allows users to verify the following for a given PR:

1. File Coverage Status: Identify all files in the Git diff for the PR and check if they have been included in the coverage report.
2. Processing Confirmation: Confirm that the coverage report for each file has been successfully processed without errors.
3. Detailed Feedback: Provide a detailed summary or diagnostic view highlighting any missing files, unprocessed reports, or other anomalies affecting patch coverage.

This could be implemented as a new section in the PR comment or a standalone endpoint in the Codecov API.

Use Case(s):

1. A customer submits a PR and wants to ensure that all files in the Git diff have test coverage accounted for.
2. Debugging cases where patch coverage seems unexpectedly low due to missing or unprocessed reports.
3. Teams working with monorepos who need to focus on specific patches without being distracted by irrelevant coverage data.

Benefits:

• Increased confidence in the accuracy of patch coverage calculations.
• Faster debugging and resolution of missing or unprocessed coverage reports.
• Improved user experience for teams focused on patch coverage in large monorepos.
• Reduced support burden as customers can self-diagnose issues related to patch coverage.

Additional Context:

• This aligns with customer feedback requesting more transparency in how patch coverage is calculated and displayed.
• The feature could integrate seamlessly into existing PR comments or act as a separate verification tool to complement Codecov's current functionality.

[kfbustam]

For our organization, we primarily rely on patch coverage that considers only the files modified in the pull request (PR) itself. This means that even if we select a base commit older than the original base commit of the branch we forked from (using pr-base-picking), we want the coverage to be calculated solely based on the files in the PR diff. We don't use the PR comments, just the Github code highlighting and merge lists status checks, so an API that would enable this would be great. Thank you!

[eliatcodecov]

we want the coverage to be calculated solely based on the files in the PR diff.

@kfbustam Full disclosure, PR base picking is a fairly niche feature that isn't widely used by our users, so it would be helpful to understand what you're seeing now in your reporting. Generally patch coverage only considers the lines of code modified in the diff (e.g., if 10 lines are added, 5 of which are covered by tests, patch coverage is 50%). It's not clear to me how this is conflicting / leading to incorrect results when PR Base Picking is used in your case. Can you elaborate a bit more on the problem you're seeing, perhaps with a concrete example?

so an API that would enable this would be great.

Just so I'm clear, are you specifically asking for an API endpoint that will display patch coverage to you when using any two commits (e.g., the HEAD of a branch and some arbitrarily chosen base)? If you use the compare endpoint, I believe the diff object returned in that response should have what you're looking for, but if something is missing / incorrect, please let us know. Once again, I could be misunderstanding your ask, so if it seems like that's the case please provide more details and I'll see what we can do on our end 😃

[kfbustam]

we want the coverage to be calculated solely based on the files in the PR diff.

@kfbustam Full disclosure, PR base picking is a fairly niche feature that isn't widely used by our users, so it would be helpful to understand what you're seeing now in your reporting. Generally patch coverage only considers the lines of code modified in the diff (e.g., if 10 lines are added, 5 of which are covered by tests, patch coverage is 50%). It's not clear to me how this is conflicting / leading to incorrect results when PR Base Picking is used in your case. Can you elaborate a bit more on the problem you're seeing, perhaps with a concrete example?

so an API that would enable this would be great.

Just so I'm clear, are you specifically asking for an API endpoint that will display patch coverage to you when using any two commits (e.g., the HEAD of a branch and some arbitrarily chosen base)? If you use the compare endpoint, I believe the diff object returned in that response should have what you're looking for, but if something is missing / incorrect, please let us know. Once again, I could be misunderstanding your ask, so if it seems like that's the case please provide more details and I'll see what we can do on our end 😃

Hey Eli thanks for the reply!

Currently, our organization lives in a world where we do not, and will not come close to at least in the near future, 100% successful builds. Not to get too much into the details, but one main reason is due to flaky tests which causes not all coverage reports to be uploaded. This causes inaccurate coverage reports for these specific builds. Sometimes, things just go wrong during processing on the Codecov side (e.g., all the reports don't process in time for the base commit of the PR). We've seen a few odd things happen, so we've been working on making our builds more resilient and we will continue to do so. In summary, we can't always depend on each commit having a successful coverage build at this moment in time.

Our current way to overcome this obstacle is to use pr-base-picking to pick a solid commit on our base branch (I.e., dev/main for our org). That is, a commit that has successfully uploaded all reports (see https://about.codecov.io/blog/getting-started-with-the-codecov-api-v2/). The problem with this is that in the diff, there could be other files being modified that have nothing to do with the HEAD commit's PR. The ask is we want some API or flag in a command (e.g., pr-base-picking --filter-on-pr) that will trigger an adjustment in the diff logic to only compute coverage changes for files included in the PR of the HEAD commit. Essentially, filtering calculating coverage for files not at all in the PR.

For example, Let's say we have 5 files in the diff. If only 2 of those files are in the PR of the HEAD commit, take the other 3 files away from the equation. Our use case is that the developer who wrote the PR should only worry about their PR's modified files' coverage. THEN do the logic you mentioned with the lines of code modified in the diff of those files.

P.S. If any of the coverage reports that include those files have failed, it would be nice to know in the merge status check list on GitHub, so we know we need to retry the builds on our end from that interface directly. Thanks again for your time 🙏🏽

[eliatcodecov]

@kfbustam thanks for the clarification, here. If you're dealing with a less than ideal testing / CI setup, I understand why you'd want to rely on base picking to find the ''last good'' commit to compare against, and I can see how that coverage information will gather up more in the coverage diff than files that were just changed on the HEAD, since there are likely several more changes between the HEAD and the chosen BASE and the HEAD and the actual BASE.

Let me touch base with the team to try to get an idea of how difficult it would be to implement a solution for this. I'll update this ticket when I know more.

As an aside, we did recently bring a flaky test detection feature to general availability that may also help with some aspects of this problem.

[eliatcodecov]

@kfbustam we've chatted a lot about this feature request internally, and I think you don't actually need this feature at all. Hear me out:

Today, in the codecov UI, if the BASE commit is in an error state, we'll give you an error in the UI stating that we cannot show you coverage because there is no coverage on the BASE. Likely this is what led you to do PR Base Picking, because then you can guarantee that you would always have some valid BASE on every commit. This valid BASE would allow the Codecov UI to display coverage. If this is what led you to PR Base Picking, I think there's actually a better way forward, here. See below 👇 :

For any given commit, what we need to compute patch and project coverage for that commit is the following:

1. The code diff between the BASE and the HEAD
2. coverage reports uploaded for the HEAD.

There's actually no great reason to require coverage on the BASE to show you any code coverage for a commit/pull in the UI. In fact, all the coverage on the BASE is really needed for is presenting the project coverage change between BASE and HEAD. While this is helpful, if you're not using project coverage nor the PR comment this coverage change isn't really helpful to you, anyway.

So, I'm proposing the following changes to the Codecov UI:

In the event of a broken BASE:

• Don't show the project coverage change metric, because we cannot calculate it.
• Show patch and project coverage, because we can compute it from a valid HEAD and the git diff

In the event of a broken HEAD:

• can't show project coverage, because there was a coverage processing error on at least one report
• can't show project coverage change compared to BASE, even if BASE is valid
• can show patch coverage for coverage calculated excluding individual report failures , but the UI needs to properly contextualize this reported patch as partial coverage. If this partial coverage is above the threshold for a passing patch coverage check we will pass the check, but notify the user that patch coverage is at least the reported value, but could potentially be greater. Otherwise we will fail the patch check.

I believe this will do several things for you (and other users). First, it will let you see patch and project coverage more often without the need for PR Base Picking, and your patch coverage will be contextualized to the proper BASE commit (because you're no longer BASE picking). Second -- depending on how flaky tests are impacting you -- your patch checks will likely pass more often, unblocking developers more often.

I don't have a precise timeline on this, but it will likely be faster than implementing the proposed featutre. This approach -- if it meets your needs -- is much more generalizable and I think benefits you more than the aforementioned feature request. Will this approach work on your end?

[kfbustam]

@kfbustam we've chatted a lot about this feature request internally, and I think you don't actually need this feature at all. Hear me out:

Today, in the codecov UI, if the BASE commit is in an error state, we'll give you an error in the UI stating that we cannot show you coverage because there is no coverage on the BASE. Likely this is what led you to do PR Base Picking, because then you can guarantee that you would always have some valid BASE on every commit. This valid BASE would allow the Codecov UI to display coverage. If this is what led you to PR Base Picking, I think there's actually a better way forward, here. See below 👇 :

For any given commit, what we need to compute patch and project coverage for that commit is the following:

1. The code diff between the BASE and the HEAD
2. coverage reports uploaded for the HEAD.

There's actually no great reason to require coverage on the BASE to show you any code coverage for a commit/pull in the UI. In fact, all the coverage on the BASE is really needed for is presenting the project coverage change between BASE and HEAD. While this is helpful, if you're not using project coverage nor the PR comment this coverage change isn't really helpful to you, anyway.

So, I'm proposing the following changes to the Codecov UI:

In the event of a broken BASE:

• Don't show the project coverage change metric, because we cannot calculate it.
• Show patch and project coverage, because we can compute it from a valid HEAD and the git diff

In the event of a broken HEAD:

• can't show project coverage, because there was a coverage processing error on at least one report
• can't show project coverage change compared to BASE, even if BASE is valid
• can show patch coverage for coverage calculated excluding individual report failures , but the UI needs to properly contextualize this reported patch as partial coverage. If this partial coverage is above the threshold for a passing patch coverage check we will pass the check, but notify the user that patch coverage is at least the reported value, but could potentially be greater. Otherwise we will fail the patch check.

I believe this will do several things for you (and other users). First, it will let you see patch and project coverage more often without the need for PR Base Picking, and your patch coverage will be contextualized to the proper BASE commit (because you're no longer BASE picking). Second -- depending on how flaky tests are impacting you -- your patch checks will likely pass more often, unblocking developers more often.

I don't have a precise timeline on this, but it will likely be faster than implementing the proposed featutre. This approach -- if it meets your needs -- is much more generalizable and I think benefits you more than the aforementioned feature request. Will this approach work on your end?

Yes this would actually work for us! When you say "notify the user..." you mean we'll see it as an appended message to the https://about.codecov.io/product/feature/status-checks/ patch coverage item row right? Thank you so much!!

[eliatcodecov]

Yes this would actually work for us! When you say "notify the user..." you mean we'll see it as an appended message to the https://about.codecov.io/product/feature/status-checks/ patch coverage item row right? Thank you so much!!

Most likely. This is pending a bit of design work, but I expect that if a patch status passes, but there were upload errors, we'll reflect this in the status text itself (e.g., "passed, even though coverage was not fully processed" or something if this nature)

[kfbustam]

Yes this would actually work for us! When you say "notify the user..." you mean we'll see it as an appended message to the https://about.codecov.io/product/feature/status-checks/ patch coverage item row right? Thank you so much!!

Most likely. This is pending a bit of design work, but I expect that if a patch status passes, but there were upload errors, we'll reflect this in the status text itself (e.g., "passed, even though coverage was not fully processed" or something if this nature)

Yes, anything of that nature would be great. Thank you so much!

[codecovdesign]

sync notes @eliatcodecov @vlad-ko @Adal3n3

• decouple the partial patch reporting and move to: Discovery: Investigate Feasibility of Incremental Reporting for Patch Coverage engineering-team#3020
• broken base scenario works and we can move to dev-ready
  • @codecovdesign ToDo follow up and align updates with the UI (commit/pulls). The problem today is that both are not showing - customer wants to see what we are able to show correctly (i.e. patch). There is also capability to show project coverage based on the head commit. So long as there are no error uploads we can show the report.
• the broken head has challenges that need further investigation
  • will do further discovery in Discovery: Investigate Feasibility of Incremental Reporting for Patch Coverage engineering-team#3020

[codecovdesign]

next steps:

• the base scenario can be moved to dev ready will break down with @adrian-codecov eng: scenario with missing base engineering-team#3272

• the head scenario requires further engineering investigation issue here as placeholder: eng investigation: scenario with missing head #632 will review with @adrian-codecov for next steps
• decouple the partial patch reporting and move to: Discovery: Investigate Feasibility of Incremental Reporting for Patch Coverage engineering-team#3020
  • discovery kickoff on jan 23rd with wider team

[eliatcodecov]

piggybacking on @codecovdesign 's update to share a bit more context. In an earlier post we outlined several scenarios where we believed we could report coverage accurately. One of those was the following:

can show patch coverage for coverage calculated excluding individual report failures , but the UI needs to properly contextualize this reported patch as partial coverage. If this partial coverage is above the threshold for a passing patch coverage check we will pass the check, but notify the user that patch coverage is at least the reported value, but could potentially be greater. Otherwise we will fail the patch check.

There's actually a bit of edge case nuance that makes this problem hard to solve in a generalizable way. So while we continue to work on this (#632 and codecov/engineering-team#3020), we're going to go ahead and ship the rest, which is captured in the above ticket referenced by @codecovdesign (codecov/engineering-team#3272).

Current estimates put codecov/engineering-team#3272 at 2 - 3 weeks out.