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

Mockito mocking of finals is broken #2677

Closed
emartynov opened this Issue Oct 12, 2016 · 17 comments

Comments

Projects
None yet
9 participants
@emartynov

emartynov commented Oct 12, 2016

Robolectric is not compatible with latest Mockito experimental feature for mocking finals:
http://site.mockito.org/mockito/docs/current/org/mockito/Mockito.html#39

Steps:

  1. Have a project with Robolectric and Mockito
  2. Enable experimental Mockito feature

I'm with Robolectric 3.1.2.

@BrianGardnerAtl

This comment has been minimized.

Show comment
Hide comment
@BrianGardnerAtl

BrianGardnerAtl Oct 18, 2016

I'm also running into this issue. Tests run with the RobolectricTestRunner will fail whenever the mocking feature is enabled. If I switch the test runner to JUnit4 then I don't get any errors. I made a small test project to verify.

https://github.com/bgardner7/MockitoTest

Using:

  • JUnit 4.12
  • Robolectric 3.1.2
  • mockito-core 2.2.3

BrianGardnerAtl commented Oct 18, 2016

I'm also running into this issue. Tests run with the RobolectricTestRunner will fail whenever the mocking feature is enabled. If I switch the test runner to JUnit4 then I don't get any errors. I made a small test project to verify.

https://github.com/bgardner7/MockitoTest

Using:

  • JUnit 4.12
  • Robolectric 3.1.2
  • mockito-core 2.2.3
@emartynov

This comment has been minimized.

Show comment
Hide comment
@emartynov

emartynov Oct 18, 2016

Robolectric and Mockito experimental feature both are using bytecode manipulation. Clear they might conflict. We had similar issues before when we tried to use Robolecric and PowerMockito. I wonder if it is possible to live them together. That would be great!

emartynov commented Oct 18, 2016

Robolectric and Mockito experimental feature both are using bytecode manipulation. Clear they might conflict. We had similar issues before when we tried to use Robolecric and PowerMockito. I wonder if it is possible to live them together. That would be great!

@BrianGardnerAtl

This comment has been minimized.

Show comment
Hide comment
@BrianGardnerAtl

BrianGardnerAtl Oct 25, 2016

Agreed, especially when trying to mock Retrofit 2 since the Retrofit and Call objects are final. Since the final class mocking in Mockito 2 is still an incubating feature I think we will have to wait until it is fully ready before finding a cause of this issue.

BrianGardnerAtl commented Oct 25, 2016

Agreed, especially when trying to mock Retrofit 2 since the Retrofit and Call objects are final. Since the final class mocking in Mockito 2 is still an incubating feature I think we will have to wait until it is fully ready before finding a cause of this issue.

@kingargyle

This comment has been minimized.

Show comment
Hide comment
@kingargyle

kingargyle Oct 25, 2016

Contributor

If you are testing out Retrofit, I highly recommend using OKHttp's MockWebserver. It will at least let you make sure your mappings are correct and the input outputs are hitting the correct endpoints. Since both Mockito and Robolectric are doing byte code manipulation it may be a while before a solution is found that works with both.

Contributor

kingargyle commented Oct 25, 2016

If you are testing out Retrofit, I highly recommend using OKHttp's MockWebserver. It will at least let you make sure your mappings are correct and the input outputs are hitting the correct endpoints. Since both Mockito and Robolectric are doing byte code manipulation it may be a while before a solution is found that works with both.

@emartynov

This comment has been minimized.

Show comment
Hide comment
@emartynov

emartynov Oct 25, 2016

@kingargyle, thanks! But we are talking about unit testing now.

emartynov commented Oct 25, 2016

@kingargyle, thanks! But we are talking about unit testing now.

@kingargyle

This comment has been minimized.

Show comment
Hide comment
@kingargyle

kingargyle Oct 26, 2016

Contributor

@emartynov Yes I realize that. And Yes MockWebserver gets away from true unit testing, but in the case where you can't get around the mocking of the final classes, (you run into similar issues when writing tests for code that uses OkHttp), the mockwebserver allows you to at least test the results, and not incure the full overhead of a true Integration Test.

Contributor

kingargyle commented Oct 26, 2016

@emartynov Yes I realize that. And Yes MockWebserver gets away from true unit testing, but in the case where you can't get around the mocking of the final classes, (you run into similar issues when writing tests for code that uses OkHttp), the mockwebserver allows you to at least test the results, and not incure the full overhead of a true Integration Test.

@mcclurej06

This comment has been minimized.

Show comment
Hide comment
@mcclurej06

mcclurej06 Dec 8, 2016

Also running into this issue.

mcclurej06 commented Dec 8, 2016

Also running into this issue.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Dec 9, 2016

Mockito maintainer here. I just debugged this and this turned out to be a combination of bugs in Mockito and Robolectrics.

Mockito throws an exception from the mock maker plugin's constructor upon an unexpected interaction with the Robolectrics class loader. This causes Mockito to attempt cleaning the stack trace what should not happen at this point as it is not a user fault but a configuration error. Cleaning stack traces runs via a plugin where the manager just failed loading the mock maker plugin what in turn results in a null pointer exception as the class initializer cannot correctly created the plugin manager where the class initializer was not executed to its end. I already fixed this in Mockito.

