[HOTFIX-#238] Fix an incompatible return type introduced in #278

scenario_player/main.py

@@ -333,11 +333,10 @@ def reclaim_eth(ctx, min_age, password, password_file, keystore_file):
 @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)
     target_dir = Path(target_dir)
-    scenario_file = Path(scenario_file.name).absolute()
-    scenario_name = Path(scenario_file.name).stem
+    data_path: Path = ctx.obj["data_path"].absolute()
+    scenario_file = Path(scenario_file).absolute()
+    scenario_name = Path(scenario_file).stem
 
     log_file_name = construct_log_file_name("pack-logs", data_path, scenario_file)
     configure_logging_for_subcommand(log_file_name)

scenario_player/utils/logs.py

@@ -7,7 +7,7 @@ def pack_n_latest_node_logs_in_dir(scenario_dir: Path, n: int) -> List[Path]:
     if n == 0:
         return []
     # Get the number of runs that have been conducted
-    run_num_file = scenario_dir.joinpath("run_num.txt")
+    run_num_file = scenario_dir.joinpath("run_number.txt")
     latest_run = 0
     if run_num_file.exists():
         latest_run = int(run_num_file.read_text())
@@ -18,12 +18,13 @@ def pack_n_latest_node_logs_in_dir(scenario_dir: Path, n: int) -> List[Path]:
     # Avoid negative indices.
     earliest_run_to_pack = max(num_of_runs - n, 0)
 
+    all_folders = sorted([p for p in scenario_dir.iterdir() if p.is_dir()], reverse=True)
+    node_folders = [p for p in all_folders if p.name.startswith("node_")]
     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)
+        for path in node_folders:
+            if path.name.startswith(f"node_{run_num}"):
+                folders.append(path)
 
     return folders
 
@@ -40,8 +41,6 @@ def pack_n_latest_logs_for_scenario_in_dir(scenario_name, scenario_log_dir: Path
 
     # 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}")

scenario_player/utils/token.py

@@ -1,6 +1,6 @@
 import json
 import pathlib
-from typing import Tuple, Union
+from typing import Optional, Tuple, Union
 
 import structlog
 from eth_utils import decode_hex, to_checksum_address
@@ -328,8 +328,6 @@ def deploy_new(self) -> Tuple[str, int]:
             resp_data["contract"],
             resp_data["deployment_block"],
         )
-        print(token_contract_data)
-        print(deployment_block)
         contract_info = self._local_contract_manager.get_contract("CustomToken")
 
         # Make deployment address and block available to address/deployment_block properties.
@@ -399,7 +397,8 @@ def mint(
             **kwargs,
         )
 
-    def update_allowance(self) -> Union[Tuple[str, int], None]:
+    def update_allowance(self) -> Tuple[Optional[str], int]:
+
         """Update the UD Token Contract allowance depending on the number of configured nodes.
 
         If the UD Token Contract's allowance is sufficient, this is a no-op.
@@ -418,7 +417,7 @@ def update_allowance(self) -> Union[Tuple[str, int], None]:
 
         if not udt_allowance < required_allowance:
             log.debug("UDTC allowance sufficient")
-            return
+            return None, required_allowance
 
         log.debug("UDTC allowance insufficient, updating")
         params = {

tests/unittests/utils/test_logs.py

@@ -26,7 +26,7 @@ def faked_scenario_dir(scenario_dir):
         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 = scenario_dir.joinpath("run_number.txt")
     run_num_file.touch()
     run_num_file.write_text("9")
 
@@ -50,6 +50,13 @@ 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)
 
+    def test_func_returns_expected_paths(self, scenario_dir):
+        all_node_folders = [p for p in scenario_dir.iterdir() if p.is_dir()]
+
+        expected = set(sorted(all_node_folders, reverse=True)[:15])
+
+        assert set(pack_n_latest_node_logs_in_dir(scenario_dir, 5)) == expected
+
 
 class TestPackNLatestLogsForScenarioInDir:
     @pytest.mark.parametrize(
@@ -66,6 +73,13 @@ def test_func_returns_expected_number_of_files(self, given, expected, scenario_d
         result = pack_n_latest_logs_for_scenario_in_dir(scenario_dir.name, scenario_dir, given)
         assert len(result) == expected
 
+    def test_func_returns_expected_files(self, scenario_dir):
+        all_logs = [p for p in scenario_dir.iterdir() if p.is_file() and p.name.endswith("log")]
+
+        expected = set(sorted(all_logs, reverse=True, key=lambda p: p.name)[:5])
+        print(expected)
+        assert set(pack_n_latest_logs_for_scenario_in_dir(scenario_dir.name, scenario_dir, 5)) == 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)