diff --git a/CHANGELOG.md b/CHANGELOG.md index 56e789c53..fb75be38d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,7 @@ Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` and `1.5.0b2` (both 2026-09-18) an #### Changed - `Probe`, `CipherSuite` and `IntegritySuite` are `Info` subclasses rather than `typing.NamedTuple`, and no `NamedTuple` remains in the package. They are Mappings now, so `len()` and iteration yield field names rather than values. -- `ModuleDescriptor.klass` reads an already-imported module out of `sys.modules` rather than re-entering `importlib.import_module`, since next layer dispatch resolves a descriptor there on a per-frame path. A registry *hit* holding a `ModuleDescriptor` is resolved once and written back; a *miss* deliberately is not -- recording a miss in a class-level `collections.defaultdict` is the defect [#421](https://github.com/JarryShaw/PyPCAPKit/issues/421)/[#426](https://github.com/JarryShaw/PyPCAPKit/pull/426) fixed at this layer, [#425](https://github.com/JarryShaw/PyPCAPKit/issues/425)/[#428](https://github.com/JarryShaw/PyPCAPKit/pull/428) for the option, chunk and block registries, and [#555](https://github.com/JarryShaw/PyPCAPKit/issues/555)/[#560](https://github.com/JarryShaw/PyPCAPKit/pull/560) at the schema layer. A resolution now costs ~117 ns rather than ~436 ns, which is **not measurable in** `extract()` **wall clock**. The change is taken for its shape: `sys.modules` stays the only module cache, whereas a memo of the class would serve the pre-reload class after an `importlib.reload` forever and fail `isinstance` against the live one. Proposed by `@Ts-Boom` in [#563](https://github.com/JarryShaw/PyPCAPKit/pull/563); the implementation differs because that one added a second, never-invalidated cache of resolved classes ([#574](https://github.com/JarryShaw/PyPCAPKit/issues/574)). +- `ModuleDescriptor.klass` reads an already-imported module out of `sys.modules` rather than re-entering `importlib.import_module`, since next layer dispatch resolves a descriptor there on a per-frame path. Neither a registry *hit* holding a `ModuleDescriptor` nor a *miss* writes the resolved class back: a memoised hit went stale on `importlib.reload` ([#1559](https://github.com/JarryShaw/PyPCAPKit/issues/1559)), and recording a miss in a class-level `collections.defaultdict` is the defect [#421](https://github.com/JarryShaw/PyPCAPKit/issues/421)/[#426](https://github.com/JarryShaw/PyPCAPKit/pull/426) fixed at this layer, [#425](https://github.com/JarryShaw/PyPCAPKit/issues/425)/[#428](https://github.com/JarryShaw/PyPCAPKit/pull/428) for the option, chunk and block registries, and [#555](https://github.com/JarryShaw/PyPCAPKit/issues/555)/[#560](https://github.com/JarryShaw/PyPCAPKit/pull/560) at the schema layer. A resolution now costs ~117 ns rather than ~436 ns, which is **not measurable in** `extract()` **wall clock**. The change is taken for its shape: `sys.modules` stays the only module cache, whereas a memo of the class would serve the pre-reload class after an `importlib.reload` forever and fail `isinstance` against the live one. Proposed by `@Ts-Boom` in [#563](https://github.com/JarryShaw/PyPCAPKit/pull/563); the implementation differs because that one added a second, never-invalidated cache of resolved classes ([#574](https://github.com/JarryShaw/PyPCAPKit/issues/574)). - `SeekableReader.truncate()` raises instead of returning a size, and the misspelled `writeable()` is now `writable()`. Both break external callers; nothing inside the package calls either. `io.IOBase` documents one gate over two methods -- "If False, write() and truncate() will raise OSError" -- and this reader's `writable()` answers False, yet `truncate` returned its new size, where `open(path, 'rb').truncate()` raises. The refusal is `UnsupportedOperation('truncate')`, the same one `write` raises; pcapkit's subclasses `io.UnsupportedOperation`. The old resizing is kept as the private `_truncate_buffer`: it resizes a private lookback window, not the stream, and it is the only route to the "position sits before the window" state that `seek` and the four buffered read paths must refuse, for which [#643](https://github.com/JarryShaw/PyPCAPKit/issues/643) and [#644](https://github.com/JarryShaw/PyPCAPKit/issues/644) landed tests. `writeable()` was never an override of anything, so `io`, `shutil` and third parties never saw it, and the value reported is unchanged ([#645](https://github.com/JarryShaw/PyPCAPKit/issues/645)). - **a breaking change to** `@final`, now enforced at runtime on `Info` and `Schema`. `info_final` and `schema_final` both end `return final(cls)`, so every finalised class carried `typing.final`'s `__final__` marker and nothing read it. **`@final` alone on an `Info` or `Schema` subclass -- without `@info_final` or `@schema_final` -- now raises `InfoError` or `SchemaError` at first construction**, the class being marked final but never finalised and so carrying no generated `__init__`. `__init_subclass__` cannot catch that, since `final` applies after class creation, so the check sits in the existing one-shot `FinalisedState.NONE` branch and a finalised class pays nothing. **Deriving from a finalised class raises too**, from new `Info.__init_subclass__` and `Schema.__init_subclass__` hooks (before, only `EnumSchema` had one). **`SchemaError` is a `ValueError`**, so code catching `TypeError` around a subclass declaration will not see it. `@info_final @final` and `@final @info_final` both finalise silently; the re-entry check keys on `__finalised__` rather than `__final__`, because only the former records *the decorator* having run. `@info_final` twice warns and returns the class unchanged. `pcapkit.utilities.compat` takes `final` from `typing_extensions` below 3.11 rather than 3.8, because `typing.final` only records `__final__` from 3.11. The guard cannot fire on the library's own classes ([#778](https://github.com/JarryShaw/PyPCAPKit/issues/778)). diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index a6c559334..24267da1a 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -306,9 +306,10 @@ Changed Mappings now, so ``len()`` and iteration yield field names rather than values. * ``ModuleDescriptor.klass`` reads an already-imported module out of ``sys.modules`` rather than re-entering ``importlib.import_module``, since next layer dispatch - resolves a descriptor there on a per-frame path. A registry *hit* holding a - ``ModuleDescriptor`` is resolved once and written back; a *miss* deliberately is not - -- recording a miss in a class-level ``collections.defaultdict`` is the defect + resolves a descriptor there on a per-frame path. Neither a registry *hit* holding a + ``ModuleDescriptor`` nor a *miss* writes the resolved class back: a memoised hit went + stale on ``importlib.reload`` (:issue:`1559`), and recording a miss in a class-level + ``collections.defaultdict`` is the defect :issue:`421`/:pr:`426` fixed at this layer, :issue:`425`/:pr:`428` for the option, chunk and block registries, and :issue:`555`/:pr:`560` at the schema layer. A resolution now costs ~117 ns rather than ~436 ns, which is **not measurable in** diff --git a/pcapkit/corekit/module.py b/pcapkit/corekit/module.py index 31615530a..5ca59a46e 100644 --- a/pcapkit/corekit/module.py +++ b/pcapkit/corekit/module.py @@ -59,14 +59,14 @@ def klass(self) -> 'Type[_T]': The module is read from :data:`sys.modules` first, and :func:`importlib.import_module` is entered only when it is not loaded yet. That matters because this property is on a *per-frame* - dispatch path: a next layer code nobody registered falls back to a - :class:`ModuleDescriptor` for - :class:`~pcapkit.protocols.misc.raw.Raw` which - :meth:`ProtocolBase._lookup_next_layer + dispatch path: :meth:`ProtocolBase._lookup_next_layer ` - deliberately does not write back, so every unrecognised frame - resolves the same descriptor again -- 48 of the 52 resolutions an - extraction of :file:`many_interfaces.pcapng` performs. + deliberately writes no resolved class back, so every next layer + lookup resolves its descriptor again -- the one registered for the + code, or the fallback for :class:`~pcapkit.protocols.misc.raw.Raw` + when nobody registered it. An extraction of + :file:`many_interfaces.pcapng` performs 296 resolutions, 48 of them + fallbacks (:issue:`1559`). :func:`~importlib.import_module` keeps real per-call work for an already-imported module (locks, :class:`~importlib.machinery.ModuleSpec` checks, the ``fromlist`` walk), so each repeat costs ~436 ns diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index 9ea07a74d..36b9fc3b3 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -2154,35 +2154,29 @@ def _lookup_next_layer(registry: 'DefaultDict[int, ModuleDescriptor[ProtocolBase The lookup itself is :meth:`self._lookup_registry `, so a miss does not grow the shared registry. What this adds is the next-layer-specific resolution step: - a registered code may hold a + the entry found -- registered, or the fallback -- may be a :class:`~pcapkit.corekit.module.ModuleDescriptor` rather than a - class, and importing it is written back so the import happens once. + class, and it is resolved to the class it names. - That write-back is deliberately confined to a *hit*. Memoising the - fallback's resolution under ``proto`` would be exactly the insertion + Nothing is written back, on a hit or a miss. A miss has no key to + write under, and recording one would be exactly the insertion :meth:`self._lookup_registry ` exists - to avoid. - - So a miss resolves its fallback descriptor again on every frame, and - what keeps that affordable is :attr:`ModuleDescriptor.klass + to avoid. A hit memoised under ``proto`` would retain a class that + :func:`importlib.reload` then makes stale, and would make parsing + mutate a registry shared by the whole process (:issue:`1559`). The + same holds for a memo in ``registry``'s default factory or in a cache + beside the registry (:issue:`421`, :issue:`425`, :issue:`555`). + + So every lookup resolves its descriptor again, and what keeps that + affordable is :attr:`ModuleDescriptor.klass ` reading :data:`sys.modules` instead of re-entering - :func:`importlib.import_module` (:issue:`574`). Memoising the - resolved class instead, whether under ``proto``, in ``registry``'s - default factory, or in a cache beside the registry, would retain a - class that :func:`importlib.reload` then makes stale (:issue:`421`, - :issue:`425`, :issue:`555`). + :func:`importlib.import_module` (:issue:`574`). """ protocol = ProtocolBase._lookup_registry(registry, proto) if isinstance(protocol, ModuleDescriptor): - klass = protocol.klass - # a descriptor can also come back from the default factory, and that - # one has no key to memoise under -- writing it back would recreate - # the insertion-on-miss this exists to avoid - if proto in registry: - registry[proto] = klass # update mapping upon import - return klass + return protocol.klass return protocol def _decode_next_layer(self, dict_: '_PT', proto: 'int', length: 'Optional[int]' = None, *, diff --git a/pcapkit/protocols/schema/link/ethernet.py b/pcapkit/protocols/schema/link/ethernet.py index 45e962f0c..02221eeaf 100644 --- a/pcapkit/protocols/schema/link/ethernet.py +++ b/pcapkit/protocols/schema/link/ethernet.py @@ -39,9 +39,9 @@ def callback_payload(self: 'PayloadField', packet: 'dict[str, Any]') -> 'None': saw would be recorded as though somebody had registered it, and :meth:`Link.register ` would afterwards report it as an overwrite. The helper reads the fallback - without recording it, and for a code that really is registered as a - :class:`~pcapkit.corekit.module.ModuleDescriptor` it writes the resolved - class back, so the import happens once. + without recording it, and resolves a + :class:`~pcapkit.corekit.module.ModuleDescriptor` without writing the + class back, so the registry is left as registered (:issue:`1559`). """ from pcapkit.protocols.link.ethernet import Ethernet # pylint: disable=import-outside-toplevel diff --git a/tests/protocols/application/test_layer_placement_unit.py b/tests/protocols/application/test_layer_placement_unit.py index 372dbb629..fb4064523 100644 --- a/tests/protocols/application/test_layer_placement_unit.py +++ b/tests/protocols/application/test_layer_placement_unit.py @@ -34,7 +34,7 @@ import tempfile import unittest -from tests._support import isolate_modules, reimport_once_per_class +from tests._support import reimport_once_per_class RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) @@ -174,12 +174,6 @@ def test_the_names_left_the_link_subpackage(self) -> None: def test_dispatch_keys_did_not_move_with_the_modules(self) -> None: """The dispatch tier is decoupled from the subpackage, deliberately.""" - # A dispatch hit writes the resolved class back over its descriptor - # (``ProtocolBase._lookup_next_layer``), and the parsing tests below - # share this class's import. So this test takes a fresh one, and reads - # the registrations as declared whatever order the tests run in. Run - # after any of them on the shared import, it failed (GitHub issue #1538). - isolate_modules(self) from pcapkit.const.reg.ethertype import EtherType from pcapkit.const.reg.transtype import TransType from pcapkit.corekit.module import ModuleDescriptor diff --git a/tests/protocols/internet/test_internet_unit.py b/tests/protocols/internet/test_internet_unit.py index 525c08d18..c1a67c636 100644 --- a/tests/protocols/internet/test_internet_unit.py +++ b/tests/protocols/internet/test_internet_unit.py @@ -184,7 +184,8 @@ def __init__(self, file_: bytes, length: int, **kwargs: object) -> None: proto._sigterm = False module = DummyInternet._import_next_layer(proto, TransType.get(251), length=4, payload=b'mod!') self.assertEqual(module.data, b'mod!') - self.assertNotIsInstance(DummyInternet.__dict__['__proto__'][TransType.get(251)], ModuleDescriptor) + # resolved for the dispatch, not written back over the descriptor (#1559) + self.assertIsInstance(DummyInternet.__dict__['__proto__'][TransType.get(251)], ModuleDescriptor) from_header = DummyInternet._import_next_layer(proto, TransType.get(250)) self.assertEqual(from_header.file, b'header-payload') diff --git a/tests/protocols/internet/test_ipv6_unit.py b/tests/protocols/internet/test_ipv6_unit.py index fb0573181..f227120cb 100644 --- a/tests/protocols/internet/test_ipv6_unit.py +++ b/tests/protocols/internet/test_ipv6_unit.py @@ -233,7 +233,8 @@ def test_ipv6_decode_next_layer_without_extensions_and_import_branches(self) -> }) imported = proto._import_next_layer(custom_proto, payload=b'module') self.assertIsInstance(imported, Raw) - self.assertIs(proto.__proto__[custom_proto], Raw) + # resolved for the dispatch, not written back over the descriptor (#1559) + self.assertIsInstance(proto.__proto__[custom_proto], ModuleDescriptor) def test_ipv6_remaining_decode_and_import_edges(self) -> None: from collections import defaultdict diff --git a/tests/protocols/link/test_link_unit.py b/tests/protocols/link/test_link_unit.py index e9f5eb69f..3a7bb32d9 100644 --- a/tests/protocols/link/test_link_unit.py +++ b/tests/protocols/link/test_link_unit.py @@ -399,9 +399,9 @@ def test_link_schema_callbacks_resolve_payload_fields(self) -> None: callback_payload(descriptor_field, {'type': custom_type}) self.assertIs(descriptor_field.protocol, Raw) - # Resolving a registered descriptor memoises the imported class, - # which the callback used to resolve afresh on every frame. - self.assertIs(registry[custom_type], Raw) + # Resolving a registered descriptor leaves the descriptor in place: + # a memoised class would go stale on a reload (#1559). + self.assertIsInstance(registry[custom_type], ModuleDescriptor) # An *unregistered* EtherType still resolves to Raw, and must not be # recorded on the way: the registry is Link.__proto__, shared by diff --git a/tests/protocols/test_dispatch_default_resolution_unit.py b/tests/protocols/test_dispatch_default_resolution_unit.py index 8acfdbeee..aff942808 100644 --- a/tests/protocols/test_dispatch_default_resolution_unit.py +++ b/tests/protocols/test_dispatch_default_resolution_unit.py @@ -8,8 +8,9 @@ defect GitHub issue #421 reported and pull request #426 fixed at this layer, pull request #428 extended to the option, chunk and block registries, and pull request #560 fixed at the schema layer. The cost of not writing it back is that -every unrecognised frame resolves the same descriptor again: 48 of the 52 -resolutions an extraction of :file:`many_interfaces.pcapng` performs. +every unrecognised frame resolves the same descriptor again. Since GitHub issue +#1559 a registered descriptor is not written back either, so an extraction of +:file:`many_interfaces.pcapng` performs 296 resolutions, 48 of them fallbacks. Proposed by @Ts-Boom in GitHub pull request #563, which paid that cost with a class-level cache of resolved classes and no invalidation -- so a diff --git a/tests/protocols/test_next_layer_hit_not_memoised_1559_unit.py b/tests/protocols/test_next_layer_hit_not_memoised_1559_unit.py new file mode 100644 index 000000000..3a36464cf --- /dev/null +++ b/tests/protocols/test_next_layer_hit_not_memoised_1559_unit.py @@ -0,0 +1,151 @@ +# -*- coding: utf-8 -*- +"""GitHub issue #1559: a next-layer hit is resolved, not memoised. + +:meth:`ProtocolBase._lookup_next_layer +` used to write the +class a registered :class:`~pcapkit.corekit.module.ModuleDescriptor` resolved to +back over that descriptor. Three consequences, one test each: + +* after :func:`importlib.reload` of the protocol's module, dispatch kept reaching + the pre-reload class, so a parsed layer failed :func:`isinstance` against the + live one -- what the method's own docstring said a memo would do; +* parsing mutated registries shared by the whole process, so what a registry + held depended on what had been parsed before; +* so whether re-registering a shipped code warned depended on it too. + +Every lookup now resolves through :attr:`ModuleDescriptor.klass +`, which reads +:data:`sys.modules`. Each test reloads or parses on a private import, under +:func:`~tests._support.isolate_modules`, so neither a reloaded module nor a +mutated registry outlives it. + +""" + +import importlib +import struct +import sys +import unittest +import warnings +from unittest import mock + +from tests._support import isolate_modules + +#: Module of the class the reload tests dispatch to. +UDP_MODULE = 'pcapkit.protocols.transport.udp' + +#: An HTTP request, so that a TCP segment to port 80 dispatches once more. +HTTP_REQUEST = b'GET / HTTP/1.1\r\nHost: x\r\n\r\n' + + +def _ipv4(proto: 'int', body: 'bytes') -> 'bytes': + """An IPv4 packet carrying ``body`` as next layer ``proto``.""" + return struct.pack('!BBHHHBBH4s4s', 0x45, 0, 20 + len(body), 0, 0x4000, 64, proto, 0, + bytes([10, 0, 0, 1]), bytes([10, 0, 0, 2])) + body + + +def _udp(body: 'bytes') -> 'bytes': + """A UDP datagram between two ports no registry dispatches on.""" + return struct.pack('!HHHH', 12345, 9999, 8 + len(body), 0) + body + + +def _tcp(dstport: 'int', body: 'bytes') -> 'bytes': + """A TCP segment, PSH/ACK, to ``dstport``.""" + return struct.pack('!HHIIBBHHH', 12345, dstport, 1, 0, 5 << 4, 0x18, 65535, 0, 0) + body + + +class NextLayerHitNotMemoisedTests(unittest.TestCase): + def setUp(self) -> None: + isolate_modules(self) + + def _parse_ipv4_udp(self) -> 'object': + """Parse an IPv4/UDP packet and return its UDP layer.""" + from pcapkit.protocols.internet.ipv4 import IPv4 + + packet = _ipv4(17, _udp(b'hello')) + return IPv4(packet, len(packet)).payload + + def test_a_reload_after_a_hit_dispatches_to_the_reloaded_class(self) -> None: + """The issue's reproduction, and the same through a real parse.""" + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.internet import Internet + from pcapkit.protocols.protocol import ProtocolBase + + registry = Internet.__proto__ + module = importlib.import_module(UDP_MODULE) + before = ProtocolBase._lookup_next_layer(registry, TransType.UDP) + self.assertIs(before, module.UDP) + self.assertIs(type(self._parse_ipv4_udp()), module.UDP) + + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + importlib.reload(module) + self.assertIsNot(module.UDP, before) # the reload really minted a class + + self.assertIs(ProtocolBase._lookup_next_layer(registry, TransType.UDP), module.UDP) + self.assertIsInstance(self._parse_ipv4_udp(), module.UDP) + + # and popping the module, which replaces the module object outright + reloaded = module.UDP + sys.modules.pop(UDP_MODULE) + fresh = importlib.import_module(UDP_MODULE) + self.assertIsNot(fresh.UDP, reloaded) + self.assertIs(ProtocolBase._lookup_next_layer(registry, TransType.UDP), fresh.UDP) + + def test_a_parse_leaves_the_shared_registries_as_declared(self) -> None: + """Ethernet/IPv4/TCP/HTTP hits each registry once; none of them changes.""" + from pcapkit.const.reg.ethertype import EtherType + from pcapkit.const.reg.transtype import TransType + from pcapkit.corekit.module import ModuleDescriptor + from pcapkit.protocols.application.http import HTTP + from pcapkit.protocols.internet.internet import Internet + from pcapkit.protocols.link.ethernet import Ethernet + from pcapkit.protocols.link.link import Link + from pcapkit.protocols.transport.tcp import TCP + + hits = ( + ('Link', Link.__proto__, EtherType.Internet_Protocol_version_4), + ('Internet', Internet.__proto__, TransType.TCP), + ('TCP', TCP.__proto__, 80), + ) + before = {name: dict(registry) for name, registry, _ in hits} + for name, registry, code in hits: + with self.subTest(registry=name, when='before'): + self.assertIsInstance(registry[code], ModuleDescriptor) + + frame = bytes(6) + bytes([2, 0, 0, 0, 0, 1]) + b'\x08\x00' + _ipv4(6, _tcp(80, HTTP_REQUEST)) + parsed = Ethernet(frame, len(frame)) + self.assertIsInstance(parsed.payload.payload.payload, HTTP) # every hit really dispatched + + for name, registry, code in hits: + with self.subTest(registry=name, when='after'): + self.assertIsInstance(registry[code], ModuleDescriptor) + self.assertEqual(dict(registry), before[name]) + + def test_registering_a_dispatched_code_warns_alike_before_and_after_a_parse(self) -> None: + """Whether ``register`` warns no longer depends on what was parsed. + + ``register`` warns when it displaces an incumbent other than the class + it is handed, and a shipped descriptor counts as different from the + class it names. While a hit wrote the class back, re-registering UDP + warned before any parse and was silent after one had reached UDP. + + """ + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.internet import Internet + from pcapkit.protocols.transport.udp import UDP + + def overwrites() -> 'list[str]': + with mock.patch.dict(Internet.__proto__), warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + Internet.register(TransType.UDP, UDP) + return [str(record.message) for record in caught + if 'already registered' in str(record.message)] + + before = overwrites() + self.assertEqual(len(before), 1) + self.assertIs(type(self._parse_ipv4_udp()), UDP) + self.assertEqual(overwrites(), before) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/test_protocol_base_unit.py b/tests/protocols/test_protocol_base_unit.py index 30d2e99fb..e6126dee9 100644 --- a/tests/protocols/test_protocol_base_unit.py +++ b/tests/protocols/test_protocol_base_unit.py @@ -261,7 +261,8 @@ class Other(Raw): ) analyzed_module = DummyProtocol.analyze(4, b'module-body', alias=4) self.assertIsInstance(analyzed_module, Raw) - self.assertIs(DummyProtocol.__proto__[4], Raw) + # resolved for the dispatch, not written back over the descriptor (#1559) + self.assertIsInstance(DummyProtocol.__proto__[4], ModuleDescriptor) DummyProtocol.__proto__ = collections.defaultdict(lambda: Raw, {1: Raw}) analyzed = DummyProtocol.analyze(1, b'raw-body', alias=1) @@ -335,7 +336,7 @@ def __index__(cls) -> int: {5: ModuleDescriptor('pcapkit.protocols.misc.raw', 'Raw')}, ) self.assertIsInstance(proto_module._import_next_layer(5, None), Raw) - self.assertIs(proto_module.__proto__[5], Raw) + self.assertIsInstance(proto_module.__proto__[5], ModuleDescriptor) # not written back (#1559) proto_stop = DummyProtocol(packet=b'abpayload', _protocol='dummyprotocol') self.assertTrue(proto_stop._check_term_threshold()) @@ -419,8 +420,8 @@ def test_lookup_next_layer_reads_the_fallback_without_recording_it(self) -> None :meth:`~pcapkit.protocols.protocol.ProtocolBase.register` call. Resolving a :class:`~pcapkit.corekit.module.ModuleDescriptor` for a code - that *is* registered still writes back, since that is memoisation of an - import rather than a new entry. + that *is* registered does not write back either: the resolved class + would go stale on :func:`importlib.reload` (GitHub issue #1559). """ DummyProtocol, _, _ = self._make_protocol_class() @@ -439,9 +440,9 @@ def test_lookup_next_layer_reads_the_fallback_without_recording_it(self) -> None self.assertNotIn(99, registry) self.assertEqual(set(registry), {7}) - # A hit resolves the descriptor once and keeps the resolved class. + # A hit resolves the descriptor and keeps the descriptor. self.assertIs(DummyProtocol._lookup_next_layer(registry, 7), Raw) - self.assertIs(registry[7], Raw) + self.assertIsInstance(registry[7], ModuleDescriptor) # A class registered directly is returned as it is. registry[8] = Raw