From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Sat, 29 Oct 2022 15:40:26 +0000 (-0700)
Subject: tests: only accept 'which' results that are adjacent to 'dot'
X-Git-Tag: 7.0.1~8^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=515c86a923601db5cb704da93046800df7da030a;p=graphviz

tests: only accept 'which' results that are adjacent to 'dot'

As described in the code comment, this addresses a problem where testing would
find binaries from a prior Graphviz installation and then spuriously fail.

This is not ideal. It is easy for an upcoming test writer to forget or be
unaware of this quirk and introduce a new direct call to `shutil.which`. But it
seems the best option we have right now.

Gitlab: closes #2201
---

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2ab04f141..e2a0a4409 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - When built with zlib support, Graphviz will unconditionally use
   `deflateBound`. The only user-visible effect should be slightly decreased
   memory usage when using a zlib-compressed output format.
+- The test suite only detects Graphviz componanion programs adjacent to the
+  irst `dot` found in `$PATH` #2201
 
 ### Fixed
 
diff --git a/ci/tests.py b/ci/tests.py
index 1ac575049..e98a67205 100644
--- a/ci/tests.py
+++ b/ci/tests.py
@@ -7,14 +7,13 @@ test cases that are only relevant to run in CI
 import os
 from pathlib import Path
 import platform
-import shutil
 import subprocess
 import sys
 from typing import Dict
 import pytest
 
 sys.path.append(os.path.join(os.path.dirname(__file__), "../tests"))
-from gvtest import dot, is_cmake, is_mingw \
+from gvtest import dot, is_cmake, is_mingw, which \
   #pylint: disable=wrong-import-position
 
 def _freedesktop_os_release() -> Dict[str, str]:
@@ -177,13 +176,13 @@ def test_existence(binary: str):
     check_that_tool_does_not_exist(binary, os_id)
     pytest.skip(f"{binary} is not installed on Windows")
 
-  assert shutil.which(binary) is not None
+  assert which(binary) is not None
 
 def check_that_tool_does_not_exist(tool, os_id):
   """
   validate that the given tool does *not* exist
   """
