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

Verify that all packages have tests #635

Merged
merged 2 commits into from
Jan 6, 2018
Merged

Verify that all packages have tests #635

merged 2 commits into from
Jan 6, 2018

Conversation

yurishkuro
Copy link
Member

Add make nocover target (that also runs as part of make cover) that checks all directories that have Go files but no test files and fails if such directories don't also have .nocover marker, which indicates that the directories are silently excluded from code coverage. If .nocover marker is found, it is printed, so that it's easy to find directories without code coverage. Directories with main.go are ignored.

The script found a number of directories:

  • Some of them have only type definitions, so an empty_test.go file was added to those
  • Some were legitimately excluded due to external requirements, like the gocql module. A .nocover file was added to those dirs with an explanation.
  • Four directories were still missing tests, even though it should be possible to add coverage. Those were also marked with .nocover file containing the word FIXME.

The current output of nocover command looks like this:

$ make nocover
Verifying that all packages have test files to count in coverage
Package excluded from coverage: ./cmd/agent/app/httpserver/thrift-0.9.2
  reason: Thrift-generated files
Package excluded from coverage: ./cmd/flags
  reason: FIXME
Package excluded from coverage: ./pkg/cassandra/config
  reason: requires connection to Cassandra
Package excluded from coverage: ./pkg/cassandra/gocql
  reason: requires connection to Cassandra
Package excluded from coverage: ./pkg/cassandra/gocql/testutils
  reason: FIXME
Package excluded from coverage: ./pkg/es
  reason: FIXME
Package excluded from coverage: ./pkg/es/config
  reason: requires connection to Elasticsearch
Package excluded from coverage: ./pkg/version
  reason: FIXME

Fixes #168

Signed-off-by: Yuri Shkuro ys@uber.com

Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling b4e974c on fix-168 into 32bc1b7 on master.

@yurishkuro yurishkuro merged commit d2f85a4 into master Jan 6, 2018
@ghost ghost removed the review label Jan 6, 2018
@yurishkuro yurishkuro deleted the fix-168 branch January 6, 2018 18:03
@black-adder black-adder mentioned this pull request Jan 31, 2018
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.

2 participants