-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
CI: Tweak our code coverage profile behavior #14967
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Matt Lord <[email protected]>
As processes seem to be getting killed Signed-off-by: Matt Lord <[email protected]>
cc20c4e
to
14c677c
Compare
Signed-off-by: Matt Lord <[email protected]>
The report.xml file in git seemed to be causing codecov issues. Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
codecov.yml
Outdated
- "go/test/endtoend" | ||
- "go/*/endtoend" | ||
- "go/vt/*/endtoend" | ||
- "go/cmd/vttestserver" # This relies on end-to-end test packages |
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.
Should we exclude all cached_size.go
files as well as generated code?
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.
Added here: 56f139f
Please let me know if you think we should remove any of the added ignore lines. Thanks!
Signed-off-by: Matt Lord <[email protected]>
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.
As discussed earlier, can we disable codecov
comments on the file changes.
This page notes how we can toggle them on and off in the PR's Changed Files view, as well as how we can disable them entirely: https://docs.codecov.com/docs/github-checks#hiding-annotations-in-the-files-view Do you think that we should disable them globally or make it more well known how to toggle them on and off during PR review? I'm OK either way. You can also see the untested lines in a better UI when you view the commit in codecov using the link added to main codecov PR comment. For example (click on a file): https://app.codecov.io/gh/vitessio/vitess/pull/15096 |
Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
It would only pass if: - You had a snap-private-tmp sub-dir in /tmp - You did not have permissions to read it Signed-off-by: Matt Lord <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14967 +/- ##
===========================================
+ Coverage 47.80% 70.63% +22.82%
===========================================
Files 1155 1375 +220
Lines 240308 182222 -58086
===========================================
+ Hits 114890 128715 +13825
+ Misses 116846 53507 -63339
+ Partials 8572 0 -8572 ☔ View full report in Codecov by Sentry. |
43890de
to
58bae66
Compare
Signed-off-by: Matt Lord <[email protected]>
58bae66
to
5e5bbd9
Compare
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 think we should ignore generated protobuf stuff, too; otherwise lgtm!
- "go/mysql/collations/supported.go" # Code generated by makecolldata | ||
|
||
comment: # https://docs.codecov.com/docs/pull-request-comments | ||
hide_project_coverage: false |
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.
just stating for posterity that i'm good with this given that https://docs.codecov.com/docs/github-checks#hiding-annotations-in-the-files-view exists
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.
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.
LGTM, thanks a bunch @mattlord
Signed-off-by: Matt Lord <[email protected]>
Description
This PR makes the following changes:
./codecov.yml
) to tweak the settings based on our early experiences-coverpkg=vitess.io/vitess/go/...
flag to ourgo test
invocation to better calculate package level coverage when testing is done across packages-covermode=atomic
flag to ourgo test
invocation to support parallel workunit race
workflows as the the code coverage workflow — which has, now cross package, coverage profiling enabled — is also heavier than the basicunit
test workflows and processes were often getting killedCould not read coverage file (coverage.out): Error: There was an error reading the coverage file: Error: Cannot create a string longer than 0x1fffffe8 characters
Related Issue(s)
Checklist