Move integration test helpers from tests/common to wireframe_testing crate

[coderabbitai]

Context

Related to PR #424 which demonstrates the pattern for modularisation by refactoring the connection module.

Request from: @leynos
Source: #424

Problem

Integration test helpers in tests/common/mod.rs:7 rely on file-level #![allow] attributes to suppress dead_code and allow_attributes warnings. This is necessary because Rust's per-test-crate compilation model means that helpers used by only some integration tests appear unused within each test crate.

Proposed Solution

Move these helpers into the wireframe_testing crate as part of the public API, then import them from integration tests. This approach:

1. Eliminates the need for #![allow(dead_code)] and #![allow(allow_attributes)] suppressions
2. Makes the helpers available as a proper public API for testing
3. Follows proper module boundaries and compilation unit structure
4. Allows the helpers to be reused across different test crates without duplication

Benefits

• Cleaner code: Remove unnecessary lint allowances
• Better structure: Proper separation between production code and test utilities
• Reusability: Helpers become available for external users of the library
• Maintainability: Helpers are compiled once as part of the library, not per-test-crate

Implementation Notes

• Extract helpers from tests/common/mod.rs into wireframe_testing crate
• Update integration tests to import from wireframe_testing
• Remove the #![allow] attributes from tests/common/mod.rs
• Ensure all integration tests continue to pass

[traycerai]

Plan

Observations

The integration test helpers in file:tests/common/mod.rs currently require file-level #![allow(dead_code)] and #![allow(allow_attributes)] suppressions due to Rust's per-test-crate compilation model. The helpers include: unused_listener() function, CommonTestEnvelope struct, TestApp and TestResult type aliases, and a factory() rstest fixture. These are used across 20+ integration test files. The wireframe_testing crate already exists with a well-structured public API in modules like helpers, echo_server, logging, and multi_packet.

Approach

Move all integration test helpers from file:tests/common/mod.rs into the wireframe_testing crate as public API exports. This eliminates the need for lint suppressions by making helpers part of a proper library crate that compiles once. Update all integration tests to import from wireframe_testing instead of the local common module. The wireframe_testing crate already has the necessary dependencies (rstest, bincode, wireframe) to support these helpers.

Implementation Steps

1. Create New Module in wireframe_testing Crate

Create a new module file file:wireframe_testing/src/integration_helpers.rs to house the moved helpers:

• Add unused_listener() function: Move the TCP listener creation function that binds to an ephemeral localhost port. This function should remain unchanged from its current implementation in file:tests/common/mod.rs:51-54.

• Add CommonTestEnvelope struct: Move the test envelope type with its bincode::Encode, bincode::BorrowDecode, PartialEq, Debug, and Clone derives. Include all three fields: id, correlation_id, and payload.

• Implement CorrelatableFrame trait: Move the implementation from file:tests/common/mod.rs:70-76 that provides correlation_id() getter and set_correlation_id() setter.

• Implement Packet trait: Move the implementation from file:tests/common/mod.rs:78-92 including id(), into_parts(), and from_parts() methods.

• Add TestApp type alias: Define as pub type TestApp = wireframe::app::WireframeApp<BincodeSerializer, (), Envelope> to match the current definition.

• Add TestResult type alias: Define as pub type TestResult<T = ()> = Result<T, Box<dyn std::error::Error + Send + Sync>> to match the current definition.

• Add factory() fixture function: Move the rstest fixture that returns a factory closure for creating TestApp instances. Keep the #[fixture] attribute and the same implementation.

• Add comprehensive module documentation: Document that these helpers are designed for integration testing and explain their purpose in reducing test boilerplate.

2. Update wireframe_testing Cargo.toml

Ensure file:wireframe_testing/Cargo.toml has all required dependencies:

• Verify rstest dependency exists (currently at version 0.18.2) for the #[fixture] attribute support
• Verify bincode dependency exists for CommonTestEnvelope serialization
• Verify wireframe dependency exists for trait implementations
• No new dependencies should be needed as all are already present

3. Export New Module in wireframe_testing lib.rs

Update file:wireframe_testing/src/lib.rs:

• Add pub mod integration_helpers; declaration after the existing module declarations (around line 24)
• Add public re-exports in the existing pub use section:

  pub use integration_helpers::{
      CommonTestEnvelope,
      TestApp,
      TestResult,
      factory,
      unused_listener,
  };

• Maintain alphabetical ordering with existing exports for consistency

4. Update Integration Tests to Use wireframe_testing

For each integration test file that currently uses mod common;:

Files using only TestResult (connection.rs, client_lifecycle.rs, client_preamble.rs, connection_actor_errors.rs, connection_actor_fairness.rs, connection_actor_shutdown.rs, connection_fragmentation.rs, correlation_id.rs, fragment_transport.rs, metadata.rs, middleware_order.rs, multi_packet.rs, multi_packet_streaming.rs, push.rs, push_policies.rs, session_registry.rs, stream_end.rs, streaming_request.rs, wireframe_protocol.rs):

• Remove mod common; declaration
• Replace use common::TestResult; with use wireframe_testing::TestResult;

Files using CommonTestEnvelope and TestResult (routes.rs, lifecycle.rs):

• Remove mod common; declaration
• Replace use common::{CommonTestEnvelope, TestResult}; with use wireframe_testing::{CommonTestEnvelope, TestResult};

Files using TestResult, factory, and unused_listener (server.rs, preamble.rs):

• Remove mod common; declaration
• Replace use common::{TestResult, factory, unused_listener}; with use wireframe_testing::{TestResult, factory, unused_listener};

Files using TestApp and TestResult (response.rs):

• Remove mod common; declaration
• Replace use common::{TestApp, TestResult}; with use wireframe_testing::{TestApp, TestResult};

5. Remove tests/common/mod.rs

After all integration tests have been updated:

• Delete file:tests/common/mod.rs entirely as it will no longer be needed
• Verify the file:tests/common directory structure - if there are other files like terminator.rs (referenced in stream_end.rs), keep the directory but only remove mod.rs
• Check if file:tests/common contains any other helper modules that should also be migrated or can be removed

6. Verification Steps

Build verification:

• Run cargo build to ensure the wireframe_testing crate compiles successfully
• Run cargo build --tests to ensure all integration tests compile

Test verification:

• Run cargo test to ensure all integration tests pass
• Run cargo test -p wireframe_testing to verify the wireframe_testing crate tests pass
• Pay special attention to tests using factory() fixture (server.rs, preamble.rs) as rstest fixtures need proper attribute handling

Lint verification:

• Run cargo clippy to ensure no new warnings are introduced
• Verify that the allow_attributes and dead_code warnings no longer appear for the moved helpers
• Confirm that the strict linting rules in file:Cargo.toml:90-125 are satisfied

7. Documentation Updates

Update file:wireframe_testing/README.md:

• Add a new section documenting the integration test helpers
• Include example usage of CommonTestEnvelope, factory(), and unused_listener()
• Explain that these helpers are designed for integration testing of wireframe applications
• Add code examples showing how to use factory() with WireframeServer and how to use unused_listener() for TCP-based tests

Benefits Summary

graph TD
    A[tests/common/mod.rs] -->|Move helpers| B[wireframe_testing crate]
    B -->|Public API| C[Integration Tests]
    B -->|Single compilation| D[No dead_code warnings]
    B -->|Proper module| E[No allow_attributes warnings]
    C -->|Import from| B
    
    style A fill:#ffcccc
    style B fill:#ccffcc
    style C fill:#ccccff
    style D fill:#ffffcc
    style E fill:#ffffcc

Loading

This refactoring:

