Skip to content

Commit 2a2f778

Browse files
committed
ci-scripts: load addon tests via unittest, not a raw import_module probe
The import-probe added for the load-failure taxonomy used importlib.import_module(modname), which is fragile: the worker appends the addon's own directory to sys.path so the addon's tests can import its top-level modules, and for an addon whose main module shares the addon's directory name (e.g. CalculateEstimatedDates/CalculateEstimatedDates.py) that appended entry shadows the namespace-package directory, so a dotted import_module resolves CalculateEstimatedDates to the .py MODULE and 'CalculateEstimatedDates.tests' then raises 'not a package'. 11 real addons failed this way on CI (gramps-independent — the fresh image would fail too). unittest's own loadTestsFromName resolves the package correctly, so load via it and classify the load failure from whichever form it takes: a RAISED exception (some Python versions raise SyntaxError) or a DEFERRED _FailedTest placeholder (ImportError, wrapped as an ImportError whose message embeds the original traceback). _dep_shaped inspects the terminal exception in that embedded traceback so a wrapped SyntaxError is still classified as a code bug, not a dependency skip. Verified across the F2/F3/F4 fixtures.
1 parent 1127965 commit 2a2f778

1 file changed

Lines changed: 77 additions & 16 deletions

File tree

.github/scripts/run_addon_tests.py

Lines changed: 77 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
from __future__ import annotations
4747

4848
import argparse
49-
import importlib
5049
import os
5150
import re
5251
import signal
@@ -84,18 +83,72 @@ def _module_timeout() -> int:
8483
_LOADERROR = "__RESULT__ loaderror"
8584

8685

86+
def _terminal_exc_name(text: str) -> str | None:
87+
"""The exception class name on the last ``Type: message`` line of a
88+
traceback string (``ModuleNotFoundError``, ``SyntaxError``, …), or None."""
89+
for line in reversed(text.strip().splitlines()):
90+
m = re.match(r"^([A-Za-z_][\w.]*(?:Error|Exception|Warning)):", line.strip())
91+
if m:
92+
return m.group(1).rsplit(".", 1)[-1]
93+
return None
94+
95+
8796
def _dep_shaped(exc: BaseException) -> bool:
8897
"""Whether a module load failure is a missing-dependency shape.
8998
9099
Only these may be excused as an expected platform skip when the addon's
91-
declared deps are unavailable: an ``ImportError`` (missing module), or the
92-
``ValueError`` ``gi.require_version`` raises for an absent typelib
93-
(``Namespace X not available``). Everything else — SyntaxError, a bug in the
94-
addon's own import-time code — is a real defect and must never be excused.
100+
declared deps are unavailable: a missing module (ImportError /
101+
ModuleNotFoundError), or the ``ValueError`` ``gi.require_version`` raises for
102+
an absent typelib (``Namespace X not available``). A SyntaxError or a bug in
103+
the addon's own import-time code is a real defect and must never be excused.
104+
105+
Complication: ``unittest``'s loader wraps EVERY import-time exception —
106+
including SyntaxError — into an ``ImportError`` whose message embeds the
107+
original traceback (``Failed to import test module: …``). So for that
108+
wrapper we cannot go by type; inspect the terminal exception in the embedded
109+
traceback instead.
95110
"""
111+
if isinstance(exc, ValueError):
112+
return "not available" in str(exc)
96113
if isinstance(exc, ImportError):
97-
return True
98-
return isinstance(exc, ValueError) and "not available" in str(exc)
114+
msg = str(exc)
115+
if "Failed to import test module" in msg:
116+
terminal = _terminal_exc_name(msg)
117+
# Unknown terminal → treat as dep (tolerant: a missing dep is the
118+
# common case and we would rather skip than falsely fail).
119+
return terminal in (None, "ImportError", "ModuleNotFoundError")
120+
return True # a bare ImportError (not the loader wrapper)
121+
return False
122+
123+
124+
def _load_failure_exception(suite: unittest.TestSuite):
125+
"""The exception behind a deferred import failure, or None.
126+
127+
``loadTestsFromName`` wraps a module that fails to import in a
128+
``unittest.loader._FailedTest`` placeholder carrying the original exception
129+
in ``_exception``. Walk the suite for one and return that exception, so the
130+
caller can classify the failure instead of running a placeholder that errors
131+
anonymously. Private-API tolerant: if the internals ever change, return None
132+
and let the suite run (the pre-taxonomy behaviour)."""
133+
try:
134+
from unittest.loader import _FailedTest
135+
except Exception:
136+
return None
137+
138+
def _walk(test):
139+
if isinstance(test, unittest.TestSuite):
140+
for child in test:
141+
found = _walk(child)
142+
if found is not None:
143+
return found
144+
return None
145+
if isinstance(test, _FailedTest):
146+
return getattr(test, "_exception", None) or ImportError(
147+
"module failed to load"
148+
)
149+
return None
150+
151+
return _walk(suite)
99152

100153

101154
def _bootstrap_gi() -> None:
@@ -136,19 +189,27 @@ def _run_worker(modname: str, root: str = ".") -> int:
136189
addon_dir = os.path.join(root, addon)
137190
if addon_dir not in sys.path:
138191
sys.path.append(addon_dir)
139-
# Probe the import EXPLICITLY first. loadTestsFromName does not raise on a
140-
# module-level ImportError/SyntaxError — since Python 3.5 it swallows the
141-
# error into a _FailedTest placeholder that only errors when run, so the
142-
# failure would reach the parent as an anonymous `broke=1` with its shape
143-
# lost. Importing here surfaces the real exception so it can be classified
144-
# (dependency-shaped vs a code bug).
192+
# Load via unittest (NOT a bare import_module probe): an addon whose
193+
# top-level module shares the addon's directory name (e.g.
194+
# CalculateEstimatedDates/CalculateEstimatedDates.py) is shadowed by
195+
# addon_dir being on sys.path, which breaks a dotted import_module but not
196+
# unittest's own resolution. A load failure surfaces two ways depending on
197+
# the Python version and error kind — loadTestsFromName may RAISE it, or
198+
# defer it into a _FailedTest placeholder that errors only when run (so the
199+
# parent would otherwise see an anonymous `broke=1` with the shape lost).
200+
# Handle both, and classify the real exception (dependency-shaped vs a code
201+
# bug) either way.
145202
try:
146-
importlib.import_module(modname)
147-
except Exception as exc: # import-time failure
203+
suite = unittest.defaultTestLoader.loadTestsFromName(modname)
204+
except Exception as exc: # raised import-time failure
148205
kind = "dep" if _dep_shaped(exc) else "other"
149206
print(f"{_LOADERROR} kind={kind} {exc!r}", flush=True)
150207
return 0
151-
suite = unittest.defaultTestLoader.loadTestsFromName(modname)
208+
load_exc = _load_failure_exception(suite) # deferred (_FailedTest) failure
209+
if load_exc is not None:
210+
kind = "dep" if _dep_shaped(load_exc) else "other"
211+
print(f"{_LOADERROR} kind={kind} {load_exc!r}", flush=True)
212+
return 0
152213
result = unittest.TextTestRunner(verbosity=2).run(suite)
153214
broke = len(result.failures) + len(result.errors)
154215
print(

0 commit comments

Comments
 (0)