test: harden the addon test suite (adversarial-review follow-ups)

Follow-ups from repeated adversarial review of the suite:

- Load test: display/GTK/environment failures are classified consistently
  (anchored signatures, unit-tested) and advisory by default; a genuine hard
  failure — or an import hang, detected via a REGISTRY_READY marker rather than
  a slow registry scan — always gates. One slow addon load can no longer abort
  the whole test.
- Strict mode (GRAMPS_ADDON_TEST_STRICT=1) turns the smoke run into a full
  gate: dependency skips, environment failures and unverified addon
  dependencies become hard failures; only slow registry-scan timeouts stay
  advisory. Covered by synthetic regression tests that don't boot the registry.
- Import/export smoke tests fail (not silently skip) when a dependency-satisfied
  plugin fails to load.
- Unlisted-addon manifest: source-based and per-registration (parses each
  .gpr.py with ast), so an accidental include_in_listing=False — even on one
  plugin inside a mixed directory like TMGimporter — fails the test.
- Neutralise blocking modal dialogs an addon may pop at import (e.g. lxml
  without python3-lxml) so a load can't hang or scatter UI.
- Silence expected locale / PyGI noise so results are readable.
- Failure messages lead with the real failures; make.py's test docstring and
  the load-test docstring document default (smoke) vs strict (full gate) modes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: eduralph

make.py

@@ -54,6 +54,21 @@
        Runs the repo-root `tests/` suite (unittest discovery for
        `test_*.py`). GRAMPSPATH must point at a Gramps checkout so the
        addon plugins are importable. Exits non-zero on any failure.
+
+       Default mode is a headless developer SMOKE run, not a full gate:
+         - only listed addons (include_in_listing=True) are load/version/
+           dependency checked; unlisted ones are covered only by the
+           listing-manifest test;
+         - failures that are really about the environment — no display, a
+           missing GI/typelib, a native GTK abort — are advisory, not fatal;
+         - tests needing extra tools skip (e.g. i18n extraction without
+           `xgettext`, GUI loads without a display).
+       For a full gate, run under a complete runtime (Xvfb + gettext + all
+       GI typelibs) with GRAMPS_ADDON_TEST_STRICT=1: unmet dependencies,
+       display/GTK failures and unverified addon dependencies then become
+       hard failures. (Slow registry-scan timeouts stay advisory — they are
+       a performance signal, not a correctness one; a genuine import hang is
+       always a hard failure.)
 """
 import shutil
 import glob
@@ -1079,6 +1094,12 @@ def register(ptype, **kwargs):
         extract_po(addon)
 
 elif command == "test":
+    import logging
+
+    # Quiet the "no compiled locale" warning Gramps logs at import: this
+    # command imports Gramps (via check_gramps_path) before the test package's
+    # own suppression runs, and translations are irrelevant to the tests.
+    logging.getLogger(".gramps.gen.utils.grampslocale").setLevel(logging.ERROR)
     check_gramps_path(command)
     import unittest
 

tests/__init__.py

@@ -6,8 +6,15 @@
 ``gramps.gui.*`` module directly never runs the launcher's own
 ``require_version``; without this the GI stack can resolve to GTK 4 on a host
 where that is the default — emitting ``PyGIWarning`` and risking the wrong stack.
+
+This module also runs before Gramps is imported, so it is where we silence
+log/warning spam that is expected when testing *uncompiled, source-tree* addons
+(no ``locale/*.mo``) and would otherwise bury the test results.
 """
 
+import logging
+import warnings
+
 try:
     import gi
 
@@ -17,3 +24,17 @@
     # No PyGObject, or the 3.0 typelibs are unavailable — leave the environment
     # untouched; this only fixes the version when it can.
     pass
+
+# Gramps warns once per addon that lacks a compiled locale dir (source-tree
+# addons ship ``po/*.po`` only), plus a startup warning about the main locale
+# dir — expected here, not a failure. Raising the logger to ERROR is robust:
+# logging levels are not reset by the test runners, so this holds on every
+# entrypoint.
+logging.getLogger(".gramps.gen.utils.grampslocale").setLevel(logging.ERROR)
+# PyGObject deprecation shout from gramps.gui.glade on import — not our concern.
+# NOTE: this warning *filter* is honoured by ``make.py test`` (TextTestRunner),
+# but a bare ``python -m unittest`` resets warning filters mid-run and may still
+# print it once. The documented entrypoint (make.py) stays clean; suppressing it
+# everywhere would mean importing gramps.gui.glade eagerly here (pulls in Gtk on
+# every run), which isn't worth it for one cosmetic line.
+warnings.filterwarnings("ignore", message=r".*shouldn't use __slots__.*")