The mock maker does not work as the InstrumentingClassLoader does not correctly delegate to its parent after we inject a class into the bootstrap class loader. Instead of asking its parent class loader for org.mockito.internal.creation.bytebuddy.MockMethodDispatcher, the class loader loads the class in question directly from the class path and therefore voids the bootstrap injection. This results in the mock maker plugin not being able to instrument types globally where it cannot work as only the bootstrap loader is globally visible.

A correct class loader implementation must request a class from its parent first. In this case, I understand why this is not happening as the system class loaders class's should be shadowed. However, this should happen by super-seeding the class loader, not by some child-first mechanism. I implemented this as a proposal pull request. I did not get the tests of your project to work, unfortunately, there are dependencies missing and I am not familiar enough with the Android space to get this working quickly.

While investigating this, I found a second bug in the Robolectrics class loader where the standard class employment is broken when loading classes via Class::forName where ClassLoader::loadClass(String, boolean) is invoked, therefore surpassing your implementation. I did this for debugging purposes but I assume real libraries can also run into that.

Let me know if anything is unclear. Cheers.

raphw commented Dec 9, 2016

Mockito maintainer here. I just debugged this and this turned out to be a combination of bugs in Mockito and Robolectrics.

Mockito throws an exception from the mock maker plugin's constructor upon an unexpected interaction with the Robolectrics class loader. This causes Mockito to attempt cleaning the stack trace what should not happen at this point as it is not a user fault but a configuration error. Cleaning stack traces runs via a plugin where the manager just failed loading the mock maker plugin what in turn results in a null pointer exception as the class initializer cannot correctly created the plugin manager where the class initializer was not executed to its end. I already fixed this in Mockito.

The mock maker does not work as the InstrumentingClassLoader does not correctly delegate to its parent after we inject a class into the bootstrap class loader. Instead of asking its parent class loader for org.mockito.internal.creation.bytebuddy.MockMethodDispatcher, the class loader loads the class in question directly from the class path and therefore voids the bootstrap injection. This results in the mock maker plugin not being able to instrument types globally where it cannot work as only the bootstrap loader is globally visible.

A correct class loader implementation must request a class from its parent first. In this case, I understand why this is not happening as the system class loaders class's should be shadowed. However, this should happen by super-seeding the class loader, not by some child-first mechanism. I implemented this as a proposal pull request. I did not get the tests of your project to work, unfortunately, there are dependencies missing and I am not familiar enough with the Android space to get this working quickly.

While investigating this, I found a second bug in the Robolectrics class loader where the standard class employment is broken when loading classes via Class::forName where ClassLoader::loadClass(String, boolean) is invoked, therefore surpassing your implementation. I did this for debugging purposes but I assume real libraries can also run into that.

Let me know if anything is unclear. Cheers.

@xian xian changed the title from Robolectric and Mockito 2.1+ to Mockito mocking of finals is broken Jan 5, 2017

@xian xian referenced this issue Jan 6, 2017

Closed

Fix Mockito mocking of finals. #2823

0 of 1 task complete

@xian xian added the defect label Jan 6, 2017

@xian xian modified the milestones: 3.2.2, 3.3.1 Jan 6, 2017

@mgoovaer

This comment has been minimized.

Show comment
Hide comment
@mgoovaer

mgoovaer Jan 13, 2017

@xian Any update on this bug? I'm still encountering it on mockito 2.6.2 and robolectric 3.2.2

mgoovaer commented Jan 13, 2017

@xian Any update on this bug? I'm still encountering it on mockito 2.6.2 and robolectric 3.2.2

@xian

This comment has been minimized.

Show comment
Hide comment
@xian

xian Jan 14, 2017

Member

We're working on this next week. :-)

Member

xian commented Jan 14, 2017

We're working on this next week. :-)

@xian xian modified the milestones: 3.3.1, 3.3 Jan 17, 2017

@aaa-ace

This comment has been minimized.

Show comment
Hide comment
@aaa-ace

aaa-ace Jan 20, 2017

Please update this thread when this bug is fixed

aaa-ace commented Jan 20, 2017

Please update this thread when this bug is fixed

@mcclurej06

This comment has been minimized.

Show comment
Hide comment
@mcclurej06

mcclurej06 Feb 3, 2017

This issue is causing is quite a few problems. Any word on if a fix is being worked on?

mcclurej06 commented Feb 3, 2017

This issue is causing is quite a few problems. Any word on if a fix is being worked on?

@jongerrish

This comment has been minimized.

Show comment
Hide comment
@jongerrish

jongerrish Feb 3, 2017

Contributor
Contributor

jongerrish commented Feb 3, 2017

@mcclurej06

This comment has been minimized.

Show comment
Hide comment
@mcclurej06

mcclurej06 Feb 3, 2017

Awesome, thanks for the update!

mcclurej06 commented Feb 3, 2017

Awesome, thanks for the update!

@xian

This comment has been minimized.

Show comment
Hide comment
@xian

xian Feb 22, 2017

Member

Fixed on master at long last, release coming soon! Thanks for your patience!

Member

xian commented Feb 22, 2017

Fixed on master at long last, release coming soon! Thanks for your patience!

@BrianGardnerAtl

This comment has been minimized.

Show comment
Hide comment
@BrianGardnerAtl

BrianGardnerAtl Feb 22, 2017

Thank you so much! Great to see this getting fixed

BrianGardnerAtl commented Feb 22, 2017

Thank you so much! Great to see this getting fixed

@mcclurej06

This comment has been minimized.

Show comment
Hide comment
@mcclurej06

mcclurej06 commented Feb 22, 2017

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment