Skip to content

Commit d0e35e3

Browse files
committed
docs(tests): state corekit, utilities and dumpkit tests' behaviour without change-relative narrative (#1103)
Rewrite comments and docstrings in tests/corekit (except _roundtrip.py), tests/utilities, tests/dumpkit, tests/interface and tests/cli so they state the behaviour pinned and the defect guarded against ("without the fix ...") rather than narrating a change ("now", "used to", "before this change"). Pure history is cut. Prose only: the AST with docstrings stripped is unchanged.
1 parent 29e82b0 commit d0e35e3

57 files changed

Lines changed: 933 additions & 1220 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/cli/test_main.py‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,25 +10,24 @@
1010
:data:`sys.modules` is process-global, so binding a stand-in over a real module
1111
name is a write that outlives the test unless something undoes it -- and these
1212
stand-ins are *emptier* than what they replace, which is the quiet kind. Issue
13-
#688: this file bound module names there -- the stand-ins above, the bare
13+
#688 is this file binding module names there -- the stand-ins above, the bare
1414
``pcapkit.foundation`` parent they hang from, and ``pcapkit.__main__`` itself
15-
-- and put none of them back. Pairing it with :mod:`tests.project.test_public_api`
16-
gave a run of ``ImportError: cannot import name 'show_flag_values' from
15+
-- and putting none of them back. Paired with :mod:`tests.project.test_public_api`,
16+
that gives a run of ``ImportError: cannot import name 'show_flag_values' from
1717
'pcapkit.utilities.compat'`` errors and one for ``SeekError``, because the real
18-
library's next import found a ``pcapkit.utilities.compat`` carrying exactly one name.
18+
library's next import finds a ``pcapkit.utilities.compat`` carrying exactly one name.
1919
2020
That is the same defect as issues #660 and #674, and it is fixed the same way:
2121
:func:`tests._support.isolate_modules` in ``setUp``, which purges the region on
22-
the way in and restores it exactly on the way out. What this file used to do
23-
instead was roll its own purge loop, which protects this file from whatever ran
24-
before it and promises nothing to whatever runs after -- see
22+
the way in and restores it exactly on the way out. A purge loop of this file's
23+
own would not do: it protects this file from whatever ran before it and promises
24+
nothing to whatever runs after -- see
2525
:func:`tests._support.purge_modules` for why restoration is owed by the code that
2626
binds.
2727
28-
:func:`tests.conftest.restore_module_table` masked the symptom under
29-
:program:`pytest` throughout, which is why it took an audit to find: it surfaces
30-
only under ``--noconftest``, under the stdlib :mod:`unittest` runner, or on any
31-
route that does not load that conftest.
28+
:func:`tests.conftest.restore_module_table` masks the symptom under
29+
:program:`pytest`: it surfaces only under ``--noconftest``, under the stdlib
30+
:mod:`unittest` runner, or on any route that does not load that conftest.
3231
:meth:`CLIMainTests.assert_module_table_restored` is checked in a cleanup
3332
registered *before* the isolation, so it runs after the restore and is not
3433
masked by that fixture -- a regression here fails under a plain
@@ -408,10 +407,10 @@ def test_layer_defaults_to_none_and_is_case_insensitive(self) -> None:
408407
"""An omitted ``-L``/``-P`` forwards :data:`None`, not a sentinel string.
409408
410409
``Extractor.__init__`` substitutes its own ``'none'``/``'null'`` for an
411-
omitted value, so the CLI has nothing to substitute. It used to pass the
412-
strings ``'None'`` and ``'null'``, which only worked because
413-
``Extractor`` happened to lowercase the former into the sentinel it
414-
wanted.
410+
omitted value, so the CLI has nothing to substitute. Forwarding the
411+
strings ``'None'`` and ``'null'`` instead works only because
412+
``Extractor`` happens to lowercase the former into the sentinel it
413+
wants.
415414
416415
"""
417416
emoji = types.SimpleNamespace(emojize=lambda text: text)

