Add test cleanup fixtures and remove global state #42
Conversation
- Create tests/conftest.py with pytest fixtures for automatic cleanup - Add output_dir fixture using tmp_path with conditional cleanup (preserves output on test failure for debugging) - Add foxes_engine fixture to replace global engine state - Add session-scoped pre-cleanup to remove leftover output directories - Register 'slow' marker for long-running tests - Update test_foxes.py to use fixtures instead of global engine - Update test_pywake.py to use output_dir fixture - Update test_cs.py to use output_dir fixture - Update test_wayve.py to use output_dir fixture and mark as slow Tests now properly clean up after themselves while preserving output on failure for debugging purposes.
bjarketol
force-pushed
the
fix-tests-cleanup
branch
from
c1c7882 to
5e069dc
Compare
|
@SchmJo can you review this? or what is the process? |
|
I am not entirely sure why you are introducing conftest.py, which looks like a complication. Was the testing not running fine originally? Usually tests run as part of CI/CD , ie., github actions, and the leftover folders don't matter. That being said, I am of course happy with cleaning up after tests. I also don't understand your special treatment of foxes engines, they should not matter for tests. My feeling is that I am missing something. Also, I do not feel comfortable reviewing the pywake part, I guess you need to catch Julian for this. The foxes part I can do, once I understand the main idea! |
|
@SchmJo The main goals are:
I am happy to skip the foxes engine resetting, that was a minor thing. |
* Ensure run_pywake correctly uses output_dir argument
|
@SchmJo I removed the foxes engine resetting and the engine argument to the tests. Can you see if you are happy with the rest? |
|
@kilojoules can you review the pywake part? |
SchmJo
left a comment
SchmJo
left a comment
There was a problem hiding this comment.
Thanks for your work on WIFA, I am still not sure why you are so interested in the tests - are they relevant for your own workflow?
tests/conftest.py
Outdated
| Provides: | ||
| - Pre-test cleanup of leftover output directories | ||
| - Output directory fixtures with conditional cleanup (preserved on failure) | ||
| - FOXES engine fixture with proper initialization and teardown |
There was a problem hiding this comment.
I think we don't need the engine part
tests/test_foxes.py
Outdated
| reset_engine() | ||
| engine = None | ||
| raise e | ||
| run_foxes(yaml_input, output_dir=output_dir, engine=None) |
There was a problem hiding this comment.
Completely remove "engine=None" part
Remove unused reset_engine import fallback from conftest, remove _noXYgrid filter and redundant engine=None arg in test_foxes, and remove if __name__ blocks from test modules. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix duplicate key in analysis_US.yaml (run_partition -> mesh_partition) and fix isort blank line in conftest.py. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@SchmJo @kilojoules can you take a look at this again? If you prefer the old style of having explicit folders, let me know, then perhaps I can just add a cleaning step to each test instead? |
SchmJo
left a comment
SchmJo
left a comment
There was a problem hiding this comment.
The foxes part looks fine for me
2 participants
Summary
Closes #41
@pytest.mark.slowProblem
Tests were leaving behind output directories (
output_pywake_4wts/,output_test_foxes/, etc.) that accumulated over multiple runs.Solution
Created
tests/conftest.pywith:output_dirfixture - Provides unique temp directory per test, cleans up on success, preserves on failureChanges
tests/conftest.pytests/test_foxes.pytests/test_pywake.pytests/test_cs.pytests/test_wayve.pyTest plan
uv run python -m pytest tests/ -m "not slow"- all 33 tests passoutput_*directories remain after test run