• Eliminates lint suppressions: Helpers become part of a proper library crate, removing the need for #![allow(dead_code)] and #![allow(allow_attributes)]
• Improves reusability: External users can leverage these helpers for their own wireframe-based applications
• Reduces compilation overhead: Helpers compile once as part of wireframe_testing instead of per-test-crate
• Maintains test compatibility: All existing tests continue to work with minimal import changes
• Follows established patterns: Aligns with PR Refactor connection module into submodules #424's modularization approach

Import In IDE

🤖 Prompt for AI Agents

## Observations

The integration test helpers in `file:tests/common/mod.rs` currently require file-level `#![allow(dead_code)]` and `#![allow(allow_attributes)]` suppressions due to Rust's per-test-crate compilation model. The helpers include: `unused_listener()` function, `CommonTestEnvelope` struct, `TestApp` and `TestResult` type aliases, and a `factory()` rstest fixture. These are used across 20+ integration test files. The `wireframe_testing` crate already exists with a well-structured public API in modules like `helpers`, `echo_server`, `logging`, and `multi_packet`.

## Approach

Move all integration test helpers from `file:tests/common/mod.rs` into the `wireframe_testing` crate as public API exports. This eliminates the need for lint suppressions by making helpers part of a proper library crate that compiles once. Update all integration tests to import from `wireframe_testing` instead of the local `common` module. The `wireframe_testing` crate already has the necessary dependencies (rstest, bincode, wireframe) to support these helpers.

## Implementation Steps

### 1. Create New Module in wireframe_testing Crate

Create a new module file `file:wireframe_testing/src/integration_helpers.rs` to house the moved helpers:

- **Add `unused_listener()` function**: Move the TCP listener creation function that binds to an ephemeral localhost port. This function should remain unchanged from its current implementation in `file:tests/common/mod.rs:51-54`.

- **Add `CommonTestEnvelope` struct**: Move the test envelope type with its `bincode::Encode`, `bincode::BorrowDecode`, `PartialEq`, `Debug`, and `Clone` derives. Include all three fields: `id`, `correlation_id`, and `payload`.

- **Implement `CorrelatableFrame` trait**: Move the implementation from `file:tests/common/mod.rs:70-76` that provides `correlation_id()` getter and `set_correlation_id()` setter.

- **Implement `Packet` trait**: Move the implementation from `file:tests/common/mod.rs:78-92` including `id()`, `into_parts()`, and `from_parts()` methods.

- **Add `TestApp` type alias**: Define as `pub type TestApp = wireframe::app::WireframeApp<BincodeSerializer, (), Envelope>` to match the current definition.

- **Add `TestResult` type alias**: Define as `pub type TestResult<T = ()> = Result<T, Box<dyn std::error::Error + Send + Sync>>` to match the current definition.

- **Add `factory()` fixture function**: Move the rstest fixture that returns a factory closure for creating `TestApp` instances. Keep the `#[fixture]` attribute and the same implementation.

- **Add comprehensive module documentation**: Document that these helpers are designed for integration testing and explain their purpose in reducing test boilerplate.

### 2. Update wireframe_testing Cargo.toml

Ensure `file:wireframe_testing/Cargo.toml` has all required dependencies:

- Verify `rstest` dependency exists (currently at version 0.18.2) for the `#[fixture]` attribute support
- Verify `bincode` dependency exists for `CommonTestEnvelope` serialization
- Verify `wireframe` dependency exists for trait implementations
- No new dependencies should be needed as all are already present

### 3. Export New Module in wireframe_testing lib.rs

Update `file:wireframe_testing/src/lib.rs`:

- Add `pub mod integration_helpers;` declaration after the existing module declarations (around line 24)
- Add public re-exports in the existing `pub use` section:
  ```rust
  pub use integration_helpers::{
      CommonTestEnvelope,
      TestApp,
      TestResult,
      factory,
      unused_listener,
  };
  ```
- Maintain alphabetical ordering with existing exports for consistency

### 4. Update Integration Tests to Use wireframe_testing

