Skip to content

Commit 26ebfda

Browse files
author
Al Rigazzi
committed
Fix lingering output files in test_symlinking and test_output_files
- Enhanced symlink_output_files to auto-create parent directories - Fixed path handling for entities with sub-entities (Orchestrator/Ensemble) - Ensured all tests use proper test directories instead of repo root - Removed unused CONFIG imports - All tests now pass without creating lingering files in repo root
1 parent 4908c50 commit 26ebfda

3 files changed

Lines changed: 88 additions & 40 deletions

File tree

smartsim/_core/control/controller.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,10 @@ def symlink_output_files(
373373
historical_err.touch()
374374
historical_out.touch()
375375

376+
# Ensure the entity directory exists for symlinks
377+
entity_out.parent.mkdir(parents=True, exist_ok=True)
378+
entity_err.parent.mkdir(parents=True, exist_ok=True)
379+
376380
if historical_err.exists() and historical_out.exists():
377381
entity_out.symlink_to(historical_out)
378382
entity_err.symlink_to(historical_err)

tests/test_output_files.py

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import pytest
3131

3232
from smartsim import Experiment
33-
from smartsim._core.config import CONFIG
3433
from smartsim._core.control.controller import Controller, _AnonymousBatchJob
3534
from smartsim._core.launcher.step import Step
3635
from smartsim.database.orchestrator import Orchestrator
@@ -106,35 +105,45 @@ def test_mutated_model_output(test_dir):
106105
def test_get_output_files_with_create_job_step(test_dir):
107106
"""Testing output files through _create_job_step"""
108107
exp_dir = pathlib.Path(test_dir)
109-
status_dir = exp_dir / ".smartsim"
110-
# Set the model path to the test directory
111-
model.path = test_dir
112-
step = controller._create_job_step(model)
113-
expected_out_path = status_dir / (model.name + ".out")
114-
expected_err_path = status_dir / (model.name + ".err")
108+
# Create a fresh model instance for this test
109+
test_model = Model("test_model", params={}, path=test_dir, run_settings=rs)
110+
# Create run_dir to avoid using current working directory
111+
run_dir = exp_dir / ".smartsim" / "run_test"
112+
step = controller._create_job_step(test_model, run_dir)
113+
expected_out_path = run_dir / (test_model.name + ".out")
114+
expected_err_path = run_dir / (test_model.name + ".err")
115115
assert step.get_output_files() == (str(expected_out_path), str(expected_err_path))
116116

117117

118118
@pytest.mark.parametrize(
119-
"entity",
120-
[pytest.param(ens, id="ensemble"), pytest.param(orc, id="orchestrator")],
119+
"entity_type",
120+
[
121+
pytest.param("ensemble", id="ensemble"),
122+
pytest.param("orchestrator", id="orchestrator"),
123+
],
121124
)
122-
def test_get_output_files_with_create_batch_job_step(entity, test_dir):
125+
def test_get_output_files_with_create_batch_job_step(entity_type, test_dir):
123126
"""Testing output files through _create_batch_job_step"""
124127
exp_dir = pathlib.Path(test_dir)
125-
# Set the entity path to test_dir
126-
entity.path = test_dir
127-
batch_step, substeps = slurm_controller._create_batch_job_step(entity)
128-
for step in substeps:
129-
# With the new simplified structure, each step should use its own entity's path
130-
# Each entity member has their own individual path, so the output goes in their own .smartsim directory
131-
step_entity_path = pathlib.Path(step.meta["status_dir"]).parent
132-
expected_out_path = pathlib.Path(step.meta["status_dir"]) / (
133-
step.entity_name + ".out"
128+
129+
# Create fresh entities for each test to avoid path conflicts
130+
if entity_type == "ensemble":
131+
entity = Ensemble(
132+
"ens", params={}, run_settings=rs, batch_settings=bs, replicas=3
134133
)
135-
expected_err_path = pathlib.Path(step.meta["status_dir"]) / (
136-
step.entity_name + ".err"
134+
else: # orchestrator
135+
entity = Orchestrator(
136+
db_nodes=3, batch=True, launcher="slurm", run_command="srun"
137137
)
138+
139+
entity.path = test_dir
140+
# Create run_dir to avoid using current working directory
141+
run_dir = exp_dir / ".smartsim" / "run_test_batch"
142+
batch_step, substeps = slurm_controller._create_batch_job_step(entity, run_dir)
143+
for step in substeps:
144+
# With timestamped runs, output files should be in the run_dir
145+
expected_out_path = run_dir / (step.entity_name + ".out")
146+
expected_err_path = run_dir / (step.entity_name + ".err")
138147
actual_out, actual_err = step.get_output_files()
139148
assert actual_out == str(expected_out_path)
140149
assert actual_err == str(expected_err_path)

tests/test_symlinking.py

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import pytest
3131

3232
from smartsim import Experiment
33-
from smartsim._core.config import CONFIG
3433
from smartsim._core.control.controller import Controller, _AnonymousBatchJob
3534
from smartsim.database.orchestrator import Orchestrator
3635
from smartsim.entity.ensemble import Ensemble
@@ -58,50 +57,84 @@
5857

5958

6059
@pytest.mark.parametrize(
61-
"entity",
62-
[pytest.param(ens, id="ensemble"), pytest.param(model, id="model")],
60+
"entity_type",
61+
[pytest.param("ensemble", id="ensemble"), pytest.param("model", id="model")],
6362
)
64-
def test_symlink(test_dir, entity):
63+
def test_symlink(test_dir, entity_type):
6564
"""Test symlinking historical output files"""
66-
entity.path = test_dir
67-
if entity.type == Ensemble:
68-
for member in ens.models:
65+
if entity_type == "ensemble":
66+
entity = Ensemble(
67+
"ens", params={}, run_settings=rs, batch_settings=bs, replicas=3
68+
)
69+
entity.path = test_dir
70+
for member in entity.models:
6971
symlink_with_create_job_step(test_dir, member)
7072
else:
73+
entity = Model("test_model", params={}, path=test_dir, run_settings=rs)
7174
symlink_with_create_job_step(test_dir, entity)
7275

7376

7477
def symlink_with_create_job_step(test_dir, entity):
7578
"""Function that helps cut down on repeated testing code"""
7679
exp_dir = pathlib.Path(test_dir)
7780
entity.path = test_dir
78-
# With simplified structure, output files go directly in .smartsim directory
79-
status_dir = exp_dir / ".smartsim"
80-
step = controller._create_job_step(entity)
81+
# Create run_dir to simulate timestamped run structure
82+
run_dir = exp_dir / ".smartsim" / "run_test"
83+
step = controller._create_job_step(entity, run_dir)
8184
controller.symlink_output_files(step, entity)
8285
assert pathlib.Path(entity.path, f"{entity.name}.out").is_symlink()
8386
assert pathlib.Path(entity.path, f"{entity.name}.err").is_symlink()
87+
# Verify symlinks point to the correct run directory
88+
expected_out = run_dir / (entity.name + ".out")
89+
expected_err = run_dir / (entity.name + ".err")
8490
assert os.readlink(pathlib.Path(entity.path, f"{entity.name}.out")) == str(
85-
status_dir / (entity.name + ".out")
91+
expected_out
8692
)
8793
assert os.readlink(pathlib.Path(entity.path, f"{entity.name}.err")) == str(
88-
status_dir / (entity.name + ".err")
94+
expected_err
8995
)
9096

9197

9298
@pytest.mark.parametrize(
93-
"entity",
99+
"entity_type",
94100
[
95-
pytest.param(ens, id="ensemble"),
96-
pytest.param(orc, id="orchestrator"),
97-
pytest.param(anon_batch_model, id="model"),
101+
pytest.param("ensemble", id="ensemble"),
102+
pytest.param("orchestrator", id="orchestrator"),
103+
pytest.param("model", id="model"),
98104
],
99105
)
100-
def test_batch_symlink(entity, test_dir):
106+
def test_batch_symlink(entity_type, test_dir):
101107
"""Test symlinking historical output files"""
102108
exp_dir = pathlib.Path(test_dir)
109+
110+
# Create fresh entities for each test to avoid path conflicts
111+
if entity_type == "ensemble":
112+
entity = Ensemble(
113+
"ens", params={}, run_settings=rs, batch_settings=bs, replicas=3
114+
)
115+
elif entity_type == "orchestrator":
116+
entity = Orchestrator(
117+
db_nodes=3, batch=True, launcher="slurm", run_command="srun"
118+
)
119+
else: # model
120+
batch_model = Model(
121+
"batch_test_model",
122+
params={},
123+
path=test_dir,
124+
run_settings=batch_rs,
125+
batch_settings=bs,
126+
)
127+
entity = _AnonymousBatchJob(batch_model)
128+
103129
entity.path = test_dir
104-
batch_step, substeps = slurm_controller._create_batch_job_step(entity)
130+
# For entities with sub-entities (like Orchestrator), set their paths too
131+
if hasattr(entity, "entities"):
132+
for sub_entity in entity.entities:
133+
sub_entity.path = test_dir
134+
135+
# Create run_dir to simulate timestamped run structure
136+
run_dir = exp_dir / ".smartsim" / "run_test_batch"
137+
batch_step, substeps = slurm_controller._create_batch_job_step(entity, run_dir)
105138

106139
# For batch entities, we need to call symlink_output_files correctly
107140
# Based on how the controller does it, we should pass the individual entities
@@ -148,7 +181,9 @@ def test_symlink_error(test_dir):
148181
path=pathlib.Path(test_dir, "badpath"),
149182
run_settings=RunSettings("echo"),
150183
)
151-
bad_step = controller._create_job_step(bad_model)
184+
# Create run_dir to avoid using current working directory
185+
run_dir = pathlib.Path(test_dir) / ".smartsim" / "run_test_error"
186+
bad_step = controller._create_job_step(bad_model, run_dir)
152187
# The new behavior should auto-create directories and symlinks without errors
153188
controller.symlink_output_files(bad_step, bad_model)
154189

0 commit comments

Comments
 (0)