add test: unit tests for MessageUtils utility class

[gong-dx]

Description

Please provide a brief description of your PR.

This PR adds comprehensive unit tests for the MessageUtils utility class in the common module, which previously had zero test coverage.

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests you ran to verify your changes.

• Unit tests covering all public methods of MessageUtils
• Edge case testing (null input, empty collection, substring matching)

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[RockteMQ-AI]

Review by github-manager-bot

Summary

This PR adds unit tests for MessageUtils in the common module, covering all 4 public methods (getShardingKeyIndex, getShardingKeyIndexByMsg, getShardingKeyIndexes, deleteProperty). Good effort filling the test coverage gap — the edge case coverage (null input, empty collection, substring matching) is thoughtful.

Findings

• [Warning] MessageUtilsTest.java — Unused import: java.nio.charset.StandardCharsets is imported but never referenced in the test class. Please remove it.

• [Info] testGetShardingKeyIndex, testGetShardingKeyIndex_EmptyKey, testGetShardingKeyIndex_ChineseKey — Weak assertions: These tests only verify isBetween(0, indexSize - 1), which is trivially true for any valid hash-mod operation. Consider adding determinism checks instead:

  // Verify same input produces same output
  int index1 = MessageUtils.getShardingKeyIndex("test-key", 10);
  int index2 = MessageUtils.getShardingKeyIndex("test-key", 10);
  assertThat(index1).isEqualTo(index2);

• [Info] testGetShardingKeyIndex_DifferentKeys — The test name implies different keys should map to different indices, but the assertions never verify index1 != index2. If the goal is to verify distribution, add:

  assertThat(index1).isNotEqualTo(index2);

  Note: this is not guaranteed for all input pairs with modular hashing, so choose inputs known to differ, or rename the test to clarify intent (e.g., testGetShardingKeyIndex_Consistency).

• [Info] testGetShardingKeyIndexByMsg — Consider adding a test case where the message has no sharding key property set, verifying the fallback to empty string:

  MessageExt msgNoKey = new MessageExt();
  int index = MessageUtils.getShardingKeyIndexByMsg(msgNoKey, 10);
  assertThat(index).isBetween(0, 9);

• [Info] Naming convention: Apache RocketMQ test methods typically use camelCase (e.g., testGetShardingKeyIndexWithEmptyKey) rather than underscore-separated names (testGetShardingKeyIndex_EmptyKey). Consider aligning with the project convention for consistency.

Suggestions

Overall a solid contribution that meaningfully improves test coverage. The suggestions above are mostly about strengthening assertions and consistency. The substring matching test (testDeleteProperty_SubstringKey) is particularly good — it correctly validates that "mykey" is not deleted when targeting "key".

---

Automated review by github-manager-bot

[RockteMQ-AI]

Review by github-manager-bot (Re-review after new commit)

Summary

Re-review triggered by new commit 86265c3e addressing previous feedback. The changes properly address all prior review comments.

Changes in Latest Commit

• ✅ Removed unused StandardCharsets import
• ✅ Renamed test methods from underscore to camelCase convention (e.g., testGetShardingKeyIndex_EmptyKey → testGetShardingKeyIndexWithEmptyKey)
• ✅ Improved testGetShardingKeyIndexConsistency to properly verify determinism (same input → same output)
• ✅ Added clarifying comments for fallback behavior

Verdict

All previous feedback addressed. Test class provides good coverage of MessageUtils utility methods including edge cases (empty key, Chinese characters, null handling, batch operations).

LGTM 👍

---

Automated re-review by github-manager-bot