Skip to content

[fix][test] Fix flaky PersistentTopicsTest setup caused by concurrent Mockito stubbing#26083

Open
void-ptr974 wants to merge 1 commit into
apache:masterfrom
void-ptr974:fix-flaky-persistent-topics-setup
Open

[fix][test] Fix flaky PersistentTopicsTest setup caused by concurrent Mockito stubbing#26083
void-ptr974 wants to merge 1 commit into
apache:masterfrom
void-ptr974:fix-flaky-persistent-topics-setup

Conversation

@void-ptr974

@void-ptr974 void-ptr974 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Motivation

Fixes #26076.

PersistentTopicsTest.setup starts a real test broker through super.internalSetup(). After the broker is started, the test replaces the broker resources with Mockito stubbing:

doReturn(resources).when(pulsar).getPulsarResources();

This is fragile because pulsar is a spy of a live PulsarService. Once the broker has started, broker services and background tasks can call methods on the same PulsarService instance concurrently. Mockito can usually handle concurrent method invocations on an already-configured mock, but it is not safe to create or modify stubs while another thread is invoking the same mock.

The failure stack points at this setup-time stubbing:

at org.apache.pulsar.broker.PulsarService.getPulsarResources(PulsarService.java:313)
at org.apache.pulsar.broker.admin.PersistentTopicsTest.setup(PersistentTopicsTest.java:175)

When another broker thread calls the spy while the setup thread is in the middle of doReturn(...).when(pulsar).getPulsarResources(), Mockito can observe an incomplete stubbing operation. That explains both failure modes from the issue:

  • AssertionError from Mockito internals while recording the method for stubbing.
  • UnfinishedStubbingException during cleanup because Mockito still sees the interrupted or incomplete stubbing state.

There are also two test methods that later update resource-related stubs through chains such as:

when(pulsar.getPulsarResources().getNamespaceResources()).thenReturn(...)
when(pulsar.getPulsarResources().getTopicResources().listPersistentTopicsAsync(...)).thenReturn(...)

Those have the same basic problem: they touch the live PulsarService spy after the broker has started, and they use chained Mockito stubbing around objects that can also be used by broker code.

Modifications

  • Stop modifying the pulsar.getPulsarResources() stub after broker startup.
  • Create the replacement PulsarResources and TopicResources spies, configure their stubs first, and then install the replacement resources on the test PulsarService.
  • Replace per-test resource re-stubbing with volatile switches:
    • namespaceResourcesOverride selects a mocked NamespaceResources only for the test that needs custom namespace policies.
    • listPersistentTopicsAsyncHandler returns custom topic listings only for the test namespace that needs them, otherwise it falls back to the real method.
  • Clear both switches in cleanup() to avoid leaking test-specific behavior to the next test method.

Result

The test still exercises the same admin paths, but Mockito stubbing is no longer created or modified on the live PulsarService spy while broker threads may be using it. Test-specific behavior is changed by updating volatile references, which is safe for concurrent readers and avoids Mockito unfinished stubbing state.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Verifications

  • ./gradlew :pulsar-broker:test --tests org.apache.pulsar.broker.admin.PersistentTopicsTest :pulsar-broker:checkstyleTest

@void-ptr974 void-ptr974 force-pushed the fix-flaky-persistent-topics-setup branch from 97ef426 to 5f39bea Compare June 24, 2026 16:37
@void-ptr974 void-ptr974 marked this pull request as ready for review June 25, 2026 14:39
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.

Flaky-test: org.apache.pulsar.broker.admin.PersistentTopicsTest.setup

2 participants