For each integration test file that currently uses `mod common;`:

**Files using only `TestResult`** (connection.rs, client_lifecycle.rs, client_preamble.rs, connection_actor_errors.rs, connection_actor_fairness.rs, connection_actor_shutdown.rs, connection_fragmentation.rs, correlation_id.rs, fragment_transport.rs, metadata.rs, middleware_order.rs, multi_packet.rs, multi_packet_streaming.rs, push.rs, push_policies.rs, session_registry.rs, stream_end.rs, streaming_request.rs, wireframe_protocol.rs):
- Remove `mod common;` declaration
- Replace `use common::TestResult;` with `use wireframe_testing::TestResult;`

**Files using `CommonTestEnvelope` and `TestResult`** (routes.rs, lifecycle.rs):
- Remove `mod common;` declaration  
- Replace `use common::{CommonTestEnvelope, TestResult};` with `use wireframe_testing::{CommonTestEnvelope, TestResult};`

**Files using `TestResult`, `factory`, and `unused_listener`** (server.rs, preamble.rs):
- Remove `mod common;` declaration
- Replace `use common::{TestResult, factory, unused_listener};` with `use wireframe_testing::{TestResult, factory, unused_listener};`

**Files using `TestApp` and `TestResult`** (response.rs):
- Remove `mod common;` declaration
- Replace `use common::{TestApp, TestResult};` with `use wireframe_testing::{TestApp, TestResult};`

### 5. Remove tests/common/mod.rs

After all integration tests have been updated:

- Delete `file:tests/common/mod.rs` entirely as it will no longer be needed
- Verify the `file:tests/common` directory structure - if there are other files like `terminator.rs` (referenced in stream_end.rs), keep the directory but only remove `mod.rs`
- Check if `file:tests/common` contains any other helper modules that should also be migrated or can be removed

### 6. Verification Steps

**Build verification**:
- Run `cargo build` to ensure the wireframe_testing crate compiles successfully
- Run `cargo build --tests` to ensure all integration tests compile

**Test verification**:
- Run `cargo test` to ensure all integration tests pass
- Run `cargo test -p wireframe_testing` to verify the wireframe_testing crate tests pass
- Pay special attention to tests using `factory()` fixture (server.rs, preamble.rs) as rstest fixtures need proper attribute handling

**Lint verification**:
- Run `cargo clippy` to ensure no new warnings are introduced
- Verify that the `allow_attributes` and `dead_code` warnings no longer appear for the moved helpers
- Confirm that the strict linting rules in `file:Cargo.toml:90-125` are satisfied

### 7. Documentation Updates

Update `file:wireframe_testing/README.md`:

- Add a new section documenting the integration test helpers
- Include example usage of `CommonTestEnvelope`, `factory()`, and `unused_listener()`
- Explain that these helpers are designed for integration testing of wireframe applications
- Add code examples showing how to use `factory()` with `WireframeServer` and how to use `unused_listener()` for TCP-based tests

## Benefits Summary

```mermaid
graph TD
    A[tests/common/mod.rs] -->|Move helpers| B[wireframe_testing crate]
    B -->|Public API| C[Integration Tests]
    B -->|Single compilation| D[No dead_code warnings]
    B -->|Proper module| E[No allow_attributes warnings]
    C -->|Import from| B
    
    style A fill:#ffcccc
    style B fill:#ccffcc
    style C fill:#ccccff
    style D fill:#ffffcc
    style E fill:#ffffcc
```

This refactoring:
- **Eliminates lint suppressions**: Helpers become part of a proper library crate, removing the need for `#![allow(dead_code)]` and `#![allow(allow_attributes)]`
- **Improves reusability**: External users can leverage these helpers for their own wireframe-based applications
- **Reduces compilation overhead**: Helpers compile once as part of wireframe_testing instead of per-test-crate
- **Maintains test compatibility**: All existing tests continue to work with minimal import changes
- **Follows established patterns**: Aligns with PR #424's modularization approach

