[FIX-#248] Limit the included node logs of pack-logs command.

scenario_player/main.py

@@ -10,7 +10,6 @@
 from enum import Enum
 from itertools import chain
 from pathlib import Path
-from typing import List
 
 import click
 import gevent
@@ -36,6 +35,11 @@
     post_task_state_to_rc,
     send_notification_mail,
 )
+from scenario_player.utils.logs import (
+    pack_n_latest_logs_for_scenario_in_dir,
+    pack_n_latest_node_logs_in_dir,
+    verify_scenario_log_dir,
+)
 
 log = structlog.get_logger(__name__)
 
@@ -265,29 +269,22 @@ def reclaim_eth(ctx, min_age, password, keystore_file):
     "Specifying 0 will pack all available logs for a scenario.",
 )
 @click.option("--post-to-rocket/--no-post-to-rocket", default=True)
-@click.argument("scenario-file", type=click.File(), required=True)
+@click.argument("scenario-file", type=click.Path(exists=True, dir_okay=False), required=True)
 @click.pass_context
 def pack_logs(ctx, scenario_file, post_to_rocket, pack_n_latest, target_dir):
     data_path: Path = ctx.obj["data_path"].absolute()
     scenario_file = Path(scenario_file.name).absolute()
     scenario_name = Path(scenario_file.name).stem
+
     log_file_name = construct_log_file_name("pack-logs", data_path, scenario_file)
     configure_logging_for_subcommand(log_file_name)
 
-    target_dir = Path(target_dir)
-    target_dir.mkdir(exist_ok=True)
-
     # The logs are located at .raiden/scenario-player/scenarios/<scenario-name>
     # - make sure the path exists.
-    scenarios_path = data_path.joinpath("scenarios")
-    scenario_log_dir = scenarios_path.joinpath(scenario_name)
-    if not scenario_log_dir.exists():
-        print(f"No log directory found for scenario {scenario_name} at {scenario_log_dir}")
-        return
-
-    # List all folders
-    folders = [path for path in scenario_log_dir.iterdir() if path.is_dir()]
+    scenarios_path, scenario_log_dir = verify_scenario_log_dir(scenario_name, data_path)
 
+    # List all node folders which fall into the range of pack_n_latest
+    folders = pack_n_latest_node_logs_in_dir(scenario_log_dir, pack_n_latest)
     # List all files that match the filters `scenario_name` and the `pack_n_latest` counter.
     files = pack_n_latest_logs_for_scenario_in_dir(scenario_name, scenario_log_dir, pack_n_latest)
 
@@ -319,31 +316,6 @@ def pack_logs(ctx, scenario_file, post_to_rocket, pack_n_latest, target_dir):
         post_to_rocket_chat(archive_fpath, **rc_message)
 
 
-def pack_n_latest_logs_for_scenario_in_dir(scenario_name, scenario_log_dir: Path, n) -> List[Path]:
-    """ Add the `n` latest log files for ``scenario_name`` in ``scenario_dir`` to a :cls:``set``
-        and return it.
-    """
-    scenario_logs = [
-        path for path in scenario_log_dir.iterdir() if (path.is_file() and "-run_" in path.name)
-    ]
-    history = sorted(scenario_logs, key=lambda x: x.stat().st_mtime, reverse=True)
-
-    # Can't pack more than the number of available logs.
-    num_of_packable_iterations = n or len(scenario_logs)
-
-    if not history:
-        raise RuntimeError(f"No Scenario logs found in {scenario_log_dir}")
-
-    if num_of_packable_iterations < n:
-        # We ran out of scenario logs to add before reaching the requested number of n latest logs.
-        print(
-            f"Only packing {num_of_packable_iterations} logs of requested latest {n} "
-            f"- no more logs found for {scenario_name}!"
-        )
-
-    return history[:num_of_packable_iterations]
-
-
 def construct_rc_message(base_dir, packed_log, log_fpath) -> str:
     """Check the result of the log file at the given `log_fpath`."""
     result = None

scenario_player/utils/logs.py

@@ -0,0 +1,70 @@
+from pathlib import Path
+from typing import List
+
+
+def pack_n_latest_node_logs_in_dir(scenario_dir: Path, n: int) -> List[Path]:
+    """Return the node log folder paths for the `n` last runs."""
+    if n == 0:
+        return []
+    # Get the number of runs that have been conducted
+    run_num_file = scenario_dir.joinpath("run_num.txt")
+    latest_run = 0
+    if run_num_file.exists():
+        latest_run = int(run_num_file.read_text())
+
+    # Run count starts at 0
+    num_of_runs = latest_run + 1
+
+    # Avoid negative indices.
+    earliest_run_to_pack = max(num_of_runs - n, 0)
+
+    folders = []
+    for run_num in range(earliest_run_to_pack, num_of_runs):
+        for path in scenario_dir.iterdir():
+            if not path.is_dir() or not path.name.startswith(f"node_{run_num}_"):
+                continue
+            folders.append(path)
+
+    return folders
+
+
+def pack_n_latest_logs_for_scenario_in_dir(scenario_name, scenario_log_dir: Path, n) -> List[Path]:
+    """ List the `n` newest scenario log files in the given `scenario_log_dir`."""
+    if n == 0:
+        return []
+    # Get all scenario run logs, sort and reverse them (newest first)
+    scenario_logs = [
+        path for path in scenario_log_dir.iterdir() if (path.is_file() and "-run_" in path.name)
+    ]
+    history = sorted(scenario_logs, reverse=True)
+
+    # Can't pack more than the number of available logs.
+    num_of_packable_iterations = min(n, len(scenario_logs))
+    print(scenario_logs)
+    print(n, len(scenario_logs), num_of_packable_iterations)
+
+    if not history:
+        raise RuntimeError(f"No Scenario logs found in {scenario_log_dir}")
+
+    if num_of_packable_iterations < n:
+        # We ran out of scenario logs to add before reaching the requested number of n latest logs.
+        print(
+            f"Only packing {num_of_packable_iterations} logs of requested latest {n} "
+            f"- no more logs found for {scenario_name}!"
+        )
+
+    return history[:num_of_packable_iterations]
+
+
+def verify_scenario_log_dir(scenario_name, data_path: Path):
+    # The logs are located at .raiden/scenario-player/scenarios/<scenario-name>
+    # - make sure the path exists.
+    scenarios_dir = data_path.joinpath("scenarios")
+    scenario_log_dir = scenarios_dir.joinpath(scenario_name)
+    if not scenario_log_dir.exists():
+        raise FileNotFoundError(
+            f"No log directory found for scenario {scenario_name} at {scenario_log_dir}"
+        )
+    if not scenario_log_dir.is_dir():
+        raise NotADirectoryError(f"Scenario Log path {scenario_log_dir} is not a directory!")
+    return scenarios_dir, scenario_log_dir

