test: re-import pcapkit only where isolation needs it (#1065)

tests/_support.py

@@ -853,6 +853,74 @@ def purge_modules(prefixes: Iterable[str]) -> None:
     _reset_abc_caches()
 
 
+#: Class attribute :func:`reimport_once_per_class` keeps a class's own import in.
+_CLASS_IMPORT = '_pcapkit_class_import'
+
+
+def reimport_once_per_class(test: 'unittest.TestCase', restore: bool = False) -> None:
+    """Give ``test``'s class one fresh :mod:`pcapkit` import, shared by its tests.
+
+    GitHub issue #1065. Calling :func:`purge_modules` from ``setUp`` re-imports
+    the package before every test -- about 0.67s and 329 modules each, which was
+    some 85% of the suite's serial time. Call this from ``setUp`` instead. The
+    first test of the class purges, and the class keeps what that test imports.
+    Every later test of the class has that same import put back in
+    :data:`sys.modules`. So a class gets a private import, which its own tests
+    may mutate, and the import is dropped when the class finishes. Nothing is
+    imported here: a class that only loads modules by path, or that never
+    imports :mod:`pcapkit` at all, keeps an empty region, as it did before.
+
+    A purge in ``setUpClass`` would not do this under :program:`pytest`.
+    :func:`tests.conftest.restore_module_table` swaps the region back to its
+    session-wide pin before every test, so such a class would run on the shared
+    import that every other class mutates as well. That is exactly the
+    cross-class leakage a per-test purge used to hide.
+
+    Under plain :mod:`unittest`, the import is also left in place after the class
+    finishes, as a per-test purge left one. ``util/run_unittest_leg.py`` relies
+    on that to catch a module still holding an earlier import (GitHub issue
+    #981).
+
+    Args:
+        test: The test being set up.
+        restore: Also put back, once the class finishes, whatever the region held
+            before its first test. This is the per-class form of a ``setUp`` that
+            snapshots, purges and restores around each test.
+
+    """
+    cls = type(test)
+    held = cls.__dict__.get(_CLASS_IMPORT)
+    if held is not None:
+        restore_modules(held, ISOLATED_PREFIXES)
+    else:
+        if restore:
+            cls.addClassCleanup(restore_modules, snapshot_modules(ISOLATED_PREFIXES), ISOLATED_PREFIXES)
+        purge_modules(['pcapkit'])
+        held = {}
+        setattr(cls, _CLASS_IMPORT, held)
+        cls.addClassCleanup(delattr, cls, _CLASS_IMPORT)
+    # Registered first, so it runs after the test's own cleanups.
+    test.addCleanup(_keep_lazy_imports, held)
+
+
+def _keep_lazy_imports(held: 'dict[str, types.ModuleType]') -> None:
+    """Add to ``held`` the real :mod:`pcapkit` modules the test imported.
+
+    This is how the class's import is built up. Without it, the conftest restore
+    drops a submodule imported during the test, and the next test of the class
+    imports it again as a second module object. The parent package still holds the first one as an attribute, and
+    :func:`unittest.mock.patch` resolves through that attribute before Python
+    3.12 -- so a patch landed on a module the code under test no longer used.
+    Only modules loaded from a file are kept; a stand-in a test bound, and its
+    own cleanup has not removed, is not.
+
+    """
+    for name, module in snapshot_modules(ISOLATED_PREFIXES).items():
+        spec = getattr(module, '__spec__', None)
+        if name not in held and spec is not None and spec.origin and getattr(module, '__name__', None) == name:
+            held[name] = module
+
+
 def _close_quietly(target: object) -> None:
     """Call ``target.close()``, swallowing any :exc:`Exception` it raises.
 

tests/const/test_const_apptype_split_unit.py

@@ -32,6 +32,29 @@
 
 __all__ = ['AppTypeSplitTests']
 
+#: Stride of :func:`_dunder_sample`: about 48 evenly spaced members of each
+#: registry, plus the ones it always keeps.
+_DUNDER_SAMPLE_SIZE = 48
+
+
+def _dunder_sample(registry: 'type') -> 'list':
+    """The members of ``registry`` the byte-identity sweep formats, in row order.
+
+    GitHub issue #1062. Sweeping every member ran 37,173 subtests, and per-test
+    coverage contexts found only **4** distinct arc sets among them: ``repr``
+    and ``str``, each with and without an ``(aliases: ...)`` suffix -- the
+    registry, the port and the service name never change which code runs. So
+    the sample keeps every member that carries an alias (the suffix branch),
+    the first and last row, and an evenly spaced stride through the rest, which
+    covers all four sets in every registry and is the same list on every run.
+
+    """
+    members = list(registry)
+    step = max(1, len(members) // _DUNDER_SAMPLE_SIZE)
+    keep = set(range(0, len(members), step)) | {len(members) - 1}
+    return [member for index, member in enumerate(members)
+            if index in keep or member.aliases]
+
 
 class AppTypeSplitTests(unittest.TestCase):
     """The shape of the split registry."""
@@ -1185,13 +1208,16 @@ def test_the_new_dunders_are_byte_identical_to_the_percent_form(self) -> None:
         """GitHub issue #798: ``AppType``'s ``__new__``/``__repr__``/``__str__`` moved
         from ``%`` formatting to f-strings, and ``__new__``'s format sets every
         real member's underlying :class:`~aenum.StrEnum` value -- a far larger
-        blast radius than an error path, so this is checked member by member
-        rather than spot-checked.
-
-        Swept over all 12,391 real members (TCP 6147, UDP 6143, SCTP 91,
-        DCCP 10, matching the population PR #783 measured), each
-        compared against what the pre-#798 ``%``-style formula would have
-        produced for that same member's own ``svc``/``port``/``proto``. This
+        blast radius than an error path. This was once checked member by
+        member; it is now a deterministic sample, because one f-string formats
+        every member and the sweep only ever reached four distinct code paths
+        (see :func:`_dunder_sample`).
+
+        The population is still counted over all 12,391 real members (TCP 6147,
+        UDP 6143, SCTP 91, DCCP 10, matching the population PR #783 measured),
+        but only :func:`_dunder_sample` is formatted, each member compared
+        against what the pre-#798 ``%``-style formula would have produced for
+        that same member's own ``svc``/``port``/``proto``. This
         is an invariance check -- it is true either side of #798's fix by
         construction, since both formulas render the same text for the same
         inputs -- rather than a regression test that fails on stock ``main``.
@@ -1224,12 +1250,14 @@ def test_the_new_dunders_are_byte_identical_to_the_percent_form(self) -> None:
         registries = {'TCP': TCP, 'UDP': UDP, 'SCTP': SCTP, 'DCCP': DCCP}
         expected_counts = {'TCP': 6147, 'UDP': 6143, 'SCTP': 91, 'DCCP': 10}
         total = 0
+        aliased = 0  # the suffix branch of both dunders; only TCP and UDP have any
 
         for name, cls in registries.items():
-            count = 0
-            for member in cls:
-                count += 1
-                total += 1
+            count = len(list(cls))
+            total += count
+            sample = _dunder_sample(cls)
+            aliased += sum(1 for member in sample if member.aliases)
+            for member in sample:
                 svc, port, proto = member.svc, member.port, member.proto
 
                 with self.subTest(registry=name, member=member.name, check='value'):
@@ -1252,6 +1280,7 @@ def test_the_new_dunders_are_byte_identical_to_the_percent_form(self) -> None:
             self.assertEqual(count, expected_counts[name], f'{name} population changed')
 
         self.assertEqual(total, 12391)
+        self.assertGreater(aliased, 0, 'the sample lost the aliased branch')
 
     def test_every_member_renders_its_own_registrys_transport_protocol(self) -> None:
         """GitHub issue #806, member by member over all 12,391.

tests/const/test_const_enum_builtin_parity.py

@@ -58,7 +58,8 @@
 
 import aenum
 
-from tests._support import ISOLATED_PREFIXES, purge_modules, restore_modules, snapshot_modules
+from tests._support import (ISOLATED_PREFIXES, purge_modules, reimport_once_per_class,
+                            restore_modules, snapshot_modules)
 
 if TYPE_CHECKING:
     from typing import Optional
@@ -546,7 +547,7 @@ class ConstFlagCompositeTests(unittest.TestCase):
     """
 
     def setUp(self) -> None:
-        purge_modules(['pcapkit'])
+        reimport_once_per_class(self)
 
     def test_tcp_flag_composites_resolve(self) -> None:
         from pcapkit.const.tcp.flags import Flags
@@ -675,6 +676,10 @@ class ConstEnumRegisterFallbackTests(unittest.TestCase):
     """
 
     def setUp(self) -> None:
+        # Per test, unlike most classes since GitHub issue #1065: these read the
+        # registries' lookup tables, which any earlier lookup in the same import
+        # can grow -- ``Flags(0)`` anywhere caches ``0`` as a pseudo-member, and
+        # ``Flags.get(UNRESOLVABLE, 0)`` then resolves instead of raising.
         purge_modules(['pcapkit'])
 
     def tearDown(self) -> None:
@@ -909,7 +914,7 @@ class ConstEnumGuardTemplateTests(unittest.TestCase):
     """
 
     def setUp(self) -> None:
-        purge_modules(['pcapkit'])
+        reimport_once_per_class(self)
 
     @unittest.skipUnless(importlib.util.find_spec('requests') is not None,
                          'pcapkit.vendor needs requests')

tests/const/test_const_enum_lookup.py

@@ -41,9 +41,9 @@
 this module carries a companion sweep over the :class:`~aenum.IntFlag` classes,
 covering the contract the :class:`~aenum.IntEnum` sweep declines to.
 
-Every test class purges :mod:`pcapkit` from :data:`sys.modules` in ``setUp``,
+Every test class re-imports :mod:`pcapkit` for itself from ``setUp``,
 matching the convention every other module in this suite uses (see
-:func:`tests._support.purge_modules`). It matters more here than usual: this
+:func:`tests._support.reimport_once_per_class`). It matters more here than usual: this
 module is the first thing in the whole suite to import *every* submodule
 under :mod:`pcapkit.const`, including ones nothing else touches (e.g.
 :mod:`pcapkit.const.reg.apptype`). ``tests/cli/test_main.py`` stubs pieces of
@@ -69,7 +69,8 @@
 
 from aenum import IntEnum, IntFlag
 
-from tests._support import ISOLATED_PREFIXES, purge_modules, restore_modules, snapshot_modules
+from tests._support import (ISOLATED_PREFIXES, purge_modules, reimport_once_per_class,
+                            restore_modules, snapshot_modules)
 
 #: Fully qualified names of the :class:`~aenum.IntEnum` classes under
 #: :mod:`pcapkit.const` for which rejecting ``0`` is *correct*, because their
@@ -249,7 +250,7 @@ class ConstMissingClassmethodTests(unittest.TestCase):
     """
 
     def setUp(self) -> None:
-        purge_modules(['pcapkit'])
+        reimport_once_per_class(self)
 
     def test_every_missing_is_a_classmethod(self) -> None:
         offenders = []  # type: list[str]
@@ -441,7 +442,7 @@ class RouterAlertPacketParseTests(unittest.TestCase):
     """Parse an on-the-wire packet carrying RFC 2113's Router Alert value 0."""
 
     def setUp(self) -> None:
-        purge_modules(['pcapkit'])
+        reimport_once_per_class(self)
 
     def test_parses_igmp_over_router_alert_zero(self) -> None:
         from pcapkit.protocols.internet.ipv4 import IPv4

tests/const/test_const_enum_no_mint.py

@@ -176,7 +176,8 @@
 import unittest
 from typing import TYPE_CHECKING
 
-from tests._support import ISOLATED_PREFIXES, purge_modules, restore_modules, snapshot_modules
+from tests._support import (ISOLATED_PREFIXES, purge_modules, reimport_once_per_class,
+                            restore_modules, snapshot_modules)
 
 if TYPE_CHECKING:
     from typing import Optional
@@ -774,9 +775,7 @@ class GetNoLongerMintsTests(unittest.TestCase):
     same ruling applied to the int path above."""
 
     def setUp(self) -> None:
-        snapshot = snapshot_modules(ISOLATED_PREFIXES)
-        purge_modules(['pcapkit'])
-        self.addCleanup(restore_modules, snapshot, ISOLATED_PREFIXES)
+        reimport_once_per_class(self, restore=True)
 
     def test_unresolvable_string_key_without_default_raises(self) -> None:
         from pcapkit.const.arp.hardware import Hardware
@@ -818,9 +817,7 @@ class RegisterStillMintsTests(unittest.TestCase):
     """The one caller-named, explicit path must still grow the registry."""
 
     def setUp(self) -> None:
-        snapshot = snapshot_modules(ISOLATED_PREFIXES)
-        purge_modules(['pcapkit'])
-        self.addCleanup(restore_modules, snapshot, ISOLATED_PREFIXES)
+        reimport_once_per_class(self, restore=True)
 
     def test_register_mints_a_real_permanent_member(self) -> None:
         from pcapkit.const.arp.hardware import Hardware
@@ -882,9 +879,7 @@ class UnregisteredMemberHelperTests(unittest.TestCase):
     """
 
     def setUp(self) -> None:
-        snapshot = snapshot_modules(ISOLATED_PREFIXES)
-        purge_modules(['pcapkit'])
-        self.addCleanup(restore_modules, snapshot, ISOLATED_PREFIXES)
+        reimport_once_per_class(self, restore=True)
 
     def test_builds_an_absent_member_on_every_registry(self) -> None:
         value = 0x6E7A0002  # arbitrary, distinct from the register() sweep's
@@ -1504,9 +1499,7 @@ class EtherTypeMixedMintTests(unittest.TestCase):
     rather than renamed, so history stays easy to follow)."""
 
     def setUp(self) -> None:
-        snapshot = snapshot_modules(ISOLATED_PREFIXES)
-        purge_modules(['pcapkit'])
-        self.addCleanup(restore_modules, snapshot, ISOLATED_PREFIXES)
+        reimport_once_per_class(self, restore=True)
 
     def test_unassigned_rows_do_not_mint(self) -> None:
         from pcapkit.const.reg.ethertype import EtherType
@@ -1572,9 +1565,7 @@ class IPXSocketMixedMintTests(unittest.TestCase):
     :meth:`test_registered_by_xerox_no_longer_mints` below."""
 
     def setUp(self) -> None:
-        snapshot = snapshot_modules(ISOLATED_PREFIXES)
-        purge_modules(['pcapkit'])
-        self.addCleanup(restore_modules, snapshot, ISOLATED_PREFIXES)
+        reimport_once_per_class(self, restore=True)
 
     def test_unassigned_rows_do_not_mint(self) -> None:
         from pcapkit.const.ipx.socket import Socket
@@ -1651,9 +1642,7 @@ class therefore overrides :meth:`_unregistered_member` to reconstruct
     """
 
     def setUp(self) -> None:
-        snapshot = snapshot_modules(ISOLATED_PREFIXES)
-        purge_modules(['pcapkit'])
-        self.addCleanup(restore_modules, snapshot, ISOLATED_PREFIXES)
+        reimport_once_per_class(self, restore=True)
 
     def test_unassigned_value_resolves_without_minting(self) -> None:
         """Swept across all 5: resolves, does not mint, repeated lookup is
@@ -1907,9 +1896,7 @@ class BespokeOpenVocabularyUnmintConvertedTests(unittest.TestCase):
     """
 
     def setUp(self) -> None:
-        snapshot = snapshot_modules(ISOLATED_PREFIXES)
-        purge_modules(['pcapkit'])
-        self.addCleanup(restore_modules, snapshot, ISOLATED_PREFIXES)
+        reimport_once_per_class(self, restore=True)
 
     def test_featcode_import_mints_nothing(self) -> None:
         """The guard against the import-time mutation coming back -- count-
@@ -2185,9 +2172,7 @@ class BespokeGetReplacementTests(unittest.TestCase):
     """
 
     def setUp(self) -> None:
-        snapshot = snapshot_modules(ISOLATED_PREFIXES)
-        purge_modules(['pcapkit'])
-        self.addCleanup(restore_modules, snapshot, ISOLATED_PREFIXES)
+        reimport_once_per_class(self, restore=True)
 
     def test_statuscode_get_omitted_default_still_raises(self) -> None:
         """The one production call site's shape: no default passed, so an
@@ -2258,7 +2243,10 @@ class BespokeGetUnchangedTests(unittest.TestCase):
     case-insensitively (GitHub issue #582 -- ``Command.get('abor')`` must
     return :attr:`Command.ABOR`, not raise), which the base's plain
     ``_member_map_``/``_value2member_map_`` lookup does not do -- swapping in
-    the base would silently reintroduce #582.
+    the base would silently reintroduce #582. That pin lives in
+    :mod:`tests.const.test_const_method_case_sensitive_896_unit`
+    (``test_command_get_stays_case_insensitive``), whose body this class used
+    to repeat verbatim.
 
     :class:`Method` used to resolve case-insensitively the same way (#583),
     but GitHub issue #896 retired that: RFC 9110 Section 9.1 makes the HTTP
@@ -2281,16 +2269,7 @@ class BespokeGetUnchangedTests(unittest.TestCase):
     """
 
     def setUp(self) -> None:
-        snapshot = snapshot_modules(ISOLATED_PREFIXES)
-        purge_modules(['pcapkit'])
-        self.addCleanup(restore_modules, snapshot, ISOLATED_PREFIXES)
-
-    def test_command_get_is_still_case_insensitive(self) -> None:
-        from pcapkit.const.ftp.command import Command
-
-        for key in ('RETR', 'retr', 'ReTr', 'rEtR'):
-            with self.subTest(key=key):
-                self.assertIs(Command.get(key), Command.RETR)  # type: ignore[attr-defined]
+        reimport_once_per_class(self, restore=True)
 
     def test_method_get_is_now_case_sensitive(self) -> None:
         """GitHub issue #896: only the exact registered casing resolves.
@@ -2404,9 +2383,7 @@ class AppTypeUnmintConvertedTests(unittest.TestCase):
     """
 
     def setUp(self) -> None:
-        snapshot = snapshot_modules(ISOLATED_PREFIXES)
-        purge_modules(['pcapkit'])
-        self.addCleanup(restore_modules, snapshot, ISOLATED_PREFIXES)
+        reimport_once_per_class(self, restore=True)
 
     def test_apptype_family_carries_the_registry_protocol(self) -> None:
         """#842's ruling is that ``get``/``get_all``/``register``/