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

Ensure spring-boot can use Mockito public API #1191

Closed
mockitoguy opened this Issue Oct 1, 2017 · 8 comments

Comments

Projects
None yet
3 participants
@mockitoguy
Member

mockitoguy commented Oct 1, 2017

Problem

Mockito framework integrators need robust public APIs from us. This way, they ship great Mockito integrations to their customers. We want to avoid the situation where framework integrators have to use Mockito internal APIs. This leads to overhead, relying on stale versions of Mockito, class conflicts in runtime.

spring-boot currently uses some internal API from Mockito. See spring-projects/spring-boot#10352 for the problem statement, solution options and chosen design.

Solution

As discussed in spring-boot ticket, here's the API we would like to prototype:

  • New "VerificationStartedListener"
  • The instance of listener can be passed on mock creation
  • Listener is notified when 'Mockito.verify' is invoked but before the verification is triggered
  • It is possible to replace the mock object that is being verified (key feature needed by spring-boot)

Example usage

mock(Foo.class, withSettings().verificationStartedListeners(new VerificationStartedListener() {
  public void onVerifyStarted(VerificationStartedEvent event) {
    event.getMock();
    event.setMock(arbitrayObject);
  }
}

mockitoguy added a commit that referenced this issue Oct 1, 2017

Added experimental API for spring-boot use case
- for design and rationale, see #1191
- for usage, see "VerificationStartedListenerTest" class

This is still experimental so refactorings, error handling, javadoc is missing.
@mockitoguy

This comment has been minimized.

Show comment
Hide comment
@mockitoguy

mockitoguy Oct 1, 2017

Member

I put together a prototype in "mockito-spring-boot" branch. Opened PR #1192 for visibility. How to use it?

  • clone mockito and check out "mockito-spring-boot" branch
  • ./gradlew publishToMavenLocal
  • grab newly published version of mockito from maven local and try it out!
  • I don't have Javadocs yet. This test class documents the new API (hopefully :) -> 1d2cd20?diff=unified#diff-c7b40da00e36958d90e247ac5f759871R23

Suggested next steps:

  • @wilkinsona, let me know if this is enough for you to try it out!
  • we can use the PR that I have opened for review comments regarding the API
Member

mockitoguy commented Oct 1, 2017

I put together a prototype in "mockito-spring-boot" branch. Opened PR #1192 for visibility. How to use it?

  • clone mockito and check out "mockito-spring-boot" branch
  • ./gradlew publishToMavenLocal
  • grab newly published version of mockito from maven local and try it out!
  • I don't have Javadocs yet. This test class documents the new API (hopefully :) -> 1d2cd20?diff=unified#diff-c7b40da00e36958d90e247ac5f759871R23

Suggested next steps:

  • @wilkinsona, let me know if this is enough for you to try it out!
  • we can use the PR that I have opened for review comments regarding the API
@wilkinsona

This comment has been minimized.

Show comment
Hide comment
@wilkinsona

wilkinsona Oct 3, 2017

@szczepiq This looks great. I've built the mockito-spring-boot branch and prototyped the updates required for Spring Boot to use it: https://github.com/wilkinsona/spring-boot/tree/gh-10352. All our tests pass and, as you can hopefully see, Boot's code is now significantly simpler. Thanks very much.

wilkinsona commented Oct 3, 2017

@szczepiq This looks great. I've built the mockito-spring-boot branch and prototyped the updates required for Spring Boot to use it: https://github.com/wilkinsona/spring-boot/tree/gh-10352. All our tests pass and, as you can hopefully see, Boot's code is now significantly simpler. Thanks very much.

@mockitoguy

This comment has been minimized.

Show comment
Hide comment
@mockitoguy

mockitoguy Oct 3, 2017

Member

Fantastic! I will polish the code / documentation and get the release out soon. Thank you for testing this out and suppling sample project!

Member

mockitoguy commented Oct 3, 2017

Fantastic! I will polish the code / documentation and get the release out soon. Thank you for testing this out and suppling sample project!

mockitoguy added a commit that referenced this issue Oct 8, 2017

Fixed documentation issue, added unit tests
- While working on new verification started listeners (#1191) I found an instance of incorrect documentation. I added unit tests and simplified the documentation.
- Currently, we do guarantee the order in which invocation listeners get notified. Some of our users might have started to depend on this behavior. I added unit tests and fixed the Javadoc. In general, it is seems useful to guarantee the order. This way, the API is more predictable, easier to use.

mockitoguy added a commit that referenced this issue Oct 8, 2017

Added an API for dual proxy use case (Spring Boot)
- For design and rationale, see #1191
- For all the details, see Javadoc in VerificationStartedListener
@mockitoguy

This comment has been minimized.

Show comment
Hide comment
@mockitoguy

mockitoguy Oct 12, 2017

Member

@wilkinsona, can you try out one more time for us please?

I've cleaned up the implementation and the PR in Mockito is ready to go. I want to make sure that we ship code that works for Spring Boot. I have added some extra validation for setMock() method. Other than that I think that the API has not changed since I provided the experimental. The code is in the same branch.

Thank you in advance! :)

Member

mockitoguy commented Oct 12, 2017

@wilkinsona, can you try out one more time for us please?

I've cleaned up the implementation and the PR in Mockito is ready to go. I want to make sure that we ship code that works for Spring Boot. I have added some extra validation for setMock() method. Other than that I think that the API has not changed since I provided the experimental. The code is in the same branch.

Thank you in advance! :)

@wilkinsona

This comment has been minimized.

Show comment
Hide comment
@wilkinsona

wilkinsona Oct 12, 2017

@szczepiq Thanks for the opportunity to double-check things. I've built the current head of the mockito-spring-boot branch and updated my prototype to use the resulting 2.11.0 binaries. The only change that was necessary was to update the version number so things look good from my perspective.

wilkinsona commented Oct 12, 2017

@szczepiq Thanks for the opportunity to double-check things. I've built the current head of the mockito-spring-boot branch and updated my prototype to use the resulting 2.11.0 binaries. The only change that was necessary was to update the version number so things look good from my perspective.

@mockitoguy

This comment has been minimized.

Show comment
Hide comment
@mockitoguy

mockitoguy Oct 15, 2017

Member

We released new version today: 2.11.0. It includes the new API that Spring Boot can use.

Member

mockitoguy commented Oct 15, 2017

We released new version today: 2.11.0. It includes the new API that Spring Boot can use.

@mockitoguy

This comment has been minimized.

Show comment
Hide comment
@mockitoguy

mockitoguy Oct 15, 2017

Member

@wilkinsona - thank you for help and nice collaboration!

Member

mockitoguy commented Oct 15, 2017

@wilkinsona - thank you for help and nice collaboration!

@TimvdLippe

This comment has been minimized.

Show comment
Hide comment
@TimvdLippe
Contributor

TimvdLippe commented Oct 18, 2017

@TimvdLippe TimvdLippe closed this Oct 18, 2017

@mockito mockito deleted a comment Jan 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment