Merge pull request #909 from infinito-nexus/fix/lint/mermaid-browser-install/795

fix(lint): provision the mermaid browser in install-lint, not in the parallel lint run

tests/unit/python/scripts/lint/test_mermaid.py

@@ -0,0 +1,62 @@
+"""``scripts/lint/mermaid.sh`` renders with the browser install-lint provisioned.
+
+``make lint`` runs its targets in parallel, so the script must not reach for a
+package manager, and a browser that cannot start must fail the lint once with
+its reason instead of once per Markdown file.
+"""
+
+from __future__ import annotations
+
+import os
+import subprocess
+import tempfile
+import unittest
+from pathlib import Path
+
+from utils.cache.files import PROJECT_ROOT
+
+SCRIPT = Path(PROJECT_ROOT) / "scripts" / "lint" / "mermaid.sh"
+LIBRARY_ERROR = (
+    "chrome-headless-shell: error while loading shared libraries: libnspr4.so"
+)
+RECORDING_STUB = '#!/bin/sh\necho "$(basename "$0")" >> "$STUB_CALLS"\n'
+
+
+class TestMermaidLint(unittest.TestCase):
+    def test_a_browser_that_cannot_start_fails_before_any_file_renders(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            bin_dir = Path(tmp)
+            calls = bin_dir / "calls"
+            calls.touch()
+            for name in ("npx", "apt-get"):
+                (bin_dir / name).write_text(RECORDING_STUB)
+            (bin_dir / "mmdc").write_text(
+                f"{RECORDING_STUB}echo '{LIBRARY_ERROR}' >&2\nexit 1\n"
+            )
+            for stub in bin_dir.iterdir():
+                stub.chmod(0o755)
+            env = {
+                k: v
+                for k, v in os.environ.items()
+                if not k.startswith("INFINITO_") and k != "BASH_ENV"
+            }
+            env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}"
+            env["STUB_CALLS"] = str(calls)
+            proc = subprocess.run(
+                ["bash", str(SCRIPT)],
+                env=env,
+                capture_output=True,
+                text=True,
+                check=False,
+            )
+            recorded = calls.read_text().split()  # nocheck: cache-read  stub call log
+
+        self.assertEqual(proc.returncode, 1, proc.stderr)
+        self.assertIn("cannot render a minimal diagram", proc.stderr)
+        self.assertIn(LIBRARY_ERROR, proc.stderr)
+        self.assertNotIn("❌", proc.stdout)
+        self.assertEqual(recorded, ["mmdc"])
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/unit/python/utils/install/lint/test_main.py

@@ -34,12 +34,30 @@ class TestStampLogic(unittest.TestCase):
             with (
                 mock.patch.object(cli, "PROJECT_ROOT", root),
                 mock.patch.object(cli.shutil, "which", return_value="/usr/bin/x"),
+                mock.patch.object(
+                    cli.mermaid_cli, "browser_installed", return_value=True
+                ),
                 mock.patch.object(cli, "_install_all") as inst,
             ):
                 rc = cli.main([])
             self.assertEqual(rc, 0)
             inst.assert_not_called()
 
+    def test_missing_mermaid_browser_invalidates_stamp(self) -> None:
+        with TemporaryDirectory() as tmp:
+            root = _make_repo_root(tmp, stamp_age_offset=60.0)
+            with (
+                mock.patch.object(cli, "PROJECT_ROOT", root),
+                mock.patch.object(cli.shutil, "which", return_value="/usr/bin/x"),
+                mock.patch.object(
+                    cli.mermaid_cli, "browser_installed", return_value=False
+                ),
+                mock.patch.object(cli, "_install_all") as inst,
+            ):
+                rc = cli.main([])
+            self.assertEqual(rc, 0)
+            inst.assert_called_once()
+
     def test_missing_tool_invalidates_stamp(self) -> None:
         with TemporaryDirectory() as tmp:
             root = _make_repo_root(tmp, stamp_age_offset=60.0)
@@ -81,9 +99,10 @@ class TestStampLogic(unittest.TestCase):
             self.assertTrue((root / cli._STAMP).exists())
 
     def test_force_drops_stamp(self) -> None:
-        with TemporaryDirectory() as tmp:
+        with TemporaryDirectory() as tmp, TemporaryDirectory() as cache:
             root = _make_repo_root(tmp, stamp_age_offset=60.0)
             with (
+                mock.patch.dict(os.environ, {"PUPPETEER_CACHE_DIR": cache}),
                 mock.patch.object(cli, "PROJECT_ROOT", root),
                 mock.patch.object(cli, "_install_all") as inst,
             ):
@@ -91,6 +110,19 @@ class TestStampLogic(unittest.TestCase):
             self.assertEqual(rc, 0)
             inst.assert_called_once()
 
+    def test_force_drops_the_mermaid_browser(self) -> None:
+        with TemporaryDirectory() as tmp, TemporaryDirectory() as cache:
+            root = _make_repo_root(tmp, stamp_age_offset=60.0)
+            browser = Path(cache) / "chrome-headless-shell" / "linux-1"
+            browser.mkdir(parents=True)
+            with (
+                mock.patch.dict(os.environ, {"PUPPETEER_CACHE_DIR": cache}),
+                mock.patch.object(cli, "PROJECT_ROOT", root),
+                mock.patch.object(cli, "_install_all"),
+            ):
+                cli.main(["--force"])
+            self.assertFalse(browser.exists())
+
 
 class TestGroupDispatch(unittest.TestCase):
     def test_group_only_bypasses_stamp(self) -> None:

tests/unit/python/utils/install/lint/test_mermaid_cli.py

@@ -0,0 +1,128 @@
+"""Unit tests for :mod:`utils.install.lint.mermaid_cli`."""
+
+from __future__ import annotations
+
+import os
+import subprocess
+import unittest
+import unittest.mock as mock
+from pathlib import Path
+from tempfile import TemporaryDirectory
+
+from utils.install.lint import mermaid_cli
+
+
+def _done(returncode: int) -> subprocess.CompletedProcess[str]:
+    return subprocess.CompletedProcess([], returncode, stdout="out\n", stderr="err\n")
+
+
+class TestBrowserProvisioning(unittest.TestCase):
+    def provision(self, *, euid: int, results: list[int]) -> tuple[list, list[str]]:
+        """Run ``ensure()`` with mmdc present and a stubbed puppeteer.
+
+        Args:
+            euid: effective uid the run pretends to have.
+            results: exit codes the successive puppeteer calls return.
+
+        Returns:
+            The recorded subprocess calls and the warnings emitted.
+        """
+        warnings: list[str] = []
+        with (
+            TemporaryDirectory() as cache,
+            mock.patch.dict(os.environ, {"PUPPETEER_CACHE_DIR": cache}),
+            mock.patch.object(mermaid_cli, "which", return_value=True),
+            mock.patch.object(mermaid_cli.os, "geteuid", return_value=euid),
+            mock.patch.object(mermaid_cli, "warn", side_effect=warnings.append),
+            mock.patch.object(
+                mermaid_cli.subprocess,
+                "run",
+                side_effect=[_done(code) for code in results],
+            ) as run,
+        ):
+            mermaid_cli.ensure()
+        return run.call_args_list, warnings
+
+    def test_root_installs_the_browser_with_its_libraries(self) -> None:
+        calls, warnings = self.provision(euid=0, results=[0])
+        self.assertEqual(len(calls), 1)
+        self.assertEqual(
+            calls[0].args[0][-2:], ["chrome-headless-shell", "--install-deps"]
+        )
+        self.assertEqual(warnings, [])
+
+    def test_a_user_installs_the_browser_without_libraries(self) -> None:
+        calls, _ = self.provision(euid=1000, results=[0])
+        self.assertNotIn("--install-deps", calls[0].args[0])
+
+    def test_the_browser_install_downloads_despite_the_mmdc_skip_flag(self) -> None:
+        with mock.patch.dict(os.environ, {"PUPPETEER_SKIP_DOWNLOAD": "1"}):
+            calls, _ = self.provision(euid=0, results=[0])
+        self.assertNotIn("PUPPETEER_SKIP_DOWNLOAD", calls[0].kwargs["env"])
+
+    def test_a_failed_install_clears_the_browser_cache_and_retries_once(self) -> None:
+        with TemporaryDirectory() as cache:
+            folder = Path(cache) / "chrome-headless-shell" / "linux-1"
+            folder.mkdir(parents=True)
+            seen: list[bool] = []
+
+            def run(*_args, **_kwargs):
+                seen.append(folder.exists())
+                return _done(1 if len(seen) == 1 else 0)
+
+            with (
+                mock.patch.dict(os.environ, {"PUPPETEER_CACHE_DIR": cache}),
+                mock.patch.object(mermaid_cli, "which", return_value=True),
+                mock.patch.object(mermaid_cli.subprocess, "run", side_effect=run),
+            ):
+                mermaid_cli.ensure()
+        self.assertEqual(seen, [True, False])
+
+    def test_a_second_failure_warns_with_puppeteers_output(self) -> None:
+        calls, warnings = self.provision(euid=0, results=[1, 1])
+        self.assertEqual(len(calls), 2)
+        self.assertEqual(len(warnings), 1)
+        self.assertIn("out\nerr\n", warnings[0])
+
+    def test_an_installed_browser_is_not_provisioned_again(self) -> None:
+        with TemporaryDirectory() as cache:
+            build = Path(cache) / "chrome-headless-shell" / "linux-1" / "linux64"
+            build.mkdir(parents=True)
+            (build / "chrome-headless-shell").write_text("", encoding="utf-8")
+            with (
+                mock.patch.dict(os.environ, {"PUPPETEER_CACHE_DIR": cache}),
+                mock.patch.object(mermaid_cli, "which", return_value=True),
+                mock.patch.object(mermaid_cli.subprocess, "run") as run,
+            ):
+                mermaid_cli.ensure()
+        run.assert_not_called()
+
+    def test_a_timeout_warns_instead_of_raising(self) -> None:
+        with (
+            TemporaryDirectory() as cache,
+            mock.patch.dict(os.environ, {"PUPPETEER_CACHE_DIR": cache}),
+            mock.patch.object(mermaid_cli, "which", return_value=True),
+            mock.patch.object(
+                mermaid_cli.subprocess,
+                "run",
+                side_effect=subprocess.TimeoutExpired("npx", 900),
+            ),
+            mock.patch.object(mermaid_cli, "warn") as warn,
+        ):
+            mermaid_cli.ensure()
+        warn.assert_called_once()
+
+
+class TestBrowserInstalled(unittest.TestCase):
+    def test_the_executable_in_the_puppeteer_cache_counts(self) -> None:
+        with TemporaryDirectory() as cache:
+            build = Path(cache) / "chrome-headless-shell" / "linux-1" / "linux64"
+            build.mkdir(parents=True)
+            with mock.patch.dict(os.environ, {"PUPPETEER_CACHE_DIR": cache}):
+                self.assertFalse(mermaid_cli.browser_installed())
+                (build / "chrome-headless-shell").write_text("", encoding="utf-8")
+                self.assertTrue(mermaid_cli.browser_installed())
+
+
+if __name__ == "__main__":
+    unittest.main()