‎tests/corekit/test_bitfield_unnamed_bits_1487_unit.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
# -*- coding: utf-8 -*-
22
"""A BitField packs back the bits its namespace does not name.
33
4-
GitHub issue #1487: :class:`~pcapkit.corekit.fields.strings.BitField` read only
5-
its named subfields and packed from all zeros, so octets with a bit outside
6-
every subfield set came back with it cleared. The unpacked value now carries
7-
those bits under ``__unnamed__`` when any is set, and a value without the key
8-
packs them as zeros, as before.
4+
GitHub issue #1487 guards against :class:`~pcapkit.corekit.fields.strings.BitField`
5+
reading only its named subfields and packing from all zeros, so that octets with a
6+
bit outside every subfield set come back with it cleared. The unpacked value
7+
carries those bits under ``__unnamed__`` when any is set, and a value without the
8+
key packs them as zeros.
99
1010
:mod:`pcapkit` is imported inside each test, after
1111
:func:`~tests._support.reimport_once_per_class`, not at module load.

‎tests/corekit/test_decorators.py‎

Lines changed: 34 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,9 @@ def _protect_global_exception_state(self) -> None:
3131
``StructError('unexpected eof', eof=True)`` does not pass ``quiet=True``,
3232
for one -- and a loud error outside development mode installs
3333
:mod:`pcapkit.utilities.exceptions`'s own exception hook on first use
34-
(GitHub issue #719). This file had no ``tearDown`` before that issue and
35-
never needed one: the equivalent side effect at the time,
36-
``sys.tracebacklimit = 0``, was already left unrestored, corrupting
37-
every test that ran afterwards in the same process -- the #981 shape,
38-
just not previously visible as a failure here. Addressed the way
34+
(GitHub issue #719). Left unrestored, that process-global corrupts every
35+
test that runs afterwards in the same process -- the #981 shape, whether
36+
or not it shows as a failure here. It is restored the way
3937
``isolate_modules`` addresses its own restore: via ``addCleanup``, so it
4038
still runs if the rest of ``setUp`` raises partway through.
4139
@@ -131,9 +129,9 @@ def _demo_schema_for_call_shapes(self):
131129
132130
Shared by the five call-shape tests below, one per row of `#444
133131
<https://github.com/JarryShaw/PyPCAPKit/issues/444>`__'s reproduction
134-
table: ``prepare`` used to subscript ``args[2]``/``args[3]``
135-
unconditionally, so only the three-positional shape worked and every
136-
shorter or keyword call raised ``IndexError`` instead of falling back
132+
table: without the fix, ``prepare`` subscripts ``args[2]``/``args[3]``
133+
unconditionally, so only the three-positional shape works and every
134+
shorter or keyword call raises ``IndexError`` instead of falling back
137135
to the documented ``length=None, packet=None`` defaults.
138136
139137
"""
@@ -187,20 +185,20 @@ def test_prepare_accepts_keyword_length_and_packet(self) -> None:
187185
self._assert_call_shape_result(DemoSchema.unpack(b'payload', length=7, packet={}))
188186

189187
def test_prepare_raises_typeerror_for_extra_positional_and_keyword_arguments(self) -> None:
190-
"""Extras used to vanish silently instead of being forwarded; see #454.
188+
"""Extras raise :exc:`TypeError` rather than vanishing silently; see #454.
191189
192-
``prepare``'s own docstring promised the decorated function receives
193-
``*args, **kwargs``, but the wrapper never populated either --
194-
reproduced from the issue::
190+
Without the fix, ``prepare``'s own docstring promises the decorated
191+
function receives ``*args, **kwargs``, but the wrapper populates neither
192+
-- reproduced from the issue::
195193
196194
Probe.unpack(b'\\x07\\x08', 2, {}, 'EXTRA_POSITIONAL', extra_kw='EXTRA_KW')
197195
-> a=7 b=8 # both extras silently gone, no error
198196
199-
The chosen fix removes the promise (nothing in the tree ever passed
200-
extras, and ``@prepare`` decorates exactly one function, whose real
201-
signature never had room for them) and rejects extras instead of
202-
forwarding them, so a misspelled or unsupported argument is a
203-
``TypeError`` rather than a parse that silently ignored it.
197+
``prepare`` makes no such promise (nothing in the tree passes extras, and
198+
the real signature of the function ``@prepare`` decorates has no room for
199+
them) and rejects extras instead of forwarding them, so a misspelled or
200+
unsupported argument is a ``TypeError`` rather than a parse that silently
201+
ignores it.
204202
205203
"""
206204
DemoSchema = self._demo_schema_for_call_shapes()
@@ -211,11 +209,11 @@ def test_prepare_raises_typeerror_for_extra_positional_and_keyword_arguments(sel
211209
def test_prepare_raises_typeerror_for_length_given_both_positionally_and_by_keyword(self) -> None:
212210
"""The narrower case #454 calls out.
213211
214-
Passing ``length`` both positionally and by keyword used to silently
215-
keep the positional value and drop the keyword one -- the keyword
216-
never reached ``kwargs.pop``, since that branch only runs when the
217-
positional slot was *not* supplied. It is now a caller error like any
218-
other unconsumed argument.
212+
Passing ``length`` both positionally and by keyword is a caller error
213+
like any other unconsumed argument. Without the fix, the positional
214+
value is silently kept and the keyword one dropped -- the keyword never
215+
reaches ``kwargs.pop``, since that branch only runs when the positional
216+
slot is *not* supplied.
219217
220218
"""
221219
DemoSchema = self._demo_schema_for_call_shapes()
@@ -241,13 +239,13 @@ def unpack(cls, data, length=None, packet=None):
241239
DemoSchema.unpack(b'', None, None)
242240

243241
def test_prepare_raises_stream_eof_error_not_a_bare_eof_error(self) -> None:
244-
"""The genuinely-exhausted case now raises an in-library exception.
242+
"""The genuinely-exhausted case raises an in-library exception.
245243
246-
A caller could not previously tell a truncated capture apart from any
247-
other ``EOFError``. ``StreamEOFError`` still *is* an ``EOFError`` --
248-
so ``pcapkit.foundation.extraction.Extractor``'s existing ``except
249-
(EOFError, StopIteration)`` keeps working unchanged -- but it can now
250-
be caught specifically via
244+
That lets a caller tell a truncated capture apart from any other
245+
``EOFError``. ``StreamEOFError`` *is* an ``EOFError`` -- so
246+
``pcapkit.foundation.extraction.Extractor``'s ``except
247+
(EOFError, StopIteration)`` catches it -- but it can also be caught
248+
specifically via
251249
:class:`pcapkit.utilities.exceptions.StreamEOFError`.
252250
253251
"""
@@ -299,7 +297,7 @@ def test_prepare_accepts_a_declared_zero_length_schema(self) -> None:
299297
class Empty(Schema):
300298
pass
301299
Empty.unpack(b'', 0, {})
302-
-> EOFError: EOFError() # before the fix
300+
-> EOFError: EOFError() # without the fix
303301
304302
Distinguishing this from the truncated-stream case above is what
305303
``length is not None`` (a *declared* length, even zero) checks for.
@@ -333,12 +331,12 @@ def test_prepare_leaves_the_schema_clean_after_post_process(self) -> None:
333331
334332
``Schema.unpack`` clears ``__updated__`` before returning, but
335333
``post_process`` runs after it and every field it assigns sets the flag
336-
again. The schema was therefore left dirty, and the next
337-
``bytes(schema)`` re-packed it -- which calls ``post_process`` a second
334+
again. Without the fix the schema is therefore left dirty, and the next
335+
``bytes(schema)`` re-packs it -- which calls ``post_process`` a second
338336
time with a packet context rebuilt from the schema's own fields, so any
339-
value ``post_process`` had derived from the *enclosing* layer's context
340-
was overwritten with the fallback. ``Schema.pack`` clears the flag after
341-
``post_process``, and this asserts the unpacking path now matches it.
337+
value ``post_process`` derived from the *enclosing* layer's context is
338+
overwritten with the fallback. ``Schema.pack`` clears the flag after
339+
``post_process``, and this asserts the unpacking path matches it.
342340
343341
"""
344342
class DemoSchema:
@@ -496,8 +494,8 @@ def test_beholder_uses_the_protocol_payload_accessor_not_the_schema(self) -> Non
496494
so neither header schema has a ``payload`` field: reaching for the schema
497495
raises ``ProtocolUnbound('unknown field: payload')`` *from the recovery
498496
path*, turning a next-layer failure that should have degraded to
499-
:class:`Raw` into a crash. It was unreachable until something was
500-
registered on an SCTP payload protocol identifier, which NGAP now is.
497+
:class:`Raw` into a crash. It is reachable once something is registered
498+
on an SCTP payload protocol identifier, as NGAP is.
501499
502500
"""
503501
exceptions = self.exceptions

