tests: fix --valgrind-memleaks option

Previously, when a valgrind memleak was discovered, would cause a
catastrophic pytest failure. Now correctly fails the current pytest as
intended.

As a result of this fix --valgrind-memleaks now works in distributed
pytest mode as well.

Signed-off-by: Christian Hopps <chopps@labn.net>

diff --git a/tests/topotests/conftest.py b/tests/topotests/conftest.py
index 7fe6a5aea11c1ed3f31d2fa9dd3608eb10632088..ce7878c24006c8815a43e6e57d0ec81469209f68 100755 (executable)
--- a/tests/topotests/conftest.py
+++ b/tests/topotests/conftest.py
@@ -21,13 +21,6 @@ from lib.topolog import logger
 from lib.topotest import g_extra_config as topotest_extra_config
 from lib.topotest import json_cmp_result
 
-try:
-    from _pytest._code.code import ExceptionInfo
-
-    leak_check_ok = True
-except ImportError:
-    leak_check_ok = False
-
 
 def pytest_addoption(parser):
     """
@@ -138,8 +131,7 @@ def pytest_addoption(parser):
 
 
 def check_for_memleaks():
-    if not topotest_extra_config["valgrind_memleaks"]:
-        return
+    assert topotest_extra_config["valgrind_memleaks"]
 
     leaks = []
     tgen = get_topogen()
@@ -151,21 +143,25 @@ def check_for_memleaks():
             existing = tgen.valgrind_existing_files
         latest = glob.glob(os.path.join(logdir, "*.valgrind.*"))
 
+    daemons = set()
     for vfile in latest:
         if vfile in existing:
             continue
-        with open(vfile) as vf:
+        existing.append(vfile)
+        with open(vfile, encoding="ascii") as vf:
             vfcontent = vf.read()
             match = re.search(r"ERROR SUMMARY: (\d+) errors", vfcontent)
             if match and match.group(1) != "0":
                 emsg = "{} in {}".format(match.group(1), vfile)
                 leaks.append(emsg)
+                daemons.add(re.match(r".*\.valgrind\.(.*)\.\d+", vfile).group(1))
+
+    if tgen is not None:
+        tgen.valgrind_existing_files = existing
 
     if leaks:
-        if leak_check_ok:
-            pytest.fail("Memleaks found:\n\t" + "\n\t".join(leaks))
-        else:
-            logger.error("Memleaks found:\n\t" + "\n\t".join(leaks))
+        logger.error("valgrind memleaks found:\n\t%s", "\n\t".join(leaks))
+        pytest.fail("valgrind memleaks found for daemons: " + " ".join(daemons))
 
 
 def pytest_runtest_logstart(nodeid, location):
@@ -178,18 +174,21 @@ def pytest_runtest_logfinish(nodeid, location):
     topolog.logfinish(nodeid, location)
 
 
-def pytest_runtest_call():
-    """
-    This function must be run after setup_module(), it does standarized post
-    setup routines. It is only being used for the 'topology-only' option.
-    """
+@pytest.hookimpl(hookwrapper=True)
+def pytest_runtest_call(item: pytest.Item) -> None:
+    "Hook the function that is called to execute the test."
+
+    # For topology only run the CLI then exit
     if topotest_extra_config["topology_only"]:
-        tgen = get_topogen()
-        if tgen is not None:
-            # Allow user to play with the setup.
-            tgen.cli()
+        get_topogen().cli()
+        pytest.exit("exiting after --topology-only")
+
+    # Let the default pytest_runtest_call execute the test function
+    yield
 
-        pytest.exit("the topology executed successfully")
+    # Check for leaks if requested
+    if topotest_extra_config["valgrind_memleaks"]:
+        check_for_memleaks()
 
 
 def pytest_assertrepr_compare(op, left, right):
@@ -333,7 +332,10 @@ def pytest_configure(config):
     topotest_extra_config["pause"] = pause
     assert_feature_windows(pause, "--pause")
 
-    topotest_extra_config["topology_only"] = config.getoption("--topology-only")
+    topology_only = config.getoption("--topology-only")
+    if topology_only and is_xdist:
+        pytest.exit("Cannot use --topology-only with distributed test mode")
+    topotest_extra_config["topology_only"] = topology_only
 
     # Check environment now that we have config
     if not diagnose_env(rundir):
@@ -373,12 +375,6 @@ def pytest_runtest_makereport(item, call):
     else:
         pause = False
 
-    if call.excinfo is None and call.when == "call":
-        try:
-            check_for_memleaks()
-        except:
-            call.excinfo = ExceptionInfo()
-
     title = "unset"
 
     if call.excinfo is None: