Skip to content
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

Remove e2e from coverage calculation #15544

Merged
merged 7 commits into from
Mar 30, 2023
Merged

Conversation

jmhbnz
Copy link
Member

@jmhbnz jmhbnz commented Mar 21, 2023

This pull request removes e2e from the coverage collected in scripts/test.sh.

Ref #15522

Signed-off-by: James Blair <mail@jamesblair.net>
@serathius
Copy link
Member

Please find and cleanup files with cov and !cov build tag. They usually have _cov_ or _nocov_ in their name. Also cleanup build target cov (and related) and coverage workflow.

@jmhbnz jmhbnz changed the title Remove e2e from coverage calculation [WIP] Remove e2e from coverage calculation Mar 24, 2023
@jmhbnz
Copy link
Member Author

jmhbnz commented Mar 26, 2023

Please find and cleanup files with cov and !cov build tag.

Ok so taking this one step at a time to get my head around it, here is our list of cov and !cov tagged files from across the codebase:

grep -Ri "//go:build" | grep cov

pkg/osutil/signal_linux.go://go:build linux && !cov
pkg/osutil/signal.go://go:build !linux || cov
tests/framework/e2e/etcd_spawn_nocov.go://go:build !cov
tests/framework/e2e/etcd_spawn_cov.go://go:build cov
tests/e2e/zap_logging_test.go://go:build !cov
tests/e2e/ctl_v3_completion_test.go://go:build !cov
tests/e2e/v3_cipher_suite_test.go://go:build !cov && !cluster_proxy
tests/e2e/ctl_v3_watch_no_cov_test.go://go:build !cov
tests/e2e/ctl_v3_watch_cov_test.go://go:build cov

@serathius are we simply removing the cov|!cov build tags from these files, or is there more detailed cleanup required for this step?

@serathius
Copy link
Member

Depends on the case. We could either:

  • Merge the cov and no_cov files. This makes sense in test files where some tests are disabled in coverage scenario. Example tests/e2e/ctl_v3_watch_cov_test.go and tests/e2e/ctl_v3_watch_no_cov_test.go
  • Remove the cov file and remove build tag from no_cov file. This should be done in alternative implementation scenario, where there is common interface that is implemented differently for cov and no_cov. Example tests/framework/e2e/etcd_spawn_nocov.go and tests/framework/e2e/etcd_spawn_cov.go

jmhbnz added 2 commits March 30, 2023 15:17
Signed-off-by: James Blair <mail@jamesblair.net>
Signed-off-by: James Blair <mail@jamesblair.net>
Signed-off-by: James Blair <mail@jamesblair.net>
@jmhbnz jmhbnz changed the title [WIP] Remove e2e from coverage calculation Remove e2e from coverage calculation Mar 30, 2023
@jmhbnz
Copy link
Member Author

jmhbnz commented Mar 30, 2023

Hey @serathius - Many thanks for the guidance above. I've had a go a the remaining cleanup you described. Hoping I have understood this correctly. Please take a look when you can and let me know what you think 🙏🏻

Signed-off-by: James Blair <mail@jamesblair.net>
Signed-off-by: James Blair <mail@jamesblair.net>
@serathius
Copy link
Member

Looks great! This is exactly what we want.

@serathius serathius merged commit e11a323 into etcd-io:main Mar 30, 2023
@jmhbnz jmhbnz deleted the remove_e2e_calc branch July 16, 2023 05:49
fuweid added a commit to fuweid/etcd that referenced this pull request Oct 17, 2023
The etcd-io#15544 has removed the `build_cov` build. And after go1.20, we use
`-cover` buildflag to enable coverage exporter. We don't need to
maintain main_test.go anymore.

```bash
➜  pwd
/home/fuwei/go/src/go.etcd.io/etcd/etcdctl

➜  go build -o /tmp/etcdctl -cover ./

➜  mkdir /tmp/etcdctl-covdata

➜  GOCOVERDIR=/tmp/etcdctl-covdata /tmp/etcdctl get /health

➜  go tool covdata percent -i=/tmp/etcdctl-covdata
        go.etcd.io/etcd/etcdctl/v3      coverage: 66.7% of statements
        go.etcd.io/etcd/etcdctl/v3/ctlv3        coverage: 83.3% of statements
        go.etcd.io/etcd/etcdctl/v3/ctlv3/command        coverage: 15.4% of statements
```

REF: https://go.dev/testing/coverage/

Signed-off-by: Wei Fu <fuweid89@gmail.com>
@fuweid fuweid mentioned this pull request Oct 17, 2023
fuweid added a commit to fuweid/etcd that referenced this pull request Oct 17, 2023
The etcd-io#15544 has removed the `build_cov` build. And after go1.20, we use
`-cover` buildflag to enable coverage exporter. We don't need to
maintain main_test.go anymore.

```bash
➜  pwd
/home/fuwei/go/src/go.etcd.io/etcd/etcdctl

➜  go build -o /tmp/etcdctl -cover ./

➜  mkdir /tmp/etcdctl-covdata

➜  GOCOVERDIR=/tmp/etcdctl-covdata /tmp/etcdctl get /health

➜  go tool covdata percent -i=/tmp/etcdctl-covdata
        go.etcd.io/etcd/etcdctl/v3      coverage: 66.7% of statements
        go.etcd.io/etcd/etcdctl/v3/ctlv3        coverage: 83.3% of statements
        go.etcd.io/etcd/etcdctl/v3/ctlv3/command        coverage: 15.4% of statements
```

REF: https://go.dev/testing/coverage/

Signed-off-by: Wei Fu <fuweid89@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants