Allow delegating to non-public methods for AdditionalAnswers#delegatesTo #1536
Conversation
Fixes an issue where doing "delegatesTo(new AnonymousClass() {})" would cause an IllegalAccessException, and other access-related issues.
Codecov Report
@@ Coverage Diff @@
## release/2.x #1536 +/- ##
=================================================
- Coverage 88.59% 88.58% -0.02%
Complexity 2402 2402
=================================================
Files 299 299
Lines 6041 6044 +3
Branches 734 734
=================================================
+ Hits 5352 5354 +2
- Misses 510 511 +1
Partials 179 179
Continue to review full report at Codecov.
|
|
Oof, what do I do with code coverage check on the "catch (SecurityException ignore)" line? |
|
|
||
| import static org.junit.Assert.assertEquals; | ||
|
|
||
| public class ForwardsInvocationsTest extends TestBase { |
There was a problem hiding this comment.
Do we need this test? Does this test improves coverage on top of the existing ForwardsInvocationsTest?
There was a problem hiding this comment.
The original problem was the inability to mock anonymous classes. In line 21 of this test, an anonymous class is created. I think this is therefore testing the appropriate codepath?
There was a problem hiding this comment.
It is. Please disregard my comment.
There was a problem hiding this comment.
Yeah, our comments were cross-posted. I saw your reply the second I posted my comment 😛
| @@ -40,6 +40,11 @@ public Object answer(InvocationOnMock invocation) throws Throwable { | |||
| } | |||
|
|
|||
| Object[] rawArguments = ((Invocation) invocation).getRawArguments(); | |||
| try { | |||
| delegateMethod.setAccessible(true); | |||
There was a problem hiding this comment.
Can you add an e2e user test? Can you put together a test class that shows that without this change Mockito indeed throws access exceptions?
|
Please ignore my earlier comment, I can see the new test is doing what we need to do! |
We prefer user e2e tests for such features
|
I reworked the test a little bit and will merge once the build is happy. Thank you @li-wjohnson! |
Fixes an issue where doing "delegatesTo(new AnonymousClass() {})" would cause an IllegalAccessException, and other access-related issues.
(See #1535 - Sorry, wasn't aware that you can't change the PR source branch.)