fix(e2e): Run each test in test suite in different namespace #2424
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dinhxuanvu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
| ) | ||
|
|
||
| BeforeEach(func() { | ||
| generatedNamespace, ns = SetUpGeneratedTestNamespace(testNamespace) |
There was a problem hiding this comment.
This is really nice and I'm wondering if we can use this pattern in other flake-prone e2e suites. There are some tests that may expect to run in openshift-operators downstream (I can think of one potentially).
If we can't rely on reliable deletion of resources in the namespace each run, then this is the next logical step.
| @@ -914,3 +915,30 @@ func HaveMessage(goal string) gtypes.GomegaMatcher { | |||
| return plan.Status.Message | |||
| }, ContainSubstring(goal)) | |||
| } | |||
|
|
|||
| func SetUpGeneratedTestNamespace(name string) (string, corev1.Namespace) { | |||
There was a problem hiding this comment.
It almost feels like this kind of functionality belongs at the TestContext level, where you'd have a method responsible for building up a new namespace given a string parameter, and return that object reference and maybe a cleanup function?
Maybe a longer term direction would be to abstract testing entirely? Would it be reasonable to create something like an interface, support a setup/test/testdown set of functions? @njhale @awgreene any thoughts?
All test cases in subscription suite shares the same test namespace. In some cases, the resource cleanup which doesn't work properly from previous test causes failure on subsequent test. This commit will create new namespace for each test so it can be run in insolation. Signed-off-by: Vu Dinh <[email protected]>
Signed-off-by: Vu Dinh <[email protected]>
Signed-off-by: Vu Dinh <[email protected]>
dinhxuanvu
changed the title
openshift-ci
bot
added
the
needs-rebase
|
@dinhxuanvu: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
There was a problem hiding this comment.
Looking at the existing e2e codebase, it looks like introducing knobs to capture individual test failures is more nuanced than simply adding a top-level BeforeEach/AfterEach clause that generates a testing namespace for us. Given there's a merge conflict right now, do we want to close this PR out and incrementally introducing support on a per-package-basis or did you have a better approach in mind. Any general thoughts?
|
It looks like this PR is fairly out-of-date at this point, and has a couple of conflicts that need to be resolved, so I'm going to close this out in the meantime. The overall test pollution is still an issue, but it's been watered down recently by breaking down the overall e2e suite into individual clusters that are responsible for running a set of jobs, and filtering out common flaky e2e tests into their own workflow. |
All test cases in subscription suite shares the same test namespace.
In some cases, the resource cleanup which doesn't work properly from
previous test causes failure on subsequent test.
This commit will create new namespace for each test so it can be run
in insolation.
Signed-off-by: Vu Dinh [email protected]
Description of the change:
Motivation for the change:
Reviewer Checklist
/doc