Expose failures to clean up after tests #342
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
When stack objects aren't deleted after tests, it can mean - resources created by the stack aren't tidied up; - the finalizer logic isn't well tested - the stack might interfere in annoying ways with other tests. Previously, all stacks in the namespace were deleted as part of a top-level `AfterEach`; but because this tried to delete all stacks and wait until they'd been finalised, any rogue test case that didn't clean up properly and couldn't be finalised would cause some _other_ test to time out. Instead, this commit uses a similar idea, but fails the test suite if there are stacks left over at the very end. This highlighted a couple of problems, which I've also fixed here: - one test was deleting its working directory before deleting the stack using it, and would fail to be finalized; - lots of stacks had random names that didn't help with tracking down the test case Finally, the helper `deleteAndWaitForFinalization` can be used to ensure a test case doesn't exit until it has truly cleaned up. It's worth using this everywhere -- next, I'm going to rewrite tests to use it, and the other helpers. Signed-off-by: Michael Bridgen <mbridgen@pulumi.com>
Signed-off-by: Michael Bridgen <mbridgen@pulumi.com>
It's a little bit faster to run a YAML program than to download dependencies and run a NodeJS program, so this saves a bit of time and reduces the possibility of timeouts. Signed-off-by: Michael Bridgen <mbridgen@pulumi.com>
Signed-off-by: Michael Bridgen <mbridgen@pulumi.com>
If the controller repeatedly retries a Stack, it will oscillate between having the "Reconciling" condition because it has been requeued, and having the same condition because it's being processed by the controller again. This makes seeing a specific reason for retrying tricky -- but, if you've observed that the stack has been processed and failed, then _either_ reason is good enough to show that it's being retried, so that race condition can be avoided. Signed-off-by: Michael Bridgen <mbridgen@pulumi.com>
squaremo
force-pushed
the
do-finalisers-work-properly
branch
from
791706f
to
bcd57fd
Compare
lblackstone
approved these changes
Oct 18, 2022
lblackstone
added
the
impact/no-changelog-required
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.
When stack objects aren't deleted after tests, it can mean
Previously, all stacks in the namespace were deleted as part of a top-level
AfterEach; but because this tried to delete all stacks and wait until they'd been finalised, any rogue test case that didn't clean up properly and couldn't be finalised would cause some other test to time out.The first commit here uses a similar idea, but fails the test suite if there are stacks left over at the very end. This highlighted a few problems, which I've also fixed here:
Deleting a stack but not waiting for it to be finalized, within a test, could mean other tests were "contaminated" (usually only meaning they might take a bit longer, and get cross-talk in the logs, but still). I've made all test cases use a helper which waits for the finalizers to have run.