Skip to content

fix: ignore library keepalive pings when asserting mock wire traffic - #9

Merged
KinseyD merged 1 commit into
mainfrom
fix/ci-wire-keepalive
Sep 30, 2026
Merged

KinseyD merged 1 commit into
mainfrom
fix/ci-wire-keepalive

Conversation

@KinseyD

@KinseyD KinseyD commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Summary

The post-merge CI run on main (36599413518) failed on windows-latest: two dynamic_channels integration tests asserted exact wire traffic and intermittently saw an extra PING <timestamp> line.

Root cause

The irc crate's Pinger builds its timer with tokio::time::interval, whose first tick fires immediately. The pinger is enabled on RPL_ENDOFMOTD, so the client sends one PING <Local::now().timestamp()> right after registration instead of after 180s. When that write lands on the wire depends on worker-thread scheduling: on a loaded CI runner it can be delayed into the wire() fixture's capture window, breaking strict equality assertions. Same tree passed the PR run and failed the main run — pure timing flake, no production impact.

Fix

wire() in tests/dynamic_channels.rs now skips PING lines whose payload is pure ASCII digits (the library keepalive). The PING :wire-barrier sentinel carries a non-numeric payload and is unaffected. No production code changes.

Verification

  • 50 consecutive dynamic_channels runs green, including 8 rebuild-then-first-run cycles reproducing the CI cold-start condition.
  • Full suite 329 passed / 0 failed on Windows; cargo fmt --all --check and cargo clippy --all-targets -- -D warnings clean.

The irc crate sends one PING (a bare local timestamp) immediately after
the MOTD because its ping interval's first tick fires right away. Under
load the worker's write is delayed into the wire() capture window, so
strict equality assertions on captured lines intermittently saw the
keepalive and failed on CI windows runners. Filter payload-numeric PINGs
in the fixture; the wire-barrier sentinel is unaffected.
@KinseyD
KinseyD merged commit 790274e into main Sep 30, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant