Skip to content

[ISSUE #10995] Stabilize client offline receipt handle test - #10996

Open
unbridled-41 wants to merge 1 commit into
apache:developfrom
unbridled-41:fix/flaky-client-offline-test
Open

[ISSUE #10995] Stabilize client offline receipt handle test#10996
unbridled-41 wants to merge 1 commit into
apache:developfrom
unbridled-41:fix/flaky-client-offline-test

Conversation

@unbridled-41

Copy link
Copy Markdown

Which Issue(s) This PR Fixes

Brief Description

DefaultReceiptHandleManagerTest#testClientOffline triggers asynchronous receipt-handle cleanup after CLIENT_UNREGISTER.

The test previously did not stub messagingProcessor.changeInvisibleTime() for the cleanup path. The unstubbed Mockito invocation returns null, and the asynchronous ReturnHandleGroupWorkerThread can then throw a NullPointerException when the test listener calls whenComplete().

When this happens, the receipt handle is not removed and returnHandleGroup() may insert the non-empty group back into receiptHandleGroupMap. Because the test immediately asserted that the map was empty after triggering CLIENT_UNREGISTER, the assertion was timing-sensitive and could fail in Coverage CI.

This change is test-only:

  • stub changeInvisibleTime() with a completed CompletableFuture for the cleanup path;
  • use Awaitility to wait for the asynchronous cleanup and verify the expected invocation;
  • keep the final map-empty assertion inside the asynchronous wait;
  • do not change production behavior.

How Did You Test This Change?

Tested with JDK 8:

  • DefaultReceiptHandleManagerTest#testClientOffline: 50/50 repeated runs passed
  • DefaultReceiptHandleManagerTest: 12/12 tests passed
  • proxy module: 308 tests run, 0 failures, 0 errors, 3 skipped
  • Maven result: BUILD SUCCESS
  • git diff --check: passed

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary

Test-only fix that stabilizes DefaultReceiptHandleManagerTest#testClientOffline by properly stubbing the async changeInvisibleTime() cleanup path and using Awaitility for the async assertion. Clean, minimal, and well-documented.

Correctness — The root cause is correctly identified: unstubbed changeInvisibleTime() returns null from Mockito, causing NPE in the async callback's whenComplete(). Stubbing with CompletableFuture.completedFuture(new AckResult()) ensures the cleanup path completes without error.

Tests — Awaitility usage is appropriate here. The 1-second timeout is reasonable since the stubbed future is already completed, so the wait should resolve nearly instantly. The Mockito.verify() inside untilAsserted correctly validates both the invocation and the final state.

Compatibility — No production code changes. Test-only scope matches the issue description exactly.

LGTM — no issues found.


Automated review by github-manager-bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] DefaultReceiptHandleManagerTest#testClientOffline is flaky during asynchronous cleanup

2 participants