---

Execution Information

Branch: main
Commit: 9b4ea6e

---

💡 Tips

Supported Commands (Inside Comments)

• Use @traycerai generate to iterate on the previous version of the implementation plan.

Supported Commands (Inside Description)

• Add @traycerai ignore anywhere in the ticket description to prevent this ticket from being processed.
• Add @traycerai branch:<branch-name> anywhere in the ticket description to specify the target branch for the implementation plan.

Community

• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[leynos]

@coderabbitai looking at the codebase, is this issue still relevant? If the issue has been fixed in the code, does the documentation reflect this?

If it is still relevant, please provide an assessment of the issue criticality (critical, high, medium or low), based on risk of the issue impacting the user and the anticipated blast radius.\n\nOnce a decision has been made, apply the appropriate label.

If the issue is documentation ONLY (i.e., no code changes needed), apply the documentation label in addition to the appropriate criticality label.

Please also provide an impact of the required changes.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Check if tests/common/mod.rs exists and show its content
echo "=== Checking tests/common/mod.rs ==="
if [ -f "tests/common/mod.rs" ]; then
    echo "File exists. Content:"
    cat tests/common/mod.rs
else
    echo "File does not exist"
fi

echo -e "\n=== Checking tests/common directory structure ==="
if [ -d "tests/common" ]; then
    ls -la tests/common/
else
    echo "Directory does not exist"
fi

echo -e "\n=== Checking wireframe_testing/src for integration_helpers ==="
if [ -f "wireframe_testing/src/integration_helpers.rs" ]; then
    echo "integration_helpers.rs exists"
    head -30 wireframe_testing/src/integration_helpers.rs
else
    echo "integration_helpers.rs does not exist"
fi

echo -e "\n=== Checking wireframe_testing/src/lib.rs exports ==="
if [ -f "wireframe_testing/src/lib.rs" ]; then
    cat wireframe_testing/src/lib.rs
else
    echo "lib.rs does not exist"
fi

echo -e "\n=== Sample integration test imports ==="
echo "--- tests/server.rs ---"
head -20 tests/server.rs 2>/dev/null || echo "File not found"

echo -e "\n--- tests/response.rs ---"
head -20 tests/response.rs 2>/dev/null || echo "File not found"

Length of output: 7383

---

🏁 Script executed:

#!/bin/bash

# Apply the "low" criticality label to issue `#433`
gh issue edit 433 --add-label "low"

# Check current labels on the issue
echo "=== Current labels on issue `#433` ==="
gh issue view 433 --json labels --jq '.labels[].name'

Length of output: 87

---

@leynos, I've assessed the issue and can confirm it is still relevant — the migration has not been completed.

Current State

The integration test helpers remain in tests/common/mod.rs with the file-level lint suppressions:

• #![allow(clippy::allow_attributes)]
• #![allow(dead_code)]

The wireframe_testing crate does not contain an integration_helpers module, and integration tests like tests/server.rs and tests/response.rs still use mod common; imports.

Note: The code references issue #333 in its FIXME comments, but this appears to be issue #433.

Criticality Assessment: LOW

I've applied the low label based on:

Risk to users: None — this affects only developers working on the codebase, not library consumers.

Blast radius: Limited to test infrastructure; the actual wireframe library functionality is unaffected.

Current impact: Tests work correctly; this is purely a code quality and maintainability improvement.

Urgency: Can be addressed when convenient as part of technical debt reduction.

Required Changes Impact: MEDIUM-HIGH EFFORT

While the risk is low, the implementation requires moderate effort:

• Scope: Create new module in wireframe_testing + update 20+ integration test files
• Complexity: Straightforward refactoring with clear requirements
• Risk: Low — tests will validate correctness; changes are mechanical
• Benefits: Eliminates lint suppressions, improves reusability, reduces compilation overhead

This is NOT documentation only — it requires code changes across the test infrastructure.