tests/gramps_test_env.py

@@ -92,6 +92,47 @@ def _has_gtk() -> bool:
 _plugin_cache: dict[str, Any] = {}
 
 
+# Dialog classes an addon might instantiate at import / load-on-reg time.
+_GUI_DIALOG_NAMES = (
+    "ErrorDialog",
+    "WarningDialog",
+    "OkDialog",
+    "InfoDialog",
+    "QuestionDialog",
+    "QuestionDialog2",
+    "DBErrorDialog",
+    "RunDatabaseRepair",
+)
+
+def neutralize_gui_dialogs() -> None:
+    """Replace ``gramps.gui.dialog`` dialogs with no-ops for the test process.
+
+    Some addons pop a *blocking* modal (``ErrorDialog(...).run()``) at import or
+    load-on-reg time when an optional dependency is missing — e.g.
+    ``lxmlGramplet`` on a host without ``python3-lxml``. Loading such an addon
+    in a test would otherwise hang on the modal (under a display) or scatter
+    dialogs on the developer's screen. Safe to call more than once.
+    """
+    try:
+        import gramps.gui.dialog as gd
+    except Exception:
+        return
+
+    class _NoDialog:
+        def __init__(self, *args: Any, **kwargs: Any) -> None:
+            self.response = 0
+
+        def run(self, *args: Any, **kwargs: Any) -> int:
+            return 0
+
+        def __getattr__(self, name: str) -> Any:
+            return lambda *a, **k: None
+
+    for name in _GUI_DIALOG_NAMES:
+        if hasattr(gd, name):
+            setattr(gd, name, _NoDialog)
+
+
 def get_plugin_manager_and_registry() -> tuple[Any, Any]:
     """Return the Gramps plugin manager and registry, initialising on first call.
 
@@ -105,6 +146,9 @@ def get_plugin_manager_and_registry() -> tuple[Any, Any]:
         from gramps.gen.const import PLUGINS_DIR
         from gramps.gen.plug import BasePluginManager, PluginRegister
 
+        # reg_plugins(load_on_reg=True) runs addon load-on-reg callbacks in
+        # this process; neutralise dialogs first so none can block or show UI.
+        neutralize_gui_dialogs()
         pmgr = BasePluginManager.get_instance()
         pmgr.reg_plugins(PLUGINS_DIR, None, None)
         pmgr.reg_plugins(ADDONS_ROOT, None, None, load_on_reg=True)
@@ -123,6 +167,24 @@ def make_gramps_user() -> Any:
     return User(auto_accept=True, quiet=True)
 
 
+def strict_mode() -> bool:
+    """Whether ``GRAMPS_ADDON_TEST_STRICT`` opts into gating on advisory results.
+
+    Off by default so a headless developer run is not flaky on failures that
+    are really about the environment (no display, missing GI/typelib, a native
+    GTK abort). A CI lane with a full runtime can set the variable to promote
+    those advisories to hard failures — see the ``make.py test`` docs.
+
+    :returns: ``True`` when the variable is set to a truthy value.
+    """
+    return os.environ.get("GRAMPS_ADDON_TEST_STRICT", "").lower() in (
+        "1",
+        "true",
+        "yes",
+        "on",
+    )
+
+
 # ------------------------------------------------------------
 #
 # GrampsTestCase

tests/test_addon_dependency_loading.py

@@ -56,7 +56,7 @@
 # ------------------------
 # Gramps specific
 # ------------------------
-from tests.gramps_test_env import GrampsTestCase
+from tests.gramps_test_env import GrampsTestCase, strict_mode
 from tests.test_plugin_registration import _get_addon_plugins
 
 LOG = logging.getLogger(__name__)
@@ -92,6 +92,20 @@
     # -I so PYTHONPATH is ignored, and a run-from-source Gramps would
     # otherwise be unimportable. addon-from-addon isolation is unaffected.
     sys.path[:0] = [{target_dir!r}] + list({dep_dirs!r}) + list({base_dirs!r})
+    # Neutralise blocking modal dialogs so an addon that pops
+    # ErrorDialog(...).run() at import (missing optional dep) can't hang us.
+    try:
+        import gramps.gui.dialog as _gd
+        class _NoDialog:
+            def __init__(self, *a, **k): self.response = 0
+            def run(self, *a, **k): return 0
+            def __getattr__(self, n): return lambda *a, **k: None
+        for _n in ("ErrorDialog", "WarningDialog", "OkDialog", "InfoDialog",
+                   "QuestionDialog", "QuestionDialog2"):
+            if hasattr(_gd, _n):
+                setattr(_gd, _n, _NoDialog)
+    except Exception:
+        pass
     try:
         importlib.import_module({module!r})
     except BaseException:
@@ -150,6 +164,8 @@ def _missing_modules(stderr: str) -> set[str]:
     return {m.split(".")[0] for m in re.findall(r"No module named ['\"]([^'\"]+)", stderr)}
 
 
+
+
 # ------------------------------------------------------------
 #
 # TestAddonDependencyLoading
@@ -267,6 +283,7 @@ def test_declared_dependencies_are_load_bearing(self) -> None:
         base_dirs = [_gramps_root()]
         addon_modules = self._addon_module_names()
         insufficient: list[str] = []  # declared deps do NOT satisfy a sibling import
+        inconclusive: list[str] = []  # load failed environmentally — unverifiable
         load_bearing = 0  # dependents whose dependency is provably needed at load
         for pdata in dependents:
             dep_dirs: list[str] = []
@@ -292,12 +309,8 @@ def test_declared_dependencies_are_load_bearing(self) -> None:
                         f"not satisfy sibling import(s) {sorted(unmet)}"
                     )
                 else:
-                    LOG.info(
-                        "%s: isolated load failed for a non-dependency reason "
-                        "(environment); cannot verify its depends_on. Tail: %s",
-                        pdata.id,
-                        (err_with.strip().splitlines() or ["<none>"])[-1],
-                    )
+                    tail = (err_with.strip().splitlines() or ["<none>"])[-1]
+                    inconclusive.append(f"{pdata.id}: {tail}")
                 continue
 
             rc_without, err_without = _isolated_load(
@@ -319,18 +332,47 @@ def test_declared_dependencies_are_load_bearing(self) -> None:
                     pdata.id,
                 )
 
+        if inconclusive:
+            # The cause is environmental (GI/display/missing pip module), not a
+            # depends_on bug, so by default this is advisory — surfaced loudly
+            # and by count so an unverified dependent is never invisible behind
+            # an overall pass. A CI job that guarantees a full runtime (Xvfb,
+            # all deps) can set GRAMPS_ADDON_TEST_STRICT=1 to make any
+            # unverified dependent a failure instead.
+            LOG.warning(
+                "%d/%d dependent(s) could not be verified — isolated load "
+                "failed for environmental reasons (GI/display/missing pip "
+                "module):\n  %s",
+                len(inconclusive),
+                len(dependents),
+                "\n  ".join(inconclusive),
+            )
+            if strict_mode():
+                self.fail(
+                    f"{len(inconclusive)} of {len(dependents)} dependent(s) "
+                    "could not be verified and strict mode "
+                    "(GRAMPS_ADDON_TEST_STRICT) is on:\n  "
+                    + "\n  ".join(inconclusive)
+                )
+
         if insufficient:
             self.fail(
-                "Addons whose declared depends_on does NOT satisfy their "
-                "sibling imports (the loader would fail to load these):\n  "
+                f"{len(insufficient)} of {len(dependents)} dependent(s) have a "
+                "declared depends_on that does NOT satisfy their sibling "
+                "imports (the loader would fail to load these):\n  "
                 + "\n  ".join(insufficient)
+                + f"\n\nRun summary: {len(dependents)} dependents, "
+                f"{load_bearing} verified load-bearing, "
+                f"{len(inconclusive)} inconclusive (environmental), "
+                f"{len(insufficient)} insufficient."
             )
         self.assertGreater(
             load_bearing,
             0,
             "No addon demonstrated a load-time depends_on: every dependent "
-            "imported fine without its declared dependency, so this test "
-            "verified nothing about the loading mechanism.",
+            "either imported fine without its declared dependency or could "
+            f"not be verified ({len(inconclusive)} inconclusive), so this "
+            "test verified nothing about the loading mechanism.",
         )
 
 

tests/test_plugin_load_gate.py

@@ -94,21 +94,36 @@ def _fake_plugin() -> SimpleNamespace:
             requires_gi=[],
         )
 
-    def _run_load_test_with(self, run_result: SimpleNamespace):
+    def _run_load_test_with(
+        self,
+        run_result: SimpleNamespace,
+        missing_deps: list | None = None,
+        strict: bool = False,
+    ):
         """Drive ``test_load_all_addon_modules`` with one synthetic plugin.
 
