cleanup test directories before running tests#826
Open
Edu92337 wants to merge 2 commits intogetfloresta:masterfrom
Open
cleanup test directories before running tests#826Edu92337 wants to merge 2 commits intogetfloresta:masterfrom
Edu92337 wants to merge 2 commits intogetfloresta:masterfrom
Conversation
Member
|
The PR title could be a bit more descriptive. Also, please squash the last two commits onto the first |
Edu92337
changed the title
Edu92337
force-pushed
the
functional-tests
branch
2 times, most recently
from
4b0b6e8 to
a8fe5ad
Compare
Fixes getfloresta#760 - Keep unconditional /data cleanup at start - Make /logs cleanup conditional on --preserve-data-dir flag
Edu92337
force-pushed
the
functional-tests
branch
from
a8fe5ad to
f88c96e
Compare
|
|
||
| fi | ||
|
|
||
| # Clean existing data/logs directories before running the tests |
Collaborator
There was a problem hiding this comment.
Why remove this comment here?
Edu92337
force-pushed
the
functional-tests
branch
from
0e806eb to
e8e8661
Compare
Edu92337
force-pushed
the
functional-tests
branch
from
e8e8661 to
8345fa4
Compare
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
3 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.
Related to #760 (mitigation)
Keep unconditional /data cleanup at start
Make /logs cleanup conditional on --preserve-data-dir flag
Prevents reusing old log state between test runs
Description and Notes
Currently, the cleanup of /logs directory happens at the END of test execution and only if tests pass. This causes:
Failed tests leave corrupted log state for next run
Tests may fail when run multiple times without manual cleanup
This PR provides a temporary mitigation by making /logs cleanup conditional on the --preserve-data-dir flag, while keeping /data unconditionally cleaned at start.
Note: This is a partial fix. Complete resolution will come in a follow-up PR after #742 is merged, where cleanup logic will be migrated to the Python runner for better control over test data management.
How to verify the changes you have done?
Test 1: Run tests multiple times
./tests/prepare.sh
./tests/run.sh
./tests/run.sh # Should work without manual cleanup
Test 2: Preserve logs for debugging
./tests/run.sh --preserve-data-dir
ls -la $FLORESTA_TEMP_DIR/logs # Logs preserved
Next Steps
After #742 is merged, I plan to submit a follow-up PR to:
Migrate cleanup logic to the Python test runner
Provide more granular control over test data management
Fully resolve #760
Contributor Checklist
I've followed the contribution guidelines
I've verified one of the following:
Ran just pcc (skipped - changes are bash-only, validated manually)
Ran just lint-features '-- -D warnings' && cargo test --release (not applicable)
Confirmed CI passed on my fork (will run after PR creation)
I've linked any related issue(s) in the sections above
Manual validation performed:
✅ Bash syntax check (bash -n)
✅ Logic tested with simulations
✅ --preserve-data-dir flag tested
✅ Consecutive test runs verified