Skip to content

[improve][test] Migrate tests to use PulsarTestContext#19376

Merged
lhotari merged 24 commits into
apache:masterfrom
lhotari:lh-use-PulsarTestContext
Feb 1, 2023
Merged

[improve][test] Migrate tests to use PulsarTestContext#19376
lhotari merged 24 commits into
apache:masterfrom
lhotari:lh-use-PulsarTestContext

Conversation

@lhotari

@lhotari lhotari commented Jan 31, 2023

Copy link
Copy Markdown
Member

Motivation

This PR continues changes made in previous PRs (#19337, #19326, #19323) to refactor Pulsar unit tests to reduce mocking which causes flakiness since Mockito isn't thread safe.

One key motivation for this PR is to extend PulsarTestContext features so that most tests can be migrated to use it to reduce the use of Mockito spies in the way that causes flakiness. Mockito Spies aren't completely removed, but it moves to a better direction.

Modifications

  • add missing features to PulsarTestContext
  • Improve MetadataStoreExtended so that it's simpler to add MetadataEventSynchronizer in tests
  • reduce unnecessary spies for ServiceConfiguration
  • migrate tests to use PulsarTestContext
  • Disable FilterEntryTest.testEntryFilterRescheduleMessageDependingOnConsumerSharedSubscription
    • this test is very flaky and the behavior depends on the number of threads in thread pool
  • Quarantine a very flaky test PendingAckPersistentTest.testDeleteUselessLogDataWhenSubCursorMoved
  • Improved Javadocs

Documentation

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

Matching PR in forked repository

PR in forked repository: lhotari#139

@lhotari lhotari force-pushed the lh-use-PulsarTestContext branch from c52801a to 05bc4ab Compare January 31, 2023 23:39
@lhotari lhotari marked this pull request as ready for review January 31, 2023 23:42
@lhotari

lhotari commented Jan 31, 2023

Copy link
Copy Markdown
Member Author

/pulsarbot rerun-failure-checks

@lhotari lhotari self-assigned this Jan 31, 2023

@michaeljmarshall michaeljmarshall left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks for the detailed Javadocs @lhotari! I left some comments that should probably be addressed. I am approving the PR since the comments are very minor.

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.

2 participants