Retain doNothing() for spies and partial mocks - #1101
Merged
Conversation
`RemoveDoNothingForDefaultMocks` treated every `@Mock` field as a plain mock, but a mock can still run real code for void methods: - `@Mock(answer = Answers.CALLS_REAL_METHODS)` is a partial mock - a `@Mock` field can be reassigned to `Mockito.spy(..)` before stubbing - a local variable holding a spy can shadow a `@Mock` field In each case removing the stubbing lets the real method execute. Fixes #1099
Only a bare identifier was matched on the left-hand side, so a spy assigned through `this` slipped past and the stubbing was still removed.
Variable identity is name plus owner, so a field and a local shadowing it are told apart without the explicit owner check, and `this.mock` resolves to the same variable as `mock`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RemoveDoNothingForDefaultMockstreated every@Mockfield as a plain mock, whose void methods do nothing by default. But a@Mockfield can still run real code for void methods, in which case removing the stubbing changes behaviour — the real method now executes, as reported in org.openrewrite.java.testing.mockito.MockitoBestPractices Unnecessary operations #1099.Three cases are now retained:
@Mock(answer = Answers.CALLS_REAL_METHODS)— a partial mock, both qualified and statically imported@Mockfield reassigned fromMockito.spy(..)before the stubbing runs@MockfieldOther answers (
RETURNS_DEEP_STUBSand friends) still do nothing for void methods, so those stubbings are still removed; there's a test pinning that.