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

Adds E2E coverage reporting #284

Merged
merged 2 commits into from
Jul 28, 2023
Merged

Conversation

m1kola
Copy link
Member

@m1kola m1kola commented Jul 3, 2023

Description

Adds E2E coverage reporting.

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 3, 2023
@m1kola m1kola force-pushed the e2e_coverage branch 3 times, most recently from 90d6d16 to b18c325 Compare July 3, 2023 15:04
@m1kola m1kola changed the title E2E coverage Adds E2E coverage reporting Jul 3, 2023
@m1kola m1kola force-pushed the e2e_coverage branch 4 times, most recently from 519b216 to 65c48f5 Compare July 7, 2023 14:48
Makefile Outdated
e2e: KUSTOMIZE_BUILD_DIR=config/e2e
e2e: GO_BUILD_FLAGS=-cover
e2e: run kind-load-test-artifacts test-e2e ## Run e2e test suite on local kind cluster
# Coverage-instrumented binary produces coverage on termination, so we scale down the manager before gathering the coverage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great use of comments ^^ thank you!

@perdasilva
Copy link
Contributor

I think this looks really good. Very tidy. You're experience as an SRE really comes through. What do you think is missing before we can take it off draft state?

It will be used to optional flags such as -cover.

Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
@m1kola m1kola force-pushed the e2e_coverage branch 2 times, most recently from 5a8f98d to 3a4c0a0 Compare July 27, 2023 14:18
@operator-framework operator-framework deleted a comment from codecov bot Jul 27, 2023
@operator-framework operator-framework deleted a comment from codecov bot Jul 27, 2023
@m1kola
Copy link
Member Author

m1kola commented Jul 27, 2023

@perdasilva thanks for the review.

What do you think is missing before we can take it off draft state?

There were few things which I wanted to change before marking ready for review:

  • I didn't want to have a bunch of kubectl commands in the Makefile They are now in ./hack/e2e-coverage.sh.
  • I didn't like the very long json in --overrides in kubectl run. I moved pod creation into config/e2e/manager_e2e_coverage_copy_pod.yaml.
  • I had to change github action to make sure that we collect coverage even if tests fail. In the previous version coverage was only collected on success due to make aborting execution on first non-zero exit code. I added -k/--keep-going flag to make -k e2e which will continue make build, but will return non-zero exit code in case of failures.

The PR should be ready now. I'm just checking few things before I actually take it off the draft state.

@m1kola m1kola marked this pull request as ready for review July 27, 2023 16:04
@m1kola m1kola requested a review from a team as a code owner July 27, 2023 16:04
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 27, 2023
.github/workflows/e2e.yaml Show resolved Hide resolved
Makefile Show resolved Hide resolved
hack/e2e-coverage.sh Show resolved Hide resolved
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #284 (6bcb822) into main (3c55b14) will increase coverage by 3.69%.
The diff coverage is n/a.

❗ Current head 6bcb822 differs from pull request most recent head 69e9c72. Consider uploading reports for the commit 69e9c72 to get more accurate results

@@            Coverage Diff             @@
##             main     #284      +/-   ##
==========================================
+ Coverage   77.00%   80.69%   +3.69%     
==========================================
  Files          15       18       +3     
  Lines         687      803     +116     
==========================================
+ Hits          529      648     +119     
+ Misses        129      112      -17     
- Partials       29       43      +14     
Flag Coverage Δ
e2e 55.79% <ø> (?)
unit 77.00% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 5 files with indirect coverage changes

@m1kola m1kola requested a review from ncdc July 28, 2023 09:31
@ncdc ncdc merged commit 669c451 into operator-framework:main Jul 28, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants