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

Remove all deprecated APIs for Mockito 4 #2418

Merged
merged 1 commit into from
Sep 9, 2021
Merged

Conversation

TimvdLippe
Copy link
Contributor

@TimvdLippe TimvdLippe commented Sep 1, 2021

All of these APIs have been marked as deprecated and have been present
in Mockito for quite a while. To cleanup our API surface and reduce
technical debt, let's remove these long-deprecated methods/classes.

An overview of now-deleted classes/methods:

  • org.mockito.Matchers which was an alias for
    org.mockito.ArgumentMatchers
  • org.mockito.ArgumentMatchers#{anyObject,anyVararg} both which were
    aliases for org.mockito.ArgumentMatchers#any
  • org.mockito.ArgumentMatchers#any*Of, which were aliases for the same
    method name without the Of and the generic parameters (which were
    ignored)
  • org.mockito.ArgumentMatchers#{is}{Not}Null(Class) which took a class
    which was ignored. Aliases for the same methods without the parameter
  • org.mockito.MockedStatic#verify which had the parameter types reversed
  • org.mockito.Mockito#verifyZeroInteractions an alias of
    verifyNoMoreInteractions
  • org.mockito.Mockito#debug framework integration API that we later
    refactored
  • org.mockito.configuration.AnnotationEngine which was leaking internal
    APIs and instead users should use org.mockito.plugins.AnnotationEngine
  • org.mockito.exceptions.verification.TooLittleActualInvocations fixed
    the grammar from "Little" to "Few"
  • Numerous internal APIs that we never officially supported and can now
    remove
  • org.mockito.plugins.InstantiatorProvider which was leaking internal
    APIs and instead users should use InstantiatorProvider2 (we should
    probably rename back to remove the number in a future major release)
  • org.mockito.runners a package that hosted several old JUnit runners
    which were no longer supported. Users should instead use
    org.mockito.junit.MockitoJUnitRunner which is our official JUnit4
    runner.

Since these APIs are removed, this change effectively defines Mockito 4.

Closes #1517

All of these APIs have been marked as deprecated and have been present
in Mockito for quite a while. To cleanup our API surface and reduce
technical debt, let's remove these long-deprecated methods/classes.

An overview of now-deleted classes/methods:
- org.mockito.Matchers which was an alias for
org.mockito.ArgumentMatchers
- org.mockito.ArgumentMatchers#{anyObject,anyVararg} both which were
aliases for org.mockito.ArgumentMatchers#any
- org.mockito.ArgumentMatchers#any*Of, which were aliases for the same
method name without the Of and the generic parameters (which were
ignored)
- org.mockito.ArgumentMatchers#{is}{Not}Null(Class) which took a class
which was ignored. Aliases for the same methods without the parameter
- org.mockito.MockedStatic#verify which had the parameter types reversed
- org.mockito.Mockito#verifyZeroInteractions an alias of
verifyNoMoreInteractions
- org.mockito.Mockito#debug framework integration API that we later
refactored
- org.mockito.configuration.AnnotationEngine which was leaking internal
APIs and instead users should use org.mockito.plugins.AnnotationEngine
- org.mockito.exceptions.verification.TooLittleActualInvocations fixed
the grammar from "Little" to "Few"
- Numerous internal APIs that we never officially supported and can now
remove
- org.mockito.plugins.InstantiatorProvider which was leaking internal
APIs and instead users should use InstantiatorProvider2 (we should
probably rename back to remove the number in a future major release)
- org.mockito.runners a package that hosted several old JUnit runners
which were no longer supported. Users should instead use
org.mockito.junit.MockitoJUnitRunner which is our official JUnit4
runner.

Since these APIs are removed, this change effectively defines Mockito 4.
@TimvdLippe TimvdLippe requested a review from a team September 1, 2021 17:26
@TimvdLippe
Copy link
Contributor Author

@mockito/core Today I was working on Mockito again and realized we are still shipping loads of deprecated APIs that have been part of Mockito for a long time. All of these APIs have valid replacements, sometimes they are even aliases of other methods.

Therefore, I propose we remove these deprecated APIs and publish that as Mockito 4. This should cleanup our codebase considerably, remove loads of tech debt and should guide our users to the officially supported classes/methods.

This PR is large, as I wasn't sure what was the best way to work on this. I can create separate PRs for each sub-item, but I wasn't sure how we do that with release. Therefore, this PR is large and can largely serve as discussion opportunity. If you want to, I can later split up the PRs and land them separately if you prefer.

Let me know what you think.

@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2021

Codecov Report

Merging #2418 (7ac03d9) into main (481639c) will increase coverage by 0.79%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #2418      +/-   ##
============================================
+ Coverage     85.45%   86.25%   +0.79%     
+ Complexity     2799     2758      -41     
============================================
  Files           330      318      -12     
  Lines          8478     8299     -179     
  Branches       1026     1015      -11     
============================================
- Hits           7245     7158      -87     
+ Misses          955      865      -90     
+ Partials        278      276       -2     
Impacted Files Coverage Δ
src/main/java/org/mockito/AdditionalMatchers.java 100.00% <ø> (ø)
src/main/java/org/mockito/Answers.java 100.00% <ø> (+8.33%) ⬆️
src/main/java/org/mockito/ArgumentMatchers.java 98.96% <ø> (-0.11%) ⬇️
src/main/java/org/mockito/BDDMockito.java 90.00% <ø> (+1.32%) ⬆️
src/main/java/org/mockito/MockedStatic.java 100.00% <ø> (ø)
src/main/java/org/mockito/Mockito.java 92.20% <ø> (+0.95%) ⬆️
...ito/configuration/DefaultMockitoConfiguration.java 100.00% <ø> (ø)
...ceptions/verification/TooFewActualInvocations.java 100.00% <ø> (ø)
...in/java/org/mockito/internal/MockedStaticImpl.java 78.66% <ø> (+2.04%) ⬆️
...nal/configuration/IndependentAnnotationEngine.java 85.36% <ø> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 481639c...7ac03d9. Read the comment docs.

Copy link
Member

@raphw raphw left a comment

Choose a reason for hiding this comment

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

Good effort. Those have been there forever. I think reducing the surface is a good call especially since the API should be easy to learn.

Copy link
Contributor

@bric3 bric3 left a comment

Choose a reason for hiding this comment

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

I would have split the removal of these APIs by domain, ie test usage, plugin loading. But maybe I'm too nit picky.

The change looks good, but since there's a long list of removal, maybe have a document that list the removal, this could be done in the wiki.

Thank you @TimvdLippe for the effort, you rock !

@TimvdLippe
Copy link
Contributor Author

@bric3 Yeah I initially wanted to do that, but then I would have to open a couple of PRs and that would hamper the discussion. I think this PR is on the edge of manageable, so I guess it is easiest to continue with it, but yeah it is not ideal.

I will make sure that the GitHub release section will have the relevant APIs listed and their replacement.

@TimvdLippe TimvdLippe marked this pull request as ready for review September 2, 2021 09:46
@TimvdLippe
Copy link
Contributor Author

This PR is now pending a final approval of @mockitoguy and I think then we can merge this 🎉 Thanks for the speedy reviews!

@TimvdLippe TimvdLippe mentioned this pull request Sep 2, 2021
7 tasks
Copy link
Contributor

@bric3 bric3 left a comment

Choose a reason for hiding this comment

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

@TimvdLippe That makes sense, plus you're the one doing the effort.

@TimvdLippe
Copy link
Contributor Author

After a quick discussion, we decided to go ahead with this change. At the moment, we don't have any other breaking changes in mind that we want to land in Mockito 4 as well. To keep churn low for our users, we will remove these APIs in a single release, to hopefully keep the cost of upgrading to Mockito 4 manageable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mockito 4
4 participants