REF reduce print statements

cortex/testing_utils.py → cortex/dependencies.py

@@ -1,19 +1,21 @@
-"""Module containing utils for testing"""
+"""Locate external tools (e.g. Inkscape) configured under ``[dependency_paths]``."""
+from __future__ import annotations
+
 import os
 import subprocess as sp
+from functools import cache
 from shutil import which
-from . import options
-import os
 
+from . import options
 from .options import config
 
 
-def has_installed(name):
+def has_installed(name: str) -> bool:
     binary_file = options.config.get('dependency_paths', name)
     return os.path.exists(binary_file) or which(name) is not None
 
 
-def inkscapePath():
+def inkscape_path() -> str | None:
     """Return the path of the inkscape executable to run, or None.
 
     The 'inkscape' option of the 'dependency_paths' config section is used
@@ -30,31 +32,42 @@ def inkscapePath():
     return which('inkscape')
 
 
-def inkscape_version():
-    inkscapeExecutable = inkscapePath()
+def inkscape_version() -> str | None:
+    """Return the version string of the inkscape executable, or None.
+
+    Nothing is cached while inkscape is missing, so installing it after
+    pycortex has been imported takes effect on the next call.
+    """
+    inkscapeExecutable = inkscape_path()
     if inkscapeExecutable is None:
         return None
+    return _query_inkscape_version(inkscapeExecutable)
+
+
+def require_inkscape() -> tuple[str, str]:
+    """Return the (path, version) of inkscape, or raise if it is not available."""
+    path = inkscape_path()
+    version = inkscape_version()
+    if path is None or version is None:
+        raise RuntimeError(
+            "Inkscape doesn't seem to be installed on this system, and is "
+            "required here. Please make sure that inkscape is installed and "
+            "accessible from the terminal, or set the 'inkscape' option in "
+            f"the [dependency_paths] section of {options.usercfg}.")
+    return path, version
+
+
+@cache
+def _query_inkscape_version(inkscapeExecutable: str) -> str | None:
     result = sp.run([inkscapeExecutable, '--version'], stdout = sp.PIPE, stderr = sp.PIPE, check = True)
     # Combine stdout and stderr; some systems print diagnostic messages
     # (e.g. "Setting _INKSCAPE_GC=disable …") before the version line.
-    combined = result.stdout + result.stderr
-    if isinstance(combined, bytes):
-        combined = combined.decode('utf-8')
+    raw = result.stdout + result.stderr
+    combined = raw.decode('utf-8') if isinstance(raw, bytes) else raw
     # Find the line that starts with 'Inkscape' to get the real version,
     # e.g. 'Inkscape 1.2.2 (b0a8486, 2022-12-01)'
     for line in combined.splitlines():
         if line.strip().startswith('Inkscape'):
             version = line.split()[1]
             return version
     return None
-
-
-INKSCAPE_PATH = inkscapePath()
-INKSCAPE_VERSION = inkscape_version()
-if INKSCAPE_VERSION is None:
-    print(("Inkscape not found. Some tests will be skipped. " 
-    "If you are on a Mac or Windows machine, please specify "
-    "the path to the Inkscape binary in the config file "
-    f"(Here: {options.usercfg})."))
-
-

cortex/svgoverlay.py

@@ -16,7 +16,7 @@
 from lxml.builder import E
 
 from .options import config
-from .testing_utils import INKSCAPE_PATH, INKSCAPE_VERSION
+from .dependencies import require_inkscape
 
 svgns = "http://www.w3.org/2000/svg"
 inkns = "http://www.inkscape.org/namespaces/inkscape"
@@ -229,12 +229,7 @@ def get_texture(self, layer_name, height, name=None, background=None, labels=Tru
         """
         # Give a more informative error in case we don't have inkscape
         # installed
-        if INKSCAPE_VERSION is None:
-            raise RuntimeError(
-                "Inkscape doesn't seem to be installed on this system."
-                "SVGOverlay.get_texture requires inkscape."
-                "Please make sure that inkscape is installed and that is "
-                "accessible from the terminal.")
+        inkscape_cmd, inkscape_ver = require_inkscape()
 
         if shadow is not None:
             for blur in self.svg.findall(".//{%s}feGaussianBlur"%svgns):
