Skip to content

Commit 5ff243c

Browse files
committed
test(pcapng,sctp): replace dead checks with ones that can fail (#2226)
Three checks could not fail: a != 104 size check no tree produces, a BitField probe gating the SCTP mixed-bit round trip that is always true, and a NOTE excusing the PCAP-NG side of the cap_len/packet equality. - test_dumped_pcap_carries_the_payload_octets: compare against the header-only size, 24 + 16 per record, which the pre-#646 dump hits. - test_sctp_unit: drop bitfield_packs_zero_bits and its skipTest; the mixed-bit test runs unconditionally. - test_block2frame_puts_the_on_wire_length_in_len: assert cap_len == len(frame.packet) and drop the NOTE.
1 parent 91147a1 commit 5ff243c

3 files changed

Lines changed: 10 additions & 42 deletions

File tree

‎tests/protocols/misc/pcap/test_frame_length_runtime.py‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -347,10 +347,7 @@ def test_block2frame_puts_the_on_wire_length_in_len(self) -> None:
347347
self.assertEqual(frame.frame_info.orig_len, block.original_len)
348348

349349
self.assertGreaterEqual(frame.len, frame.cap_len)
350-
# NOTE: no assertion that ``cap_len == len(frame.packet)`` here,
351-
# which is the PCAP side's independent check. The octets a
352-
# PCAP-NG block carries are #646's concern, pinned in
353-
# tests/protocols/test_pcapng_regression.py rather than here.
350+
self.assertEqual(frame.cap_len, len(frame.packet))
354351

355352
if block.captured_len != block.original_len:
356353
truncated += 1

‎tests/protocols/test_pcapng_regression.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -405,12 +405,12 @@ def test_dumped_pcap_carries_the_payload_octets(self) -> None:
405405
self.assertGreaterEqual(orig_len, incl_len)
406406

407407
# 24 octets of global header, then 16 of record header and the payload per
408-
# record, with nothing left over. Without the #646 fix this file is 88 octets
409-
# for these four blocks -- the global header and four record headers, and not
410-
# one octet of payload -- which is both the wrong size and, worse, a size a
411-
# reader cannot tell is wrong until it has already lost frame sync.
408+
# record, with nothing left over. Without the #646 fix the file is the
409+
# global header and one record header per block, and not one octet of
410+
# payload -- which is both the wrong size and, worse, a size a reader cannot
411+
# tell is wrong until it has already lost frame sync.
412412
self.assertEqual(len(dumped), 24 + sum(16 + len(payload) for payload in expected))
413-
self.assertNotEqual(len(dumped), 104)
413+
self.assertNotEqual(len(dumped), 24 + 16 * len(expected))
414414

415415

416416
if __name__ == '__main__':

‎tests/protocols/transport/test_sctp_unit.py‎

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -31,30 +31,6 @@
3131
HAS_SCAPY = importlib.util.find_spec('scapy') is not None
3232

3333

34-
def bitfield_packs_zero_bits() -> bool:
35-
"""Whether :class:`~pcapkit.corekit.fields.strings.BitField` packs a clear bit.
36-
37-
It guards against a ``BitField.pre_process`` that seeds its working buffer
38-
with NUL bytes and then writes the ASCII characters ``b'0'`` and ``b'1'``
39-
into it, before collapsing the buffer by truth-testing each byte -- ``b'0'``
40-
is ``0x30``, which is truthy, so every named bit comes out **set** regardless
41-
of its value, and a flag word constructed with any bit clear does not survive
42-
a round trip.
43-
44-
That is a library-wide concern of :mod:`pcapkit.corekit`, not an SCTP one.
45-
The tests that need clear bits on the *construction* side are gated on this
46-
probe, so that they run wherever :mod:`pcapkit.corekit` packs clear bits
47-
correctly. Parsing does not depend on it -- ``BitField.post_process`` reads
48-
the bits separately -- so the wire-conformance and scapy cross-check tests
49-
below exercise mixed flags unconditionally.
50-
51-
"""
52-
from pcapkit.corekit.fields.strings import BitField
53-
54-
field = BitField(length=1, namespace={'a': (0, 1), 'b': (7, 1)})
55-
return field.pre_process({'a': 0, 'b': 1}, {}) == b'\x01'
56-
57-
5834
@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed')
5935
class SCTPUnitTests(unittest.TestCase):
6036
# NOTE: Unlike the sibling TCP/UDP unit tests, this module does not purge
@@ -246,11 +222,10 @@ def test_every_chunk_type_round_trips(self) -> None:
246222
from pcapkit.const.sctp.chunk import Chunk
247223
from pcapkit.const.sctp.parameter import Parameter
248224

249-
# Every flag in the flag-carrying chunks is set, so that these tests do
250-
# not depend on the BitField construction probe
251-
# :func:`bitfield_packs_zero_bits`; mixed flags are covered on the
252-
# parsing side by the wire-conformance and scapy tests below, and on the
253-
# construction side by the gated test that follows this one.
225+
# Every flag in the flag-carrying chunks is set; mixed flags are covered
226+
# on the parsing side by the wire-conformance and scapy tests below, and
227+
# on the construction side by
228+
# ``test_chunk_flags_round_trip_with_mixed_bits``.
254229
cases = [
255230
(Chunk.Payload_Data,
256231
dict(I=True, U=True, B=True, E=True, tsn=0x0A0B0C0D, stream_id=1,
@@ -329,10 +304,6 @@ def test_every_chunk_type_round_trips(self) -> None:
329304
def test_chunk_flags_round_trip_with_mixed_bits(self) -> None:
330305
from pcapkit.const.sctp.chunk import Chunk
331306

332-
if not bitfield_packs_zero_bits():
333-
self.skipTest('BitField.pre_process sets every named bit regardless '
334-
'of value; see bitfield_packs_zero_bits()')
335-
336307
raw = self._build([(Chunk.Payload_Data,
337308
dict(I=False, U=False, B=True, E=True, tsn=1, data=b'x'))])
338309
flags = self._packet(raw).info.chunks[Chunk.Payload_Data].flags

0 commit comments

Comments
 (0)