diff --git a/src/virtualship/cli/_run.py b/src/virtualship/cli/_run.py index 2feb43a3..f2622be3 100644 --- a/src/virtualship/cli/_run.py +++ b/src/virtualship/cli/_run.py @@ -255,36 +255,35 @@ def _unique_id(expedition: Expedition, cache_dir: Path) -> str: """ Return a unique id for the expedition (marked by datetime), which can be used to determine whether the expedition has 'changed' since the last run. - Simultaneously, log to .txt file if first run or if there have been additions of instruments since last run. + Returns the previous id if no instruments have been added since the last run, allowing re-use of previously encountered problems. + Otherwise generates and persists a new id. """ - current_instruments = expedition.get_instruments() - new_id = datetime.now().strftime("%Y%m%d%H%M%S") - previous_id = None + cache_dir.mkdir(exist_ok=True) - if not cache_dir.exists(): - cache_dir.mkdir() id_path = cache_dir.joinpath(EXPEDITION_IDENTIFIER) last_expedition_path = cache_dir.joinpath(EXPEDITION_LATEST) + new_id = datetime.now().strftime("%Y%m%d%H%M%S") - if id_path.exists(): - previous_id = id_path.read_text().strip() - try: - last_expedition = Expedition.from_yaml(last_expedition_path) - except FileNotFoundError as e: - raise RuntimeError( - f"Previous expedition data is present but incomplete in {cache_dir}. This may be because a previous expedition run was interrupted/failed unexpectedly. Deleting the '{cache_dir}' directory and re-running the expedition should resolve this issue." - ) from e - last_instruments = last_expedition.get_instruments() + if not id_path.exists(): + id_path.write_text(new_id) + return new_id - added_instruments = set(current_instruments) - set(last_instruments) - if not added_instruments: - return previous_id # if no additions, keep previous id to allow re-use of previously encountered problems - else: - id_path.write_text(new_id) + previous_id = id_path.read_text().strip() - else: + try: + last_expedition = Expedition.from_yaml(last_expedition_path) + except FileNotFoundError: + # cache is not useful in this case as it implies the previous run was interrupted and is incomplete; update passively id_path.write_text(new_id) + return new_id + + added_instruments = set(expedition.get_instruments()) - set( + last_expedition.get_instruments() + ) + if not added_instruments: + return previous_id # if no additions, keep previous id to allow re-use of previously encountered problems + id_path.write_text(new_id) return new_id diff --git a/tests/cli/test_run.py b/tests/cli/test_run.py index bbdc2fa9..b537e405 100644 --- a/tests/cli/test_run.py +++ b/tests/cli/test_run.py @@ -1,12 +1,14 @@ from datetime import datetime from pathlib import Path +from unittest.mock import MagicMock -from virtualship.cli._run import _run +from virtualship.cli._run import _run, _unique_id from virtualship.expedition.simulate_schedule import ( MeasurementsToSimulate, ScheduleOk, ) -from virtualship.utils import EXPEDITION, get_example_expedition +from virtualship.instruments.types import InstrumentType +from virtualship.utils import EXPEDITION, EXPEDITION_IDENTIFIER, get_example_expedition def _simulate_schedule(projection, expedition): @@ -71,3 +73,24 @@ def test_run(tmp_path, monkeypatch): if difficulty_level == "easy": cache_dir = expedition_dir / "cache" assert not cache_dir.exists() + + +def test_unique_id_incomplete_cache_does_not_raise(tmp_path): + """When expedition_latest.yaml is missing (incomplete cache from an interrupted run), _unique_id must not raise error for any 'difficulty_level', it should return a new id.""" + ORIGINAL_TIMESTAMP = "20240101120000" + + cache_dir = tmp_path / "cache" + cache_dir.mkdir() + (cache_dir / EXPEDITION_IDENTIFIER).write_text(ORIGINAL_TIMESTAMP) + # EXPEDITION_LATEST intentionally absent to simulate an interrupted run + + expedition = MagicMock() + expedition.get_instruments.return_value = {InstrumentType.CTD} + + result = _unique_id(expedition, cache_dir) + + assert result != ORIGINAL_TIMESTAMP + assert ( + abs(datetime.strptime(result, "%Y%m%d%H%M%S") - datetime.now()).seconds < 30 + ), "new ID should be timestamp close to the current time" + assert (cache_dir / EXPEDITION_IDENTIFIER).read_text().strip() == result