Skip to content

test+fix: recover MockGateway-deletion coverage; fix async cancel deadlock#482

Merged
wboayue merged 3 commits into
mainfrom
coverage-fill-gaps
May 2, 2026
Merged

test+fix: recover MockGateway-deletion coverage; fix async cancel deadlock#482
wboayue merged 3 commits into
mainfrom
coverage-fill-gaps

Conversation

@wboayue
Copy link
Copy Markdown
Owner

@wboayue wboayue commented May 2, 2026

Summary

After PR #480 deleted the MockGateway-driven test files, coverage dropped in a handful of files. This PR recovers it via two phases:

  1. Targeted unit tests (MessageBusStub-driven) for trivial Client accessors and MessageBus-trait impl methods on the real TcpMessageBus / AsyncTcpMessageBus.
  2. Minimal handshake-replay listeners (spawn_handshake_listener per side, ~50 LOC each) that bind to 127.0.0.1:0 and let Client::connect* exercise the production TCP paths without restoring MockGateway's per-API-call surface.

Also fixes a real production deadlock found while writing the trait-level cancel tests.

Coverage delta

cargo llvm-cov --all-features --summary-only:

File Region % Line %
TOTAL 82.65% → 83.80% (+1.15) 87.21% → 88.21% (+1.00)
client/sync.rs 52.74 → 90.41 (+37.67) 61.54 → 93.27 (+31.73)
client/async.rs 62.59 → 95.92 (+33.33) 73.45 → 97.35 (+23.90)
transport/async_io.rs 0.00 → 57.89 (+57.89) 0.00 → 71.05 (+71.05)
connection/async.rs 66.67 → 73.64 (+6.97) 72.14 → 77.86 (+5.72)
connection/sync.rs 84.08 → 88.57 (+4.49) 92.14 → 95.00 (+2.86)
transport/sync/mod.rs 59.02 → 69.81 (+10.79) 62.06 → 72.52 (+10.46)
transport/async.rs 44.26 → 51.83 (+7.57) 46.14 → 53.88 (+7.74)

What's added

Phase 1 — MessageBusStub-driven unit tests

  • src/client/{sync,async}_tests.rs — sibling test files exercising Client::stubbed-built clients: accessor round-trip, check_server_version branches, builder factories, send_* helpers, create_order_update_subscription uniqueness.
  • transport/sync/tests.rs — 5 tests calling MessageBus-trait methods on the real TcpMessageBus<MemoryStream>: cancel_subscription, cancel_order_subscription, cancel_shared_subscription, send_message, create_order_update_subscription.
  • transport/async_tests.rs — 5 mirroring tests on the real AsyncTcpMessageBus<MemoryStream> plus an is_connected reflection check.

Phase 2 — Handshake-replay TCP listener

  • src/transport/sync/test_listener.rs + src/transport/async_test_listener.rsspawn_handshake_listener(frames) -> (SocketAddr, JoinHandle). Binds 127.0.0.1:0, accepts once, consumes the API\0 magic + version range + start_api writes, replays length-prefixed frames, then drains further writes until the client closes.
  • 3 sync + 3 async tests at client/{sync,async}_tests.rs:
    • connect_handshakes_against_real_socketClient::connect succeeds, metadata populated.
    • connect_with_callback_receives_unsolicited_messages — injects an OpenOrder mid-handshake; asserts the callback fires.
    • connect_with_options_applies_tcp_no_delayconnect_with_options(tcp_no_delay=true) returns Ok.

What's fixed

