Skip to content

Improve test coverage #1840

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

Merged
merged 6 commits into from
Mar 20, 2019

Conversation

dgageot
Copy link
Contributor

@dgageot dgageot commented Mar 20, 2019

This improves the coverage of a few pieces of code.

It also configures go test so that test coverage is counted on every package, not the package that the test belongs to. This should show our real test coverage.

dgageot added 2 commits March 20, 2019 07:40
Signed-off-by: David Gageot <[email protected]>
Signed-off-by: David Gageot <[email protected]>
dgageot added 2 commits March 20, 2019 09:18
Signed-off-by: David Gageot <[email protected]>
It’s currently waiting for all the tests to end
to print the output

Signed-off-by: David Gageot <[email protected]>
@dgageot dgageot force-pushed the improve-test-coverage branch from d0255a7 to ec9239c Compare March 20, 2019 09:18
@codecov-io
Copy link

codecov-io commented Mar 20, 2019

Codecov Report

Merging #1840 into master will increase coverage by 1.72%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1840      +/-   ##
==========================================
+ Coverage   47.91%   49.64%   +1.72%     
==========================================
  Files         144      165      +21     
  Lines        6499     7145     +646     
==========================================
+ Hits         3114     3547     +433     
- Misses       3101     3270     +169     
- Partials      284      328      +44
Impacted Files Coverage Δ
pkg/skaffold/docker/parse.go 79.71% <ø> (+4.6%) ⬆️
pkg/skaffold/docker/validate.go 100% <100%> (ø)
integration/util.go
hack/schemas/main.go
pkg/skaffold/sources/upload.go 0% <0%> (ø)
pkg/skaffold/sync/kubectl/kubectl.go 9.67% <0%> (ø)
pkg/webhook/kubernetes/deployment.go 0% <0%> (ø)
cmd/skaffold/skaffold.go 0% <0%> (ø)
pkg/webhook/kubernetes/service.go 0% <0%> (ø)
pkg/skaffold/schema/latest/config.go 100% <0%> (ø)
... and 60 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e4c4a9...ca96952. Read the comment docs.

@dgageot
Copy link
Contributor Author

dgageot commented Mar 20, 2019

I still don't understand how Codecov is computing coverage...

@dgageot dgageot force-pushed the improve-test-coverage branch from ec9239c to beed7c2 Compare March 20, 2019 09:58
test.sh Outdated
@@ -21,7 +21,8 @@ GREEN='\033[0;32m'
RESET='\033[0m'

echo "Running go tests..."
go test -count=1 -race -cover -short -timeout 60s -coverprofile=out/coverage.txt -covermode=atomic ./... | grep -v 'no test files' | sed ''/PASS/s//$(printf "${GREEN}PASS${RESET}")/'' | sed ''/FAIL/s//$(printf "${RED}FAIL${RESET}")/''
go test -cover -short -timeout=60s -coverprofile=out/coverage.txt -coverpkg="./pkg/...,./cmd/..." ./... | awk -v FAIL="${RED}FAIL${RESET}" '! /no test files/ { gsub("FAIL", FAIL, $0); print $0 }'
Copy link
Contributor

Choose a reason for hiding this comment

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

this removes the race detector - is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

@dgageot dgageot force-pushed the improve-test-coverage branch from beed7c2 to d2abf81 Compare March 20, 2019 17:50
Not just on the package the test belongs to

Signed-off-by: David Gageot <[email protected]>
@dgageot dgageot force-pushed the improve-test-coverage branch from d2abf81 to ca96952 Compare March 20, 2019 17:56
@dgageot
Copy link
Contributor Author

dgageot commented Mar 20, 2019

@balopat should be all good now

@balopat balopat merged commit d43367e into GoogleContainerTools:master Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants