Fix/bug 1551 cce on smart not null answers #1576
Merged
Conversation
This file contains 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
Codecov Report
@@ Coverage Diff @@
## release/2.x #1576 +/- ##
=================================================
+ Coverage 88.49% 88.57% +0.07%
- Complexity 2408 2425 +17
=================================================
Files 299 299
Lines 6059 6099 +40
Branches 739 752 +13
=================================================
+ Hits 5362 5402 +40
+ Misses 515 514 -1
- Partials 182 183 +1
Continue to review full report at Codecov.
|
TimvdLippe
approved these changes
Jan 6, 2019
| for (int i = 0; i < parameterTypes.length; i++) { | ||
| Type argType = parameterTypes[i]; | ||
| if (returnType.equals(argType)) { | ||
| return invocation.getArgument(i).getClass(); |
There was a problem hiding this comment.
@Patouche This will throw a NPE if the argument value is null! Can you write a test?
| if (argType instanceof GenericArrayType) { | ||
| argType = ((GenericArrayType) argType).getGenericComponentType(); | ||
| if (returnType.equals(argType)) { | ||
| return invocation.getArgument(i).getClass(); |
There was a problem hiding this comment.
@Patouche This will throw a NPE if the argument value is null! Can you write a test?
TimvdLippe
added a commit
that referenced
this pull request
Feb 8, 2019
This solves a large number of edge-cases where `null` will actually remove the runtime ClassCastException. This essentially negates the whole MockitoCast ErrorProne check. We can still not support every use case, but causing a NPE instead of a CCE does not seem to make this worse. I am still running internal tests within Google to see if there are any regressions, but I already saw that some of the test failures we had with ByteBuddy were resolved with this particular patch. Note that this now fully closes #357. A previous PR resolved the same issue with ReturnsSmartNulls: #1576. Fixes #357
TimvdLippe
added a commit
that referenced
this pull request
Feb 12, 2019
This solves a large number of edge-cases where `null` will actually remove the runtime ClassCastException. This essentially negates the whole MockitoCast ErrorProne check. We can still not support every use case, but causing a NPE instead of a CCE does not seem to make this worse. I am still running internal tests within Google to see if there are any regressions, but I already saw that some of the test failures we had with ByteBuddy were resolved with this particular patch. Note that this now fully closes #357. A previous PR resolved the same issue with ReturnsSmartNulls: #1576. Fixes #357
SamBarker
pushed a commit
to SamBarker/mockito
that referenced
this pull request
Feb 25, 2019
* Fix mockito#1551 : Add support for Generic in ReturnsSmartNulls * Fix mockito#1551 : Integrate support for generic using varargs * mockito#1551 : Fix typo and some missing documentation
SamBarker
pushed a commit
to SamBarker/mockito
that referenced
this pull request
Feb 25, 2019
This solves a large number of edge-cases where `null` will actually remove the runtime ClassCastException. This essentially negates the whole MockitoCast ErrorProne check. We can still not support every use case, but causing a NPE instead of a CCE does not seem to make this worse. I am still running internal tests within Google to see if there are any regressions, but I already saw that some of the test failures we had with ByteBuddy were resolved with this particular patch. Note that this now fully closes mockito#357. A previous PR resolved the same issue with ReturnsSmartNulls: mockito#1576. Fixes mockito#357
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR intends to fix #1551. For fix it, the
ReturnsSmartNullsanswer use now some reflection to find the correct return type instead of thejava.util.Object.When the return type can be found, it will start to retrieve empty values using the answer
ReturnsMoreEmptyValues&ReturnsEmptyValuesbefore creating a new mock instance.As you may notice in tests, there is still one case where the return type cannot be found. In this case, the
Answerwill return anullinstead ofObjectmock.I think, it may be possible to improve this code. Any suggestion will be welcome !