airship
/
airshipctl
Code Issues Proposed changes

Fix the `make cover` target 

This change has multiple benefits:

* Allows testing/coverage Makefile flags to be set from the command
  line. This allows a dev to easily test both correctness and coverage of
  a subset of packages by simply providing a value for the `PKG` make
  variable.

* Fixes an issue where an error message was being printed to stderr on
  each Zuul run, since the `COVER_PKG` variable required that Go is
  installed, and yet we don't install Go to the Zuul VM.

* Fixes an issue where Zuul VMs were crashing due to an Out Of Memory
  error. This was happening because of the way that the `coverpkg` flag
  works, which caused tests to occasionally require more than 8G of
  memory, exceeding the amount of memory on each Zuul VM.

* Cleans up the output when running `make cover`. Previously, the list
  of all packages was printed for coverage for each package, creating a
  lot of noise on the console.

However, it has the following negative:

* With the removal of the `coverpkg` flag, the `go test` command will no
  longer check coverage of packages which do not include any `_test.go`
  files. This puts the responsibility of assuring that coverage is
  correctly reflected onto the shoulders of developers and reviewers.

Change-Id: I7fef1dd26ef19b5f2ab4cf2d1be223bf82210492
This commit is contained in:
Ian Howell 2019-10-16 16:42:01 -05:00
parent 3101e642ce
commit 6fb4af06a1
1 changed files with 6 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
Makefile
View File

@ -21,12 +21,11 @@ DOCKER_IMAGE ?= $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_PREFIX)/$(DOCKER_IMAGE_
DOCKER_TARGET_STAGE ?= release
# go options
PKG := ./...
TESTS := .
TEST_FLAGS :=
COVER_FLAGS :=
COVER_PROFILE := cover.out
COVER_PKG := $(shell go list ./... | tail -n+2 | grep -v "opendev.org/airship/airshipctl/testutil" | paste -sd"," -)
PKG ?= ./...
TESTS ?= .
TEST_FLAGS ?=
COVER_FLAGS ?=
COVER_PROFILE ?= cover.out
.PHONY: get-modules
get-modules:
@ -48,7 +47,7 @@ unit-tests:
@echo "All unit tests passed"
.PHONY: cover
cover: COVER_FLAGS = -covermode=atomic -coverprofile=$(COVER_PROFILE) -coverpkg=$(COVER_PKG)
cover: COVER_FLAGS = -covermode=atomic -coverprofile=$(COVER_PROFILE)
cover: unit-tests
@./tools/coverage_check $(COVER_PROFILE)