acceptance: add destroy-bundle to clean up the bundle directory - #6318
Open
shreyas-goenka wants to merge 4 commits into
Open
acceptance: add destroy-bundle to clean up the bundle directory#6318shreyas-goenka wants to merge 4 commits into
shreyas-goenka wants to merge 4 commits into
Conversation
`bundle destroy` removes `~/.bundle/<name>/<target>` but not the `<name>` parent, and it does not run at all when a script fails before reaching it. Acceptance tests use a unique bundle name per run, so nothing reuses what is left behind: the shared CI workspace accumulates the leftovers until they hit its child-node limit. Add a `destroy-bundle` helper that destroys the bundle and then removes the `~/.bundle/<name>` directory it was deployed under. The cleanup runs even when destroy fails, which is when a deployment is most likely to be left behind, and the destroy exit code is still propagated so `set -e` behaves as before. The `bundle destroy` call is traced exactly as a direct invocation would be and the cleanup only runs against a real workspace, so no expected output changes. Cleanup failures are ignored: they must never turn a passing test red. Only the `~/.bundle/<name>/<target>` layout is touched, since `root_path` is configurable and the parent of an arbitrary path is not ours to remove. Converted the 58 scripts whose single destroy is the plain form and that use no `-t` target, leaving the shapes where a rewrite could change behaviour (`errcode`, `musterr`, `trap`, several destroys in one script) for a follow-up. Co-authored-by: Isaac
Collaborator
Integration test reportCommit: 0c1f59b
8 interesting tests: 4 SKIP, 3 RECOVERED, 1 flaky
Top 20 slowest tests (at least 2 minutes):
|
Co-authored-by: Isaac
The first pass only matched destroy invocations at the start of a line, which
skipped every one indented inside a `cleanup() { ... }` body registered with
`trap cleanup EXIT` — the most common shape in the suite.
Convert those too, along with the one-line `cleanup()` definitions and the
`trap "errcode trace '$CLI' bundle destroy --auto-approve" EXIT` form, all of
which produce identical trace output through the helper.
Only the last destroy in a script is converted: an earlier one would remove the
parent directory recursively while a later destroy still needs it. The
invocation also has to be terminal, meaning it sits in a trap, in a function a
trap references, or at top level with no deploy after it.
Left alone are the shapes whose recorded output would move: `musterr`, a traced
`errcode` wrapper, untraced or redirected invocations, and flags placed before
`--auto-approve`.
Co-authored-by: Isaac
That test asserts the CLI itself removes the .bundle/<name> directory. Routing it through destroy-bundle would satisfy the assertion from the helper's own cleanup instead, hiding the behaviour under test. Co-authored-by: Isaac
shreyas-goenka
marked this pull request as ready for review
August 19, 2026 11:11
Contributor
Approval status: pending
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Why
As we run more tests and create more PRs we are causing the production DB for webapp to slow down because of the number of treenodes. This PR adds a best-effort cleanup step that was added to existing tests to lower the amount of folder treenodes we end up with over a day.
The cleanup nightly job still handles the final cleanup and actually pruning all nodes after a day.