Fix agent side test_move_results

While working on PR #1948, refactoring the common logging infrastructure,
the tests in `test_move_results` were failing for odd reasons.  Looking
at them a bit more closely it became apparent the test was not actually
invoking the API's PUT method.  This commit creates a test that allows
the API to be invoked, fixing the move results code to work correctly.

Author: portante

lib/pbench/cli/agent/commands/results/__init__.py

@@ -1,11 +1,12 @@
 import os
-import sys
 import requests
+import shutil
+import sys
 import tempfile
 
-from pbench.common.logger import get_pbench_logger
 from pbench.agent import PbenchAgentConfig
 from pbench.agent.results import MakeResultTb, CopyResultTb
+from pbench.common.logger import get_pbench_logger
 
 
 def move_results(ctx, _user, _prefix, _show_server):
@@ -47,9 +48,6 @@ def move_results(ctx, _user, _prefix, _show_server):
         )
         sys.exit(1)
 
-    runs_copied = 0
-    failures = 0
-
     try:
         temp_dir = tempfile.mkdtemp(
             dir=config.pbench_tmp, prefix="pbench-move-results."
@@ -58,42 +56,42 @@ def move_results(ctx, _user, _prefix, _show_server):
         logger.error("Failed to create temporary directory")
         sys.exit(1)
 
-    dirs = [
-        _dir
-        for _dir in next(os.walk(config.pbench_run))[1]
-        if not _dir.startswith("tools-") and not _dir.startswith("tmp")
-    ]
+    runs_copied = 0
+    failures = 0
 
-    for _dir in dirs:
-        result_dir = config.pbench_run / _dir
+    for dirent in config.pbench_run.iterdir():
+        if not dirent.is_dir():
+            continue
+        if dirent.name.startswith("tools-") or dirent.name == "tmp":
+            continue
+        result_dir = dirent
         mrt = MakeResultTb(result_dir, temp_dir, _user, _prefix, config, logger)
         result_tb_name = mrt.make_result_tb()
-        if result_tb_name:
-            crt = CopyResultTb(controller, result_tb_name, config, logger)
-            copy_result = crt.copy_result_tb()
-            try:
-                os.remove(result_tb_name)
-                os.remove(f"{result_tb_name}.md5")
-            except OSError:
-                logger.error("rm failed to remove %s and its .md5 file", result_tb_name)
-                sys.exit(1)
-            if not copy_result:
-                failures += 1
-                continue
+        assert (
+            result_tb_name
+        ), "Logic bomb!  make_result_tb() always returns a tar ball name"
+        crt = CopyResultTb(controller, result_tb_name, config, logger)
+        crt.copy_result_tb()
+        try:
+            # We always remove the constructed tar ball, regardless of success
+            # or failure, since we keep the result directory below on failure.
+            os.remove(result_tb_name)
+            os.remove(f"{result_tb_name}.md5")
+        except OSError:
+            logger.error("rm failed to remove %s and its .md5 file", result_tb_name)
+            sys.exit(1)
 
-            try:
-                os.remove(result_dir)
-            except OSError:
-                logger.error(
-                    "rm failed to remove the %s directory hierarchy", result_dir
-                )
-                sys.exit(1)
+        try:
+            shutil.rmtree(result_dir)
+        except OSError:
+            logger.error("rm failed to remove the %s directory hierarchy", result_dir)
+            sys.exit(1)
 
-            runs_copied += 1
+        runs_copied += 1
 
     if runs_copied + failures > 0:
         logger.debug(
             "successfully moved %s runs, encountered %s failures", runs_copied, failures
         )
 
-    return failures
+    return runs_copied, failures

lib/pbench/test/unit/agent/task/test_move_results.py

@@ -1,15 +1,45 @@
-import os
 import datetime
+import logging
+import os
 import responses
 
+from pathlib import Path
 from pbench.cli.agent.commands.results import move_results
 from pbench.test.unit.agent.task.common import MockDatetime
 
 
+# Template for a mocked "metadata.log" file; the caller needs to
+# provide variables for the name, script, config, and date values.
+mdlog_tmpl = """[pbench]
+name = {name}
+script = {script}
+config = {config}
+date = {date}
+rpm-version = 0.00.0-1
+iterations = 1, 1
+
+[tools]
+hosts = agent.example.com
+group = default
+
+[tools/agent.example.com]
+sar = --interval=3
+
+[run]
+controller = agent.example.com
+start_run = YYYY-MM-DDTHH:MM:SS.000000000
+end_run = YYYY-MM-DDTHH:MM:SS.000000000
+
+[iterations/1]
+iteration_name = 1
+user_script = sleep
+"""
+
+
 class TestMoveResults:
     @staticmethod
     @responses.activate
-    def test_move_results(monkeypatch):
+    def test_move_results(monkeypatch, caplog, pytestconfig):
         monkeypatch.setenv("_pbench_full_hostname", "localhost")
         monkeypatch.setattr(datetime, "datetime", MockDatetime)
 
@@ -21,14 +51,32 @@ def test_move_results(monkeypatch):
         )
         responses.add(
             responses.PUT,
-            "http://pbench.example.com/api/v1/upload/ctrl/controller",
+            "http://pbench.example.com/api/v1/upload/ctrl/localhost",
             status=200,
         )
 
         ctx = {"args": {"config": os.environ["_PBENCH_AGENT_CONFIG"]}}
 
+        # In order for a pbench tar ball to be moved/copied to a pbench-server
+        # the run directory has to have one file in it, a "metadata.log" file.
+        # We make a run directory and populate it with our test specific
+        # information.
+        TMP = pytestconfig.cache.get("TMP", None)
+        pbrun = Path(TMP) / "var" / "lib" / "pbench-agent"
+        script = "pbench-user-benchmark"
+        config = "test-move-results"
+        date = "YYYY.MM.DDTHH.MM.SS"
+        name = "{script}_{config}_{date}"
+        run_dir = pbrun / name
+        run_dir.mkdir()
+        mlog = run_dir / "metadata.log"
+        mlog.write_text(mdlog_tmpl.format(**locals()))
+
+        caplog.set_level(logging.DEBUG)
+
         try:
-            move_results(ctx, "pbench", "", True)
+            runs_copied, failures = move_results(ctx, "pbench", "", True)
         except SystemExit:
             assert False
-        assert True
+        assert failures == 0, f"Unexpected failures, {failures}"
+        assert runs_copied == 1, f"Unexpected runs_copied, {runs_copied}"