Make go test ./... succeed by default #16914
Conversation
twpayne
added
release-note/misc
|
Commit 98e2a21de19a9cb29cc0f1f46565c8bade4d0f72 does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
maintainer-s-little-helper
bot
added
the
dont-merge/needs-sign-off
|
Commit 98e2a21de19a9cb29cc0f1f46565c8bade4d0f72 does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
|
Oh, @maintainer-s-little-helper, commit 98e2a21 is no longer part of this PR, so I'm not sure why you're complaining about it. |
|
test-me-please |
|
test-me-please Job 'Cilium-PR-K8s-1.19-kernel-5.4' failed and has not been observed before, so may be related to your PR: Click to show.Test NameFailure OutputIf it is a flake, comment Job 'Cilium-PR-K8s-1.16-net-next' failed and has not been observed before, so may be related to your PR: Click to show.Test NameFailure OutputIf it is a flake, comment |
|
|
|
ConformanceEKS (ci-eks) failure looks like a flake, I've opened #17102. |
|
k8s-1.19-kernel-5.4 (test-1.19-5.4) failure is #16852. |
| // Copyright 2017 Authors of Cilium | ||
| // Copyright 2017-2021 Authors of Cilium | ||
|
|
||
| // +build integration_tests |
There was a problem hiding this comment.
It'd be nice if we could figure out a way to allow both go test ./... and also ginkgo -focus ... without adding this extra -tags integration_tests boilerplate argument for every time a developer wants to run ginkgo 🤔
At the same time, I personally have macros wrapped around the ginkgo invocations anyway so it doesn't directly affect me. And I'd probably rather that unit-testing is the easiest type of testing in order to encourage developers to write more of them.
There was a problem hiding this comment.
It'd be nice if we could figure out a way to allow both
go test ./...and alsoginkgo -focus ...without adding this extra-tags integration_testsboilerplate argument for every time a developer wants to run ginkgo 🤔
Agree, I'd prefer this as well, but I can't find a mechanism to do it, short of providing our own ginkgo binary.
In my personal experience, yes, the tree has been this unstable, but perhaps I've just been unlucky. |
Before this commit, running the standard go command
go test ./...
would fail because a number of unit tests were in fact integration
tests, typically requiring an etcd server to be running.
This PR adds an integration_test build flag to all such tests and
renames the unit-tests make target to integration-tests and sets this
flag.
With this commit "go test ./..." succeeds and runs a large subset of the
unit tests, completing in less than 15 seconds on a eight core machine
and a fully populated cache. "make integration-tests" in the same
configuration takes about nine minutes as it tests packages serially and
does not use or Go's test result cache.
Unfortunately, the bundling of many tests into gopkg.in/check.v1 suites
means that unit tests in many packages are coupled to integration
tests. This commit makes no attempt to unpick such coupling and so the
set of tests run by go test ./... is smaller than it could potentially
be. Improving this is left for a follow-up commit.
Signed-off-by: Tom Payne <tom@isovalent.com>
twpayne
added
the
ready-to-merge
|
Gilberto is on PTO for a bit, this has multiple approvals, so marking as |
PR #16914 missed a couple of places where the integration_test build tag needs to be set. Signed-off-by: Tom Payne <tom@isovalent.com>
PR cilium#16914 missed a couple of places where the integration_test build tag needs to be set. Signed-off-by: Tom Payne <tom@isovalent.com>
This will skip the test when running the tests standalone (i.e. via `go test` and not via Makefile). See cilium#17536 for more details about this particular file, which applied the same principle to the benchmark in that test suite. See also cilium#16914 Reported-by: Hemanth Malla <hemanth.malla@datadoghq.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This will skip the test when running the tests standalone (i.e. via `go test` and not via Makefile). See #17536 for more details about this particular file, which applied the same principle to the benchmark in that test suite. See also #16914 Reported-by: Hemanth Malla <hemanth.malla@datadoghq.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This PR makes
$ go test ./...succeed by default. Before, it would fail due to many unit tests actually being integration tests needing a running etcd server and the requirement for various linker flags to be present to set various test configuration variables.
See the individual commits for more details.