Conversation
Summary by CodeRabbit
WalkthroughAdd comprehensive Rustdoc documentation and usage examples to all public asynchronous helper functions in Changes
Sequence Diagram(s)sequenceDiagram
participant Tester
participant Helpers
participant WireframeApp
Tester->>Helpers: Call drive_with_bincode(msg)
Helpers->>Helpers: Encode msg with bincode
Helpers->>WireframeApp: Drive app with framed bytes
WireframeApp-->>Helpers: Return output
Helpers-->>Tester: Return result
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: applies to docs/wireframe_testing/src/lib.rs : expose asynchronous test helper functions (such as `d...Applied to files:
📚 Learning: applies to docs/wireframe_testing/src/helpers.rs : provide mutable variants of helper functions (suc...Applied to files:
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Reviewer's GuideThis PR enriches the wireframe_testing helpers module by adding comprehensive Rustdoc comments and usage examples to all public async test helper functions, improving discoverability and guiding developers on how to integrate these utilities in their tests. Class diagram for documented async test helpers in wireframe_testingclassDiagram
class WireframeApp
class TestSerializer
class wireframe_testing_helpers {
<<module>>
+drive_with_frame(app, frame)
+drive_with_frame_with_capacity(app, frame, capacity)
+drive_with_frames(app, frames)
+drive_with_frames_with_capacity(app, frames, capacity)
+drive_with_frame_mut(&mut app, frame)
+drive_with_frame_with_capacity_mut(&mut app, frame, capacity)
+drive_with_frames_mut(&mut app, frames)
+drive_with_frames_with_capacity_mut(&mut app, frames, capacity)
+drive_with_bincode(app, msg)
+run_app_with_frame(app, frame)
+run_app_with_frame_with_capacity(app, frame, capacity)
+run_app_with_frames(app, frames)
+run_app_with_frames_with_capacity(app, frames, capacity)
+run_with_duplex_server(app)
}
wireframe_testing_helpers ..> WireframeApp : uses
wireframe_testing_helpers ..> TestSerializer : uses
WireframeApp <|.. TestSerializer : trait bound
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `wireframe_testing/src/helpers.rs:51` </location>
<code_context>
+/// # Ok(())
+/// # }
+/// ```
pub async fn drive_with_frame<S, C, E>(
app: WireframeApp<S, C, E>,
frame: Vec<u8>,
</code_context>
<issue_to_address>
Consider replacing the many nearly identical forwarding helper functions with a small set of macros to generate them automatically.
Here’s one way to collapse all of those identical “forwarding” helpers into a tiny `macro_rules!` rather than hand‐writing each one. You still keep the exact same public API, docs, and calls under the hood, but you remove dozens of near‐duplicate lines:
```rust
// before your helpers:
const DEFAULT_CAPACITY: usize = 4096;
// 1) define a small macro for the two shapes of forwarding functions
macro_rules! forward_default {
// `drive_with_frame(mut)?`, `run_app_with_frame(mut)?`, etc.
(
/// $docs:tt
$vis:vis fn $fn_name:ident ( $app:ident : $app_ty:ty, $arg:ident : $arg_ty:ty ) -> $ret:ty
=> $inner:ident ( $app, $arg, DEFAULT_CAPACITY )
) => {
#[doc = $docs]
$vis async fn $fn_name<S, C, E>(
$app: $app_ty,
$arg: $arg_ty,
) -> $ret
where
S: TestSerializer,
C: Send + 'static,
E: Packet,
{
$inner($app, $arg, DEFAULT_CAPACITY).await
}
};
}
macro_rules! forward_with_capacity {
(
/// $docs:tt
$vis:vis fn $fn_name:ident ( $app:ident : $app_ty:ty, $arg:ident : $arg_ty:ty, capacity: usize ) -> $ret:ty
=> $inner:ident ( $app, $arg, capacity )
) => {
#[doc = $docs]
$vis async fn $fn_name<S, C, E>(
$app: $app_ty,
$arg: $arg_ty,
capacity: usize,
) -> $ret
where
S: TestSerializer,
C: Send + 'static,
E: Packet,
{
$inner($app, $arg, capacity).await
}
};
}
// 2) invoke it for each variant. Note how all the `drive_*`, `run_*`, `*_mut`
// helpers collapse into four macro‐invocations:
forward_default! {
/// Drive `app` with a single length‐prefixed `frame` and return the bytes
pub fn drive_with_frame(app: WireframeApp<S, C, E>, frame: Vec<u8>) -> io::Result<Vec<u8>>
=> drive_with_frame_with_capacity(app, frame, DEFAULT_CAPACITY)
}
forward_with_capacity! {
/// Drive `app` with a single frame using a duplex buffer of `capacity` bytes.
pub fn drive_with_frame_with_capacity(app: WireframeApp<S, C, E>, frame: Vec<u8>, capacity: usize) -> io::Result<Vec<u8>>
=> drive_with_frames_with_capacity(app, frame, capacity)
}
// …and repeat for `drive_with_frames`, `drive_with_frames_with_capacity`,
// `drive_with_frame_mut`, `drive_with_frame_with_capacity_mut`, etc.
// Since each block is just forwarding, you now get all of them
// from 4–8 lines of macro invocations instead of 50+ near‐dupes.
```
You can apply the same two macros for your `run_*` set, too. This keeps the signatures and docs exactly the same but collapses your “dozens of almost identical functions” to just a handful of macro invocations.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| /// # Ok(()) | ||
| /// # } | ||
| /// ``` | ||
| pub async fn drive_with_frame<S, C, E>( |
There was a problem hiding this comment.
issue (complexity): Consider replacing the many nearly identical forwarding helper functions with a small set of macros to generate them automatically.
Here’s one way to collapse all of those identical “forwarding” helpers into a tiny macro_rules! rather than hand‐writing each one. You still keep the exact same public API, docs, and calls under the hood, but you remove dozens of near‐duplicate lines:
// before your helpers:
const DEFAULT_CAPACITY: usize = 4096;
// 1) define a small macro for the two shapes of forwarding functions
macro_rules! forward_default {
// `drive_with_frame(mut)?`, `run_app_with_frame(mut)?`, etc.
(
/// $docs:tt
$vis:vis fn $fn_name:ident ( $app:ident : $app_ty:ty, $arg:ident : $arg_ty:ty ) -> $ret:ty
=> $inner:ident ( $app, $arg, DEFAULT_CAPACITY )
) => {
#[doc = $docs]
$vis async fn $fn_name<S, C, E>(
$app: $app_ty,
$arg: $arg_ty,
) -> $ret
where
S: TestSerializer,
C: Send + 'static,
E: Packet,
{
$inner($app, $arg, DEFAULT_CAPACITY).await
}
};
}
macro_rules! forward_with_capacity {
(
/// $docs:tt
$vis:vis fn $fn_name:ident ( $app:ident : $app_ty:ty, $arg:ident : $arg_ty:ty, capacity: usize ) -> $ret:ty
=> $inner:ident ( $app, $arg, capacity )
) => {
#[doc = $docs]
$vis async fn $fn_name<S, C, E>(
$app: $app_ty,
$arg: $arg_ty,
capacity: usize,
) -> $ret
where
S: TestSerializer,
C: Send + 'static,
E: Packet,
{
$inner($app, $arg, capacity).await
}
};
}
// 2) invoke it for each variant. Note how all the `drive_*`, `run_*`, `*_mut`
// helpers collapse into four macro‐invocations:
forward_default! {
/// Drive `app` with a single length‐prefixed `frame` and return the bytes
pub fn drive_with_frame(app: WireframeApp<S, C, E>, frame: Vec<u8>) -> io::Result<Vec<u8>>
=> drive_with_frame_with_capacity(app, frame, DEFAULT_CAPACITY)
}
forward_with_capacity! {
/// Drive `app` with a single frame using a duplex buffer of `capacity` bytes.
pub fn drive_with_frame_with_capacity(app: WireframeApp<S, C, E>, frame: Vec<u8>, capacity: usize) -> io::Result<Vec<u8>>
=> drive_with_frames_with_capacity(app, frame, capacity)
}
// …and repeat for `drive_with_frames`, `drive_with_frames_with_capacity`,
// `drive_with_frame_mut`, `drive_with_frame_with_capacity_mut`, etc.
// Since each block is just forwarding, you now get all of them
// from 4–8 lines of macro invocations instead of 50+ near‐dupes.You can apply the same two macros for your run_* set, too. This keeps the signatures and docs exactly the same but collapses your “dozens of almost identical functions” to just a handful of macro invocations.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
wireframe_testingTesting
make fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_688bf64c69288322a53b65b07e773ce8
Summary by Sourcery
Document all public asynchronous test helper functions in wireframe_testing by adding descriptive doc comments and usage examples.
Documentation: