ci-scripts: fix same-named-addon-module shadowing; honour module-level SkipTest

Two real defects the fork CI run surfaced, both pre-existing (the original
runner failed identically) but newly hit by the post-merge addon set:

1. Same-name shadowing. The worker appends the addon's directory to sys.path
   so its tests can import sibling modules by bare name (WebSearch's
   'from models import …'). But most addons also ship <Addon>/<Addon>.py, and
   once <Addon>/ is on sys.path that regular module wins the bare name over the
   namespace-package directory — so loading the dotted test name
   <Addon>.tests.test_x died with "module '<Addon>' has no attribute 'tests'".
   11 real addons failed this way (CalculateEstimatedDates, WebSearch,
   libaccess, …). Import the addon package from the repo root FIRST, pinning it
   in sys.modules, before the addon dir joins sys.path; the dotted load then
   resolves and the bare sibling imports still work. Reproduced and killed with
   a synthetic same-named-module fixture.

2. A module-level 'raise SkipTest(...)' — an addon's own "needs a display /
   PyGObject" guard — was classified as a code bug and failed the run. It is an
   explicit opt-out and is now honoured as a skip on every platform, regardless
   of declared-dep satisfiability.

The load classifier gained a third outcome (skip | dep | other) and now reads
the terminal exception out of unittest's wrapped-ImportError message, so a
wrapped SkipTest/SyntaxError is classified by what actually happened.

Author: eduralph

.github/scripts/run_addon_tests.py

@@ -28,7 +28,9 @@
    lives in ``addon_system_deps.py``). A module that fails to LOAD is excused as
    a platform skip only when the failure is dependency-shaped (ImportError, or
    gi's absent-typelib ValueError); a SyntaxError or a bug in the addon's own
-   import-time code is a real defect and always FAILS, on every platform.
+   import-time code is a real defect and always FAILS, on every platform. A
+   module that raises ``SkipTest`` at import is opting out explicitly and is
+   always honoured as a skip.
 
 Usage::
 
@@ -46,6 +48,7 @@
 from __future__ import annotations
 
 import argparse
+import importlib
 import os
 import re
 import signal
@@ -85,14 +88,28 @@ def _module_timeout() -> int:
 
 def _terminal_exc_name(text: str) -> str | None:
     """The exception class name on the last ``Type: message`` line of a
-    traceback string (``ModuleNotFoundError``, ``SyntaxError``, …), or None."""
+    traceback string (``ModuleNotFoundError``, ``SyntaxError``, ``SkipTest``…),
+    or None. Scans from the end, so the first match is the terminal line."""
     for line in reversed(text.strip().splitlines()):
-        m = re.match(r"^([A-Za-z_][\w.]*(?:Error|Exception|Warning)):", line.strip())
+        m = re.match(r"^([A-Za-z_][\w.]*)\s*:", line.strip())
         if m:
             return m.group(1).rsplit(".", 1)[-1]
     return None
 
 
+def _is_skip_request(exc: BaseException) -> bool:
+    """Whether a module opted out at import time via ``raise SkipTest(...)``.
+
+    A module-level SkipTest is an explicit, deliberate opt-out (the addon's own
+    guard for "this needs a display / PyGObject / a backend I don't have"), so it
+    is honoured as a skip on every platform — never a failure, and never
+    dependent on the addon's declared system deps."""
+    if isinstance(exc, unittest.SkipTest):
+        return True
+    # unittest's loader may wrap it (see _dep_shaped) — check the terminal type.
+    return isinstance(exc, ImportError) and _terminal_exc_name(str(exc)) == "SkipTest"
+
+
 def _dep_shaped(exc: BaseException) -> bool:
     """Whether a module load failure is a missing-dependency shape.
 
@@ -121,6 +138,13 @@ def _dep_shaped(exc: BaseException) -> bool:
     return False
 
 
+def _load_kind(exc: BaseException) -> str:
+    """Classify a module load failure: skip | dep | other."""
+    if _is_skip_request(exc):
+        return "skip"
+    return "dep" if _dep_shaped(exc) else "other"
+
+
 def _load_failure_exception(suite: unittest.TestSuite):
     """The exception behind a deferred import failure, or None.
 
@@ -187,6 +211,20 @@ def _run_worker(modname: str, root: str = ".") -> int:
     # working too.
     addon = modname.split(".", 1)[0]
     addon_dir = os.path.join(root, addon)
+    # Resolve the addon PACKAGE (its directory, from the repo root) before the
+    # addon dir joins sys.path. Many addons ship a top-level module named after
+    # the addon itself (<Addon>/<Addon>.py); the moment <Addon>/ is on sys.path
+    # that regular module wins the bare name <Addon> over the namespace package,
+    # and the dotted test name <Addon>.tests.test_x then dies with "module
+    # '<Addon>' has no attribute 'tests'". Importing the package first pins it in
+    # sys.modules so the dotted load resolves, while the addon dir added below
+    # still serves the tests' bare sibling imports (e.g. `from models import …`).
+    try:
+        importlib.import_module(addon)
+    except Exception:
+        # Not importable as a package (single-file addon, or its __init__ needs
+        # deps): leave it; the dotted load below reports the real failure.
+        pass
     if addon_dir not in sys.path:
         sys.path.append(addon_dir)
     # Load via unittest (NOT a bare import_module probe): an addon whose
@@ -202,12 +240,12 @@ def _run_worker(modname: str, root: str = ".") -> int:
     try:
         suite = unittest.defaultTestLoader.loadTestsFromName(modname)
     except Exception as exc:  # raised import-time failure
-        kind = "dep" if _dep_shaped(exc) else "other"
+        kind = _load_kind(exc)
         print(f"{_LOADERROR} kind={kind} {exc!r}", flush=True)
         return 0
     load_exc = _load_failure_exception(suite)  # deferred (_FailedTest) failure
     if load_exc is not None:
-        kind = "dep" if _dep_shaped(load_exc) else "other"
+        kind = _load_kind(load_exc)
         print(f"{_LOADERROR} kind={kind} {load_exc!r}", flush=True)
         return 0
     result = unittest.TextTestRunner(verbosity=2).run(suite)
@@ -276,7 +314,12 @@ def _classify(modname: str, platform: str, root: str) -> tuple[bool, str]:
 
     if result_line.startswith(_LOADERROR):
         kind_m = re.search(r"\bkind=(\w+)", result_line)
-        dep_shaped = bool(kind_m) and kind_m.group(1) == "dep"
+        kind = kind_m.group(1) if kind_m else ""
+        if kind == "skip":
+            # The module raised SkipTest at import: an explicit opt-out, honoured
+            # on every platform regardless of declared-dep satisfiability.
+            return False, f"  skip  {modname} — module opted out (SkipTest at import)"
+        dep_shaped = kind == "dep"
         if not dep_shaped:
             # A non-dependency load failure (SyntaxError, a bug in the addon's
             # import-time code) is a real defect on every platform — never

tests/test_run_addon_tests_paths.py

@@ -228,6 +228,45 @@ def test_non_integer_timeout_env_is_tolerated(self) -> None:
         self.assertEqual(result.returncode, 0, out)
         self.assertIn("ignoring non-integer", result.stderr)
 
+    def test_same_named_addon_module_does_not_shadow_package(self) -> None:
+        # Regression: many addons ship <Addon>/<Addon>.py. Once <Addon>/ is on
+        # sys.path (for the tests' bare sibling imports) that regular module wins
+        # the bare name over the namespace-package directory, and the dotted test
+        # name <Addon>.tests.test_x died with "module '<Addon>' has no attribute
+        # 'tests'" — 11 real addons failed this way on CI.
+        self._write("ShadowAddon/shadowaddon.gpr.py", 'register(GRAMPLET, id="s")\n')
+        self._write("ShadowAddon/ShadowAddon.py", "MAIN = 'addon main module'\n")
+        self._write("ShadowAddon/sibling.py", "SIB = 5\n")
+        self._write("ShadowAddon/tests/__init__.py", "")
+        self._write(
+            "ShadowAddon/tests/test_x.py",
+            "import unittest\n"
+            "from sibling import SIB\n"  # bare sibling import needs addon dir
+            "\n"
+            "class T(unittest.TestCase):\n"
+            "    def test_sibling(self):\n"
+            "        self.assertEqual(SIB, 5)\n",
+        )
+        result = self._run("ShadowAddon.tests.test_x", platform="apt")
+        out = result.stdout + result.stderr
+        self.assertEqual(result.returncode, 0, out)
+        self.assertIn("ok    ShadowAddon.tests.test_x", result.stdout, out)
+
+    def test_module_level_skiptest_is_honored(self) -> None:
+        # A module that raises SkipTest at import is explicitly opting out (the
+        # addon's own "needs a display / PyGObject" guard) — honour it as a skip
+        # on every platform, never a failure.
+        self._write("SkipAddon/skipaddon.gpr.py", 'register(GRAMPLET, id="s")\n')
+        self._write("SkipAddon/tests/__init__.py", "")
+        self._write(
+            "SkipAddon/tests/test_x.py",
+            "import unittest\nraise unittest.SkipTest('no display here')\n",
+        )
+        result = self._run("SkipAddon.tests.test_x", platform="apt")
+        out = result.stdout + result.stderr
+        self.assertEqual(result.returncode, 0, out)
+        self.assertIn("opted out", out)
+
     @unittest.skipUnless(os.name == "posix", "process-group kill is POSIX-only")
     def test_timeout_reaps_grandchild_holding_stdout(self) -> None:
         # A test that spawns a long-lived child inheriting the worker's stdout