Skip to content

Commit b9425e0

Browse files
committed
fix(test): restore the parent-package attribute after a blocked re-import
The last two CI failures. test_routing.py's live-detection cases failed only in a full-suite run, and which cases failed depended on whether the machine had a GPU - so they passed locally and failed on the runner, which read as a CI-only mystery. It was an ordering bug. `_reimport_without` deletes a submodule from sys.modules, re-imports it with a dependency blocked, and restores sys.modules in a finally. But importing `pkg.sub` also rebinds the fresh module onto its parent as `pkg.sub`, and nothing undid that. The throwaway module therefore outlived the helper as an attribute of the real package, permanently disagreeing with sys.modules for the rest of the session: sys.modules["qector_decoder_v3.gpu_backend"] -> original qector_decoder_v3.gpu_backend -> throwaway `routing` holds `from . import gpu_backend as _gb`, bound at import to the original, and that is what `HardwareProfile.detect()` calls. monkeypatch reached the throwaway - by either spelling, since pytest's resolve() walks parents with getattr, which is why switching to the dotted-path form in b0f5456 changed nothing. The patch was applied to a module nothing under test read, so detect() kept reporting real hardware. Restore the parent attribute alongside sys.modules. Verified by causation, not by hope: with the fix, the polluting file followed by test_routing passes 91/91; with it stashed, the same command fails 2 - locally the [False-*] cases, on CI the [*-True] ones, the difference being the host's actual GPU, exactly as the mechanism predicts. Also corrects the comment added in b0f5456, whose explanation was wrong.
1 parent 1a1af6e commit b9425e0

2 files changed

Lines changed: 32 additions & 6 deletions

File tree

python/tests/test_optional_import_degradation.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,26 @@ def _reimport_without(module_name: str, blocked: list[str]):
5858
finally:
5959
sys.meta_path.remove(finder)
6060
sys.modules.update(saved)
61+
# Restoring sys.modules is not enough. Importing `pkg.sub` also rebinds
62+
# the fresh module onto its PARENT as `pkg.sub`, and nothing above undoes
63+
# that - so the throwaway module outlives this helper as an attribute of
64+
# the real package while sys.modules holds the original. The two then
65+
# disagree, permanently, for the rest of the session:
66+
#
67+
# sys.modules["qector_decoder_v3.gpu_backend"] -> original
68+
# qector_decoder_v3.gpu_backend -> throwaway
69+
#
70+
# That broke test_routing.py's live-detection cases, and did so only in a
71+
# full-suite run and only on a machine without a GPU, which is why it
72+
# looked like a CI-only mystery. `routing` holds `from . import
73+
# gpu_backend as _gb` (the original), while monkeypatch - by either
74+
# spelling, since pytest's resolve() walks parents with getattr - reached
75+
# the throwaway. The patch applied to a module nothing under test used.
76+
for name, module in saved.items():
77+
parent_name, _, child = name.rpartition(".")
78+
parent = sys.modules.get(parent_name) if parent_name else None
79+
if parent is not None and getattr(parent, child, None) is not module:
80+
setattr(parent, child, module)
6181

6282

6383
@pytest.mark.parametrize(("module_name", "blocked"), OPTIONAL_IMPORT_MATRIX)

python/tests/test_routing.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,18 @@ def test_code_object_is_classified_structurally():
137137
# ---------------------------------------------------------------------------
138138
@pytest.mark.parametrize("cuda_rust,gpu", [(False, False), (False, True), (True, True)])
139139
def test_detected_hardware_drives_huge_batch(monkeypatch, cuda_rust, gpu):
140-
# Patch the canonical module by dotted path, NOT via `qd.gpu_backend`.
141-
# `HardwareProfile.detect()` reads `routing._gb.has_cuda_rust()`, where `_gb`
142-
# is `qector_decoder_v3.gpu_backend` bound from sys.modules. The package-level
143-
# attribute `qd.gpu_backend` is not reliably that same object - in CI it is
144-
# shadowed, so patching it left `detect()` reading the real hardware and these
145-
# two cases failed on any machine without a GPU while passing on one with.
140+
# `HardwareProfile.detect()` reads `routing._gb.<probe>()`, where `_gb` is the
141+
# `qector_decoder_v3.gpu_backend` entry in sys.modules.
142+
#
143+
# Either spelling below works, but only because test_optional_import_degradation
144+
# now restores the parent-package attribute it rebinds. Before that fix its
145+
# re-import left `qector_decoder_v3.gpu_backend` pointing at a throwaway module
146+
# while sys.modules kept the original, and monkeypatch - which resolves a dotted
147+
# path with getattr on the parent, so both spellings land in the same place -
148+
# patched the throwaway. `detect()` went on reading the real hardware, and these
149+
# cases failed in a full-suite run while passing in isolation. Whichever cases
150+
# failed depended on the machine's actual GPU, which is what made it look like a
151+
# CI-only mystery rather than an ordering bug.
146152
monkeypatch.setattr("qector_decoder_v3.gpu_backend.has_cuda_rust", lambda: cuda_rust)
147153
monkeypatch.setattr("qector_decoder_v3.gpu_backend.gpu_available", lambda: gpu)
148154

0 commit comments

Comments
 (0)