tests: fix wx sys.modules isolation in linux_launcher/radiothread mocks

Root cause: test_wxui_linux_launcher.py and test_wxui_radiothread.py
each mock sys.modules['wx'] (and related entries) at import time to
run without a real wx installation, but never restored it. Importing
either one transitively imports chirp.wxui.common, which subclasses
real wx types -- so those classes get built against whatever wx (real
or fake) was current at that moment, and stay cached that way in
sys.modules (and as an attribute of the chirp.wxui package object,
which "from chirp.wxui import common" consults before sys.modules).
Any test file collected afterward that triggers a fresh
`from chirp.wxui import memedit` (chirp/wxui/memedit.py:1021 combines
common.ChirpEditor and common.ChirpSyncEditor) inherits whatever was
left cached, raising "TypeError: metaclass conflict" if it was fake.
This is why PR #7's required "Unit tests" CI check was failing:
master's own test_wxui_linux_launcher.py (unrelated to the Programming
Assistant work in that PR) collides with any real-wx test file
collected after it.

Confirmed via direct reproduction, not assumption: a minimal repro
(these two files' module-level code, executed directly) reproduces
the exact conflict; restoring only sys.modules['wx'] afterward still
reproduces it (the chirp.wxui.common caching is the actual culprit);
restoring the sys.modules entry without also clearing the parent
package's attribute reference still reproduces it one level further
down the import chain (chirp/wxui/developer.py); and the reverse
collection order (a real-wx import first, then linux_launcher.py)
independently breaks in the other direction if chirp.wxui.common
isn't evicted *before* this file's own mock-based imports too, not
just after.

Fix, applied to both files: evict chirp.wxui.* submodules (and their
parent-package attribute references, but never the bare chirp.wxui
package itself -- it defines no wx-based classes at import time, and
evicting it broke chirp/wxui/__init__.py's own maybe_install_desktop()
in an unrelated way) both before this file's own mocked imports run
and immediately after, synchronously as part of this file's own
collection rather than via a pytest fixture (which only runs during
test execution, well after pytest has already finished collecting
every file, too late to protect a later file's own collection).

test_wxui_radiothread.py needed one more piece: its own TestStartup
tests exercise chirp/wxui/__init__.py's maybe_install_desktop(),
which does a lazy `import wx` inside the function body at call time,
so those specific tests need sys.modules['wx'] to still be this
file's own mock during execution, not just collection -- immediately
restoring it (safe for every other test in this file) broke them.
Fixed by re-installing that same mock object scoped to just
TestStartup's own tests, via plain setUp()/addCleanup() save-restore
(mock.patch.dict was tried first and confirmed, by direct
reproduction, to trip an unrelated pytest/importlib.resources
caching interaction -- manual save/restore does not).

No production code changed: this is entirely a test-isolation defect,
confirmed by reproducing it with unmodified production code across
several combinations before touching anything.

Added tests/unit/test_wx_module_isolation_regression.py: since the
defect is fundamentally about what one test file's module-level code
leaves in sys.modules for whichever file pytest collects next, it can
only be verified across process/collection boundaries -- not from
calls made within an already-running test. Each new test launches an
isolated subprocess exercising a specific file combination and order
(individually, both orders against chirp.wxui.memedit -- production
code, present regardless of any particular feature's own test file --
and the exact three-file combination and order from the original
report) and asserts a clean exit with no collection errors. Confirmed
these tests actually catch the defect: 3 of the 7 fail with the exact
originally-reported symptom when run against this commit's parent.

Validation: pytest tests/unit -k "not network" -ra: 680 passed (up
from 673), 12 skipped, 10 deselected. cpep8 and mypy clean (one
pre-existing, unrelated mypy note in tests/unit/base.py, confirmed
present on origin/master before this change). check_commit.sh
origin/master: exit 0.
This commit is contained in:
David Davis 2026-07-28 10:33:05 -07:00 committed by GitHub
parent 24900311d8
commit f174ff2ceb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 397 additions and 0 deletions

View file

@ -0,0 +1,164 @@
"""Regression coverage for the wx sys.modules isolation defect fixed
in test_wxui_linux_launcher.py and test_wxui_radiothread.py.
Both of those files mock sys.modules['wx'] (and related entries) at
module import time, to let their own tests run without a real wx
installation. The defect: importing either of them also transitively
imports chirp.wxui.common, which subclasses real wx types --
producing classes built against whichever wx (real or fake) happened
to be in sys.modules at that moment, cached under chirp.wxui.common's
own sys.modules entry (and as an attribute of the chirp.wxui package
object) for the rest of the process. Any test file collected
afterward that triggers a fresh `from chirp.wxui import memedit`
(chirp/wxui/memedit.py:1021 defines `class ChirpMemEdit(common.
ChirpEditor, common.ChirpSyncEditor)`) inherited whatever was cached,
including a fake one -- raising "TypeError: metaclass conflict" if
chirp.wxui.common was left built against a MagicMock.
This can only be verified across process/collection boundaries -- the
underlying bug is entirely about what state a *separate* test file's
module-level code leaves in sys.modules for whichever file pytest
collects next, which isn't observable from calls made within a single
already-running test. Each test here launches a fresh, isolated
pytest subprocess with a specific combination and ordering of test
files (mirroring the exact CI invocation: `pytest tests/unit -k "not
network"`, scoped down to just the files relevant to this defect) and
asserts it exits 0 with no collection errors.
"""
import os
import subprocess
import sys
import unittest
_REPO_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(
os.path.abspath(__file__))))
class WxModuleIsolationRegressionTest(unittest.TestCase):
def _run_pytest(self, *relative_test_paths):
env = dict(os.environ)
env['CHIRP_TESTENV'] = '1'
env['PYTHONPATH'] = _REPO_ROOT
result = subprocess.run(
[sys.executable, '-m', 'pytest', '-q', *relative_test_paths],
cwd=_REPO_ROOT, env=env,
stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
text=True, timeout=120)
return result
def _assert_clean_pass(self, result, context):
self.assertEqual(
0, result.returncode,
'%s: expected a clean pytest run (exit 0), got %i:\n%s' % (
context, result.returncode, result.stdout))
self.assertNotIn('metaclass conflict', result.stdout, context)
self.assertNotIn('ERROR collecting', result.stdout, context)
def test_linux_launcher_individually(self):
result = self._run_pytest('tests/unit/test_wxui_linux_launcher.py')
self._assert_clean_pass(result, 'linux_launcher alone')
def test_radiothread_individually(self):
result = self._run_pytest('tests/unit/test_wxui_radiothread.py')
self._assert_clean_pass(result, 'radiothread alone')
def test_linux_launcher_then_real_memedit_import_succeeds(self):
# A stand-in for "a real-wx test file collected after
# linux_launcher.py, that imports chirp.wxui.memedit" --
# chirp.wxui.memedit is production code, present regardless of
# whether any particular feature's own test file exists on
# this branch, so this doesn't depend on one.
result = self._run_pytest(
'tests/unit/test_wxui_linux_launcher.py', '-p', 'no:cacheprovider',
'--co')
self._assert_clean_pass(result, 'linux_launcher collect-only')
result = subprocess.run(
[sys.executable, '-c',
'import sys\n'
'import tests.unit.test_wxui_linux_launcher\n'
'from chirp.wxui import memedit\n'
'assert "MagicMock" not in repr(type(memedit.common.'
'ChirpEditor)), memedit.common.ChirpEditor\n'
'sys.stdout.write("OK\\n")'],
cwd=_REPO_ROOT,
env=dict(os.environ, CHIRP_TESTENV='1', PYTHONPATH=_REPO_ROOT),
stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True,
timeout=60)
self.assertEqual(0, result.returncode, result.stdout)
self.assertIn('OK', result.stdout)
def test_real_memedit_import_then_linux_launcher_succeeds(self):
# The reverse order: a real-wx module imported first, then
# linux_launcher.py collected afterward -- confirmed to be a
# distinct failure mode from the forward order (linux_launcher.
# py's own tests broke, exercising the real wx.MessageDialog,
# if chirp.wxui.common was already cached for real).
result = subprocess.run(
[sys.executable, '-c',
'import sys\n'
'from chirp.wxui import memedit\n'
'import tests.unit.test_wxui_linux_launcher\n'
'sys.stdout.write("OK\\n")'],
cwd=_REPO_ROOT,
env=dict(os.environ, CHIRP_TESTENV='1', PYTHONPATH=_REPO_ROOT),
stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True,
timeout=60)
self.assertEqual(0, result.returncode, result.stdout)
self.assertIn('OK', result.stdout)
result = self._run_pytest('tests/unit/test_wxui_linux_launcher.py')
self._assert_clean_pass(
result, 'linux_launcher after a real chirp.wxui.memedit import')
def test_radiothread_then_linux_launcher_then_real_memedit_import(self):
# The exact three-file combination and order from the original
# report: test_wxui_radiothread.py (the older, previously-
# accepted pre-existing pollution source) collected first,
# then test_wxui_linux_launcher.py, then a real-wx import.
result = subprocess.run(
[sys.executable, '-c',
'import sys\n'
'import tests.unit.test_wxui_radiothread\n'
'import tests.unit.test_wxui_linux_launcher\n'
'from chirp.wxui import memedit\n'
'assert "MagicMock" not in repr(type(memedit.common.'
'ChirpEditor)), memedit.common.ChirpEditor\n'
'sys.stdout.write("OK\\n")'],
cwd=_REPO_ROOT,
env=dict(os.environ, CHIRP_TESTENV='1', PYTHONPATH=_REPO_ROOT),
stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True,
timeout=60)
self.assertEqual(0, result.returncode, result.stdout)
self.assertIn('OK', result.stdout)
result = self._run_pytest(
'tests/unit/test_wxui_radiothread.py',
'tests/unit/test_wxui_linux_launcher.py')
self._assert_clean_pass(result, 'radiothread + linux_launcher, run')
def test_radiothread_and_linux_launcher_together_both_orders(self):
result = self._run_pytest(
'tests/unit/test_wxui_radiothread.py',
'tests/unit/test_wxui_linux_launcher.py')
self._assert_clean_pass(result, 'radiothread, then linux_launcher')
result = self._run_pytest(
'tests/unit/test_wxui_linux_launcher.py',
'tests/unit/test_wxui_radiothread.py')
self._assert_clean_pass(result, 'linux_launcher, then radiothread')
def test_relevant_files_pass_together_with_full_directory_deselection(
self):
# A lighter-weight proxy for "passes as part of the full
# `pytest tests/unit -k not network` collection": collecting
# the whole tests/unit directory (so every other file's own
# module-level imports run too, in their normal relative
# order) but only *running* the files relevant to this
# defect, via -k, keeps this fast while still exercising the
# real, full collection order the CI job uses.
result = self._run_pytest(
'tests/unit', '-k',
'TestStartup or DoInstallLinuxLauncherTest or '
'ReportOutcomeTest or MenuWiringSourceTest')
self._assert_clean_pass(result, 'full tests/unit collection order')

