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

MockitoSession API does not clean up listener when initMocks fails #1475

Merged
merged 3 commits into from Aug 19, 2018

Conversation

mockitoguy
Copy link
Member

When initMocks() fails we did not clean up the session listener. This led to a confusing error message on the next attempt to create a session (next test method run). I stumbled upon this problem when attempting to configure TestNG to use strict stubbing (mockito/mockito-testng#1).

In order to fix this cleanly I need to keep the existing functionality that protects the user from forgetting to use 'finishMocking()'. Hence, I still need to throw an exception if the user adds the same listener multiple times. However, in the event that the listener is dirty, I clean it up automatically instead of failing.

@mockitoguy mockitoguy added the bug label Aug 13, 2018
@codecov-io
Copy link

codecov-io commented Aug 13, 2018

Codecov Report

Merging #1475 into release/2.x will increase coverage by 0.13%.
The diff coverage is 95.45%.

Impacted file tree graph

@@                Coverage Diff                @@
##             release/2.x    #1475      +/-   ##
=================================================
+ Coverage          88.43%   88.56%   +0.13%     
- Complexity          2390     2397       +7     
=================================================
  Files                298      299       +1     
  Lines               6020     6036      +16     
  Branches             729      732       +3     
=================================================
+ Hits                5324     5346      +22     
+ Misses               517      511       -6     
  Partials             179      179
Impacted Files Coverage Δ Complexity Δ
...java/org/mockito/internal/exceptions/Reporter.java 93.68% <100%> (ø) 91 <1> (ø) ⬇️
...kito/exceptions/misusing/InjectMocksException.java 100% <100%> (ø) 1 <1> (?)
.../mockito/internal/junit/UniversalTestListener.java 93.54% <100%> (+0.69%) 13 <2> (+2) ⬆️
...kito/internal/framework/DefaultMockitoSession.java 96.15% <100%> (+0.5%) 6 <0> (ø) ⬇️
...mockito/internal/progress/MockingProgressImpl.java 97.53% <90%> (+0.27%) 33 <7> (+4) ⬆️
...ByteBuddyCrossClassLoaderSerializationSupport.java 84% <0%> (+8%) 6% <0%> (ø) ⬇️

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 4e5af65...91c2899. Read the comment docs.

When initMocks() fails we did not clean up the session listener. This led to a confusing error message on the next attempt to create session. I stumbled upon this problem when attempting to configure TestNG to use strict stubbing.

In order to fix this cleanly I need to keep the existing functionality that protects the user from forgetting to use 'finishMocking()'. Hence, I still need to throw an exception if the user adds the same listener multiple times. However, in the event that the listener is dirty, I clean it up automatically instead of failing.
@mockitoguy
Copy link
Member Author

I'm merging this because it blocks mockito/mockito-testng#1. Happy to address code review feedback after this is merged. The change does not involved public API changes, it fixes a bug. All existing tests pass, and the new tests pass, too. Let's ship it!

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

Successfully merging this pull request may close these issues.

None yet

2 participants