[ISSUE #6155][Test🧪] Add unit tests for UpdateConsumerOffsetResponseHeader

[oopscompiled]

Which Issue(s) This PR Fixes(Closes)

• Fixes [Test🧪] Add unit tests for UpdateConsumerOffsetResponseHeader #6155

Brief Description

How Did You Test This Change?

Summary by CodeRabbit

• Tests
  • Added comprehensive unit tests for consumer-offset update responses covering creation/equality, zero-sized behavior, JSON serialization/deserialization (empty JSON, round-trip), handling of unknown JSON fields, and encoding to empty representations.
  • Tests increase confidence in data handling and edge cases without modifying runtime behavior or any public APIs.

[rocketmq-rust-bot]

🔊@oopscompiled 🚀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💥.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

Walkthrough

Adds unit tests for UpdateConsumerOffsetResponseHeader (empty response header): creation, Default, zero-sized check, JSON serde (empty object, round-trip, unknown fields), equivalence, and encoding; no production code or public API changes. (≤50 words)

Changes

Cohort / File(s)	Summary
Unit tests — UpdateConsumerOffsetResponseHeader rocketmq-remoting/src/protocol/header/update_consumer_offset_header.rs	Appended ~67 lines of unit tests covering struct creation, Default, Debug formatting, zero-sized assertion, serde JSON serialization/deserialization (empty object, round-trip), ignoring unknown fields, instance equality, and encoding to an empty map. No production code modified.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

• mxsm
• TeslaRustor

Poem

🐰 I hop through tests with nimble feet,
Empty header, tidy and neat,
JSON whispers {} in flight,
Round-trips pass by moonlit night,
Rabbity cheers for code done right.

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (21 files): ⚔️ CHANGELOG.md (content) ⚔️ rocketmq-broker/src/out_api/broker_outer_api.rs (content) ⚔️ rocketmq-broker/src/processor/admin_broker_processor.rs (content) ⚔️ rocketmq-broker/src/processor/admin_broker_processor/offset_request_handler.rs (content) ⚔️ rocketmq-broker/src/processor/end_transaction_processor.rs (content) ⚔️ rocketmq-client/src/admin/default_mq_admin_ext_impl.rs (content) ⚔️ rocketmq-client/src/implementation/mq_client_api_impl.rs (content) ⚔️ rocketmq-client/src/producer/producer_impl/default_mq_producer_impl.rs (content) ⚔️ rocketmq-example/Cargo.toml (content) ⚔️ rocketmq-remoting/src/clients.rs (content) ⚔️ rocketmq-remoting/src/clients/rocketmq_tokio_client.rs (content) ⚔️ rocketmq-remoting/src/protocol/header/update_consumer_offset_header.rs (content) ⚔️ rocketmq-store/src/queue.rs (content) ⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/admin/default_mq_admin_ext.rs (content) ⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands.rs (content) ⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/auth_commands.rs (content) ⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/broker_commands.rs (content) ⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/consumer_commands.rs (content) ⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/controller_commands.rs (content) ⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/namesrv_commands.rs (content) ⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/topic_commands.rs (content) These conflicts must be resolved before merging into main.	Resolve conflicts locally and push changes to this branch.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically references the issue number (#6155) and accurately describes the main change: adding unit tests for UpdateConsumerOffsetResponseHeader.
Linked Issues check	✅ Passed	The PR implements most core test requirements from issue #6155 including struct creation, Default/Debug traits, JSON serialization/deserialization, round-trip consistency, unknown field handling, and instance equivalence, though integration tests with RequestHeaderCodecV2 macro and malformed JSON error handling are not explicitly covered.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to adding unit tests for UpdateConsumerOffsetResponseHeader as specified in issue #6155; no unrelated code modifications or production logic changes are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

⚔️ Resolve merge conflicts (beta)

• Auto-commit resolved conflicts to branch test/add-test-case-UpdateConsumerOffsetResponseHeader
• Post resolved changes as copyable diffs in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🤖 Fix all issues with AI agents

Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.

In `@rocketmq-remoting/src/protocol/header/update_consumer_offset_header.rs`:
- Around line 409-466: The tests repeatedly compare Debug strings because
UpdateConsumerOffsetResponseHeader lacks PartialEq; add #[derive(PartialEq, Eq)]
to the UpdateConsumerOffsetResponseHeader struct definition (the type named
UpdateConsumerOffsetResponseHeader) so tests can use direct assert_eq! on
instances, then replace the format!("{:?}", ...) comparisons in tests (e.g.,
response_header_can_be_created, response_header_deserializes_from_empty_json,
response_header_round_trip, response_header_ignores_extra_fields,
all_instances_are_equivalent) with straightforward assert_eq!(a, b) or
assert_eq!(header, default) assertions.

🧹 Nitpick comments (1)

🤖 Fix all nitpicks with AI agents

Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.

In `@rocketmq-remoting/src/protocol/header/update_consumer_offset_header.rs`:
- Around line 409-466: The tests repeatedly compare Debug strings because
UpdateConsumerOffsetResponseHeader lacks PartialEq; add #[derive(PartialEq, Eq)]
to the UpdateConsumerOffsetResponseHeader struct definition (the type named
UpdateConsumerOffsetResponseHeader) so tests can use direct assert_eq! on
instances, then replace the format!("{:?}", ...) comparisons in tests (e.g.,
response_header_can_be_created, response_header_deserializes_from_empty_json,
response_header_round_trip, response_header_ignores_extra_fields,
all_instances_are_equivalent) with straightforward assert_eq!(a, b) or
assert_eq!(header, default) assertions.

rocketmq-remoting/src/protocol/header/update_consumer_offset_header.rs (1)

409-466: Consider deriving PartialEq on UpdateConsumerOffsetResponseHeader to simplify assertions.

Multiple tests use format!("{:?}", a) == format!("{:?}", b) as a workaround for the missing PartialEq derive. This is verbose and fragile (Debug output is not contractually stable). Adding PartialEq to line 23 would let all of these collapse to direct assert_eq! calls.

Proposed production code change (line 23)

-#[derive(Debug, Serialize, Deserialize, Default, RequestHeaderCodecV2)]
+#[derive(Debug, PartialEq, Serialize, Deserialize, Default, RequestHeaderCodecV2)]
 pub struct UpdateConsumerOffsetResponseHeader {}

Then simplify the tests, e.g.:

-assert_eq!(format!("{:?}", header), format!("{:?}", default));
+assert_eq!(header, default);

🤖 Prompt for AI Agents

Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@rocketmq-remoting/src/protocol/header/update_consumer_offset_header.rs`
around lines 409 - 466, The tests repeatedly compare Debug strings because
UpdateConsumerOffsetResponseHeader lacks PartialEq; add #[derive(PartialEq, Eq)]
to the UpdateConsumerOffsetResponseHeader struct definition (the type named
UpdateConsumerOffsetResponseHeader) so tests can use direct assert_eq! on
instances, then replace the format!("{:?}", ...) comparisons in tests (e.g.,
response_header_can_be_created, response_header_deserializes_from_empty_json,
response_header_round_trip, response_header_ignores_extra_fields,
all_instances_are_equivalent) with straightforward assert_eq!(a, b) or
assert_eq!(header, default) assertions.

[Copilot]

Pull request overview

This PR adds comprehensive unit tests for the UpdateConsumerOffsetResponseHeader struct, an empty response header in the RocketMQ remoting module. The tests verify the struct's basic functionality, serialization/deserialization behavior, and integration with the RequestHeaderCodecV2 macro.

Changes:

• Added 8 unit tests covering struct creation, memory size, JSON serialization/deserialization, round-trip consistency, extra field handling, instance equivalence, and header encoding to map format

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rocketmq-rust-bot]

LGTM - All CI checks passed ✅

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.48%. Comparing base (8b35652) to head (91435b7).
⚠️ Report is 26 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6354      +/-   ##
==========================================
- Coverage   42.62%   42.48%   -0.15%     
==========================================
  Files         912      919       +7     
  Lines      128056   129303    +1247     
==========================================
+ Hits        54590    54935     +345     
- Misses      73466    74368     +902     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[rocketmq-rust-bot]

LGTM - All CI checks passed ✅

[mxsm]

LGTM