Skip to content

Conversation

@carlfriedrich
Copy link
Contributor

📚 Description

Fixes #482

This PR resolves an issue where temporary files and directories created in set_up_before_script were not properly cleaned up.

It adds both a unit test an an acceptance test that fail without the fix and now pass.

🔖 Changes

  • Extend temp_file and temp_dir functions so that they
    • detect whether they are called in testcase context or script setup context
    • use different target paths for each
  • Improve clean up routines to
    • clean up testcase temp data at the end of a test case
    • clean up script temp data at the end of a test script (using different implementations for sequential and parallel modes)

✅ To-do list

  • I updated the CHANGELOG.md to reflect the new feature or fix
  • I updated the documentation to reflect the changes

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.
@carlfriedrich
Copy link
Contributor Author

carlfriedrich commented Sep 5, 2025

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).

@Chemaclass Chemaclass added the bug Something isn't working label Sep 5, 2025
Copy link
Member

@Chemaclass Chemaclass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thank you!

@Chemaclass Chemaclass merged commit a18a6ff into TypedDevs:main Sep 5, 2025
18 checks passed
@carlfriedrich carlfriedrich deleted the fix/always-correctly-clean-up-temp-contents branch September 5, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

temp_file and temp_dir are not cleaned up when called from set_up_before_script()

2 participants