-
Notifications
You must be signed in to change notification settings - Fork 10
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: don't require BASE report for patch in comparison #650
Conversation
This changes is related to codecov/engineering-team#2387. Currently we require that the BASE report exists to compute the comparison, which includes patch data. In fact we only need the HEAD report and a base commit, to have a diff from the git provider. These changes calculate the patch coverage with those assumptions, thus we no longer need a BASE report to have patch coverage. I also removed some timers in favor of sentry tracing
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
@@ Coverage Diff @@
## main #650 +/- ##
==========================================
- Coverage 98.12% 98.11% -0.02%
==========================================
Files 437 437
Lines 36751 36727 -24
==========================================
- Hits 36063 36034 -29
- Misses 688 693 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
@@ Coverage Diff @@
## main #650 +/- ##
==========================================
- Coverage 98.12% 98.11% -0.02%
==========================================
Files 437 437
Lines 36751 36727 -24
==========================================
- Hits 36063 36034 -29
- Misses 688 693 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #650 +/- ##
==========================================
+ Coverage 98.16% 98.17% +0.01%
==========================================
Files 476 476
Lines 38072 38639 +567
==========================================
+ Hits 37374 37935 +561
- Misses 698 704 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 9 files with indirect coverage changes
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #650 +/- ##
==========================================
- Coverage 98.12% 98.11% -0.02%
==========================================
Files 437 437
Lines 36751 36727 -24
==========================================
- Hits 36063 36034 -29
- Misses 688 693 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
calculated_patch_totals = impacted_files.get("changes_summary").get( | ||
"patch_totals" | ||
) | ||
if calculated_patch_totals != comparison.patch_totals: | ||
log.warning( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
under which circumstances would this ever happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, to be honest. Hopefully never.
But as I'm switching from that calculation to a copy of the one we use for the notifications I thought it would be prudent to add this just in case.
I also thought it was a good idea to always keep the result that come from the same place
This changes is related to codecov/engineering-team#2387.
Currently we require that the BASE report exists to compute the comparison, which includes patch data.
In fact we only need the HEAD report and a base commit, to have a diff from the git provider.
These changes calculate the patch coverage with those assumptions, thus we no longer need a BASE report to have patch coverage.
I also removed some timers in favor of sentry tracing