Skip to content

Add test-coverage & codecov target and update circleci #109

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.

Sign up for GitHub

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
dnephin merged 2 commits into from
May 23, 2017
+31 −3
Merged

Add test-coverage & codecov target and update circleci #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d79d903
Add test-coverage & codecov target and update circleci
vdemeester May 18, 2017
f6d148c
Upload coverage report from outside of test container.
dnephin May 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@
/build/
cli/winresources/rsrc_386.syso
cli/winresources/rsrc_amd64.syso
coverage.txt
profile.out
6 changes: 5 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ clean:
# the "-tags daemon" part is temporary
.PHONY: test
test:
go test -tags daemon -v $(shell go list ./... | grep -v /vendor/)
./scripts/test/unit $(shell go list ./... | grep -v /vendor/)

.PHONY: test-coverage
test-coverage:
./scripts/test/unit-with-coverage

.PHONY: lint
lint:
Expand Down
7 changes: 5 additions & 2 deletions circle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,16 @@ jobs:
docker run --name cross cli-builder make cross
docker cp cross:/go/src/github.com/docker/cli/build /work/build
- run:
name: "Unit Test"
name: "Unit Test with Coverage"
command: |
if [ "$CIRCLE_NODE_INDEX" != "2" ]; then exit; fi
dockerfile=dockerfiles/Dockerfile.dev
echo "COPY . ." >> $dockerfile
docker build -f $dockerfile --tag cli-builder .
docker run cli-builder make test
docker run --name test cli-builder make test-coverage
docker cp test:/go/src/github.com/docker/cli/coverage.txt coverage.txt
apk add -U bash curl
curl -s https://codecov.io/bash | bash
- run:
name: "Validate Vendor and Code Generation"
command: |
Expand Down
4 changes: 4 additions & 0 deletions scripts/test/unit
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/usr/bin/env bash
set -eu -o pipefail

go test -tags daemon -v $@
Copy link
Contributor

@stevvooe stevvooe May 22, 2017

Choose a reason for hiding this comment

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

Why aren't these just in the Makefile? These kinds of systems are extremely hard to maintain.

Copy link
Contributor

@dnephin dnephin May 23, 2017

Choose a reason for hiding this comment

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

What are "these kinds of systems" ? And what makes them hard to maintain?

Copy link
Contributor

@stevvooe stevvooe May 23, 2017

Choose a reason for hiding this comment

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

A mess of bash scripts. It contributes to a horrid build experience and is very hard to modify.

This particular example is absurd. Why is this in its own file?

15 changes: 15 additions & 0 deletions scripts/test/unit-with-coverage
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/usr/bin/env bash
set -eu -o pipefail

for pkg in $(go list ./... | grep -v /vendor/); do
./scripts/test/unit \
-cover \
-coverprofile=profile.out \
-covermode=atomic \
${pkg}

if test -f profile.out; then
Copy link
Contributor

@stevvooe stevvooe May 22, 2017

Choose a reason for hiding this comment

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

Please include a comment as to why this required.

Copy link
Contributor

@stevvooe stevvooe May 23, 2017

Choose a reason for hiding this comment

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

Please address this. Why is this not an argument to the original command?

Copy link
Contributor

@dnephin dnephin May 23, 2017

Choose a reason for hiding this comment

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

What is "this"? The profile.out filename?

https://lk4d4.darth.io/posts/multicover/

Copy link
Contributor

@stevvooe stevvooe May 23, 2017

Choose a reason for hiding this comment

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

The file is given as one argument, then copied to the other. Why not give the original file to the command and have it write directly? Are you trying to append it to a global coverage file?

Copy link
Contributor

@dnephin dnephin May 23, 2017

Choose a reason for hiding this comment

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

The file isn't copied. The file is appended to the global coverage file yes. That's what it's doing.

cat profile.out >> coverage.txt
rm profile.out
fi
done