AsyncTcpMessageBus::cancel_subscription and cancel_order_subscription self-deadlocked: each acquired a read guard, then awaited a write guard on the same RwLock while the read guard was still alive (Rust shadowing doesn't drop the prior binding before the new RHS evaluates).

// before
let channels = self.request_channels.read().await;          // read guard alive
if let Some(sender) = channels.get(...) { ... }
let mut channels = self.request_channels.write().await;     // deadlock here

The bug was latent because the production cancel flow goes through Subscription::cancel().await → message_bus.send_message(...), not through cancel_subscription. The new test_cancel_subscription_writes_and_clears_channel was the first caller to hit the path on the real bus and hung. Fix collapses to a single write guard.

Backport tracked in #483 (v2-stable has the identical bug).

Remaining gaps (intentionally accepted)

  • transport/async.rs 51.83% — process_messages task body branches (timeout-loop, connection-error reconnect) need more sophisticated test harnesses; deferred.
  • transport/async_io.rs 57.89% — AsyncTcpSocket::reconnect and sleep (the listener accepts once, can't drive reconnect cleanly without listener swap).
  • transport/routing.rs 72.33% — routing decision branches; orthogonal to MockGateway deletion.

These are covered by the live-Gateway integration tests under tests/.

Test plan

  • cargo fmt --check
  • cargo clippy --all-targets -- -D warnings
  • cargo clippy --all-targets --features sync -- -D warnings
  • cargo clippy --all-features
  • cargo test (default): 877 passed (+13 from main)
  • cargo test --no-default-features --features sync: 881 passed (+13)
  • cargo test --all-features: 1058 passed (+26)

…dlock

After PR 5 deleted client/{sync,async}/tests.rs, several files dropped
in coverage because the deleted MockGateway tests were the only callers
of certain trivial getters and MessageBus-trait impl methods.

Adds:
- src/client/{sync,async}_tests.rs: stubbed-Client tests covering
  is_connected, client_id, server_version, time_zone, connection_time,
  next_request_id, next_order_id, set_next_order_id, decoder_context,
  market_data, order, send_*, create_order_update_subscription,
  check_server_version (10 tests, sync+async).
- transport/sync/tests.rs: 5 tests against the real TcpMessageBus via
  the MessageBus trait — cancel_subscription, cancel_order_subscription,
  cancel_shared_subscription, send_message,
  create_order_update_subscription.
- transport/async_tests.rs: 5 mirroring tests on AsyncMessageBus.

Fixes (transport/async.rs):
- AsyncTcpMessageBus::cancel_subscription and cancel_order_subscription
  acquired a read guard, then awaited a write guard on the same RwLock
  while the read guard was still alive (Rust shadowing doesn't drop
  the prior binding). Self-deadlock on the same task. The new
  cancel_subscription test was the first caller exercising this path
  on the real bus and exposed the hang. Fix is a single write guard.

Coverage delta (cargo llvm-cov --all-features --summary-only):
  TOTAL                 region 82.65% -> 83.11%, line 87.21% -> 87.67%
  client/sync.rs        region 52.74% -> 60.27%, line 61.54% -> 70.19%
  client/async.rs       region 62.59% -> 70.07%, line 73.45% -> 81.42%
  transport/async.rs    region 44.26% -> 51.83%, line 46.14% -> 53.88%
  transport/sync/mod.rs region 59.02% -> 65.46%, line 62.06% -> 68.04%

Remaining gaps are concentrated in production-TCP paths (Client::connect,
AsyncTcpSocket, TcpMessageBus<TcpSocket> monomorphization) that cannot
be unit-tested without restoring socket scaffolding.
wboayue added 2 commits May 2, 2026 12:33
Adds minimal one-shot TCP listener helpers (sync + async, ~50 LOC each)
that bind to 127.0.0.1:0, accept once, replay scripted handshake frames,
and drain further writes until the client closes. No per-API-call mock
surface — used only at the Client::connect* seam.

Tests added per side (sync + async, 3 each):
- connect_handshakes_against_real_socket: verify Client::connect
  succeeds and metadata is populated.
- connect_with_callback_receives_unsolicited_messages: inject an
  OpenOrder mid-handshake; assert the callback receives it.
- connect_with_options_applies_tcp_no_delay: verify
  connect_with_options(tcp_no_delay=true) returns Ok.

Coverage delta from this PR's combined work (cargo llvm-cov
--all-features --summary-only):

  TOTAL                     region 82.65% -> 83.80%, line 87.21% -> 88.21%
  client/sync.rs            region 52.74% -> 90.41%, line 61.54% -> 93.27%
  client/async.rs           region 62.59% -> 95.92%, line 73.45% -> 97.35%
  transport/async_io.rs     region  0.00% -> 57.89%, line  0.00% -> 71.05%
  connection/sync.rs        region 84.08% -> 88.57%, line 92.14% -> 95.00%
  connection/async.rs       region 66.67% -> 73.64%, line 72.14% -> 77.86%
  transport/sync/mod.rs     region 59.02% -> 69.81%, line 62.06% -> 72.52%
  transport/async.rs        region 44.26% -> 51.83%, line 46.14% -> 53.88%

Remaining gaps (e.g. AsyncTcpSocket::reconnect, dispatcher-loop branches)
require more sophisticated test harnesses; deferred.
After PRs #473-#480 eliminated MockGateway and PR #482 introduced the
handshake-replay listener, the testing docs were stale.

- testing-patterns.md: full rewrite. Describes the three-fixture
  stratification (MessageBusStub for domain logic, MemoryStream for
  transport/connection, spawn_handshake_listener for Client::connect*).
- build-and-test.md: replace MockGateway integration test snippet with
  MessageBusStub example; defer to testing-patterns.md for the full
  story.
- troubleshooting.md: drop the obsolete "MockGateway tests failing"
  section.
- CLAUDE.md: update doc index label.
@wboayue wboayue merged commit 09e106f into main May 2, 2026
3 checks passed
@wboayue wboayue deleted the coverage-fill-gaps branch May 2, 2026 19:50
wboayue added a commit that referenced this pull request May 2, 2026
)

* fix(async): self-deadlock in cancel_subscription/cancel_order_subscription

AsyncTcpMessageBus::cancel_subscription and cancel_order_subscription
acquired a read guard, then awaited a write guard on the same RwLock
while the read guard was still alive. Rust shadowing doesn't drop the
prior binding before the new RHS evaluates, so the task self-deadlocks.

Collapse to a single write guard.

Backport of #482 (main). v2-stable bug is latent: production cancel
flow routes through Subscription::cancel().await -> send_message, not
through cancel_subscription, but the trait method is part of the
pub(crate) AsyncMessageBus surface and any future caller would hang.

Production fix only; the accompanying tests on main depend on the
MemoryStream infrastructure that didn't land on v2-stable.

Closes #483

* bump version to 2.11.4
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