From 9a312e5126c45cb7cf2b1ae95aa9ebb81ac98683 Mon Sep 17 00:00:00 2001 From: Lillem4n Date: Sun, 6 Sep 2026 17:38:14 +0200 Subject: [PATCH] Harden the interop capture analysis and smppsim capture readiness (#89) * Fail an interop run on an empty capture or a missing bind/bind_resp, and always tear down on analysis failure * smppsim: wait for the capture sidecar to actually be capturing before node starts * Clear the stale textdlr capture file too, and tidy the bind-check ordering * Split the unlink comment so each line matches the unlink it explains --- interop-tests/compose.smppsim.yaml | 16 +++++++++++++-- interop-tests/run.py | 31 +++++++++++++++++++++++++----- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/interop-tests/compose.smppsim.yaml b/interop-tests/compose.smppsim.yaml index 7232788..05c09bc 100644 --- a/interop-tests/compose.smppsim.yaml +++ b/interop-tests/compose.smppsim.yaml @@ -86,6 +86,13 @@ services: smppsim: condition: service_started command: ["dumpcap", "-i", "any", "-f", "tcp port 2775", "-w", "/captures/smppsim.pcapng"] + # dumpcap writes the pcapng section header as soon as it opens the interface, before any + # packet arrives, so a non-empty file means the capture is actually running. + healthcheck: + test: ["CMD-SHELL", "test -s /captures/smppsim.pcapng"] + interval: 1s + retries: 30 + timeout: 2s volumes: - ./interop-tests/captures:/captures @@ -101,15 +108,20 @@ services: smppsim-textdlr: condition: service_started command: ["dumpcap", "-i", "any", "-f", "tcp port 2775", "-w", "/captures/smppsim-textdlr.pcapng"] + healthcheck: + test: ["CMD-SHELL", "test -s /captures/smppsim-textdlr.pcapng"] + interval: 1s + retries: 30 + timeout: 2s volumes: - ./interop-tests/captures:/captures node: depends_on: capture: - condition: service_started + condition: service_healthy capture-textdlr: - condition: service_started + condition: service_healthy smppsim: condition: service_healthy smppsim-textdlr: diff --git a/interop-tests/run.py b/interop-tests/run.py index ad836a9..c8eba29 100755 --- a/interop-tests/run.py +++ b/interop-tests/run.py @@ -21,6 +21,10 @@ PORT_BY_PEER = { "smscsim": 2775, } +# Every scenario in every peer's test file binds before doing anything else, so a capture missing +# either side of that handshake never saw real traffic - the same signal as an empty capture. +BIND_COMMANDS = ("bind_receiver", "bind_transceiver", "bind_transmitter") + # The 33 SMPP commands (SMPP 3.4), by numeric command_id, for the tshark histogram. COMMAND_NAMES = { 0x00000001: "bind_receiver", @@ -135,6 +139,9 @@ def analyse_capture(peer: str, port: int) -> tuple[int, dict[str, object]]: 1 for severity in collect(frame, "_ws.expert.severity") if severity == EXPERT_SEVERITY_ERROR ) + has_bind = any(histogram[name] for name in BIND_COMMANDS) + has_bind_resp = any(histogram[f"{name}_resp"] for name in BIND_COMMANDS) + print(f"frames: {len(frames)}") print("commands:") for name, count in sorted(histogram.items()): @@ -142,7 +149,18 @@ def analyse_capture(peer: str, port: int) -> tuple[int, dict[str, object]]: print(f"malformed: {malformed}") print(f"expert errors: {expert_errors}") - status = 1 if malformed > 0 or expert_errors > 0 else 0 + if not frames: + print("empty capture: no frames decoded", file=sys.stderr) + if not has_bind or not has_bind_resp: + print("no bind/bind_resp pair in capture", file=sys.stderr) + + status = 1 if any(( + not frames, + not has_bind, + not has_bind_resp, + malformed > 0, + expert_errors > 0, + )) else 0 return status, { "commands": dict(histogram), @@ -169,6 +187,8 @@ def main() -> int: # root; overwriting one from a previous run then fails, since root here has no DAC override # either - so the stale file has to go before a fresh one can be written in its place. (CAPTURES_DIR / f"{args.peer}.pcapng").unlink(missing_ok=True) + # Also clears smppsim's textdlr capture so its healthcheck can't pass against a stale leftover. + (CAPTURES_DIR / f"{args.peer}-textdlr.pcapng").unlink(missing_ok=True) tests = sh(compose_cmd( args.peer, "run", "--rm", "--use-aliases", "node", @@ -177,10 +197,11 @@ def main() -> int: sh(compose_cmd(args.peer, "stop", "capture")) fix_capture_ownership() - analysis_status, _ = analyse_capture(args.peer, port) - - if not args.keep: - sh(compose_cmd(args.peer, "down", "-v")) + try: + analysis_status, _ = analyse_capture(args.peer, port) + finally: + if not args.keep: + sh(compose_cmd(args.peer, "down", "-v")) return 1 if tests.returncode != 0 else analysis_status