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

feat add coverage tests #1207

Merged
merged 1 commit into from Jan 14, 2020
Merged

Conversation

@camilamacedo86
Copy link
Member

camilamacedo86 commented Nov 17, 2019

Description

  • Add coveralls to check the % of code covered.

See: https://coveralls.io/github/kubernetes-sigs/kubebuilder

@k8s-ci-robot k8s-ci-robot requested review from droot and mengqiy Nov 17, 2019
@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:coveralls branch from c41fec9 to 7ac13be Nov 17, 2019
@k8s-ci-robot k8s-ci-robot added size/M and removed size/S labels Nov 17, 2019
@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:coveralls branch 5 times, most recently from 314fb89 to 4d383ab Nov 17, 2019
@camilamacedo86 camilamacedo86 changed the title feat add coverage tests WIP: feat add coverage tests Nov 17, 2019
@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:coveralls branch 3 times, most recently from 6bf284b to 3bd5acb Nov 17, 2019
@k8s-ci-robot k8s-ci-robot added size/S and removed size/M labels Nov 18, 2019
@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:coveralls branch from 3bd5acb to 330a868 Nov 18, 2019
@camilamacedo86 camilamacedo86 changed the title WIP: feat add coverage tests feat add coverage tests Nov 18, 2019
@camilamacedo86

This comment has been minimized.

Copy link
Member Author

camilamacedo86 commented Nov 18, 2019

/assign @mengqiy
/assign @droot

.travis.yml Outdated Show resolved Hide resolved
@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:coveralls branch from 1662061 to 330a868 Nov 21, 2019
.travis.yml Outdated Show resolved Hide resolved
@camilamacedo86

This comment has been minimized.

Copy link
Member Author

camilamacedo86 commented Dec 4, 2019

HI @mengqiy and @@alexeldeib,

WDYT, could we move forward with this one?

@alexeldeib

This comment has been minimized.

Copy link
Contributor

alexeldeib commented Dec 12, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Dec 12, 2019
@camilamacedo86

This comment has been minimized.

Copy link
Member Author

camilamacedo86 commented Dec 12, 2019

Hi @alexeldeib,

For we are able to merge we need the /approved one as well
And would be great someone with access to Travis create the env var with the token there for we use the commented line instead. See; #1207 (comment)

@camilamacedo86

This comment has been minimized.

Copy link
Member Author

camilamacedo86 commented Jan 6, 2020

Hi @mengqiy ,

Please, let me know if you are ok with we move now with and if we can add the token in the trevis in another PR.

Copy link
Contributor

DirectXMan12 left a comment

added the token in travis. please update the PR accordingly.

@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:coveralls branch from 78a142f to 76fa5ea Jan 7, 2020
@k8s-ci-robot k8s-ci-robot removed the lgtm label Jan 7, 2020
@camilamacedo86 camilamacedo86 requested a review from DirectXMan12 Jan 7, 2020
Makefile Outdated
@@ -19,6 +19,8 @@
#
export GOPROXY?=https://proxy.golang.org/
CONTROLLER_GEN_BIN_PATH := $(shell which controller-gen)
PKGS_COVER ?= $(shell go list ./... | grep -v /vendor/ | grep -v /testdata/ | grep -v /test/ | sort | uniq)

This comment has been minimized.

Copy link
@DirectXMan12

DirectXMan12 Jan 7, 2020

Contributor

sort -u can replace sort | uniq. Additionally, we can skip the grep -v vendor and grep -v testdata, since we don't have a vendor directory and go list skips testdata already.

This comment has been minimized.

Copy link
@camilamacedo86

camilamacedo86 Jan 7, 2020

Author Member

done

.travis.yml Outdated Show resolved Hide resolved
Makefile Outdated
echo "mode: count" > coverage-all.out
$(foreach pkg,$(PKGS_COVER),\
go test -failfast -tags=integration -coverprofile=coverage.out -covermode=count $(pkg) || exit 1;\
tail -n +2 coverage.out >> coverage-all.out;)

This comment has been minimized.

Copy link
@DirectXMan12

DirectXMan12 Jan 7, 2020

Contributor

where does this come from? The goveralls docs don't seem to think this is necessary. It at least needs a comment.

This comment has been minimized.

Copy link
@camilamacedo86

camilamacedo86 Jan 7, 2020

Author Member

I simplified and added the comments. Please, let me know what do you think now.

This comment has been minimized.

Copy link
@DirectXMan12

DirectXMan12 Jan 7, 2020

Contributor

actually, I think we can simplify this further: go test -failfast -tags=integration -coverprofile=coverages.out -covermode=count ./pkg/... ./cmd/...

This comment has been minimized.

Copy link
@camilamacedo86

camilamacedo86 Jan 9, 2020

Author Member

It is true 👍 Getter better how it works too :-)

@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:coveralls branch from 76fa5ea to bf1d081 Jan 7, 2020
.travis.yml Outdated
before_script:
# The following module is used to integrate the projct with coveralls.io. It allow us to easily sent the data.
# More info: https://github.com/mattn/goveralls
- go get github.com/mattn/goveralls

This comment has been minimized.

Copy link
@DirectXMan12

DirectXMan12 Jan 7, 2020

Contributor

can you pin this to a particular version please? e.g. go get github.com/mattn/goveralls@vXYZ

This comment has been minimized.

Copy link
@camilamacedo86

camilamacedo86 Jan 7, 2020

Author Member

good spot. Done.

@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:coveralls branch 5 times, most recently from d7a72f7 to d155c03 Jan 7, 2020
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Jan 9, 2020

@camilamacedo86: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-kubebuilder-e2e 330a868 link /test pull-kubebuilder-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:coveralls branch from d155c03 to 24d31be Jan 10, 2020
@camilamacedo86 camilamacedo86 requested a review from DirectXMan12 Jan 10, 2020
@DirectXMan12

This comment has been minimized.

Copy link
Contributor

DirectXMan12 commented Jan 14, 2020

/lgtm
/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Jan 14, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, DirectXMan12

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 6357643 into kubernetes-sigs:master Jan 14, 2020
7 of 8 checks passed
7 of 8 checks passed
tide Not mergeable. Needs approved, lgtm labels.
Details
cla/linuxfoundation camilamacedo86 authorized
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
netlify/kubebuilder/deploy-preview Deploy preview ready!
Details
pull-kubebuilder-e2e-k8s-1-14-1 Job succeeded.
Details
pull-kubebuilder-e2e-k8s-1-15-3 Job succeeded.
Details
pull-kubebuilder-e2e-k8s-1-16-2 Job succeeded.
Details
pull-kubebuilder-test Job succeeded.
Details
@camilamacedo86 camilamacedo86 deleted the camilamacedo86:coveralls branch Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.