-        The synthetic addon has no declared dependencies, so it is not skipped;
         ``subprocess.run`` is stubbed to ``run_result`` to simulate a chosen load
-        outcome. Returns the exception the production method raised, or ``None``
-        if it returned without raising (the silent-pass behaviour).
+        outcome; ``_check_dependencies`` returns ``missing_deps`` (empty by
+        default, so the plugin is not skipped). ``GRAMPS_ADDON_TEST_STRICT`` is
+        pinned — to ``"1"`` when ``strict`` else ``""`` — so the outcome never
+        depends on the ambient environment. Returns the exception the production
+        method raised, or ``None`` if it returned without raising.
         """
         loader = prod.TestPluginLoading("test_load_all_addon_modules")
         with mock.patch.object(
             prod, "_get_addon_plugins", return_value=[self._fake_plugin()]
         ), mock.patch.object(
-            prod, "_check_dependencies", return_value=[]
+            prod, "_check_dependencies", return_value=(missing_deps or [])
         ), mock.patch.object(
             prod.subprocess, "run", return_value=run_result
+        ), mock.patch.dict(
+            os.environ, {"GRAMPS_ADDON_TEST_STRICT": "1" if strict else ""}
+        ), mock.patch.object(
+            # Silence the production advisory logger: every warning here is about
+            # the synthetic fixture, so it must not leak next to the real tree's
+            # warnings in the test output.
+            prod,
+            "LOG",
         ):
             try:
                 loader.test_load_all_addon_modules()
@@ -145,6 +160,56 @@ def test_clean_load_does_not_gate(self) -> None:
             outcome, f"a clean load must not gate, but raised: {outcome!r}"
         )
 
+    def test_strict_mode_gates_missing_dependency(self) -> None:
+        """In strict mode a declared-but-missing dependency must gate.
+
+        By default a dependency skip is advisory; with
+        ``GRAMPS_ADDON_TEST_STRICT=1`` the full-runtime gate must promote it to a
+        hard failure. ``subprocess.run`` is never reached (the plugin is skipped
+        before load), so the outcome is driven purely by the missing dependency.
+        """
+        clean = SimpleNamespace(returncode=0, stderr="")
+        missing = ["gi:GExiv2-0.10"]
+
+        default = self._run_load_test_with(clean, missing_deps=missing, strict=False)
+        self.assertIsNone(
+            default, "a dependency skip must stay advisory in default mode"
+        )
+
+        strict = self._run_load_test_with(clean, missing_deps=missing, strict=True)
+        self.assertIsInstance(
+            strict,
+            prod.TestPluginLoading.failureException,
+            "strict mode must gate on a missing declared dependency",
+        )
+        self.assertIn("synthetic_broken_addon", str(strict))
+        self.assertIn("unmet dependency", str(strict))
+
+    def test_strict_mode_gates_environmental_failure(self) -> None:
+        """In strict mode an environment-classified load failure must gate.
+
+        A GTK/display init failure is advisory by default (it names a known
+        environmental signature); strict mode promotes it to a hard failure,
+        since a full runtime should provide the display/GTK stack.
+        """
+        env_fail = SimpleNamespace(
+            returncode=1, stderr="RuntimeError: Gtk couldn't be initialized"
+        )
+
+        default = self._run_load_test_with(env_fail, strict=False)
+        self.assertIsNone(
+            default, "an environmental failure must stay advisory in default mode"
+        )
+
+        strict = self._run_load_test_with(env_fail, strict=True)
+        self.assertIsInstance(
+            strict,
+            prod.TestPluginLoading.failureException,
+            "strict mode must gate on an environment-classified failure",
+        )
+        self.assertIn("synthetic_broken_addon", str(strict))
+        self.assertIn("environmental", str(strict))
+
 
 if __name__ == "__main__":
     unittest.main()