test(client): expand blocking and async conformance coverage #96

Merged
Lochnair merged 2 commits from test/client-driver-conformance into main 2026-08-15 03:11:37 +00:00
Lochnair commented 2026-08-14 18:41:30 +00:00 (Migrated from github.com)

Summary

Test-only PR. No production code changed. The purpose is evidence, not architecture.

The existing conformance tests in async_client/tests.rs compared Client and AsyncClient on open negotiation with HMAC and DSCP, no-test completion, and peer close. Three externally-observable gaps are now covered:

  • Failed open rollback, then a valid retry. Against a peer that ignores exactly one open attempt, both drivers fail the first open() with the same promised error (OpenTimeout) and both accept a retry on the same connected client, negotiating identical parameters and the same token, then probing from sequence 0 and closing cleanly.
  • Probe scheduling and completion. Three caller-paced probes against a real in-tree irtt-server: both drivers emit EchoSent for sequences 0, 1, 2 and receive the matching replies, sequence for sequence and shape for shape.
  • Error equivalence for out-of-state operations. Probing before open, opening twice, and probing or closing after close produce the same promised error in both drivers (NotOpen, AlreadyOpen, AlreadyClosed).

Assertions compare what each driver promises — outcomes, event sequences, error variants — never internal state, readiness machinery, retained in-flight operations, or buffer mechanics, which legitimately differ.

No blocking-vs-async divergence was found. The suite passes against current intended production semantics, so nothing is being fixed or worked around here.

Behavior

No behavior change. A real irtt-server is used for compliant peer behavior; the one fake peer added is deliberately mute for a single open attempt, which a compliant server has no way to be. No production test hooks were added.

Scope

Explicitly not changed:

  • Client / AsyncClient were not redesigned, and no production code was touched at all.
  • No production observability hook was added to make a test convenient.
  • No production refactor was done to share test setup; the new tests reuse the file's existing peer helpers.
  • Architecture candidate 1 is untouched — this PR only gathers evidence about it.

Verification

Run from the task worktree, all successful:

  • cargo fmt --all -- --check
  • cargo check --workspace --all-targets --all-features --locked
  • cargo test --workspace --all-targets --all-features --locked
  • cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
  • RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps --locked
  • cargo check -p irtt-cli --no-default-features --features client --all-targets --locked
  • cargo check -p irtt-cli --no-default-features --features tui --all-targets --locked
  • cargo check -p irtt-cli --no-default-features --features server --all-targets --locked
  • git diff --check

The new tests were also run five times in a row to check they do not depend on timing slack.

## Summary **Test-only PR.** No production code changed. The purpose is evidence, not architecture. The existing conformance tests in `async_client/tests.rs` compared `Client` and `AsyncClient` on open negotiation with HMAC and DSCP, no-test completion, and peer close. Three externally-observable gaps are now covered: - **Failed open rollback, then a valid retry.** Against a peer that ignores exactly one open attempt, both drivers fail the first `open()` with the same promised error (`OpenTimeout`) and both accept a retry on the same connected client, negotiating identical parameters and the same token, then probing from sequence 0 and closing cleanly. - **Probe scheduling and completion.** Three caller-paced probes against a real in-tree `irtt-server`: both drivers emit `EchoSent` for sequences 0, 1, 2 and receive the matching replies, sequence for sequence and shape for shape. - **Error equivalence for out-of-state operations.** Probing before open, opening twice, and probing or closing after close produce the same promised error in both drivers (`NotOpen`, `AlreadyOpen`, `AlreadyClosed`). Assertions compare what each driver *promises* — outcomes, event sequences, error variants — never internal state, readiness machinery, retained in-flight operations, or buffer mechanics, which legitimately differ. **No blocking-vs-async divergence was found.** The suite passes against current intended production semantics, so nothing is being fixed or worked around here. ## Behavior No behavior change. A real `irtt-server` is used for compliant peer behavior; the one fake peer added is deliberately mute for a single open attempt, which a compliant server has no way to be. No production test hooks were added. ## Scope Explicitly not changed: - `Client` / `AsyncClient` were not redesigned, and no production code was touched at all. - No production observability hook was added to make a test convenient. - No production refactor was done to share test setup; the new tests reuse the file's existing peer helpers. - Architecture candidate 1 is untouched — this PR only gathers evidence about it. ## Verification Run from the task worktree, all successful: - `cargo fmt --all -- --check` - `cargo check --workspace --all-targets --all-features --locked` - `cargo test --workspace --all-targets --all-features --locked` - `cargo clippy --workspace --all-targets --all-features --locked -- -D warnings` - `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps --locked` - `cargo check -p irtt-cli --no-default-features --features client --all-targets --locked` - `cargo check -p irtt-cli --no-default-features --features tui --all-targets --locked` - `cargo check -p irtt-cli --no-default-features --features server --all-targets --locked` - `git diff --check` The new tests were also run five times in a row to check they do not depend on timing slack.
chatgpt-codex-connector[bot] (Migrated from github.com) reviewed 2026-08-14 18:45:19 +00:00
chatgpt-codex-connector[bot] (Migrated from github.com) left a comment

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 150ee4fd3b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `150ee4fd3b` <details> <summary>ℹ️ About Codex in GitHub</summary> <br/> [Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you - Open a pull request for review - Mark a draft as ready - Comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". </details>
@@ -1214,0 +1364,4 @@
// the sequence helpers keep: an extra event on either side is a difference.
assert_eq!(blocking_sent.len(), async_sent.len());
assert_eq!(blocking_replies.len(), async_replies.len());
for (blocking, asynchronous) in blocking_sent.iter().zip(&async_sent) {
chatgpt-codex-connector[bot] (Migrated from github.com) commented 2026-08-14 18:45:19 +00:00

P2 Badge Assert event-list lengths before zipping

If either driver emits an additional trailing event, these zip loops silently stop at the shorter list, while probe_sequences and reply_sequences filter out non-probe events; the conformance test can therefore pass despite observably different event streams. Assert equal vector lengths before comparing their elements so the test protects full emitted-event equivalence.

AGENTS.md reference: AGENTS.md:L89-L91

Useful? React with 👍 / 👎.

**<sub><sub>![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat)</sub></sub> Assert event-list lengths before zipping** If either driver emits an additional trailing event, these `zip` loops silently stop at the shorter list, while `probe_sequences` and `reply_sequences` filter out non-probe events; the conformance test can therefore pass despite observably different event streams. Assert equal vector lengths before comparing their elements so the test protects full emitted-event equivalence. AGENTS.md reference: [AGENTS.md:L89-L91](https://github.com/Lochnair/irtt-rs/blob/150ee4fd3b8a16976cadc74f8edc694a397b0ffd/AGENTS.md#L89-L91) Useful? React with 👍 / 👎.
Lochnair (Migrated from github.com) reviewed 2026-08-14 18:55:04 +00:00
@@ -1214,0 +1364,4 @@
// the sequence helpers keep: an extra event on either side is a difference.
assert_eq!(blocking_sent.len(), async_sent.len());
assert_eq!(blocking_replies.len(), async_replies.len());
for (blocking, asynchronous) in blocking_sent.iter().zip(&async_sent) {
Lochnair (Migrated from github.com) commented 2026-08-14 18:55:04 +00:00

Agreed — event-list lengths are now asserted before both zip loops (and for the sent/reply/close lists in the retry test), so an extra event the sequence helpers filter out no longer passes silently.

🤖 Addressed by Claude Code

Agreed — event-list lengths are now asserted before both zip loops (and for the sent/reply/close lists in the retry test), so an extra event the sequence helpers filter out no longer passes silently. _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
Sign in to join this conversation.