@@ -293,11 +288,10 @@ def get_texture(self, layer_name, height, name=None, background=None, labels=Tru
         svgtemp.close()
         svgfile = svgtemp.name
 
-        inkscape_cmd = INKSCAPE_PATH
         # The command is built as an argument list rather than as a string that
         # is then split, so that a configured inkscape path containing spaces
         # or backslashes is passed through unchanged.
-        if LooseVersion(INKSCAPE_VERSION) < LooseVersion('1.0'):
+        if LooseVersion(inkscape_ver) < LooseVersion('1.0'):
             cmd = [inkscape_cmd, '-z', '-h', str(height), '-e', pngfile, svgfile]
         else:
             cmd = [inkscape_cmd, '-h', str(height), '--export-filename', pngfile, svgfile]
@@ -326,7 +320,7 @@ def get_texture(self, layer_name, height, name=None, background=None, labels=Tru
             except SyntaxError as e:
                 os.unlink(pngfile)
                 raise RuntimeError(f"Error reading image from {pngfile}: {e}"
-                                   f" (inkscape version: {INKSCAPE_VERSION})"
+                                   f" (inkscape version: {inkscape_ver})"
                                    f" (inkscape command: {inkscape_cmd})"
                                    f" (stdout: {stdout})"
                                    f" (stderr: {stderr})")

cortex/tests/test_dataset.py

@@ -4,11 +4,11 @@
 import pytest
 
 from cortex import db, dataset
-from cortex.testing_utils import inkscapePath
+from cortex.dependencies import inkscape_path
 
 subj, xfmname, nverts, volshape = "S1", "fullhead", 304380, (31, 100, 100)
 
-no_inkscape = inkscapePath() is None
+no_inkscape = inkscape_path() is None
 
 
 def test_braindata():

cortex/tests/test_dependencies.py

@@ -0,0 +1,141 @@
+"""Tests for cortex/dependencies.py"""
+import subprocess
+from unittest import mock
+
+import pytest
+
+from cortex.dependencies import (
+    _query_inkscape_version,
+    inkscape_path,
+    inkscape_version,
+    require_inkscape,
+)
+
+
+@pytest.fixture(autouse=True)
+def _clear_cache():
+    _query_inkscape_version.cache_clear()
+    yield
+    _query_inkscape_version.cache_clear()
+
+
+def _make_run_result(stdout=b'', stderr=b'', returncode=0):
+    """Helper to build a mock subprocess.CompletedProcess."""
+    result = mock.MagicMock()
+    result.stdout = stdout
+    result.stderr = stderr
+    result.returncode = returncode
+    return result
+
+
+@mock.patch('cortex.dependencies.inkscape_path', return_value=None)
+def test_inkscape_version_not_installed(mock_path):
+    """Returns None when inkscape is neither configured nor on PATH."""
+    assert inkscape_version() is None
+
+
+@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape')
+@mock.patch('cortex.dependencies.sp.run')
+def test_inkscape_version_clean_stdout(mock_run, mock_path):
+    """Parses version correctly from clean stdout output."""
+    mock_run.return_value = _make_run_result(
+        stdout=b'Inkscape 1.0 (4035a4f, 2020-05-01)\n'
+    )
+    assert inkscape_version() == '1.0'
+
+
+@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape')
+@mock.patch('cortex.dependencies.sp.run')
+def test_inkscape_version_newer(mock_run, mock_path):
+    """Parses a newer version number correctly."""
+    mock_run.return_value = _make_run_result(
+        stdout=b'Inkscape 1.2.2 (b0a8486, 2022-12-01)\n'
+    )
+    assert inkscape_version() == '1.2.2'
+
+
+@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape')
+@mock.patch('cortex.dependencies.sp.run')
+def test_inkscape_version_with_diagnostic_noise(mock_run, mock_path):
+    """Returns correct version even when a diagnostic message precedes it.
+
+    This is the regression test for the bug where systems that print
+    "Setting _INKSCAPE_GC=disable as a workaround for broken libgc"
+    caused INKSCAPE_VERSION to be set to '_INKSCAPE_GC=disable'.
+    """
+    mock_run.return_value = _make_run_result(
+        stdout=(
+            b'Setting _INKSCAPE_GC=disable as a workaround for broken libgc\n'
+            b'Inkscape 1.2.2 (b0a8486, 2022-12-01)\n'
+        )
+    )
+    assert inkscape_version() == '1.2.2'
+
+
+@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape')
+@mock.patch('cortex.dependencies.sp.run')
+def test_inkscape_version_noise_in_stderr(mock_run, mock_path):
+    """Returns correct version when the diagnostic message is on stderr."""
+    mock_run.return_value = _make_run_result(
+        stderr=b'Setting _INKSCAPE_GC=disable as a workaround for broken libgc\n',
+        stdout=b'Inkscape 1.2.2 (b0a8486, 2022-12-01)\n',
+    )
+    assert inkscape_version() == '1.2.2'
+
+
+@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape')
+@mock.patch('cortex.dependencies.sp.run')
+def test_inkscape_version_no_inkscape_line(mock_run, mock_path):
+    """Returns None when no 'Inkscape …' line is found in output."""
+    mock_run.return_value = _make_run_result(stdout=b'some unexpected output\n')
+    assert inkscape_version() is None
+
+
+@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape')
+@mock.patch('cortex.dependencies.sp.run')
+def test_inkscape_version_subprocess_error(mock_run, mock_path):
+    """Propagates CalledProcessError when inkscape exits non-zero."""
+    mock_run.side_effect = subprocess.CalledProcessError(1, 'inkscape')
+    with pytest.raises(subprocess.CalledProcessError):
+        inkscape_version()
+
+
+@mock.patch('cortex.dependencies.sp.run')
+@mock.patch('cortex.dependencies.which')
+@mock.patch('cortex.dependencies.config')
+def test_inkscape_lookup_retries_until_found(mock_config, mock_which, mock_run):
+    """A failed lookup is not cached, so a later install is picked up."""
+    mock_config.get.return_value = ''
+    mock_which.return_value = None
+    assert inkscape_path() is None
+    assert inkscape_version() is None
+
+    # Inkscape gets installed while the process is still running.
+    mock_which.return_value = '/usr/bin/inkscape'
+    mock_run.return_value = _make_run_result(
+        stdout=b'Inkscape 1.2.2 (b0a8486, 2022-12-01)\n'
+    )
+    assert inkscape_path() == '/usr/bin/inkscape'
+    assert inkscape_version() == '1.2.2'
+
+    # Once found, the version is cached and the subprocess is not run again.
+    mock_run.reset_mock()
+    assert inkscape_version() == '1.2.2'
+    mock_run.assert_not_called()
+
+
+@mock.patch('cortex.dependencies.inkscape_path', return_value=None)
+def test_require_inkscape_missing(mock_path):
+    """Raises an error that points at the config option when inkscape is missing."""
+    with pytest.raises(RuntimeError, match=r'\[dependency_paths\]'):
+        require_inkscape()
+
+
+@mock.patch('cortex.dependencies.inkscape_path', return_value='/usr/bin/inkscape')
+@mock.patch('cortex.dependencies.sp.run')
+def test_require_inkscape_found(mock_run, mock_path):
+    """Returns the path and version together when inkscape is available."""
+    mock_run.return_value = _make_run_result(
+        stdout=b'Inkscape 1.2.2 (b0a8486, 2022-12-01)\n'
+    )
+    assert require_inkscape() == ('/usr/bin/inkscape', '1.2.2')

cortex/tests/test_nan_alpha.py

@@ -19,7 +19,7 @@
 
 import cortex
 from cortex import dataset
-from cortex.testing_utils import has_installed
+from cortex.dependencies import has_installed
 import cortex.utils
 from cortex.webgl.data import Package
 from cortex.webgl.serve import NPEncode

cortex/tests/test_quickflat.py

@@ -7,10 +7,10 @@
 from cortex import dataset
 import cortex.quickflat.utils  # for ty
 from cortex.svgoverlay import svgns
-from cortex.testing_utils import inkscapePath
+from cortex.dependencies import inkscape_path
 from cortex.webgl.data import Package
 
-no_inkscape = inkscapePath() is None
+no_inkscape = inkscape_path() is None
 
 
 def random_volume(with_nan=False, **kwargs):