Add infrastructure for running unit tests #19
Conversation
honestly I don't believe, as you, this belongs upstream, I'll merge after you drop it |
|
VS Code commit dropped. |
| @@ -20,6 +20,8 @@ ifeq ($(INTERACTIVE), 1) | |||
| DOCKER_FLAGS += -t | |||
| endif | |||
| DOCKER_RUN_DOCKER := $(DOCKER_FLAGS) "$(DOCKER_IMAGE)" | |||
| # Use =, not :=, so that this is not expanded when we are not running unit tests. | |||
| UNIT_TEST_PACKAGES = $(shell find -name '*_test.go' | sed 's,/[^/]*$$,,' | sort -u | grep -v '^\./\(integration\|vendor\)$$') | |||
There was a problem hiding this comment.
should go list ./... take care of this filtering?
There was a problem hiding this comment.
go list ./...does not automatically filterintegrationandvendorsubdirectories; are you saying that should somehow happen?go listlists all packages; this only finds packages which contain tests. I am unsure about which is better;go listcauses? $package_name [no test files]lines to be output, arguably both useful and useless. Right now thefind -name '*.test.go'variant means$(UNIT_TEST_PACKAGES)is empty andmake test-unitdoes not even start the container, but this difference will go away with the first added unit test.
There was a problem hiding this comment.
go list ./... does not automatically filter integration and vendor subdirectories; are you saying that should somehow happen?
Nope
go list ./... | grep -v ^vendor/|integration/ (or something like this)
go list lists all packages; this only finds packages which contain tests. I am unsure about which is better; go list causes ?
$package_name [no test files] lines to be output, arguably both useful and useless. Right now the find -name '*.test.go' variant means $ (UNIT_TEST_PACKAGES) is empty and make test-unit does not even start the container, but this difference will go away with the first added unit test.
I'm ok with the "no tests found" line (cause somehow helps figuring instantly when running tests coverage)
There was a problem hiding this comment.
Getting go list to work was surprisingly non-trivial (e.g. it doesn't work within sudo when GOPATH is not set), but it allowed to drop the "empty package list" case and generally simplified the Makefile.
Thanks!
There was a problem hiding this comment.
unfortunately anything dealing with sudo and env variables is fine with go :(
Running unit tests without the integration tests is non-trivial, so add a Makefile target to help with this.
- (make check): GNU coding standards-compliant primary entry point, running all available tests in the best environment (i.e. Docker container). - (make test-all-local): Local entry point, running only tests which do not require a special environment; intended for IDE integration and quick turnaround cycles. Also modifies the Travis configuration to run (make check), to prevent duplication.
|
LGTM |
Add infrastructure for running unit tests
There haven’t been any unit tests in the code base so far; after adding them,
go test ., soMakefiletargets are useful.Makefiletargets.The VS Code aspect is honestly a bit dubious; I’ll gladly drop it if you think it does not belong upstream.