Add test cleanup fixtures and remove global state#42
Add test cleanup fixtures and remove global state#42bjarketol wants to merge 9 commits intoEUFLOW:mainfrom
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
There was a problem hiding this comment.
engine.initialize() was actually removed in foxes v1.7.0
There was a problem hiding this comment.
Maybe just leave out the mentioning of the engine alltogether, then it should always fall back to using a default (also for older versions)
There was a problem hiding this comment.
Good point. I changed it so it only resets state.
|
@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?
|
|
||
| import pytest | ||
|
|
||
| # Handle different foxes versions - reset_engine location varies |
There was a problem hiding this comment.
Can we remove the engine related parts?
| 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
| print("SETTING ENGINE:", engine) | ||
|
|
||
| for yaml_input in wes_dir.glob("system.yaml"): | ||
| if "_noXYgrid" not in str(yaml_input): |
There was a problem hiding this comment.
This "_noXYgrid" part is deprecated, this "if" clause can be removed
| 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
| test_foxes_simple_wind_rose() | ||
| import pytest | ||
|
|
||
| pytest.main([__file__, "-v"]) |
There was a problem hiding this comment.
Why do you need these two last lines? Usually the tests are run via "pytest tests" or similar, see github workflow
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