-  assert shutil.which(tool) is None, f"{tool} has been resurrected in the " \
+  assert which(tool) is None, f"{tool} has been resurrected in the " \
     f'{os.getenv("build_system")} build on {os_id}. Please remove skip.'
 
 @pytest.mark.xfail(is_cmake() and not is_centos()
diff --git a/tests/gvtest.py b/tests/gvtest.py
index cfe5344c0..54386042e 100644
--- a/tests/gvtest.py
+++ b/tests/gvtest.py
@@ -104,8 +104,7 @@ def dot(T: str, source_file: Optional[Path] = None, source: Optional[str] = None
 def gvpr(program: Path) -> str:
   """run a GVPR program on empty input"""
 
-  assert shutil.which("gvpr") is not None, \
-    "attempt to run GVPR without it available"
+  assert which("gvpr") is not None, "attempt to run GVPR without it available"
 
   return subprocess.check_output(["gvpr", "-f", program],
                                  stdin=subprocess.DEVNULL,
@@ -167,3 +166,33 @@ def run_c(src: Path, args: List[str] = None, input: str = "",
     p.check_returncode()
 
     return p.stdout, p.stderr
+
+def which(cmd: str) -> Optional[Path]:
+  """
+  `shutil.which` but only return results that are adjacent to the main Graphviz
+  executable.
+
+  Graphviz has numerous optional components. So if you choose to e.g. disable
+  `mingle` during compilation and installation, you may find that a naive
+  `shutil.which("mingle")` after this returns the `mingle` from a prior
+  system-wide installation of the full Graphviz. This is undesirable during
+  testing, as this older `mingle` will then load shared libraries from the
+  installation you just created, most likely crashing.
+  """
+
+  # try a straightforward lookup of the command
+  abs_cmd = shutil.which(cmd)
+  if abs_cmd is None:
+    return None
+  abs_cmd = Path(abs_cmd)
+
+  # where does the main Graphviz program live?
+  abs_dot = shutil.which("dot")
+  assert abs_dot is not None, "dot not in $PATH"
+  abs_dot = Path(abs_dot)
+
+  # discard the result we found if it does not live in the same directory
+  if abs_cmd.parent != abs_dot.parent:
+    return None
+
+  return abs_cmd
diff --git a/tests/test_examples.py b/tests/test_examples.py
index f82aa5769..6ce262735 100644
--- a/tests/test_examples.py
+++ b/tests/test_examples.py
@@ -3,13 +3,12 @@
 import os
 from pathlib import Path
 import platform
-import shutil
 import subprocess
 import sys
 import pytest
 
 sys.path.append(os.path.dirname(__file__))
-from gvtest import run_c #pylint: disable=wrong-import-position
+from gvtest import run_c, which #pylint: disable=wrong-import-position
 
 @pytest.mark.parametrize("src", ["demo.c", "dot.c", "example.c", "neatopack.c",
   "simple.c"])
@@ -43,7 +42,7 @@ def test_compile_example(src):
   "addranks", "anon", "bb", "chkclusters", "cliptree", "col", "color",
   "dechain", "deledges", "delnodes", "dijkstra", "get-layers-list", "knbhd",
   "maxdeg", "rotate", "scalexy", "topon"])
-@pytest.mark.skipif(shutil.which("gvpr") is None, reason="GVPR not available")
+@pytest.mark.skipif(which("gvpr") is None, reason="GVPR not available")
 def test_gvpr_example(src):
   """check GVPR can parse the given example"""
 
@@ -64,7 +63,7 @@ def test_gvpr_example(src):
   # run GVPR with the given script
   subprocess.check_call(["gvpr", "-f", path], stdin=subprocess.DEVNULL, cwd=wd)
 
-@pytest.mark.skipif(shutil.which("gvpr") is None, reason="GVPR not available")
+@pytest.mark.skipif(which("gvpr") is None, reason="GVPR not available")
 # FIXME: Remove skip when
 # https://gitlab.com/graphviz/graphviz/-/issues/1882 is fixed
 @pytest.mark.skipif(
diff --git a/tests/test_regression.py b/tests/test_regression.py
index efd8f0ce5..111753889 100644
--- a/tests/test_regression.py
+++ b/tests/test_regression.py
@@ -10,7 +10,6 @@ import os
 from pathlib import Path
 import platform
 import re
-import shutil
 import signal
 import stat
 import subprocess
@@ -21,7 +20,7 @@ import xml.etree.ElementTree as ET
 import pytest
 
 sys.path.append(os.path.dirname(__file__))
-from gvtest import dot, gvpr, ROOT, remove_xtype_warnings, run_c \
+from gvtest import dot, gvpr, ROOT, remove_xtype_warnings, run_c, which \
   #pylint: disable=wrong-import-position
 
 def is_ndebug_defined() -> bool:
@@ -120,7 +119,7 @@ def test_131():
   # ask Graphviz to process this to PIC
   pic = dot("pic", source=src)
 
-  if shutil.which("gpic") is None:
+  if which("gpic") is None:
     pytest.skip("GNU PIC not available")
 
   # ask GNU PIC to process the Graphviz output
@@ -316,8 +315,7 @@ def test_358():
     assert m is not None, \
       f"font characteristic {1 << i} not enabled in xdot 1.7"
 
-@pytest.mark.skipif(shutil.which("gv2gxl") is None or
-                    shutil.which("gxl2gv") is None,
+@pytest.mark.skipif(which("gv2gxl") is None or which("gxl2gv") is None,
                     reason="GXL tools not available")
 def test_517():
   """
@@ -429,8 +427,7 @@ def test_1221():
   # process this with dot
   dot("svg", input)
 
-@pytest.mark.skipif(shutil.which("gv2gml") is None,
-                    reason="gv2gml not available")
+@pytest.mark.skipif(which("gv2gml") is None, reason="gv2gml not available")
 def test_1276():
   """
   quotes within a label should be escaped in translation to GML
@@ -574,7 +571,7 @@ def test_1449():
 
   assert stderr.strip() == "", "SVG color scheme use caused warnings"
 
-@pytest.mark.skipif(shutil.which("gvpr") is None, reason="GVPR not available")
+@pytest.mark.skipif(which("gvpr") is None, reason="GVPR not available")
 def test_1594():
   """
   GVPR should give accurate line numbers in error messages
@@ -685,7 +682,7 @@ def test_1767():
   #                  "cluster_2 contains 3 nodes\n" \
   #                  "cluster_3 contains 3 nodes\n"
 
-@pytest.mark.skipif(shutil.which("gvpr") is None, reason="GVPR not available")
+@pytest.mark.skipif(which("gvpr") is None, reason="GVPR not available")
 @pytest.mark.skipif(platform.system() != "Windows",
   reason="only relevant on Windows")
 def test_1780():
@@ -717,7 +714,7 @@ def test_1783():
 
   assert ret != -signal.SIGSEGV, "Graphviz segfaulted"
 
-@pytest.mark.skipif(shutil.which("gvedit") is None, reason="Gvedit not available")
+@pytest.mark.skipif(which("gvedit") is None, reason="Gvedit not available")
 def test_1813():
   """
   gvedit -? should show usage
@@ -797,7 +794,7 @@ def test_1856():
 
   assert all(y >= top_of_five for y in waypoints_y), "edge dips below 5"
 
-@pytest.mark.skipif(shutil.which("fdp") is None, reason="fdp not available")
+@pytest.mark.skipif(which("fdp") is None, reason="fdp not available")
 def test_1865():
   """
   fdp should not read out of bounds when processing node names
@@ -813,10 +810,8 @@ def test_1865():
   # fdp should not crash when processing this file
   subprocess.check_call(["fdp", "-o", os.devnull, input])
 
-@pytest.mark.skipif(shutil.which("gv2gml") is None,
-                    reason="gv2gml not available")
-@pytest.mark.skipif(shutil.which("gml2gv") is None,
-                    reason="gml2gv not available")
+@pytest.mark.skipif(which("gv2gml") is None, reason="gv2gml not available")
+@pytest.mark.skipif(which("gml2gv") is None, reason="gml2gv not available")
 @pytest.mark.parametrize("penwidth", ("1.0", "1"))
 def test_1871(penwidth: str):
   """
@@ -839,7 +834,7 @@ def test_1871(penwidth: str):
   assert has_1 or has_1_0, \
     f"incorrect penwidth from round tripping through GML (output {gml})"
 
-@pytest.mark.skipif(shutil.which("fdp") is None, reason="fdp not available")
+@pytest.mark.skipif(which("fdp") is None, reason="fdp not available")
 def test_1876():
   """
   fdp should not rename nodes with internal names
@@ -859,7 +854,7 @@ def test_1876():
   # we should not see any internal names like "%3"
   assert "%" not in output, "internal name in fdp output"
 
-@pytest.mark.skipif(shutil.which("fdp") is None, reason="fdp not available")
+@pytest.mark.skipif(which("fdp") is None, reason="fdp not available")
 def test_1877():
   """
   fdp should not fail an assertion when processing cluster edges
@@ -962,7 +957,7 @@ def test_1855():
   assert len(y) > 4, "insufficient precision in y scale"
 
 @pytest.mark.parametrize("variant", [1, 2])
-@pytest.mark.skipif(shutil.which("gml2gv") is None, reason="gml2gv not available")
+@pytest.mark.skipif(which("gml2gv") is None, reason="gml2gv not available")
 def test_1869(variant: int):
   """
   gml2gv should be able to parse the style, outlineStyle, width and
@@ -1037,7 +1032,7 @@ def test_1906():
 
   assert "area too large" in stderr, "missing/incorrect error message"
 
-@pytest.mark.skipif(shutil.which("twopi") is None, reason="twopi not available")
+@pytest.mark.skipif(which("twopi") is None, reason="twopi not available")
 def test_1907():
   """
   SVG edges should have title elements that match their names
@@ -1054,7 +1049,7 @@ def test_1907():
   assert "<title>A&#45;&gt;B</title>" in output, \
     "element title not found in SVG"
 
-@pytest.mark.skipif(shutil.which("gvpr") is None, reason="gvpr not available")
+@pytest.mark.skipif(which("gvpr") is None, reason="gvpr not available")
 def test_1909():
   """
   GVPR should not output internal names
@@ -1180,7 +1175,7 @@ def test_1931():
   assert "line 3\nline 4" in xdot
   assert "line 5\nline 6" in xdot
 
-@pytest.mark.skipif(shutil.which("edgepaint") is None,
+@pytest.mark.skipif(which("edgepaint") is None,
                     reason="edgepaint not available")
 def test_1971():
   """
@@ -1343,8 +1338,7 @@ def test_2089_2():
   # run it
   _, _ = run_c(c_src, link=["cgraph"])
 
-@pytest.mark.skipif(shutil.which("dot2gxl") is None,
-                    reason="dot2gxl not available")
+@pytest.mark.skipif(which("dot2gxl") is None, reason="dot2gxl not available")
 def test_2092():
   """
   an empty node ID should not cause a dot2gxl NULL pointer dereference
@@ -1357,8 +1351,7 @@ def test_2092():
 
   assert p.returncode == 1, "dot2gxl crashed"
 
-@pytest.mark.skipif(shutil.which("dot2gxl") is None,
-                    reason="dot2gxl not available")
+@pytest.mark.skipif(which("dot2gxl") is None, reason="dot2gxl not available")
 def test_2093():
   """
   dot2gxl should handle elements with no ID
@@ -1387,8 +1380,7 @@ def test_2095():
   # ask Graphviz to process it
   dot("pdf", input)
 
-@pytest.mark.skipif(shutil.which("gv2gml") is None,
-                    reason="gv2gml not available")
+@pytest.mark.skipif(which("gv2gml") is None, reason="gv2gml not available")
 def test_2131():
   """
   gv2gml should be able to process basic Graphviz input
@@ -1405,8 +1397,7 @@ def test_2131():
   except subprocess.CalledProcessError as e:
     raise RuntimeError("gv2gml rejected a basic graph") from e
 
-@pytest.mark.skipif(shutil.which("gvpr") is None,
-                    reason="gvpr not available")
+@pytest.mark.skipif(which("gvpr") is None, reason="gvpr not available")
 @pytest.mark.parametrize("examine", ("indices", "tokens"))
 def test_2138(examine: str):
   """
@@ -1488,7 +1479,7 @@ def test_2179_1():
   assert "Warning: no hard-coded metrics for" not in stderr, \
     "incorrect warning triggered"
 
-@pytest.mark.skipif(shutil.which("nop") is None, reason="nop not available")
+@pytest.mark.skipif(which("nop") is None, reason="nop not available")
 def test_2184_1():
   """
   nop should not reposition labelled graph nodes
@@ -1652,7 +1643,7 @@ def test_2193():
   new = dot("canon", source=canonical)
   assert canonical == new, "canonical translation is not stable"
 
-@pytest.mark.skipif(shutil.which("gvpr") is None, reason="GVPR not available")
+@pytest.mark.skipif(which("gvpr") is None, reason="GVPR not available")
 def test_2211():
   """
   GVPR’s `index` function should return correct results
@@ -1819,7 +1810,7 @@ def test_vcxproj_inclusive(vcxproj: Path):
       "mismatch between sources in {str(vcxproj)} and {str(filters)}"
 
 @pytest.mark.xfail() # FIXME: fails on CentOS 7/8, macOS Autotools, MSBuild
-@pytest.mark.skipif(shutil.which("gvmap") is None, reason="gvmap not available")
+@pytest.mark.skipif(which("gvmap") is None, reason="gvmap not available")
 def test_gvmap_fclose():
   """
   gvmap should not attempt to fclose(NULL). This example will trigger a crash if
@@ -1851,7 +1842,7 @@ def test_gvmap_fclose():
   # pass this through gvmap
   subprocess.run(["gvmap"], input=input.encode("utf-8"), check=True)
 
-@pytest.mark.skipif(shutil.which("gvpr") is None, reason="gvpr not available")
+@pytest.mark.skipif(which("gvpr") is None, reason="gvpr not available")
 def test_gvpr_usage():
   """
   gvpr usage information should be included when erroring on a malformed command
@@ -1995,8 +1986,7 @@ def test_2282():
   # confirm this is valid JSON
   json.loads(output)
 
-@pytest.mark.skipif(shutil.which("gxl2gv") is None,
-                    reason="gxl2gv not available")
+@pytest.mark.skipif(which("gxl2gv") is None, reason="gxl2gv not available")
 @pytest.mark.xfail()
 def test_2300_1():
   """
diff --git a/tests/test_tools.py b/tests/test_tools.py
index 458df9160..686090e22 100644
--- a/tests/test_tools.py
+++ b/tests/test_tools.py
@@ -9,13 +9,12 @@ property of one of the tools has been broken.
 import os
 import platform
 import re
-import shutil
 import subprocess
 import sys
 import pytest
 
 sys.path.append(os.path.dirname(__file__))
-from gvtest import is_cmake, remove_xtype_warnings \
+from gvtest import is_cmake, remove_xtype_warnings, which \
   #pylint: disable=wrong-import-position
 
 @pytest.mark.parametrize("tool", [
@@ -65,7 +64,7 @@ def test_tools(tool):
   check the help/usage output of the given tool looks correct
   """
 
-  if shutil.which(tool) is None:
+  if which(tool) is None:
     pytest.skip(f"{tool} not available")
 
   # FIXME: Remove skip when
@@ -117,7 +116,7 @@ def test_tools(tool):
 
   assert returncode != 0, f"{tool} accepted unsupported option -$"
 
-@pytest.mark.skipif(shutil.which("edgepaint") is None,
+@pytest.mark.skipif(which("edgepaint") is None,
                     reason="edgepaint not available")
 @pytest.mark.parametrize("arg", ("-accuracy=0.01", "-angle=15",
                                  "-random_seed=42", "-random_seed=-42",