tests/unittests/utils/test_logs.py

@@ -0,0 +1,101 @@
+import pytest
+
+from scenario_player.utils.logs import (
+    pack_n_latest_logs_for_scenario_in_dir,
+    pack_n_latest_node_logs_in_dir,
+    verify_scenario_log_dir,
+)
+
+
+
+@pytest.fixture
+def scenario_dir(tmp_path):
+    scenario_dir = tmp_path.joinpath("test_scenario")
+    scenario_dir.mkdir()
+    return scenario_dir
+
+
+@pytest.fixture(autouse=True)
+def faked_scenario_dir(scenario_dir):
+
+    # Create 10 fake scenario run logs, and 3 fake node folders per run
+    for n in range(10):
+        scenario_dir.joinpath(f"{scenario_dir.stem}-run_2000-08-{n}.log").touch()
+        scenario_dir.joinpath(f"node_{n}_001").mkdir()
+        scenario_dir.joinpath(f"node_{n}_002").mkdir()
+        scenario_dir.joinpath(f"node_{n}_003").mkdir()
+
+    # Create the run_num text file.
+    run_num_file = scenario_dir.joinpath("run_num.txt")
+    run_num_file.touch()
+    run_num_file.write_text("9")
+
+
+class TestPackNLatestNodeLogsInDir:
+    @pytest.mark.parametrize(
+        "given, expected",
+        argvalues=[(0, 0), (100, 30), (5, 15), (10, 30)],
+        ids=[
+            "n=0 returns empty list",
+            "n>current run number returns all available",
+            "n<run num returns n paths",
+            "n==run num returns n",
+        ],
+    )
+    def test_func_returns_expected_number_of_paths(self, given, expected, scenario_dir):
+        result = pack_n_latest_node_logs_in_dir(scenario_dir, given)
+        assert len(result) == expected
+
+    def test_func_returns_directories_only(self, scenario_dir):
+        result = pack_n_latest_node_logs_in_dir(scenario_dir, 10)
+        assert all(path.is_dir() for path in result)
+
+
+class TestPackNLatestLogsForScenarioInDir:
+    @pytest.mark.parametrize(
+        "given, expected",
+        argvalues=[(0, 0), (100, 10), (5, 5), (10, 10)],
+        ids=[
+            "n=0 returns empty list",
+            "n>current run number returns all available",
+            "n<run num returns n paths",
+            "n==run num returns n",
+        ],
+    )
+    def test_func_returns_expected_number_of_files(self, given, expected, scenario_dir):
+        result = pack_n_latest_logs_for_scenario_in_dir(scenario_dir.name, scenario_dir, given)
+        assert len(result) == expected
+
+    def test_func_returns_files_only(self, scenario_dir):
+        result = pack_n_latest_logs_for_scenario_in_dir(scenario_dir.name, scenario_dir, 10)
+        assert all(path.is_file() for path in result)
+
+    def test_func_raises_runtimeerror_if_no_logs_are_found(self, scenario_dir):
+        # remove created log files.
+        for path in scenario_dir.iterdir():
+            if path.is_file():
+                path.unlink()
+
+        with pytest.raises(RuntimeError):
+            pack_n_latest_logs_for_scenario_in_dir(scenario_dir.name, scenario_dir, 1)
+
+
+class TestVerifyScenarioLogDir:
+    def test_func_raises_not_a_directory_if_scenario_log_dir_is_not_a_directory(self, tmp_path):
+        f = tmp_path.joinpath("scenarios")
+        f.mkdir(parents=True)
+        f= f.joinpath("wolohoo")
+        f.write_text("something")
+        with pytest.raises(NotADirectoryError):
+            verify_scenario_log_dir("wolohoo", tmp_path)
+
+    def test_func_raises_file_not_found_if_log_dir_does_not_exist(self, tmp_path):
+        with pytest.raises(FileNotFoundError):
+            verify_scenario_log_dir("wolohoo", tmp_path)
+
+    def test_func_returns_exepcted_paths(self, tmp_path):
+        f = tmp_path.joinpath("scenarios", "test_scenario")
+        f.mkdir(parents=True)
+        scenarios_dir, log_dir = verify_scenario_log_dir("test_scenario", tmp_path)
+        assert scenarios_dir == tmp_path.joinpath("scenarios")
+        assert log_dir == tmp_path.joinpath("scenarios", "test_scenario")