Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix][flaky-test]PersistentSubscriptionTest.testCanAcknowledgeAndCommitForTransaction #17845

Conversation

poorbarcode
Copy link
Contributor

Fixes:

Motivation

Concurrent execution of these two instructions will throw an exception:

  • Mocked objects are executing methods
  • Bind the new mock behavior again

In this test, cursorMock triggers this error.

Modifications

make "binding the new mock behavior" always running after "all methods execute finish"

Documentation

  • doc-not-needed

Matching PR in forked repository

PR in forked repository:

Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM

@poorbarcode
Copy link
Contributor Author

After discussion with @congbobo184 , the scenario of this test was covered by PendingAckPersistentTest, so deleted it

@poorbarcode poorbarcode force-pushed the flaky/testCanAcknowledgeAndCommitForTransaction branch from cca82b7 to 60a3d62 Compare September 27, 2022 12:50
@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

1 similar comment
@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@lhotari lhotari merged commit 5f59f8b into apache:master Sep 28, 2022
@poorbarcode poorbarcode deleted the flaky/testCanAcknowledgeAndCommitForTransaction branch September 28, 2022 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs type/flaky-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants