Skip to content

Commit d8d28d1

Browse files
authored
docs(tests): state foundation, toolkit and root tests' behaviour without change-relative narrative (#1103) (#2195)
Rewrite comments and docstrings that described behaviour relative to a fix ("now", "used to", "before this change") as timeless statements of behaviour or of the defect guarded against. Prose only; AST unchanged once docstrings are stripped.
1 parent 0583a96 commit d8d28d1

53 files changed

Lines changed: 337 additions & 374 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎tests/_dependency_gates.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,8 @@
178178
# :file:`pyproject.toml` extra declares it at all -- so
179179
# :func:`extras_providing` resolves it to the empty set and every pytest
180180
# job reaching a ``HAS_MYPY`` gate is reported as a :class:`Gap`. That is
181-
# the honest answer rather than the :exc:`KeyError` a missing entry used to
182-
# raise: the gate really is dark in CI, and
181+
# the honest answer rather than a :exc:`KeyError` for a missing entry: the
182+
# gate really is dark in CI, and
183183
# :data:`DEPENDENCY_GATE_EXCLUSIONS`'s ``HAS_MYPY`` entry is where the reason
184184
# for declining to close it lives (#779). Spelled as a requirement string
185185
# anyway, so the day some extra does carry mypy the gap closes itself
@@ -190,8 +190,8 @@
190190
# (#766), so :func:`extras_providing` resolves it to ``{'test'}``, every
191191
# pytest job installs it, and ``HAS_ISORT`` is a satisfied gate with no
192192
# :data:`DEPENDENCY_GATE_EXCLUSIONS` entry at all -- which is the whole point
193-
# of listing it here: the guard now *counts* a gate it could not see before,
194-
# and finds nothing to report.
193+
# of listing it here: the guard *counts* a gate it could not see without
194+
# the entry, and finds nothing to report.
195195
#
196196
# Why the opposite ruling from ``mypy``, whose gap is declined above: that
197197
# entry's reason turns on lint.yml already running mypy over the whole
@@ -539,7 +539,7 @@ class Job(NamedTuple):
539539
That is deliberate rather than an oddity to special-case away:
540540
:func:`dependency_gate_gaps` aggregates by ``(flag, job.name)``, and a gate
541541
reached by *any* of a job's per-engine variants is exactly what "this job
542-
reaches this gate" has to mean once one job's cells no longer all install
542+
reaches this gate" has to mean when one job's cells do not all install
543543
and select the same thing.
544544
545545
"""

‎tests/conftest.py‎

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -137,23 +137,22 @@ def pytest_sessionstart(session: 'pytest.Session') -> 'None':
137137
happens here decides what every test is restored *to*. Left cold, the
138138
region is empty at the pin and restored to empty after every test, which
139139
makes each of the tests that does not purge for itself -- :mod:`tests.project`
140-
is the bulk of them -- pay a fresh ``import pcapkit`` it used to get from the
141-
warm table. Measured over ``tests/project/``: 1.59s with no
140+
is the bulk of them -- pay a fresh ``import pcapkit`` it would otherwise get
141+
from the warm table. Measured over ``tests/project/``: 1.59s with no
142142
guard at all, **9.34s** guarded from a cold table, **0.35s** guarded from a
143143
warm one. The last is the fastest of the three because the import it would
144144
otherwise do inside the first test has moved here instead.
145145
146-
This used **not** to help a class that purges in
147-
:meth:`~unittest.TestCase.setUpClass`, for the reason
148-
:func:`_pin_module_snapshot` explains: such a class purges *after* this hook
149-
and *before* the first snapshot the old, entry-scoped fixture took, so that
150-
snapshot was whatever the class left -- warming the table earlier changed
151-
nothing about it. Restoring to a *pinned* snapshot before each test as well
152-
as after, rather than to one taken at that test's own entry, is what makes
153-
the warm-up matter to a purging class too now: the pin below still captures
154-
this import, and every later test -- including a purging class's own first
155-
one -- is restored to it going in as well as coming out, regardless of
156-
whether that class reloads anything for itself.
146+
It helps a class that purges in :meth:`~unittest.TestCase.setUpClass` too.
147+
Such a class purges *after* this hook and *before* its first test, so a
148+
snapshot taken at that test's own entry would be whatever the class left --
149+
warming the table earlier would change nothing about it, as
150+
:func:`_pin_module_snapshot` explains. Restoring to a *pinned* snapshot
151+
before each test as well as after is what makes the warm-up matter to a
152+
purging class: the pin below captures this import, and every later test --
153+
including a purging class's own first one -- is restored to it going in as
154+
well as coming out, regardless of whether that class reloads anything for
155+
itself.
157156
So a class that purges and reimports for itself, as
158157
:class:`tests.integration._helpers.EndToEndTestCase` does, still runs its
159158
tests on the pin. Only a module object a class attribute holds a direct
@@ -269,13 +268,11 @@ def restore_module_table() -> 'Iterator[None]':
269268
that will be written, which is the difference between the failure being
270269
unlikely and being impossible.
271270
272-
The others were fixed at their call sites when this guard landed, but
273-
:mod:`tests.cli.test_main` was not: it kept its own purge loop, which
274-
purges and restores nothing, and this fixture went on quietly healing it for
275-
every test. That is issue #688 -- it took an audit rather than a failing run
276-
to find, because a guard that repairs a leak also hides it. It is now fixed
277-
at its call site too, which is what makes the paragraph above true of all
278-
three rather than of two.
271+
A guard that repairs a leak also hides it. Issue #688 -- a purge loop in
272+
:mod:`tests.cli.test_main` that purged and restored nothing, which this
273+
fixture quietly healed for every test -- took an audit rather than a
274+
failing run to find. That is why every leak is fixed at its call site as
275+
well.
279276
280277
Deliberately *not* fixed by moving ``test_protochain.py`` so it sorts
281278
elsewhere, or by leaning on a neighbouring test's purge to heal the state.
@@ -603,7 +600,7 @@ def _tier_guard_lock(exclusive: 'bool') -> 'Iterator[None]':
603600
writes a real violation to disk, under :data:`~tests._tiers.PROBE_MODULE_NAME`, to
604601
reproduce the tier guard's own collection-time diagnostic end to end.
605602
Serially the two never overlap; under the ``-n auto --dist load`` this
606-
suite's CI jobs now use, pytest-xdist may hand the two tests to different
603+
suite's CI jobs use, pytest-xdist may hand the two tests to different
607604
worker processes at the same moment, and the scan can then observe the
608605
probe mid-life and report a violation nobody committed -- a real defect in
609606
the test suite's own concurrency, not a false positive, since the guard is

‎tests/foundation/engines/test_dpkt_raw_null_linktypes_unit.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
GitHub issues #1502 and #1574. :meth:`DPKT._get_protocol
55
<pcapkit.foundation.engines.dpkt.DPKT._get_protocol>` knew Ethernet, IPv4 and
66
IPv6 only, so a ``LINKTYPE_RAW`` (101), ``LINKTYPE_NULL`` (0) or
7-
``LINKTYPE_LOOP`` (108) frame was kept as raw data with a warning. Raw IP is now
7+
``LINKTYPE_LOOP`` (108) frame was kept as raw data with a warning. Raw IP is
88
read as IPv4 or IPv6 by its version nibble, and both loopback link types by the
99
address family, as :class:`~pcapkit.protocols.link.loopback.Loopback` reads
1010
them, with the network layer where :mod:`pcapkit.toolkit.dpkt` looks for it.

‎tests/foundation/engines/test_engine_base.py‎

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -69,25 +69,22 @@ def read_frame(self) -> str:
6969
def test_engine_subclass_registration_is_opt_in(self) -> None:
7070
"""Registration happens if and only if ``engine`` is given.
7171
72-
This is the #514 opt-in contract. Before it, ``__init_subclass__`` fell
73-
back to ``cls.name`` when the keyword was absent, so *every* subclass of
74-
the public class was registered -- which is why no built-in could
75-
subclass it and every one of them inherited :class:`EngineBase` under an
76-
alias instead.
72+
This is the #514 opt-in contract. It guards against ``__init_subclass__``
73+
falling back to ``cls.name`` when the keyword is absent, which registers
74+
*every* subclass of the public class.
7775
7876
The ``Default`` case is the load-bearing half: it sets
79-
``__engine_name__``, so under the old fallback it registered under
80-
``'defaultengine'``. A class attribute is not a registry key, and it
77+
``__engine_name__``, so a fallback to ``cls.name`` would register it under
78+
``'defaultengine'``. A class attribute is not a registry key, so it
8179
registers nothing.
8280
83-
No version guard, and that is the point of the rename. The keyword was
84-
``name`` when opt-in registration landed, which made this half of the test
85-
unrunnable below Python 3.11 -- ``mcls``/``name``/``bases``/``namespace``
86-
collide with :meth:`abc.ABCMeta.__new__`'s own parameters, which are
87-
positional-or-keyword before 3.11 and positional-only from 3.11, so the
88-
class statement failed in the metaclass before ``__init_subclass__`` ran.
89-
``engine`` is outside that set, so this runs everywhere. Verified on
90-
3.10.21 and 3.14.7.
81+
No version guard, and that is why the keyword is ``engine`` rather than
82+
``name``: ``mcls``/``name``/``bases``/``namespace`` collide with
83+
:meth:`abc.ABCMeta.__new__`'s own parameters, which are
84+
positional-or-keyword before 3.11 and positional-only from 3.11, so below
85+
Python 3.11 such a class statement fails in the metaclass before
86+
``__init_subclass__`` runs. ``engine`` is outside that set, so this runs
87+
everywhere. Verified on 3.10.21 and 3.14.7.
9188
9289
"""
9390
from pcapkit.foundation.engines.engine import Engine
@@ -123,10 +120,9 @@ def test_engine_subclass_rejects_unrecognised_keyword(self) -> None:
123120
124121
``Engine`` spells the registry key ``engine`` while ``Reassembly`` and
125122
``TraceFlow`` spell the same idea ``protocol``, so guessing the wrong one
126-
is the expected mistake. It used to land in ``**kwargs``, get dropped by
127-
the bare ``super().__init_subclass__()``, and leave the class registered
128-
under its own class name. With registration now opt-in the same typo
129-
would instead skip registration silently, which is quieter still.
123+
is the expected mistake. Without the guard it lands in ``**kwargs``, is
124+
dropped by a bare ``super().__init_subclass__()``, and, registration being
125+
opt-in, the class silently goes unregistered.
130126
131127
Unguarded by version on purpose: ``protocol`` is not one of the four
132128
names that collide with :meth:`abc.ABCMeta.__new__` below Python 3.11, so
@@ -138,7 +134,7 @@ def test_engine_subclass_rejects_unrecognised_keyword(self) -> None:
138134
version-dependent: below 3.11 the metaclass collision fires before the
139135
guard, so it is a :exc:`TypeError` rather than an
140136
:exc:`~pcapkit.utilities.exceptions.UnsupportedCall`. Loud either way,
141-
which is why the rename did not need to chase it.
137+
which is why the guard needs no special case for it.
142138
143139
"""
144140
from pcapkit.foundation.engines.engine import Engine
@@ -179,17 +175,18 @@ def test_registration_is_not_inherited_by_a_subclass(self) -> None:
179175
``final`` marker -- so that subclassing them again cannot register the
180176
subclass under a name nobody chose. That worked, but it is a property of
181177
*which base you inherited*, so it cannot be varied per subclass, and
182-
under the old fallback a subclass of a registered class re-registered
183-
itself under its own class name.
178+
under a ``cls.name`` fallback a subclass of a registered class
179+
re-registers itself under its own class name.
184180
185181
Keying registration on the keyword instead gives both properties at once:
186182
the parent stays registered, the subclass does not re-register, and a
187183
subclass that *wants* registering can still ask for it. Which is why
188184
``HTTP``, ``L2TP`` and ``IP`` can be open to inheritance without the
189185
anti-re-registration guarantee being weakened.
190186
191-
Runs on every supported version since the keyword became ``engine``;
192-
it was skipped below 3.11 while the keyword was ``name``.
187+
Runs on every supported version: ``engine`` does not collide with
188+
:meth:`abc.ABCMeta.__new__`'s own parameters below 3.11, as ``name``
189+
would.
193190
194191
"""
195192
from pcapkit.foundation.engines.engine import Engine

‎tests/foundation/engines/test_engine_verbose_callback_1874_unit.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
:class:`~pcapkit.foundation.extraction.Extractor` keeps a callable ``verbose``
55
as its ``_vfunc``, but each third-party engine installed its own printer
66
whenever verbose output was on, so the caller's callback was replaced and never
7-
called. An engine now installs its printer only for ``verbose=True``.
7+
called. An engine installs its printer only for ``verbose=True``.
88
99
Each case runs a real extraction of :file:`in.pcap`, which git tracks, so the
1010
module is unit tier. It proves first that the requested engine ran: the

‎tests/foundation/engines/test_pcapng_engine.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -414,10 +414,10 @@ def test_packet_blocks_without_an_interface_description_are_format_errors(self)
414414
self.assertIn('interface description block', message)
415415

416416
def test_out_of_range_interface_id_is_a_format_error(self) -> None:
417-
# Regression for #367: a section with at least one IDB and a block naming
418-
# an interface it does not describe used to escape as a bare
419-
# ``IndexError`` out of ``PCAPNG._get_timezone``, because the engine's
420-
# bounds guards ran only after the block had already been parsed.
417+
# Regression for #367: guards against a section with at least one IDB and
418+
# a block naming an interface it does not describe escaping as a bare
419+
# ``IndexError`` out of ``PCAPNG._get_timezone``, which happens when the
420+
# engine's bounds guards run only after the block has been parsed.
421421
from pcapkit.utilities.exceptions import FormatError
422422

423423
blocks = {

‎tests/foundation/engines/test_pcapng_spb_zero_snaplen_1558_unit.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
0's snaplen, and a snaplen of 0 means no limit. The default engine handed
66
interface 0's snaplen to the parser as ``__packet__['snaplen']`` as it stood, so
77
a 0 read as a limit of no octets, and the block's padding was read as packet
8-
data: 57 data octets came out as 60. The engine now hands over no limit for a
8+
data: 57 data octets came out as 60. The engine hands over no limit for a
99
zero snaplen, and :class:`~pcapkit.protocols.misc.pcapng.PCAPNG` reads a zero
1010
snaplen handed in as no limit too.
1111

‎tests/foundation/engines/test_pypcap_unusable_module_2132_unit.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131

3232
#: What the fake ``pcap`` package raises on import: the libpcap loader's message.
3333
LOADER_ERROR = 'Cannot find libpcap.so library'
34-
#: The verdict ``PCAP_CT`` already gave on that failure, which ``PyPCAP`` shares.
34+
#: The verdict ``PCAP_CT`` and ``PyPCAP`` both give on that failure.
3535
REASON = f'the `pcap` module is installed but unusable -- OSError: {LOADER_ERROR}'
3636
#: Hang guard for the child interpreter, in seconds.
3737
TIMEOUT = 120

‎tests/foundation/engines/test_pyshark_engine.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
* **the** :program:`tshark` **binary** -- ``pyshark`` shells out to it and parses
2020
nothing itself.
2121
22-
What the running host provides is no longer assumed either way, and that is worth
22+
What the running host provides is not assumed either way, and that is worth
2323
stating plainly rather than leaving to a silent skip: :program:`tshark` is absent
2424
on most of this file's legs, but #751's ``engine-tests`` job -- which collects this
2525
same module -- installs it via ``apt-get``, so neither "present" nor "absent" can

‎tests/foundation/engines/test_scapy_engine.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
public API resets them, and ``sys.modules`` surgery does not un-run a
2222
``bind_layers`` call. So an in-process assertion here would pass whenever
2323
something earlier in the run had imported a `Scapy`_ layer -- which is exactly
24-
what happened before #406 was found. :mod:`tests.toolkit.test_scapy_unit` imports
24+
how #406 went unnoticed. :mod:`tests.toolkit.test_scapy_unit` imports
2525
:mod:`scapy.layers.l2`, :mod:`~scapy.layers.inet` and :mod:`~scapy.layers.inet6`
2626
to build its fixtures, so ``pytest tests/toolkit tests/interface/test_misc.py``
2727
saw correct dissection while ``pytest tests/interface/test_misc.py`` alone did

0 commit comments

Comments
 (0)