You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@pulsar.apache.org by "lhotari (via GitHub)" <gi...@apache.org> on 2023/01/25 12:53:53 UTC

[GitHub] [pulsar] lhotari opened a new pull request, #19323: [fix][test] Replace PulsarService Mockito spy that has a thread safety issues

lhotari opened a new pull request, #19323:
URL: https://github.com/apache/pulsar/pull/19323

   Fixes #13620
   Fixes #16444
   Fixes #16427
   
   ### Motivation
   
   Mockito is not thread safe and there's a problem with mocking PulsarService which is described in #16821 description:
   > I found the cause of the problem: while executing doReturn(pulsarResources).when(pulsar).getPulsarResources(), Meta Store Thread also accesses variable PulsarService.getPulsarResources() asynchronously in logic: notification by zk-watcher(Concurrent access will be problematic if the object is being mock bound).
   
   Unfortunately, the solution made in PR #16821 didn't fix the problem. A better solution is needed to fix the problem.
   
   ### Modifications
   
   Replace the Mockito spy based PulsarService used in a few unit tests with a subclass of PulsarService which reduces the use of Mockito. Mockito spies can be continued to be used, but the getters are replaced with overridden methods in a new class TestPulsarService which extends PulsarService.
   The test code is also simplified by moving the required collaborator objects and resources to the TestPulsarService.Factory class which contains a `close()` method to close and release the allocated resources.
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] lhotari merged pull request #19323: [fix][test] Replace PulsarService Mockito spy solution for overriding getters in tests

Posted by "lhotari (via GitHub)" <gi...@apache.org>.

lhotari merged PR #19323:
URL: https://github.com/apache/pulsar/pull/19323


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] lhotari commented on pull request #19323: [fix][test] Replace PulsarService Mockito spy that has thread safety issues

Posted by "lhotari (via GitHub)" <gi...@apache.org>.

lhotari commented on PR #19323:
URL: https://github.com/apache/pulsar/pull/19323#issuecomment-1403572707

   /pulsarbot rerun-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org