View file

@ -3,6 +3,50 @@ from pathlib import Path
import sys
from unittest import mock
# Snapshot sys.modules before mocking so it can be restored exactly,
# immediately below, once this file's own imports are done with it --
# see the restore block after the imports for why both the snapshot
# and the immediate (not fixture-based) timing are needed.
_PRE_MOCK_SYS_MODULES = dict(sys.modules)
def _evict_chirp_wxui_modules():
"""Remove every already-imported chirp.wxui.* submodule (and the
parent package's attribute reference to it -- see the restore
block below for why that second part matters) so that whatever
imports chirp.wxui submodules next -- our own mock installation
below, or a real one after it -- gets a fresh import rather than
reusing a module cached from however wx looked the last time it
was genuinely imported.
This runs twice: once here, before our own mocking, in case an
earlier-collected test file already imported chirp.wxui.common
(etc.) for real, which would otherwise make this file's own tests
exercise the real wx.MessageDialog instead of our mock; and once
after our own imports, to avoid leaving *our* fake-wx-built
modules cached for a later-collected file that needs the real
thing (e.g. tests/unit/test_wxui_programming_assistant.py).
Confirmed by direct reproduction that both directions are
necessary: running this file before a real-wx test file fails
without the after-case, and running it after one fails without
this before-case.
"""
for name in list(sys.modules):
if name != 'chirp.wxui' and not name.startswith('chirp.wxui.'):
continue
if name == 'chirp.wxui':
# Never evict the bare package itself -- see the restore
# block below for why.
continue
del sys.modules[name]
parent_name, _, attr = name.rpartition('.')
parent = sys.modules.get(parent_name)
if parent is not None:
vars(parent).pop(attr, None)
_evict_chirp_wxui_modules()
sys.modules['wx'] = wx = mock.MagicMock()
sys.modules['wx.adv'] = mock.MagicMock()
sys.modules['wx.aui'] = mock.MagicMock()
@ -33,6 +77,99 @@ import chirp # noqa
from chirp import linux_desktop # noqa
from chirp.wxui import linux_launcher # noqa
# Restore sys.modules immediately -- synchronously, still as part of
# this file's own collection -- rather than via a pytest fixture.
# pytest fully COLLECTS (imports) every test file before it EXECUTES
# any test; a fixture's teardown only runs during that later execution
# phase, which is already too late to help another test file whose
# own top-level import (e.g. `from chirp.wxui import memedit`) runs
# during ITS collection, potentially before this module's tests ever
# execute. This mirrors test_wxui_recentfiles.py's stash/restore
# pattern, extended to cover every sys.modules entry this file's
# mocking and imports touch, not just 'wx' itself:
#
# Restoring sys.modules['wx'] alone is not enough. Importing
# chirp.wxui.linux_launcher above also transitively imports
# chirp.wxui.common, and chirp.wxui.common defines classes
# (ChirpEditor, ChirpSyncEditor) that subclass real wx types. Since
# that import happens while sys.modules['wx'] is still our MagicMock,
# those classes get built against the *fake* wx and stay cached in
# sys.modules that way -- putting the real 'wx' back afterward does
# not retroactively fix a class object that already exists. Any test
# file collected later in the same pytest session that triggers a
# fresh `from chirp.wxui import memedit` (which also subclasses
# common.ChirpEditor/ChirpSyncEditor) hits "TypeError: metaclass
# conflict" as a result, since chirp.wxui.common never gets
# re-imported with the real wx once it's cached this way. Confirmed by
# direct reproduction that restoring only sys.modules['wx'] still
# reproduces that conflict, and that this broader restore does not.
#
# This evicts every 'wx'/'wx.*' and 'chirp.wxui.*' (submodules only --
# not the bare 'chirp.wxui' package itself, see below) entry that is
# new since this file started, restoring the ones that already existed
# to their prior value, so the next real import of any of them
# re-executes from scratch against the real wx.
#
# The bare 'chirp.wxui' package (chirp/wxui/__init__.py) is
# deliberately excluded: unlike its submodules, it defines no wx-based
# classes at import time -- maybe_install_desktop() does `import wx`
# lazily, inside the function body, so it always resolves
# sys.modules['wx'] fresh at call time regardless of caching. Evicting
# it anyway was tried and confirmed harmful: it forces a second,
# later, non-idempotent import of chirp/wxui/__init__.py, which raises
# wx._core.PyNoAppError deep inside test_wxui_radiothread.py's
# TestStartup tests (no real wx.App exists in this headless test
# environment).
#
# Safe for this file's own tests: chirp.wxui.linux_launcher and
# chirp.linux_desktop both only `import wx` at module scope (captured
# once, at this file's own import time) -- never lazily inside a
# function body -- so nothing in this file's own tests re-resolves
# sys.modules['wx'] later, after this restore has already run.
#
# Removing a submodule from sys.modules is not sufficient on its own:
# `from chirp.wxui import common` first checks whether the chirp.wxui
# *package* object already has a `common` attribute, and uses that
# directly without consulting sys.modules at all if so -- Python sets
# that attribute automatically as a side effect of the first import,
# the same way `import chirp.wxui.common` also makes `common`
# accessible as `chirp.wxui.common`. Since chirp.wxui itself is
# deliberately kept (not evicted, per above), it keeps that stale
# attribute pointing at the fake-wx-built submodule even after the
# submodule's own sys.modules entry is removed, which reproduces the
# exact same metaclass conflict one level further down the import
# chain (chirp/wxui/developer.py, imported by memedit.py, doing its
# own `from chirp.wxui import common`). Confirmed by direct
# reproduction. So each evicted chirp.wxui.* submodule's attribute
# must also be cleared from its parent package object -- and likewise
# fixed up (not just left as whatever this file's own imports left
# behind) on the restore path.
#
# The set of names to consider is the *union* of what's in
# _PRE_MOCK_SYS_MODULES and what's in sys.modules now, not just
# whichever of the two is convenient to iterate: something present
# before but evicted by _evict_chirp_wxui_modules() above and never
# reimported by this file's own `from chirp.wxui import
# linux_launcher` chain (e.g. chirp.wxui.programming_assistant, when
# this file runs after a real-wx test module that imported it) would
# otherwise never be considered at all, silently staying evicted
# forever instead of being restored. Confirmed by direct reproduction
# that iterating only `sys.modules` misses exactly this case.
_affected = {n for n in set(_PRE_MOCK_SYS_MODULES) | set(sys.modules)
if n == 'wx' or n.startswith('wx.') or
n.startswith('chirp.wxui.')}
for _name in _affected:
_parent_name, _, _attr = _name.rpartition('.')
_parent = sys.modules.get(_parent_name)
if _name in _PRE_MOCK_SYS_MODULES:
sys.modules[_name] = _PRE_MOCK_SYS_MODULES[_name]
if _parent is not None:
setattr(_parent, _attr, _PRE_MOCK_SYS_MODULES[_name])
else:
sys.modules.pop(_name, None)
if _parent is not None:
vars(_parent).pop(_attr, None)
class DoInstallLinuxLauncherTest(base.BaseTest):
def setUp(self):

