Rewrite coverage test execution to use new Go workspace functions#21145
Rewrite coverage test execution to use new Go workspace functions#21145ivanvc wants to merge 1 commit intoetcd-io:mainfrom
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ivanvc The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test pull-etcd-coverage-report |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted filessee 276 files with indirect coverage changes @@ Coverage Diff @@
## main #21145 +/- ##
==========================================
- Coverage 68.47% 68.05% -0.42%
==========================================
Files 429 449 +20
Lines 35291 37913 +2622
==========================================
+ Hits 24165 25802 +1637
- Misses 9733 10701 +968
- Partials 1393 1410 +17 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
ivanvc
force-pushed
the
update-coverage-tests-to-use-new-functions
branch
from
db6be2f to
627126c
Compare
|
/test pull-etcd-coverage-report |
| function not_test_packages { | ||
| for m in $(modules); do | ||
| if [[ $m =~ .*/etcd/tests/v3 ]]; then continue; fi | ||
| if [[ $m =~ .*/etcd/v3 ]]; then continue; fi | ||
| echo "${m}/..." | ||
| done | ||
| } |
There was a problem hiding this comment.
Note that this function was only used by the coverage pass. So, I moved this to inside the coverage function, updating how we get the modules, and removing the confusing name "not_test_packages": https://github.com/etcd-io/etcd/pull/21145/files#diff-52afcc694c0ff8ff253441f42ff44d6b4734fe30ae9070f2ba07a668a10e7baaR296-R303
| function merge_cov { | ||
| log_callout "[$(date)] Merging coverage files ..." | ||
| coverdir="${1}" | ||
| for d in "${coverdir}"/*/; do | ||
| d=${d%*/} # remove the trailing "/" | ||
| merge_cov_files "${d}" "${d}.coverprofile" & | ||
| done | ||
| wait | ||
| merge_cov_files "${coverdir}" "${coverdir}/all.coverprofile" | ||
| } |
There was a problem hiding this comment.
Because of how we execute the go tests now, there's no need to merge multi-directory-level coverage files. In the end, we only have four coverage files: unit, integration, integration v2 store, and integration cluster proxy. So, this is simplified in https://github.com/etcd-io/etcd/pull/21145/files#diff-52afcc694c0ff8ff253441f42ff44d6b4734fe30ae9070f2ba07a668a10e7baaR322
| function pkg_to_coverprofileflag { | ||
| local prefix="${1}" | ||
| local pkgs="${2}" | ||
| local pkgs_normalized | ||
| prefix_normalized=$(echo "${prefix}" | tr "./ " "__+") | ||
| if [ "${pkgs}" == "./..." ]; then | ||
| pkgs_normalized="all" | ||
| else | ||
| pkgs_normalized=$(echo "${pkgs}" | tr "./ " "__+") | ||
| fi | ||
| mkdir -p "${coverdir}/${prefix_normalized}" | ||
| echo -n "-coverprofile=${coverdir}/${prefix_normalized}/${pkgs_normalized}.coverprofile" | ||
| } |
There was a problem hiding this comment.
Refer to: https://github.com/etcd-io/etcd/pull/21145/files#r2761487207. We no longer need this complex function, as we can set the coverprofile filename during test execution.
| sed --in-place -E "s|go.etcd.io/etcd/api/v3/|api/|g" "${cover_out_file}" || true | ||
| sed --in-place -E "s|go.etcd.io/etcd/client/v3/|client/v3/|g" "${cover_out_file}" || true | ||
| sed --in-place -E "s|go.etcd.io/etcd/client/pkg/v3|client/pkg/v3/|g" "${cover_out_file}" || true | ||
| sed --in-place -E "s|go.etcd.io/etcd/etcdctl/v3/|etcdctl/|g" "${cover_out_file}" || true | ||
| sed --in-place -E "s|go.etcd.io/etcd/etcdutl/v3/|etcdutl/|g" "${cover_out_file}" || true | ||
| sed --in-place -E "s|go.etcd.io/etcd/pkg/v3/|pkg/|g" "${cover_out_file}" || true | ||
| sed --in-place -E "s|go.etcd.io/etcd/server/v3/|server/|g" "${cover_out_file}" || true | ||
| sed --in-place -E "s|go.etcd.io/etcd/(.*)/v3|\1|g" "${cover_out_file}" || true |
There was a problem hiding this comment.
This is just a simplification (and consolidation) of the sed command.
ivanvc
force-pushed
the
update-coverage-tests-to-use-new-functions
branch
from
627126c to
aa04491
Compare
|
/retitle Rewrite coverage test execution to use new Go workspace functions |
k8s-ci-robot
changed the title
|
Please note that the error in the CI job is due to the cache test flake. Even though it failed, the report is correctly generated, as reported by CodeCov (#21145 (comment)) |
ivanvc
force-pushed
the
update-coverage-tests-to-use-new-functions
branch
from
aa04491 to
d2257af
Compare
|
/retest |
Use the new test functions, simplify generating the coverage files and merge at a single level by generating less coverage files. Signed-off-by: Ivan Valdes <ivan@vald.es>
ivanvc
force-pushed
the
update-coverage-tests-to-use-new-functions
branch
from
d2257af to
f0aab33
Compare
2 participants
Use the new test functions, simplify generating the coverage files, and
merge at a single level by generating fewer coverage files.
Part of: #18409
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.