Skip to content

[fix][test] Fix UnfinishedStubbing issue in AuthZTests #24165

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

Merged

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Apr 9, 2025

Motivation

Mockito is not thread safe and modifying mocks in multi-threaded code will result in failures such as this one:

   [ERROR] Failures:
    [ERROR]   NamespaceAuthZTest.testMaxConsumersPerTopic:1537->setAuthorizationPolicyOperationChecker:183 » UnfinishedStubbing
    Unfinished stubbing detected here:
    -> at org.apache.pulsar.broker.admin.NamespaceAuthZTest.setAuthorizationPolicyOperationChecker(NamespaceAuthZTest.java:173)

    E.g. thenReturn() may be missing.
    Examples of correct stubbing:
        when(mock.isOk()).thenReturn(true);
        when(mock.isOk()).thenThrow(exception);
        doThrow(exception).when(mock).someVoidMethod();
    Hints:
     1. missing thenReturn()
     2. you are trying to stub a final method, which is not supported
     3. you are stubbing the behaviour of another mock inside before 'thenReturn' instruction is completed

Modifications

  • refactor the test code to setup the mock once and switch the implementation dynamically

Documentation

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

lhotari added 2 commits April 9, 2025 16:58
[ERROR] Failures:
[ERROR]   NamespaceAuthZTest.testMaxConsumersPerTopic:1537->setAuthorizationPolicyOperationChecker:183 » UnfinishedStubbing
Unfinished stubbing detected here:
-> at org.apache.pulsar.broker.admin.NamespaceAuthZTest.setAuthorizationPolicyOperationChecker(NamespaceAuthZTest.java:173)

E.g. thenReturn() may be missing.
Examples of correct stubbing:
    when(mock.isOk()).thenReturn(true);
    when(mock.isOk()).thenThrow(exception);
    doThrow(exception).when(mock).someVoidMethod();
Hints:
 1. missing thenReturn()
 2. you are trying to stub a final method, which is not supported
 3. you are stubbing the behaviour of another mock inside before 'thenReturn' instruction is completed
@lhotari lhotari added this to the 4.1.0 milestone Apr 9, 2025
@lhotari lhotari self-assigned this Apr 9, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 9, 2025
@nodece nodece merged commit e8be56d into apache:master Apr 10, 2025
52 of 53 checks passed
poorbarcode pushed a commit to poorbarcode/pulsar that referenced this pull request Apr 15, 2025
lhotari added a commit that referenced this pull request Apr 17, 2025
@lhotari
Copy link
Member Author

lhotari commented Apr 17, 2025

depends on #22546 which isn't included in branch-3.0, dropping cherry-picking to branch-3.0 for now

manas-ctds pushed a commit to datastax/pulsar that referenced this pull request May 2, 2025
(cherry picked from commit e8be56d)
(cherry picked from commit 843bcbd)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 6, 2025
(cherry picked from commit e8be56d)
(cherry picked from commit 843bcbd)
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.

5 participants