[ISSUE #5376]🧪Add test case for RpcClientError

[WaterWhisperer]

Which Issue(s) This PR Fixes(Closes)

Fixes #5376

Brief Description

How Did You Test This Change?

Summary by CodeRabbit

• Tests
  • Enhanced test coverage for RPC client error handling.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[rocketmq-rust-bot]

🔊@WaterWhisperer 🚀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]

Walkthrough

A test module for RpcClientError was added to validate error variant string representations and construction mechanisms. Five test cases cover specific error types: broker_not_found, request_failed, unexpected_response_code, unsupported_request_code, and remote_error. No production logic was modified.

Changes

Cohort / File(s)	Summary
RpcClientError Tests rocketmq-error/src/unified/rpc.rs	Added test module with five test cases validating error variant construction and string representation for each RpcClientError type

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A test module hops into view,
Five error cases, shiny and new,
RpcClientError strings shine bright,
Each assertion proven right! ✨

Pre-merge checks and finishing touches

✅ 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 indicates the main change: adding a test case for RpcClientError, which directly matches the changeset that adds tests for this error type.
Linked Issues check	✅ Passed	The PR successfully addresses linked issue #5376 by adding comprehensive test cases for RpcClientError covering multiple error variants and their string representations.
Out of Scope Changes check	✅ Passed	All changes are within scope; only test code for RpcClientError was added with no unrelated modifications to production logic or other components.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

---

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]

Actionable comments posted: 0

🧹 Nitpick comments (2)

rocketmq-error/src/unified/rpc.rs (2)

90-116: Consider verifying the error source chain for RequestFailed.

The tests successfully verify string representations and constructor helpers for all error variants. However, the RequestFailed variant has a #[source] attribute (line 32) that enables error chaining, but this behavior is not tested.

💡 Add test for error source chain

Consider adding a test that verifies the source error is accessible:

#[test]
fn test_request_failed_preserves_source() {
    let source = io::Error::other("network error");
    let err = RpcClientError::request_failed("127.0.0.1:10911", 10, 3000, source);
    
    // Verify the source chain is preserved
    assert!(err.source().is_some());
    assert_eq!(err.source().unwrap().to_string(), "network error");
}

---

90-116: Consider splitting into separate test functions.

All error variants are tested in a single function, which makes it harder to identify which specific variant fails if a test breaks.

💡 Split into focused test functions

Consider splitting into separate test functions for better clarity:

#[test]
fn test_broker_not_found() {
    let err = RpcClientError::broker_not_found("broker-a");
    assert_eq!(
        err.to_string(),
        "Broker 'broker-a' address not found in client metadata"
    );
}

#[test]
fn test_request_failed() {
    let source = io::Error::other("network error");
    let err = RpcClientError::request_failed("127.0.0.1:10911", 10, 3000, source);
    assert_eq!(
        err.to_string(),
        "RPC request failed: addr=127.0.0.1:10911, request_code=10, timeout=3000ms"
    );
}

// ... similar pattern for other variants

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b12f7a6 and 001dcf0.

📒 Files selected for processing (1)

• rocketmq-error/src/unified/rpc.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: Code Coverage
• GitHub Check: Build & Test (macos-latest)
• GitHub Check: Build & Test (ubuntu-latest)
• GitHub Check: Build & Test (windows-latest)
• GitHub Check: auto-approve

🔇 Additional comments (1)

rocketmq-error/src/unified/rpc.rs (1)

84-89: LGTM! Test module setup is correct.

The test module structure and imports are appropriate for testing the RpcClientError variants.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 37.86%. Comparing base (b12f7a6) to head (001dcf0).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5446      +/-   ##
==========================================
+ Coverage   37.83%   37.86%   +0.03%     
==========================================
  Files         811      811              
  Lines      109905   109921      +16     
==========================================
+ Hits        41582    41625      +43     
+ Misses      68323    68296      -27     

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