tests: Fix flaky TestWatchNamespaceSelector #10160
Conversation
github-actions
bot
added
keep pr updated
davidjumani
removed
the
work in progress
|
Visit the preview URL for this PR (updated for commit f5c4dfd): https://gloo-edge--pr10160-fix-watch-ns-mjgy3a5i.web.app (expires Thu, 10 Oct 2024 15:19:16 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 77c2b86e287749579b7ff9cadb81e099042ef677 |
| @@ -59,6 +59,11 @@ func (s *testingSuite) testWatchNamespaceSelector(key, value string) { | |||
|
|
|||
| // The VS defined in the random namespace should be translated | |||
| s.TestInstallation.Assertions.CurlEventuallyRespondsWithStatus(s.Ctx, "random/", http.StatusOK) | |||
|
|
|||
| s.unwatchNamespace(key) | |||
There was a problem hiding this comment.
Is this a "cleanup" step and therefore we should consider performing it in the AfterTest method, or in a deferred function so it is always executed?
There was a problem hiding this comment.
It's only for this specific test and it also serves as a way to verify that unlabelling a namespace leads to the resources defined in it to not be translated
| @@ -210,7 +210,7 @@ analyze: | |||
|
|
|||
| GINKGO_VERSION ?= $(shell echo $(shell go list -m github.com/onsi/ginkgo/v2) | cut -d' ' -f2) | |||
| GINKGO_ENV ?= GOLANG_PROTOBUF_REGISTRATION_CONFLICT=ignore ACK_GINKGO_RC=true ACK_GINKGO_DEPRECATIONS=$(GINKGO_VERSION) | |||
| GINKGO_FLAGS ?= -tags=purego --trace -progress -race --fail-fast -fail-on-pending --randomize-all --compilers=5 | |||
| GINKGO_FLAGS ?= -tags=purego --trace -progress -race --fail-fast -fail-on-pending --randomize-all --compilers=5 --flake-attempts=3 | |||
There was a problem hiding this comment.
This will affect all times we invoke these tests, on PRs and nightlies. I'm good with this, though initially i had envisioned we would only add this for PRs, so that nightlies still give us a good signal. wdyt?
There was a problem hiding this comment.
Since I assume the plan is to eventually move away from ginkgo, this should be fine as long as we make strides to migrate tests
|
@davidjumani do you mind adding Fixes #10160 to the PR description to close out the associated issue? |
I'll close out the issue when this is backported to 1.17 |
| @@ -210,7 +210,7 @@ analyze: | |||
|
|
|||
| GINKGO_VERSION ?= $(shell echo $(shell go list -m github.com/onsi/ginkgo/v2) | cut -d' ' -f2) | |||
| GINKGO_ENV ?= GOLANG_PROTOBUF_REGISTRATION_CONFLICT=ignore ACK_GINKGO_RC=true ACK_GINKGO_DEPRECATIONS=$(GINKGO_VERSION) | |||
| GINKGO_FLAGS ?= -tags=purego --trace -progress -race --fail-fast -fail-on-pending --randomize-all --compilers=5 | |||
| GINKGO_FLAGS ?= -tags=purego --trace -progress -race --fail-fast -fail-on-pending --randomize-all --compilers=5 --flake-attempts=3 | |||
There was a problem hiding this comment.
i'm in favor of reverting this change so we can catch flakes more easily
Description
Fixes the flaky TestMatchLabels that could be caused by not removing the label on a namespace after the test.
The presence of the label on the namespace (from the prior test even though the namespace was deleted???) could be present which causes the test to flake - maybe translation takes a bit of time to process the deleted namespace and its resources which is longer than it takes for the next test to begin ??
I've run this test ~20 times without flakes so it appears to be stable now
Fixes #10160 once backported
Checklist: