Skip to content

Conversation

Kramermp
Copy link
Contributor

@Kramermp Kramermp commented Oct 16, 2024

This change makes a couple of changes surronding the the clearMock and clearMock apis.

  1. All disabled mocks gets the INSTANCE of DisabledMockCreationSettings. This prevents any NPEs from attempting to access the MockCreationSettings, but will throw a MockDisabledException if attempting to actually read the settings. This will block calls such as: isMock(), etc.
  2. This change syncs the logic between the clearMocks() and clearMock() logic to make the customer experience the same whether you are attempting to clear all mocks or just and individual mock.

These changes are being made mainly to introduce some more consistent behavior for mocks that have been cleared. Currently, the user experience is less than desirable to have NPEs be tripped depending on which clear method was called on the mock. Additionally, I think this disabling approach brings the design more in-line with some of the documentation about the behavior of a cleared mock.

Checklist

  • Read the contributing guide
  • PR should be motivated, i.e. what does it fix, why, and if relevant how
  • If possible / relevant include an example in the description, that could help all readers
    including project members to get a better picture of the change
  • [X ] Avoid other runtime dependencies
  • Meaningful commit history ; intention is important please rebase your commit history so that each
    commit is meaningful and help the people that will explore a change in 2 years
  • The pull request follows coding style
  • Mention Fixes #<issue number> in the description if relevant
  • At least one commit should mention Fixes #<issue number> if relevant

@codecov-commenter
Copy link

codecov-commenter commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 28.00000% with 18 lines in your changes missing coverage. Please review.

Project coverage is 85.59%. Comparing base (7834859) to head (5186436).
Report is 22 commits behind head on main.

Files with missing lines Patch % Lines
...ockito/internal/framework/DisabledMockHandler.java 18.18% 18 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3476      +/-   ##
============================================
- Coverage     85.74%   85.59%   -0.16%     
- Complexity     2951     2952       +1     
============================================
  Files           341      341              
  Lines          8946     8967      +21     
  Branches       1114     1114              
============================================
+ Hits           7671     7675       +4     
- Misses          988     1007      +19     
+ Partials        287      285       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This change makes a couple of changes surronding the the clearMock and
clearMock() apis.
1. All disabled mocks gets the INSTANCE of DisabledMockCreationSettings.
   This prevents any NPEs from attempting to access the
   MockCreationSettings, but will throw a MockDisabledException if
   attempting to actually read the settings. This will block calls such
   as: isMock(), etc.
2. This change syncs the logic between the clearMocks() and clearMock()
   logic to make the customer experience the same whether you are
   attempting to clear all mocks or just and individual mock.
Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the improvements!

@TimvdLippe TimvdLippe merged commit 915ed6e into mockito:main Oct 19, 2024
18 checks passed
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.

3 participants