[Test🧪] Add test case for GetLiteGroupInfoRequestHeader#6615
Merged
Conversation
Add unit tests covering: - Header creation with field values - Serialization to map (to_map) - Deserialization from map (from_map) - Clone behavior - Debug formatting - Edge cases for top_k (zero, negative, max) Closes mxsm#6569
Collaborator
|
🔊@slegarraga 🚀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
added
AI review first
Contributor
WalkthroughAdded a unit test module for GetLiteGroupInfoRequestHeader verifying instantiation, map serialization/deserialization, cloning, Debug formatting, and boundary cases for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
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)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rocketmq-remoting/src/protocol/header/get_lite_group_info_request_header.rs (1)
107-120: Consider testing clone with a populatedrpcfield.The clone test only verifies behavior when
rpc: None. For completeness, consider adding a case whererpcisSome(...)to ensure all nested fields are cloned correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketmq-remoting/src/protocol/header/get_lite_group_info_request_header.rs` around lines 107 - 120, Add a second case to the get_lite_group_info_request_header_clone test that sets GetLiteGroupInfoRequestHeader.rpc to Some(...) (e.g., a populated CheetahRpc or equivalent struct used in this codebase), clone the header, and assert equality on rpc as well as group, lite_topic, and top_k to verify the nested rpc field is correctly cloned; update the test function get_lite_group_info_request_header_clone to create both None and Some(...) scenarios (or add a new test) and compare header.rpc == cloned.rpc.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rocketmq-remoting/src/protocol/header/get_lite_group_info_request_header.rs`:
- Around line 37-163: The CI failure is due to rustfmt formatting issues in the
tests module for GetLiteGroupInfoRequestHeader; run rustfmt (cargo fmt --all)
locally, reformat the file containing the tests (the tests module, the impls for
GetLiteGroupInfoRequestHeader and any related trait impls like
FromMap/CommandCustomHeader), verify formatting passes with cargo fmt --all --
--check, then stage and commit the reformatted file so the CI formatting check
succeeds.
---
Nitpick comments:
In `@rocketmq-remoting/src/protocol/header/get_lite_group_info_request_header.rs`:
- Around line 107-120: Add a second case to the
get_lite_group_info_request_header_clone test that sets
GetLiteGroupInfoRequestHeader.rpc to Some(...) (e.g., a populated CheetahRpc or
equivalent struct used in this codebase), clone the header, and assert equality
on rpc as well as group, lite_topic, and top_k to verify the nested rpc field is
correctly cloned; update the test function
get_lite_group_info_request_header_clone to create both None and Some(...)
scenarios (or add a new test) and compare header.rpc == cloned.rpc.
rocketmq-remoting/src/protocol/header/get_lite_group_info_request_header.rs
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
rocketmq-remoting/src/protocol/header/get_lite_group_info_request_header.rs (2)
129-153: Edge-case test should round-trip through codec, not only struct assignment.Line 129-Line 153 currently proves
i32values can be stored, but it does not validate boundary behavior forto_map/FromMap::from. Consider a table-driven round-trip assertion for0,-1, andi32::MAX.Suggested round-trip edge-case test
#[test] fn get_lite_group_info_request_header_with_different_top_k_values() { - let header_zero = GetLiteGroupInfoRequestHeader { - group: CheetahString::from("g"), - lite_topic: CheetahString::from("t"), - top_k: 0, - rpc: None, - }; - assert_eq!(header_zero.top_k, 0); - - let header_negative = GetLiteGroupInfoRequestHeader { - group: CheetahString::from("g"), - lite_topic: CheetahString::from("t"), - top_k: -1, - rpc: None, - }; - assert_eq!(header_negative.top_k, -1); - - let header_large = GetLiteGroupInfoRequestHeader { - group: CheetahString::from("g"), - lite_topic: CheetahString::from("t"), - top_k: i32::MAX, - rpc: None, - }; - assert_eq!(header_large.top_k, i32::MAX); + for top_k in [0, -1, i32::MAX] { + let header = GetLiteGroupInfoRequestHeader { + group: CheetahString::from("g"), + lite_topic: CheetahString::from("t"), + top_k, + rpc: None, + }; + + let map = header.to_map().unwrap(); + let decoded = <GetLiteGroupInfoRequestHeader as FromMap>::from(&map).unwrap(); + assert_eq!(decoded.top_k, top_k); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketmq-remoting/src/protocol/header/get_lite_group_info_request_header.rs` around lines 129 - 153, The test get_lite_group_info_request_header_with_different_top_k_values currently only assigns values to GetLiteGroupInfoRequestHeader fields but must round-trip through the header codec to validate map conversion; modify the test to iterate over values [0, -1, i32::MAX], create a GetLiteGroupInfoRequestHeader with each top_k, call its to_map (or equivalent serialization) and then parse back with FromMap::from (or equivalent deserialization), and assert the deserialized.header.top_k equals the original value (use the GetLiteGroupInfoRequestHeader name and the to_map / FromMap::from code paths to locate the logic to exercise).
113-126: Tighten debug assertions to avoid false positives.Line 123-Line 125 currently check broad substrings (
"group","lite","7"), which can still pass if debug formatting regresses. Prefer checking field labels.Suggested assertion tightening
- assert!(debug_str.contains("group")); - assert!(debug_str.contains("lite")); - assert!(debug_str.contains("7")); + assert!(debug_str.contains("group:")); + assert!(debug_str.contains("lite_topic:")); + assert!(debug_str.contains("top_k: 7"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketmq-remoting/src/protocol/header/get_lite_group_info_request_header.rs` around lines 113 - 126, The test get_lite_group_info_request_header_debug is using loose substring checks that can produce false positives; update the assertions in that test to verify the debug output contains the field labels together with their values for the GetLiteGroupInfoRequestHeader fields (e.g., the "group" field label and its value, the "lite_topic" field label and its value, and the "top_k" label and value) instead of broad substrings like "group", "lite", and "7" so the test will fail if Debug formatting regresses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rocketmq-remoting/src/protocol/header/get_lite_group_info_request_header.rs`:
- Around line 129-153: The test
get_lite_group_info_request_header_with_different_top_k_values currently only
assigns values to GetLiteGroupInfoRequestHeader fields but must round-trip
through the header codec to validate map conversion; modify the test to iterate
over values [0, -1, i32::MAX], create a GetLiteGroupInfoRequestHeader with each
top_k, call its to_map (or equivalent serialization) and then parse back with
FromMap::from (or equivalent deserialization), and assert the
deserialized.header.top_k equals the original value (use the
GetLiteGroupInfoRequestHeader name and the to_map / FromMap::from code paths to
locate the logic to exercise).
- Around line 113-126: The test get_lite_group_info_request_header_debug is
using loose substring checks that can produce false positives; update the
assertions in that test to verify the debug output contains the field labels
together with their values for the GetLiteGroupInfoRequestHeader fields (e.g.,
the "group" field label and its value, the "lite_topic" field label and its
value, and the "top_k" label and value) instead of broad substrings like
"group", "lite", and "7" so the test will fail if Debug formatting regresses.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6615 +/- ##
==========================================
- Coverage 41.52% 41.48% -0.05%
==========================================
Files 961 974 +13
Lines 134645 136197 +1552
==========================================
+ Hits 55909 56496 +587
- Misses 78736 79701 +965 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rocketmq-rust-bot
approved these changes
Mar 3, 2026
Collaborator
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Closes #6569
Modification
Added unit tests for
GetLiteGroupInfoRequestHeadercovering:to_map)from_map)top_k(zero, negative, i32::MAX)Result
All 6 tests pass. Improves test coverage for the remoting protocol headers.
Summary by CodeRabbit