Add unit tests for HeartbeatRequestHeader#6234
Conversation
|
🔊@willwang-io 🚀Thanks for your contribution🎉！ 💡CodeRabbit(AI) will review your code first🔥！ Note 🚨The code review suggestions from CodeRabbit are to be used as a reference only, and the PR submitter can decide whether to make changes based on their own judgment. Ultimately, the project management personnel will conduct the final code review💥. |
rocketmq-rust-bot
requested review from
SpaceXCN,
TeslaRustor and
rocketmq-rust-bot
rocketmq-rust-bot
added
AI review first
WalkthroughAdds a comprehensive test module for HeartbeatRequestHeader covering default initialization, JSON (camelCase) serialization/deserialization, roundtrips, handling of optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@rocketmq-remoting/src/protocol/header/heartbeat_request_header.rs`:
- Around line 96-98: Replace clippy-flagged equality-to-bool assertions in this
file by converting assert_eq!(expr, true) to assert!(expr) and assert_eq!(expr,
false) to assert!(!expr); specifically update the assertions referencing
rpc.namespaced, rpc.oneway and any other assert_eq! calls comparing to
true/false (also present elsewhere in the file at the other occurrences
mentioned) so they use assert! or assert!(!) respectively to satisfy the
bool_assert_comparison lint.
🧹 Nitpick comments (2)
rocketmq-remoting/src/protocol/header/heartbeat_request_header.rs (2)
116-128: Roundtrip withNoneisn't truly a roundtrip — consider noting this explicitly.Serializing
Noneproduces{}, but deserializing{}yieldsSome(...)due to the flatten behavior. The test already documents this, but the test nameroundtrip_with_noneis slightly misleading since the value doesn't survive the roundtrip unchanged. Consider renaming to something likeroundtrip_with_none_becomes_some_due_to_flatten.
304-389: Consider consolidating individual field tests using a parameterized helper.The four per-field tests (lines 304–368) and the camelCase test (lines 370–389) each construct nearly identical
HeartbeatRequestHeadervalues. A small helper or macro could reduce boilerplate while retaining coverage.
| assert_eq!(rpc.namespaced.unwrap(), true); | ||
| assert_eq!(rpc.broker_name.unwrap(), "test_broker"); | ||
| assert_eq!(rpc.oneway.unwrap(), false); |
There was a problem hiding this comment.
Fix clippy bool_assert_comparison errors — CI is failing.
Replace assert_eq!(..., true) with assert!(...) and assert_eq!(..., false) with assert!(!...) across all affected lines. This is flagged by clippy and blocks CI.
🔧 Proposed fix (showing all affected lines)
- assert_eq!(rpc.namespaced.unwrap(), true);
+ assert!(rpc.namespaced.unwrap());
assert_eq!(rpc.broker_name.unwrap(), "test_broker");
- assert_eq!(rpc.oneway.unwrap(), false);
+ assert!(!rpc.oneway.unwrap());- assert_eq!(rpc.namespaced.unwrap(), true);
+ assert!(rpc.namespaced.unwrap());
assert_eq!(rpc.broker_name.unwrap(), "test_broker");
- assert_eq!(rpc.oneway.unwrap(), false);
+ assert!(!rpc.oneway.unwrap());- assert_eq!(rpc.namespaced.unwrap(), true);
+ assert!(rpc.namespaced.unwrap());
assert_eq!(rpc.broker_name.unwrap(), "test_broker");
- assert_eq!(rpc.oneway.unwrap(), false);
+ assert!(!rpc.oneway.unwrap());- assert_eq!(rpc.namespaced.unwrap(), true);
+ assert!(rpc.namespaced.unwrap());
assert_eq!(rpc.broker_name.unwrap(), "broker-master");
- assert_eq!(rpc.oneway.unwrap(), false);
+ assert!(!rpc.oneway.unwrap());- assert_eq!(header.rpc_request.as_ref().unwrap().namespaced.unwrap(), true);
+ assert!(header.rpc_request.as_ref().unwrap().namespaced.unwrap());- assert_eq!(header.rpc_request.as_ref().unwrap().oneway.unwrap(), true);
+ assert!(header.rpc_request.as_ref().unwrap().oneway.unwrap());Also applies to: 146-148, 223-225, 299-301, 334-334, 367-367
🧰 Tools
🪛 GitHub Actions: RocketMQ Rust CI
[error] 96-96: clippy: bool_assert_comparison - use 'assert!' instead of 'assert_eq!(..., true)'.
[error] 98-98: clippy: bool_assert_comparison - use 'assert!' instead of 'assert_eq!(..., false)'.
🤖 Prompt for AI Agents
In `@rocketmq-remoting/src/protocol/header/heartbeat_request_header.rs` around
lines 96 - 98, Replace clippy-flagged equality-to-bool assertions in this file
by converting assert_eq!(expr, true) to assert!(expr) and assert_eq!(expr,
false) to assert!(!expr); specifically update the assertions referencing
rpc.namespaced, rpc.oneway and any other assert_eq! calls comparing to
true/false (also present elsewhere in the file at the other occurrences
mentioned) so they use assert! or assert!(!) respectively to satisfy the
bool_assert_comparison lint.
- Default and Debug trait implementations - JSON serialization/deserialization with camelCase fields - Roundtrip serialization with None, Some, and partial fields - Flattened serde attribute behavior - RequestHeaderCodecV2 macro-generated FromMap trait - Individual RpcRequestHeader field validation These tests ensure the header correctly serializes to JSON with flattened RpcRequestHeader fields at the top level, properly handles optional nested structures, and maintains compatibility with the protocol's camelCase naming convention.
willwang-io
force-pushed
the
issue-6218
branch
from
d948b84 to
fb96d46
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6234 +/- ##
==========================================
+ Coverage 41.72% 41.85% +0.12%
==========================================
Files 896 896
Lines 124905 125180 +275
==========================================
+ Hits 52115 52391 +276
+ Misses 72790 72789 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rocketmq-rust-bot
left a comment
rocketmq-rust-bot
left a comment
There was a problem hiding this comment.
LGTM - All CI checks passed ✅
rocketmq-rust-bot
added
approved
4 participants
Which Issue(s) This PR Fixes(Closes)
Brief Description
Implement 22 comprehensive test cases covering:
These tests ensure the header correctly serializes to JSON with flattened RpcRequestHeader fields at the top level, properly handles optional nested structures, and maintains compatibility with the protocol's camelCase naming convention.
How Did You Test This Change?
cargo fmt --all -- --checkcargo test --all-features --workspacecargo clippy --all-targets --all-features --workspaceSummary by CodeRabbit