Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 26 additions & 30 deletions tests/_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,11 @@ def time_limit(seconds: int = 30) -> Iterator[None]:
missing altogether. That one-second floor covers every case where the body ran
for longer than the enclosing deadline had left.

The deadline is a wall-clock one, so it measures the runner as much as the
body: a 5-second default, against bodies that take single-digit milliseconds,
still expired on a loaded 4-core CI runner under :program:`pytest-xdist`
(#1058). Hence the 30-second default, the same bound the extraction tests
already used -- several thousand times a working body, and still a fast failure
next to a run that never ends -- and :func:`scale_timeout` on top, for a runner
slower than that.
The deadline is wall-clock, so it measures the runner as much as the body: five
seconds over a body of single-digit milliseconds has expired on a loaded 4-core
CI runner under :program:`pytest-xdist` (#1058). Hence the 30-second default --
several thousand times a working body, and still a fast failure next to a run
that never ends -- and :func:`scale_timeout` for a runner slower than that.

Args:
seconds: Whole seconds to allow the body, before
Expand Down Expand Up @@ -139,9 +137,8 @@ def expire(signum: int, frame: object) -> None:

previous_handler = signal.signal(signal.SIGALRM, expire)

# Armed inside the ``try``, so that anything raised while arming -- the
# ``OverflowError`` an oversized value once produced, say -- still puts the
# previous handler back on the way out rather than leaking ``expire``.
# Armed inside the ``try``, so that anything raised while arming still puts
# the previous handler back on the way out rather than leaking ``expire``.
armed = False
pending = 0
started = time.monotonic()
Expand Down Expand Up @@ -936,26 +933,25 @@ def purge_modules(prefixes: Iterable[str]) -> None:
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.
GitHub issue #1065. Call this from ``setUp`` in place of
:func:`purge_modules`, under which every test re-imports the package -- about
0.67s and 329 modules each, some 85% of the suite's serial time. The first
test of the class purges, and the class keeps what its tests import; every
later test has that import put back in :data:`sys.modules`. So a class gets a
private import, which its own tests may mutate, and lets go of it when the
class finishes. Nothing is imported here: a class that only loads modules by
path, or never imports :mod:`pcapkit` at all, keeps an empty region.

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.
import that every other class mutates as well -- the cross-class leakage a
private import rules out.

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).
Under plain :mod:`unittest`, unless ``restore`` is set, the import is also
left in place after the class finishes, as a per-test purge leaves the last
test's. ``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.
Expand Down Expand Up @@ -984,11 +980,11 @@ def _keep_lazy_imports(held: 'dict[str, types.ModuleType]') -> None:

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.
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 would land on a module
the code under test no longer uses. 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():
Expand Down
21 changes: 9 additions & 12 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,9 @@
def _pin_module_snapshot() -> None:
"""Capture the :mod:`pcapkit` region as the restore target, once.

:func:`restore_module_table` used to restore to a snapshot taken at its own
entry -- "whatever the region held when this test began" -- which is exact
for a leak the fixture's own window can see, but blind to one planted
outside it. :meth:`~unittest.TestCase.setUpClass` runs *before* the first
:func:`restore_module_table` restores to this pinned snapshot, not to one
taken at its own entry. An entry snapshot is exact for a leak the fixture's
own window can see, but blind to one planted outside it. :meth:`~unittest.TestCase.setUpClass` runs *before* the first
test's fixture entry and a class-level cleanup runs *after* the last one's
fixture exit, so a purge in either place is invisible to an entry-snapshot
restore: the entry snapshot for that first test is already the post-purge
Expand Down Expand Up @@ -140,12 +139,11 @@ def pytest_sessionstart(session: 'pytest.Session') -> 'None':
this import, and every later test -- including a purging class's own first
one -- is restored to it going in as well as coming out, regardless of
whether that class reloads anything for itself.
:class:`tests.integration._helpers.EndToEndTestCase` does reload immediately
after its own purge, for the unrelated reason given in its own docstring --
saving roughly 45s across its 92 test methods -- and nothing here disturbs
that: rebinding :data:`sys.modules` to the pin does not invalidate a module
object a class attribute already holds a direct reference to, so a class
that reimports for itself keeps exactly what it reimported.
So a class that purges and reimports for itself, as
:class:`tests.integration._helpers.EndToEndTestCase` does, still runs its
tests on the pin. Only a module object a class attribute holds a direct
reference to survives the rebinding, since restoring :data:`sys.modules`
does not invalidate it.

Warming it also means the restore is a no-op for the common case. A test
that imports the library and nothing else leaves the region exactly as it
Expand Down Expand Up @@ -260,8 +258,7 @@ def pytest_timeout_cancel_timer(item: 'pytest.Item') -> 'Iterator[None]':
installs fresh tbtrim hooks there, so each retained timer pins the
:mod:`pcapkit` generation that was live when its test started. Measured on
``tests/const/test_const_registry_protocol.py`` under ``thread``: past 9 GiB
and killed, against 283 MiB with this hook. That growth is what took every
``test`` leg's runner down at about 13% on CI. ``optionalhook``: without
and killed, against 283 MiB with this hook. ``optionalhook``: without
pytest-timeout installed the hookspec does not exist.

"""
Expand Down
16 changes: 8 additions & 8 deletions tests/const/test_const_apptype_split_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@

__all__ = ['AppTypeSplitTests']

#: Stride of :func:`_dunder_sample`: about 48 evenly spaced members of each
#: registry, plus the ones it always keeps.
#: Divisor of :func:`_dunder_sample`'s stride, ``len(registry) // 48``: about 50
#: evenly spaced members of TCP and UDP, and every member of a registry under 96.
_DUNDER_SAMPLE_SIZE = 48


Expand All @@ -46,7 +46,7 @@ def _dunder_sample(registry: 'type') -> 'list':
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.
covers all four sets across the registries and is the same list on every run.

"""
members = list(registry)
Expand Down Expand Up @@ -1208,12 +1208,12 @@ 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. 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`).
blast radius than an error path. It is checked on a deterministic
sample rather than member by member, because one f-string formats every
member and a full sweep reaches only four distinct code paths (see
:func:`_dunder_sample`).

The population is still counted over all 12,391 real members (TCP 6147,
The population is 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
Expand Down
2 changes: 1 addition & 1 deletion tests/const/test_const_enum_builtin_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ class ConstEnumRegisterFallbackTests(unittest.TestCase):
"""

def setUp(self) -> None:
# Per test, unlike most classes since GitHub issue #1065: these read the
# Per test, not per class (``reimport_once_per_class``): 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.
Expand Down
6 changes: 3 additions & 3 deletions tests/const/test_const_enum_lookup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 re-imports :mod:`pcapkit` for itself from ``setUp``,
matching the convention every other module in this suite uses (see
:func:`tests._support.reimport_once_per_class`). It matters more here than usual: this
Every test class purges :mod:`pcapkit` before its tests import it -- from
``setUp`` through :func:`tests._support.reimport_once_per_class`, or in
``setUpClass`` -- matching the rest of the suite. 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
Expand Down
3 changes: 1 addition & 2 deletions tests/const/test_const_enum_no_mint.py
Original file line number Diff line number Diff line change
Expand Up @@ -2245,8 +2245,7 @@ class BespokeGetUnchangedTests(unittest.TestCase):
``_member_map_``/``_value2member_map_`` lookup does not do -- swapping in
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.
(``test_command_get_stays_case_insensitive``).

: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
Expand Down
23 changes: 10 additions & 13 deletions tests/const/test_const_registry_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ def test_tcp_flags_now_inherits_the_base(self) -> None:

Resolves ``EnumRegistry`` freshly from the just-reimported
``pcapkit.corekit.enum`` rather than the module-scope import above --
``setUp`` re-imported ``pcapkit`` for this class, so ``Flags`` now
``setUp`` gave this class its own ``pcapkit`` import, so ``Flags`` now
inherits a *new* ``EnumRegistry`` class object, and identity against
the stale outer one would fail for a reason that says nothing about
the product (see ``ProtocolIsInheritedTests.setUpClass`` above, which
Expand Down Expand Up @@ -1403,14 +1403,12 @@ def test_missing_name_with_default_negative_one_is_now_a_real_default(self) -> N
"""``-1`` and ``-1.0`` are honoured as defaults by
:meth:`~pcapkit.corekit.enum.EnumRegistry.get` itself.

This used to probe :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader`,
and its docstring conceded that it no longer discriminated #857: no
shipped registry's domain reaches ``-1``, so since #864 the name
A throwaway registry holding ``-1`` is what discriminates #857. No
shipped registry's domain reaches ``-1``, so on one of those the name
lookup's :exc:`KeyError` propagates whether ``-1`` is read as a default
or as the old marker. A throwaway registry holding ``-1`` tells the two
apart again: it resolves to that member now, while the pre-#857
``default == -1`` check read the caller's ``-1`` (and ``-1.0``, which
compares equal) as *no default* and raised (GitHub issue #1062).
or as the old marker. Here it resolves to that member, while the
pre-#857 ``default == -1`` check read the caller's ``-1`` (and ``-1.0``,
which compares equal) as *no default* and raised.
"""
from aenum import IntEnum

Expand All @@ -1433,12 +1431,11 @@ def test_negative_one_defaults_on_a_shipped_registry_raise_and_mint_nothing(self
Neither is a registered value of any shipped registry, whose domains
all start at ``0``, so ``default`` does not resolve: there is no
``cls(default)`` fallback left to attempt, and the name lookup's own
:exc:`KeyError` propagates, as with no default at all. Pinned from the
pre-#864 tree, which raised :exc:`ValueError` from ``cls(-1)``. This
does *not* discriminate #857, which raised the same :exc:`KeyError`;
:exc:`KeyError` propagates, as with no default at all; the pre-#864
tree raised :exc:`ValueError` from ``cls(-1)`` instead. This does *not*
discriminate #857, which raised the same :exc:`KeyError`;
:meth:`test_missing_name_with_default_negative_one_is_now_a_real_default`
does. Merged from separate ``-1`` and ``-1.0`` tests (GitHub issue
#1062).
does.
"""
from pcapkit.const.ipv6.extension_header import ExtensionHeader

Expand Down
43 changes: 19 additions & 24 deletions tests/integration/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,30 +64,25 @@ class EndToEndTestCase(unittest.TestCase):
def setUpClass(cls) -> None:
"""Drop the imported library so the class starts from a clean state.

The surrounding tiers used to purge in :meth:`setUp`, i.e. once per
test, and since GitHub issue #1065 mostly purge once per class too. A
fresh :mod:`pcapkit` import measures at roughly 0.7s on this machine,
which across this tier would cost more than the extractions themselves,
so the purge happens once per class instead. That is equivalent here:
every test below imports :mod:`pcapkit` inside the test method, so none
of them depends on what an earlier test left in :data:`sys.modules`.

The re-import on the last line is what keeps it once per class, and it
has to happen here rather than being left to the first test.
:func:`tests.conftest.restore_module_table` snapshots the module table
immediately after this method and restores to that snapshot after every
test, so purging without re-importing makes the snapshot an *empty*
table -- and then all 92 of this tier's test methods pay the 0.7s each,
rather than one per class across its 28 classes. Measured with a
throwaway subclass of this class whose second and third test methods
report whether ``pcapkit`` is still in :data:`sys.modules`: ``True`` on
``mainline`` and ``True`` with this line, ``False`` without it. The other
``setUpClass`` methods in the suite are unaffected because each already
loads something immediately after its own purge, which populates the
table before the snapshot is taken.

The re-import is guarded because it is an optimisation and nothing more,
so it must not be able to turn a test failure into a class error. Most
Under plain :mod:`unittest` this gives each class one fresh
:mod:`pcapkit` import, which its tests then share. A fresh import
measures at roughly 0.7s, which per test would cost this tier more than
the extractions themselves. That is equivalent here: every test below
imports :mod:`pcapkit` inside the test method, so none of them depends on
what an earlier test left in :data:`sys.modules`.

Under :program:`pytest` neither the purge nor the re-import reaches the
tests. :func:`tests.conftest.restore_module_table` restores the region
before every test to the warm snapshot
:func:`tests.conftest._pin_module_snapshot` took at session start, so
each test runs on that session-wide import instead. Measured with a
throwaway subclass whose tests report the ``id`` of
``sys.modules['pcapkit']``: under :program:`pytest` every test saw the
object from before this method ran, under :mod:`unittest` every test saw
the one imported here.

The re-import is guarded because nothing depends on it, so it must not be
able to turn a test failure into a class error. Most
subclasses carry ``@skipUnless(HAS_RUNTIME, ...)`` and never reach this
method without the runtime dependencies installed, but
``PlistRoundTripTests`` and ``PcapngUnescapedKeyTests`` do not, so on a
Expand Down
7 changes: 4 additions & 3 deletions tests/utilities/test_compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@
#: :mod:`pcapkit.protocols.misc.pcap.frame`, :mod:`pcapkit.protocols.misc.pcapng`,
#: :mod:`pcapkit.protocols.link.ethernet` and :mod:`pcapkit.protocols.link.arp`.
#: None of them is reachable from the branches faked here, and if one became
#: reachable it would still be harmless: the region is purged and restored around
#: every test, so the next import recomputes the value from source. A third-party
#: reachable it would still be harmless: the region is restored around every
#: test, so the next import recomputes the value from source. A third-party
#: module is never purged, which is exactly why its cache is permanent and why the
#: assertion is aimed there.
OWN_MODULE_PREFIX = 'pcapkit'
Expand Down Expand Up @@ -172,7 +172,8 @@ class CompatTests(unittest.TestCase):
def setUp(self) -> None:
reimport_once_per_class(self)

# Registered before anything else, and deliberately first: cleanups run
# Registered deliberately early -- only ``reimport_once_per_class``'s own
# cleanup, which has to run last, comes before it. Cleanups run
# last-in-first-out, so this one runs *after* the module-table restore
# that ``load_module`` arranges below and after the test body however it
# ended -- including when it ended by raising. It is a sweep of a handful
Expand Down
Loading