Always correctly clean up temp contents #483
Merged
Conversation
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
Add a unit test and an acceptance test to verify that temp resources created in the `set_up_before_script` function are correctly cleaned up.
This commit resolves an issue where temporary files created in `set_up_before_script` were not properly cleaned up due to test ID contamination between test files. Unset BASHUNIT_CURRENT_TEST_ID and set BASHUNIT_CURRENT_SCRIPT_ID at the start of a script in order to make sure `temp_file()` and `temp_dir()` use the correct prefix for their path names. Remove `cleanup_temp_files()` in favor of more explicit `cleanup_testcase_temp_files()` and `cleanup_script_temp_files()` functions. Call `cleanup_testcase_temp_files()` in the testcase exit trap handler to make sure that testcase temp files are always cleaned. Call `cleanup_script_temp_files()` explicitly at the end of test file runs, as well as in cleanup handlers. This makes the general cleaning at the end of `main::exec_tests()` and `main::exec_benchmarks` obsolete. The fallback branch in the previous implementation (`rm -rf "${TMPDIR:-/tmp}/bashunit/tmp"/*`) was not a good idea anyway, as it potentially broke parallel running instances of bashunit using the same TMPDIR. Contributor Author
| I just realized that the current implementation will leave temp files when bashunit is interrupted via Ctrl-C in parallel mode. To safely clean everything in this use case we should remove bashunit's complete temp folder as it was done before. This is not safe for running multiple bashunit instances in parallel, though, but that is probably a different issue which we could fix afterwards (by adding an ID to the base temporary path). |
carlfriedrich deleted the fix/always-correctly-clean-up-temp-contents branch 2 tasks
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.
📚 Description
Fixes #482
This PR resolves an issue where temporary files and directories created in
set_up_before_scriptwere not properly cleaned up.It adds both a unit test an an acceptance test that fail without the fix and now pass.
🔖 Changes
temp_fileandtemp_dirfunctions so that they✅ To-do list
CHANGELOG.mdto reflect the new feature or fix