Keep k8s resources on test failure #236
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pablochacin
force-pushed
the
keep-k8s-resources-on-test-failure
branch
2 times, most recently
from
7761f58 to
20a90bb
Compare
pablochacin
force-pushed
the
keep-k8s-resources-on-test-failure
branch
from
20a90bb to
7763aa9
Compare
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
pablochacin
force-pushed
the
keep-k8s-resources-on-test-failure
branch
from
7763aa9 to
a148748
Compare
nadiamoe
approved these changes
Jun 29, 2023
| if err != nil { | ||
| t.Errorf("error creating test namespace: %v", err) | ||
| t.Errorf("failed to create test namespace: %v", err) |
There was a problem hiding this comment.
Probably a discussion for another PR, but t.Fatalf would be preferred in this cases, as it protects us from forgetting the return line below.
There was a problem hiding this comment.
I would prefer to unify this in another PR.
| options: []TestNamespaceOption{WithPrefix("prefix-")}, | ||
| expectError: false, | ||
| check: func(k8s kubernetes.Interface, ns string) error { | ||
| return nil |
There was a problem hiding this comment.
Should we check here that strings.HasPrefix(ns, "prefix-")?
There was a problem hiding this comment.
Good catch, I forgot to implement the checks 😅
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds a helper for e2e tests that allows retaining the namespace used for a test if the test fails.
Fixes #228
Note to reviewers
Testing this PR is tricky as the expected behavior is triggered by a failed test. Therefore, a unit test was added that actually fails, but still checks if the expected behavior happened.
Another important note is that this test requires the sub-test not to run in parallel or otherwise the validation would happen before the subtests ends. However, the
tparallellinter would complain if the sub-test does not specifiest.Parallel()and it does not accept thenolintdirective.A workaround for the two issues about is to make this test build only if the
keeponfailtag is set in thego testcommandThe command for running this test and the expected output is shown below:
Checklist:
make lint) and all checks pass.make test) and all tests pass.make e2e-xxxforagent,disruptors,kubernetesorclusterrelated changes)