View file

@ -5,6 +5,37 @@ from unittest import mock
import ddt
# Snapshot sys.modules before mocking, so it can be restored exactly,
# immediately below, once this file's own collection-time imports
# (clone.py, radiothread.py) are done with it. See the restore block
# after the imports, and TestStartup.setUp() further down, for why
# this needs to be a two-part fix rather than a single restore.
_PRE_MOCK_SYS_MODULES = dict(sys.modules)
def _evict_chirp_wxui_modules():
"""Remove every already-imported chirp.wxui.* submodule (and the
parent package's attribute reference to it) so that whatever
imports chirp.wxui submodules next gets a fresh import rather than
reusing a module cached from however wx looked the last time it
was genuinely imported. See test_wxui_linux_launcher.py, which
has the identical helper (and the identical underlying problem)
with a more detailed explanation of why both the sys.modules
removal and the parent-attribute cleanup are needed."""
for name in list(sys.modules):
if name != 'chirp.wxui' and not name.startswith('chirp.wxui.'):
continue
if name == 'chirp.wxui':
continue
del sys.modules[name]
parent_name, _, attr = name.rpartition('.')
parent = sys.modules.get(parent_name)
if parent is not None:
vars(parent).pop(attr, None)
_evict_chirp_wxui_modules()
sys.modules['wx'] = wx = mock.MagicMock()
sys.modules['wx.lib'] = mock.MagicMock()
sys.modules['wx.lib.scrolledpanel'] = mock.MagicMock()
@ -22,6 +53,38 @@ from chirp.wxui import clone # noqa
from chirp.wxui import config # noqa
from chirp.wxui import radiothread # noqa
# Restore sys.modules immediately -- synchronously, still as part of
# this file's own collection -- for the same reason and via the same
# mechanism as test_wxui_linux_launcher.py: pytest fully collects
# every test file before executing any test, so restoring only once
# this file's own tests finish executing (e.g. via a pytest fixture)
# is too late to stop a later-collected real-wx test file (like
# test_wxui_programming_assistant.py) from seeing this mock during
# *its* collection.
#
# clone.py and radiothread.py (the modules under test for
# TestRadioThread/TestClone below) both only `import wx` at module
# scope, captured once at this file's own import time, so restoring
# immediately is safe for those two classes' own tests. TestStartup is
# the exception -- see its setUp() below, which re-installs a scoped
# copy of this same mock for exactly that class's own tests, instead
# of relying on it staying installed globally for the rest of the
# session the way this file used to leave it.
_affected = {n for n in set(_PRE_MOCK_SYS_MODULES) | set(sys.modules)
if n == 'wx' or n.startswith('wx.') or
n.startswith('chirp.wxui.')}
for _name in _affected:
_parent_name, _, _attr = _name.rpartition('.')
_parent = sys.modules.get(_parent_name)
if _name in _PRE_MOCK_SYS_MODULES:
sys.modules[_name] = _PRE_MOCK_SYS_MODULES[_name]
if _parent is not None:
setattr(_parent, _attr, _PRE_MOCK_SYS_MODULES[_name])
else:
sys.modules.pop(_name, None)
if _parent is not None:
vars(_parent).pop(_attr, None)
class TestRadioThread(base.BaseTest):
def setUp(self):
@ -177,6 +240,33 @@ class TestException(Exception):
class TestStartup(base.BaseTest):
def setUp(self):
super().setUp()
# maybe_install_desktop() (chirp/wxui/__init__.py) does a
# lazy `import wx` inside its own function body, resolved
# fresh every call -- unlike this file's own collection-time
# imports (clone.py etc.), which only ever needed the
# module-level mock during collection, and have already been
# restored by the module-level cleanup above. Re-install the
# *same* wx mock object this file's own module-level `wx`
# variable refers to (so the wx.MessageBox.assert_*() calls
# below, which check that same object, keep working), scoped
# to just this test -- not left mocked globally for the rest
# of the session the way this file used to.
#
# Deliberately not mock.patch.dict(sys.modules, {'wx': wx}):
# confirmed by direct reproduction that using it here, even
# though it is a no-op in terms of the *value* sys.modules['wx']
# ends up holding, makes some later test in this class trip
# pytest's own assertion-rewrite import hook into calling
# os.path.normcase() on a real path while os.path is
# separately mocked below, raising
# "TypeError: expected string or bytes-like object, got
# 'MagicMock'" resolving importlib.resources.files('chirp.
# share') -- a mock.patch.dict-specific interaction with
# pytest/importlib's own caching, unrelated to wx isolation.
# Plain manual save/restore does not trigger it.
self._prior_wx = sys.modules.get('wx')
sys.modules['wx'] = wx
self.addCleanup(self._restore_wx_module)
self.use(mock.patch('os.path'))
self.use(mock.patch('os.makedirs'))
self.use(mock.patch('chirp.wxui.CONF'))
@ -187,6 +277,12 @@ class TestStartup(base.BaseTest):
self.maybe_install_desktop = maybe_install_desktop
self.conf = CONF
def _restore_wx_module(self):
if self._prior_wx is not None:
sys.modules['wx'] = self._prior_wx
else:
sys.modules.pop('wx', None)
@ddt.data(
# No arguments, no file, no previous, answer no
[False, False, False, False, False],