Skip to content

Commit

Permalink
BF(TST)+RF: make execution_summary robust to case where Report is not…
Browse files Browse the repository at this point in the history
… yet complete
  • Loading branch information
yarikoptic committed Aug 15, 2024
1 parent bec292e commit 8b8186f
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 9 deletions.
21 changes: 15 additions & 6 deletions src/con_duct/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from enum import Enum
from functools import cached_property
import json
import math
import os
import shutil
import subprocess
Expand Down Expand Up @@ -268,6 +269,7 @@ def __init__(
log_paths: LogPaths,
summary_format: str,
clobber: bool = False,
process: subprocess.Popen | None = None,
) -> None:
self._command = command
self.arguments = arguments
Expand All @@ -276,7 +278,7 @@ def __init__(
self.clobber = clobber
# Defaults to be set later
self.start_time = None
self.process: subprocess.Popen | None = None
self.process = process
self.session_id: int | None = None
self.gpus: list[dict[str, str]] | None = None
self.env: dict[str, str] | None = None
Expand All @@ -298,9 +300,14 @@ def elapsed_time(self) -> float:
return time.time() - self.start_time

@property
def wall_clock_time(self) -> float:
assert self.start_time
assert self.end_time
def wall_clock_time(self) -> Optional[float]:
if self.start_time is None:
return math.nan
if self.end_time is None:
# if no end_time -- must be still ongoing
# Cannot happen ATM but could in "library mode" later
return time.time() - self.start_time

Check warning on line 309 in src/con_duct/__main__.py

View check run for this annotation

Codecov / codecov/patch

src/con_duct/__main__.py#L309

Added line #L309 was not covered by tests
# we reached the end
return self.end_time - self.start_time

def collect_environment(self) -> None:
Expand Down Expand Up @@ -378,10 +385,12 @@ def write_subreport(self) -> None:

@cached_property
def execution_summary(self) -> dict[str, Any]:
# prepare the base, but enrich if we did get process
# running
return {
"exit_code": self.process.returncode,
"exit_code": self.process.returncode if self.process else None,
"command": self.command,
"logs_prefix": self.log_paths.prefix,
"logs_prefix": self.log_paths.prefix if self.log_paths else "",
"wall_clock_time": self.wall_clock_time,
"peak_rss": self.max_values.total_rss,
"average_rss": self.averages.rss,
Expand Down
10 changes: 7 additions & 3 deletions test/test_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,14 @@ def test_execution_summary_formatted(
mock_popen: mock.MagicMock, mock_log_paths: mock.MagicMock
) -> None:
mock_log_paths.prefix = "mock_prefix"
report = Report(
"_cmd", [], None, mock_popen, mock_log_paths, EXECUTION_SUMMARY_FORMAT, False
)
report = Report("_cmd", [], None, EXECUTION_SUMMARY_FORMAT, clobber=False)
# It should not crash and it would render even where no wallclock time yet
assert "wall clock time: nan" in report.execution_summary_formatted.lower()

# Test with process
report.process = mock_popen
output = report.execution_summary_formatted
assert "None" not in output
assert "unknown" in output
# Process did not finish, we didn't set start_time, so remains nan but there
assert "wall clock time: nan" in report.execution_summary_formatted.lower()

0 comments on commit 8b8186f

Please sign in to comment.