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

Allow spying on interfaces so that it is convenient to work with Java 8 default methods #942

Merged
merged 3 commits into from Feb 28, 2017

Conversation

fluentfuture
Copy link
Contributor

Fixes #680

@codecov-io
Copy link

codecov-io commented Feb 12, 2017

Codecov Report

Merging #942 into release/2.x will increase coverage by 0.02%.
The diff coverage is n/a.

@@                Coverage Diff                @@
##             release/2.x     #942      +/-   ##
=================================================
+ Coverage          86.56%   86.58%   +0.02%     
+ Complexity          2254     2252       -2     
=================================================
  Files                289      289              
  Lines               5738     5733       -5     
  Branches             663      662       -1     
=================================================
- Hits                4967     4964       -3     
+ Misses               582      581       -1     
+ Partials             189      188       -1
Impacted Files Coverage Δ Complexity Δ
...to/internal/configuration/SpyAnnotationEngine.java 98.18% <ø> (+3.18%) 24 <ø> (-2)

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 64ab2f7...87f3a5e. Read the comment docs.

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.

Intention is good. Could you update the core Mockito documentation regarding this change? Also would like to see some tests specifically for Java 8. Unsure how we can do that for release/2.x though :(

@@ -59,19 +59,16 @@ public void should_init_spy_and_automatically_create_instance() throws Exception
}

@Test
public void should_prevent_spying_on_interfaces() throws Exception {
public void should_allow_spying_on_interfaces() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

Nice! Thank you for updating the test!

Copy link
Member

@mockitoguy mockitoguy left a comment

Choose a reason for hiding this comment

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

I almost merged it when I read @TimvdLippe comment on documentation :) I request documentation update only. See the checkbox in #680 regarding docs.

Thank you for the code! It's a nice improvement for Java 8 users.

@mockitoguy
Copy link
Member

@TimvdLippe, I'm ok with not having dedicated coverage for Java8 in release/2.x. We don't have a way of doing it right now so I would not want to block this PR.

@TimvdLippe
Copy link
Contributor

We did have some tests for Java 8 in release 2. So it can be done. But lets drop it as requirement for this PR.

@fluentfuture
Copy link
Contributor Author

Thanks for review!

Let me know if the javadoc change looks okay to you.

@@ -1008,7 +1008,7 @@
*
*
*
* <h3 id="30">30. <a class="meaningful_link" href="#spying_abstract_classes" name="spying_abstract_classes">Spying or mocking abstract classes (Since 1.10.12)</a></h3>
* <h3 id="30">30. <a class="meaningful_link" href="#spying_abstract_classes" name="spying_abstract_classes">Spying or mocking abstract classes (Since 1.10.12) and Java 8 default methods (Since release 2.x)</a></h3>
Copy link
Member

Choose a reason for hiding this comment

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

Nice and clean! Thank you!

Copy link
Member

@mockitoguy mockitoguy left a comment

Choose a reason for hiding this comment

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

Nice!

@mockitoguy mockitoguy changed the title Relax validation on spy(AnInterface.class) Allow spying on interfaces so that it is convenient to work with Java 8 default methods Feb 28, 2017
@mockitoguy
Copy link
Member

Restarted the Travis CI jobs and will merge as soon as they are green!

@mockitoguy mockitoguy merged commit bba71f6 into mockito:release/2.x Feb 28, 2017
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.

None yet

4 participants