Skip to content

[ISSUE #6344][Enhancement✨] Remove useless code from end_transaction_processor.rs#6346

Merged
mxsm merged 1 commit intomxsm:mainfrom
oopscompiled:enhancement/remove-code-end_transaction_processor
Feb 16, 2026
Merged

[ISSUE #6344][Enhancement✨] Remove useless code from end_transaction_processor.rs#6346
mxsm merged 1 commit intomxsm:mainfrom
oopscompiled:enhancement/remove-code-end_transaction_processor

Conversation

@oopscompiled
Copy link
Contributor

@oopscompiled oopscompiled commented Feb 16, 2026

Which Issue(s) This PR Fixes(Closes)

Brief Description

How Did You Test This Change?

Summary by CodeRabbit

  • Tests
    • Removed outdated test assertions and cleaned up commented code in transaction processing tests for improved code maintainability.

Copilot AI review requested due to automatic review settings February 16, 2026 11:24
@rocketmq-rust-bot
Copy link
Collaborator

🔊@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
Copy link
Contributor

coderabbitai bot commented Feb 16, 2026

No actionable comments were generated in the recent review. 🎉


Walkthrough

This PR removes 8 lines of unused test code from the end_transaction_processor.rs file, including a test assertion validating tags_code computation for SingleTag and a commented-out test line. The underlying implementation logic remains unchanged.

Changes

Cohort / File(s) Summary
Test Code Cleanup
rocketmq-broker/src/processor/end_transaction_processor.rs
Removed unused test assertion comparing tags_code output and a commented-out no-op test line in end_message_transaction_with_empty_body test case.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 Whiskers twitching with delight,
Old test assertions fade from sight,
Useless code we bid adieu,
Cleaner tests, oh what a view!

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (16 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-example/Cargo.toml (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 references issue #6344 and accurately describes the main change: removing useless code from end_transaction_processor.rs.
Linked Issues check ✅ Passed The PR removes useless code from end_transaction_processor.rs as requested in issue #6344, removing test assertions and commented-out lines.
Out of Scope Changes check ✅ Passed All changes are within scope: they only remove test assertions and commented-out code from end_transaction_processor.rs as specified in the linked issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 enhancement/remove-code-end_transaction_processor
  • 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

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes stale commented-out code from end_transaction_processor.rs test module, addressing issue #6344 by cleaning up “useless” commented assertions/lines.

Changes:

  • Deleted a commented-out tags_code assertion block in the end_message_transaction_with_valid_message test.
  • Removed a commented-out line in the end_message_transaction_with_empty_body test.

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

Comment on lines 549 to 553
fn end_message_transaction_with_empty_body() {
let mut msg_ext = MessageExt::default();
//msg_ext.set_body(None);
let msg_inner = end_message_transaction(&mut msg_ext);
assert!(!msg_inner.get_body().is_some_and(|b| b.is_empty()));
}
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

end_message_transaction_with_empty_body currently constructs a default MessageExt (which has body == None) and then asserts !msg_inner.get_body().is_some_and(|b| b.is_empty()), which will always pass when the body is None. This makes the test effectively non-assertive for the “empty body” scenario. Consider either renaming the test to reflect the None body case, or explicitly setting an empty body (e.g., Bytes::new()) and asserting the intended behavior for empty-but-present bodies.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Feb 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.42%. Comparing base (8b35652) to head (424502b).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6346      +/-   ##
==========================================
- Coverage   42.62%   42.42%   -0.21%     
==========================================
  Files         912      918       +6     
  Lines      128056   129040     +984     
==========================================
+ Hits        54590    54750     +160     
- Misses      73466    74290     +824     

☔ 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.

Copy link
Collaborator

@rocketmq-rust-bot rocketmq-rust-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - All CI checks passed ✅

@mxsm
Copy link
Owner

mxsm commented Feb 16, 2026

@oopscompiled Thank you for your continued contributions to the RocketMQ-Rust project. Wishing you and your family a healthy and prosperous Year of the Horse this Lunar New Year's Eve! May all your endeavors be wildly successful!

Copy link
Owner

@mxsm mxsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mxsm mxsm merged commit 05bfb2f into mxsm:main Feb 16, 2026
16 of 27 checks passed
@rocketmq-rust-bot rocketmq-rust-bot added approved PR has approved and removed ready to review waiting-review waiting review this PR labels Feb 16, 2026
@oopscompiled oopscompiled deleted the enhancement/remove-code-end_transaction_processor branch February 16, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement✨] Remove useless code from end_transaction_processor.rs

5 participants