-
Notifications
You must be signed in to change notification settings - Fork 4
Add test cleanup fixtures and remove global state #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3ab28ff
dc2da37
e78313c
1fa81e1
e1d9b02
5e069dc
6f91354
2906526
d535903
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| """ | ||
| Pytest configuration and fixtures for WIFA tests. | ||
|
|
||
| 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we don't need the engine part |
||
| """ | ||
|
|
||
| import shutil | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| # Handle different foxes versions - reset_engine location varies | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we remove the engine related parts? |
||
| try: | ||
| from foxes import reset_engine | ||
| except ImportError: | ||
| try: | ||
| from foxes.core import reset_engine | ||
| except ImportError: | ||
| try: | ||
| from foxes.core.engine import reset_engine | ||
| except ImportError: | ||
| # Older foxes versions don't have reset_engine - use no-op | ||
| def reset_engine(): | ||
| pass | ||
|
|
||
|
|
||
| # Store test outcomes for conditional cleanup | ||
| _test_outcomes = {} | ||
|
|
||
|
|
||
| @pytest.hookimpl(hookwrapper=True) | ||
| def pytest_runtest_makereport(item, call): | ||
| """Track test outcomes to conditionally preserve output on failure.""" | ||
| outcome = yield | ||
| report = outcome.get_result() | ||
| if report.when == "call": | ||
| _test_outcomes[item.nodeid] = report.failed | ||
|
|
||
|
|
||
| def pytest_configure(config): | ||
| """Register custom markers.""" | ||
| config.addinivalue_line( | ||
| "markers", "slow: marks tests as slow (deselect with '-m \"not slow\"')" | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session", autouse=True) | ||
| def cleanup_old_outputs(): | ||
| """Remove output directories from previous test runs at session start.""" | ||
| patterns = [ | ||
| "output_pywake_*", | ||
| "output_test_*", | ||
| "output", | ||
| ] | ||
| for pattern in patterns: | ||
| for path in Path(".").glob(pattern): | ||
| if path.is_dir(): | ||
| shutil.rmtree(path) | ||
| elif path.is_file(): | ||
| path.unlink() | ||
| yield | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def output_dir(request, tmp_path): | ||
| """ | ||
| Provide a unique temporary output directory for tests. | ||
|
|
||
| Cleans up automatically on test success, preserves on failure for debugging. | ||
| """ | ||
| yield tmp_path | ||
|
|
||
| # Only clean up if test passed | ||
| test_failed = _test_outcomes.get(request.node.nodeid, False) | ||
| if not test_failed: | ||
| if tmp_path.exists(): | ||
| shutil.rmtree(tmp_path) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def named_output_dir(request): | ||
| """ | ||
| Provide a named output directory based on test name. | ||
|
|
||
| Use this when tests need output in the current working directory | ||
| (e.g., for Code Saturne integration). | ||
|
|
||
| Cleans up automatically on test success, preserves on failure. | ||
| """ | ||
| test_name = request.node.name.replace("[", "_").replace("]", "_").rstrip("_") | ||
| output_path = Path(f"output_test_{test_name}") | ||
| output_path.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| yield output_path | ||
|
|
||
| # Only clean up if test passed | ||
| test_failed = _test_outcomes.get(request.node.nodeid, False) | ||
| if not test_failed: | ||
| if output_path.exists(): | ||
| shutil.rmtree(output_path) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def cleanup_output_dir(request): | ||
| """ | ||
| Cleanup fixture for tests that write to the default 'output/' directory. | ||
|
|
||
| Use this when tests can't control their output location (e.g., when YAML | ||
| specifies output_folder). Cleans up on success, preserves on failure. | ||
| """ | ||
| output_path = Path("output") | ||
|
|
||
| yield output_path | ||
|
|
||
| # Only clean up if test passed | ||
| test_failed = _test_outcomes.get(request.node.nodeid, False) | ||
| if not test_failed: | ||
| if output_path.exists(): | ||
| shutil.rmtree(output_path) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| import os | ||
| from pathlib import Path | ||
| from shutil import rmtree | ||
|
|
||
| from foxes import Engine, reset_engine | ||
| from windIO import __path__ as wiop | ||
| from windIO import validate as validate_yaml | ||
|
|
||
|
|
@@ -10,96 +10,77 @@ | |
| test_path = Path(os.path.dirname(__file__)) | ||
| windIO_path = Path(wiop[0]) | ||
|
|
||
| engine = None | ||
|
|
||
|
|
||
| def _run_foxes(wes_dir): | ||
| def _run_foxes(wes_dir, output_dir): | ||
| """Run FOXES on all system.yaml files in the given directory.""" | ||
| assert wes_dir.is_dir(), f"{wes_dir} is not a directory" | ||
|
|
||
| global engine | ||
| if engine is None: | ||
| engine = Engine.new("default", verbosity=0) | ||
| engine.initialize() | ||
| print("SETTING ENGINE:", engine) | ||
|
|
||
| for yaml_input in wes_dir.glob("system.yaml"): | ||
| if "_noXYgrid" not in str(yaml_input): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This "_noXYgrid" part is deprecated, this "if" clause can be removed |
||
| print("\nRUNNING FOXES ON", yaml_input, "\n") | ||
| validate_yaml(yaml_input, Path("plant/wind_energy_system")) | ||
| output_dir_name = Path("output_test_foxes") | ||
| output_dir_name.mkdir(parents=True, exist_ok=True) | ||
| try: | ||
| run_foxes(yaml_input, output_dir=output_dir_name, engine=None) | ||
| except Exception as e: | ||
| reset_engine() | ||
| engine = None | ||
| raise e | ||
| run_foxes(yaml_input, output_dir=output_dir, engine=None) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Completely remove "engine=None" part |
||
|
|
||
|
|
||
| def test_foxes_KUL(): | ||
| def test_foxes_KUL(output_dir): | ||
| wes_dir = test_path / "../examples/cases/KUL_LES/wind_energy_system/" | ||
| _run_foxes(wes_dir) | ||
| _run_foxes(wes_dir, output_dir) | ||
|
|
||
|
|
||
| def test_foxes_4wts(): | ||
| def test_foxes_4wts(output_dir): | ||
| wes_dir = test_path / "../examples/cases/windio_4turbines/wind_energy_system/" | ||
| _run_foxes(wes_dir) | ||
| _run_foxes(wes_dir, output_dir) | ||
|
|
||
|
|
||
| def test_foxes_abl(): | ||
| def test_foxes_abl(output_dir): | ||
| wes_dir = test_path / "../examples/cases/windio_4turbines_ABL/wind_energy_system/" | ||
| _run_foxes(wes_dir) | ||
| _run_foxes(wes_dir, output_dir) | ||
|
|
||
|
|
||
| def test_foxes_abl_stable(): | ||
| def test_foxes_abl_stable(output_dir): | ||
| wes_dir = ( | ||
| test_path / "../examples/cases/windio_4turbines_ABL_stable/wind_energy_system/" | ||
| ) | ||
| _run_foxes(wes_dir) | ||
| _run_foxes(wes_dir, output_dir) | ||
|
|
||
|
|
||
| def test_foxes_profiles(): | ||
| def test_foxes_profiles(output_dir): | ||
| wes_dir = ( | ||
| test_path | ||
| / "../examples/cases/windio_4turbines_profiles_stable/wind_energy_system/" | ||
| ) | ||
| _run_foxes(wes_dir) | ||
| _run_foxes(wes_dir, output_dir) | ||
|
|
||
|
|
||
| def test_foxes_heterogeneous_wind_rose_at_turbines(): | ||
| def test_foxes_heterogeneous_wind_rose_at_turbines(output_dir): | ||
| wes_dir = ( | ||
| test_path | ||
| / "../examples/cases/heterogeneous_wind_rose_at_turbines/wind_energy_system/" | ||
| ) | ||
| _run_foxes(wes_dir) | ||
| _run_foxes(wes_dir, output_dir) | ||
|
|
||
|
|
||
| def test_foxes_heterogeneous_wind_rose_map(): | ||
| def test_foxes_heterogeneous_wind_rose_map(output_dir): | ||
| wes_dir = ( | ||
| test_path / "../examples/cases/heterogeneous_wind_rose_map/wind_energy_system/" | ||
| ) | ||
| _run_foxes(wes_dir) | ||
| _run_foxes(wes_dir, output_dir) | ||
|
|
||
|
|
||
| def test_foxes_simple_wind_rose(): | ||
| def test_foxes_simple_wind_rose(output_dir): | ||
| wes_dir = test_path / "../examples/cases/simple_wind_rose/wind_energy_system/" | ||
| _run_foxes(wes_dir) | ||
| _run_foxes(wes_dir, output_dir) | ||
|
|
||
|
|
||
| def test_foxes_timeseries_with_operating_flag(): | ||
| def test_foxes_timeseries_with_operating_flag(output_dir): | ||
| wes_dir = ( | ||
| test_path | ||
| / "../examples/cases/timeseries_with_operating_flag/wind_energy_system/" | ||
| ) | ||
| _run_foxes(wes_dir) | ||
| _run_foxes(wes_dir, output_dir) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| test_foxes_KUL() | ||
| test_foxes_4wts() | ||
| test_foxes_abl() | ||
| test_foxes_abl_stable() | ||
| test_foxes_profiles() | ||
| test_foxes_heterogeneous_wind_rose_at_turbines() | ||
| test_foxes_heterogeneous_wind_rose_map() | ||
| test_foxes_simple_wind_rose() | ||
| import pytest | ||
|
|
||
| pytest.main([__file__, "-v"]) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need these two last lines? Usually the tests are run via "pytest tests" or similar, see github workflow |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
engine.initialize()was actually removed in foxes v1.7.0There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I changed it so it only resets state.