Merge branch '4252-pytest-symlink-to-test-artifacts' into 'main'

Create symlinks to test artifacts for pytest runner

Closes #4252

See merge request isc-projects/bind9!8194

bin/tests/system/.gitignore

@@ -2,6 +2,7 @@
 .hypothesis
 .mypy_cache
 __pycache__
+_last_test_run
 dig.out*
 rndc.out*
 nsupdate.out*
@@ -19,4 +20,10 @@ named.run
 /start.sh
 /stop.sh
 /ifconfig.sh
-/*_tmp_*
+
+# Ignore file names with underscore in their name except python or shell files.
+# This is done to ignore the temporary directories and symlinks created by the
+# pytest runner, which contain underscore in their file names.
+/*_*
+!/*_*.py
+!/*_*.sh

bin/tests/system/Makefile.am

@@ -238,3 +238,6 @@ AM_LOG_FLAGS = -r
 $(TESTS): legacy.run.sh
 
 test-local: check
+
+clean-local::
+	-find $(builddir) -maxdepth 1 -type d -name "*_*" | xargs rm -rf

bin/tests/system/conftest.py

@@ -104,6 +104,9 @@ else:
     ]
     PRIORITY_TESTS_RE = re.compile("|".join(PRIORITY_TESTS))
     CONFTEST_LOGGER = logging.getLogger("conftest")
+    SYSTEM_TEST_DIR_GIT_PATH = "bin/tests/system"
+    SYSTEM_TEST_NAME_RE = re.compile(f"{SYSTEM_TEST_DIR_GIT_PATH}" + r"/([^/]+)")
+    SYMLINK_REPLACEMENT_RE = re.compile(r"/tests(_sh(?=_))?(.*)\.py")
 
     # ---------------------- Module initialization ---------------------------
 
@@ -227,8 +230,16 @@ else:
         # bin/tests/system. These temporary directories contain all files
         # needed for the system tests - including tests_*.py files. Make sure to
         # ignore these during test collection phase. Otherwise, test artifacts
-        # from previous runs could mess with the runner.
-        return "_tmp_" in str(path)
+        # from previous runs could mess with the runner. Also ignore the
+        # convenience symlinks to those test directories. In both of those
+        # cases, the system test name (directory) contains an underscore, which
+        # is otherwise and invalid character for a system test name.
+        match = SYSTEM_TEST_NAME_RE.search(str(path))
+        if match is None:
+            CONFTEST_LOGGER.warning("unexpected test path: %s (ignored)", path)
+            return True
+        system_test_name = match.groups()[0]
+        return "_" in system_test_name
 
     def pytest_collection_modifyitems(items):
         """Schedule long-running tests first to get more benefit from parallelism."""
@@ -345,8 +356,8 @@ else:
         """Dictionary containing environment variables for the test."""
         env = os.environ.copy()
         env.update(ports)
-        env["builddir"] = f"{env['TOP_BUILDDIR']}/bin/tests/system"
-        env["srcdir"] = f"{env['TOP_SRCDIR']}/bin/tests/system"
+        env["builddir"] = f"{env['TOP_BUILDDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
+        env["srcdir"] = f"{env['TOP_SRCDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
         return env
 
     @pytest.fixture(scope="module")
@@ -367,7 +378,9 @@ else:
         return logging.getLogger(f"{system_test_name}.{request.node.name}")
 
     @pytest.fixture(scope="module")
-    def system_test_dir(request, env, system_test_name, mlogger):
+    def system_test_dir(
+        request, env, system_test_name, mlogger
+    ):  # pylint: disable=too-many-statements,too-many-locals
         """
         Temporary directory for executing the test.
 
@@ -409,14 +422,26 @@ else:
             assert all(res.outcome == "passed" for res in test_results.values())
             return "passed"
 
+        def unlink(path):
+            try:
+                path.unlink()  # missing_ok=True isn't available on Python 3.6
+            except FileNotFoundError:
+                pass
+
         # Create a temporary directory with a copy of the original system test dir contents
-        system_test_root = Path(f"{env['TOP_BUILDDIR']}/bin/tests/system")
+        system_test_root = Path(f"{env['TOP_BUILDDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}")
         testdir = Path(
             tempfile.mkdtemp(prefix=f"{system_test_name}_tmp_", dir=system_test_root)
         )
         shutil.rmtree(testdir)
         shutil.copytree(system_test_root / system_test_name, testdir)
 
+        # Create a convenience symlink with a stable and predictable name
+        module_name = SYMLINK_REPLACEMENT_RE.sub(r"\2", request.node.name)
+        symlink_dst = system_test_root / module_name
+        unlink(symlink_dst)
+        symlink_dst.symlink_to(os.path.relpath(testdir, start=system_test_root))
+
         # Configure logger to write to a file inside the temporary test directory
         mlogger.handlers.clear()
         mlogger.setLevel(logging.DEBUG)
@@ -428,7 +453,7 @@ else:
         # System tests are meant to be executed from their directory - switch to it.
         old_cwd = os.getcwd()
         os.chdir(testdir)
-        mlogger.info("switching to tmpdir: %s", testdir)
+        mlogger.debug("switching to tmpdir: %s", testdir)
         try:
             yield testdir  # other fixtures / tests will execute here
         finally:
@@ -438,19 +463,34 @@ else:
             result = get_test_result()
 
             # Clean temporary dir unless it should be kept
+            keep = False
             if request.config.getoption("--noclean"):
-                mlogger.debug("--noclean requested, keeping temporary directory")
+                mlogger.debug(
+                    "--noclean requested, keeping temporary directory %s", testdir
+                )
+                keep = True
             elif result == "failed":
-                mlogger.debug("test failure detected, keeping temporary directory")
+                mlogger.debug(
+                    "test failure detected, keeping temporary directory %s", testdir
+                )
+                keep = True
             elif not request.node.stash[FIXTURE_OK]:
                 mlogger.debug(
-                    "test setup/teardown issue detected, keeping temporary directory"
+                    "test setup/teardown issue detected, keeping temporary directory %s",
+                    testdir,
+                )
+                keep = True
+
+            if keep:
+                mlogger.info(
+                    "test artifacts in: %s", symlink_dst.relative_to(system_test_root)
                 )
             else:
                 mlogger.debug("deleting temporary directory")
                 handler.flush()
                 handler.close()
                 shutil.rmtree(testdir)
+                unlink(symlink_dst)
 
     def _run_script(  # pylint: disable=too-many-arguments
         env,