‎tests/corekit/test_decorators_path_1735_unit.py‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
44
GitHub issue #1735, applying #1712's placement rule. ``seekset``, ``beholder``
55
and ``prepare`` act on a protocol's file and the schema it unpacks, and
6-
``beholder`` falls back to protocol classes, so they moved from
7-
``pcapkit.utilities.decorators`` to :mod:`pcapkit.corekit.decorators`. Per the
8-
ruling on #1715 nothing is left at the old path, and :mod:`pcapkit.utilities`
9-
no longer exports the three names. This pins both halves of that: the old path
6+
``beholder`` falls back to protocol classes, so they live in
7+
:mod:`pcapkit.corekit.decorators` rather than ``pcapkit.utilities.decorators``.
8+
Per the ruling on #1715 nothing is left at the old path, and
9+
:mod:`pcapkit.utilities` does not export those names. This pins both halves of
10+
that: the old path
1011
and the old exports are gone, and every importer uses the new module.
1112
1213
"""
@@ -23,7 +24,7 @@
2324
#: Where the decorators live.
2425
NEW_PATH = 'pcapkit.corekit.decorators'
2526

26-
#: Where they lived before #1735; removed, not re-exported.
27+
#: The path they moved from (#1735); nothing is left there, not even a re-export.
2728
OLD_PATH = 'pcapkit.utilities.decorators'
2829

2930
#: The three decorators.

‎tests/corekit/test_deferred_packet_1516_unit.py‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@
22
"""GitHub issue #1516: one :class:`~pcapkit.corekit.packet.DeferredPacket` for
33
reassembly and flow tracing.
44
5-
There used to be two copies of the mixin, in
6-
``pcapkit/foundation/reassembly/data/data.py`` and
7-
``pcapkit/foundation/traceflow/data/data.py``. Their method bodies were
8-
AST-identical, and the one thing that differed was which module-level
9-
``Deferred`` the ``isinstance`` test named. The merged class takes that
5+
The mixin serves the models of both ``pcapkit/foundation/reassembly/data/data.py``
6+
and ``pcapkit/foundation/traceflow/data/data.py``, which differ only in which
7+
module-level ``Deferred`` the ``isinstance`` test names. The class takes that
108
difference as the ``__deferred__`` hook each subclass sets, so a model resolves
119
its own placeholder class and nothing else.
1210

0 commit comments

Comments
 (0)