-
Notifications
You must be signed in to change notification settings - Fork 437
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
tests: Fix flaky TestWatchNamespaceSelector #10160
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'm in favor of reverting this change so we can catch flakes more easily |
||
GINKGO_REPORT_FLAGS ?= --json-report=test-report.json --junit-report=junit.xml -output-dir=$(OUTPUT_DIR) | ||
GINKGO_COVERAGE_FLAGS ?= --cover --covermode=atomic --coverprofile=coverage.cov | ||
TEST_PKG ?= ./... # Default to run all tests | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
changelog: | ||
- type: NON_USER_FACING | ||
description: Fixes the flaky TestMatchLabels that could be caused by not removing the label on a namespace after the test. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
||
// Ensure CRs defined in non watched-namespaces are not translated | ||
s.TestInstallation.Assertions.CurlConsistentlyRespondsWithStatus(s.Ctx, "random/", http.StatusNotFound) | ||
} | ||
|
||
func (s *testingSuite) TestUnwatchedNamespaceValidation() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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