Skip to content

Commit 3066177

Browse files
sbryngelsonclaude
andcommitted
Fix 3 issues from AI review: timeout after restart, stale case.py, float consistency
- Use common.delete_file instead of raw os.remove for intermediate step cleanup (prevents filesystem errors from crashing a passing test) - Guard finally block's create_directory so it cannot mask the original exception on failure - Use direct case.restart_check instead of getattr with default (fail loudly if attribute is removed rather than silently skipping tests) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6f62db5 commit 3066177

2 files changed

Lines changed: 6 additions & 3 deletions

File tree

toolchain/mfc/test/case.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,12 +176,15 @@ def run_restart(self, targets, gpus):
176176
d_dir = os.path.join(dirpath, "D")
177177
mid_tag = f"{mid_step:06d}"
178178
for f in glob.glob(os.path.join(d_dir, f"*.{mid_tag}.dat")):
179-
os.remove(f)
179+
common.delete_file(f)
180180

181181
return result2
182182
finally:
183183
self.params = orig
184-
self.create_directory()
184+
try:
185+
self.create_directory()
186+
except Exception:
187+
pass
185188

186189
def get_trace(self) -> str:
187190
return self.trace

toolchain/mfc/test/test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ def _handle_case(case: TestCase, devices: typing.Set[int]):
386386

387387
# Restart roundtrip verification: run to midpoint, restart,
388388
# and compare restarted output against the straight run.
389-
if getattr(case, 'restart_check', False) and not ARG("add_new_variables"):
389+
if case.restart_check and not ARG("add_new_variables"):
390390
straight_pack = pack
391391

392392
if timeout_flag.is_set():

0 commit comments

Comments
 (0)