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

Cannot mock/spy class error, after adding quarkus-junit5-mockito dependency #37927

Closed
ennishol opened this issue Dec 23, 2023 · 13 comments · Fixed by #46119
Closed

Cannot mock/spy class error, after adding quarkus-junit5-mockito dependency #37927

ennishol opened this issue Dec 23, 2023 · 13 comments · Fixed by #46119
Labels
area/testing kind/bug Something isn't working
Milestone

Comments

@ennishol
Copy link
Contributor

Describe the bug

After adding quarkus-junit5-mockito dependency, tests start to fail with Cannot mock/spy class error

Expected behavior

Test should work

Actual behavior

Tests fail

How to Reproduce?

con-testing.tar.gz

  1. Run ./mvnw quarkus:dev and press r to start testing
  2. Tests are failing
  3. When quarkus-junit5-mockito is removed with the test using @InjectMock, the failing tests become green

Output of uname -a or ver

No response

Output of java -version

openjdk version "21.0.1" 2023-10-17 LTS Corretto-21.0.1.12.1 (build 21.0.1+12-LTS)

Quarkus version or git rev

5.6.4

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.6

Additional information

No response

@ennishol ennishol added the kind/bug Something isn't working label Dec 23, 2023
Copy link

quarkus-bot bot commented Dec 23, 2023

/cc @geoand (testing)

@famod
Copy link
Member

famod commented Dec 27, 2023

The problem is that Quarkus 3.x is fixating the old subclass mockmaker, which cannot mock e.g. record (like you do in your example).
There is a hint and a workaround (!) in the 3.0 migration guide: https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.0#fixation-of-the-mockito-subclass-mockmaker

FTR, this also happens with Java 17.

@geoand are you planning on supporting the default inline mockmaker properly? I fear that the issue of Quarkus relying on the old and inferior mockmaker will only become worse.
Until then I think the testing guide should contain a similar hint as the 3.0 migration guide (see above).

@ennishol
Copy link
Contributor Author

ennishol commented Dec 28, 2023

@famod but then I will run again in the memory usage problem, right? #31251
I'll give it a try

@ennishol
Copy link
Contributor Author

@famod unfortunately exclusion of mockito-subclass leads to OOM in integration tests, But at least for unit tests I can use it as a workaround

@geoand
Copy link
Contributor

geoand commented Jan 3, 2024

Right, we'll have to tackle the memory leak at some point and support inline properly.

@famod
Copy link
Member

famod commented Oct 28, 2024

FWIW, I was just hit by subclass mock maker limitations when trying to @InjectSpy a spring-data-jpa JpaRepository that has a default method - error from mockito: "Cannot call abstract real method on java object!"

So +1 (again) to move on to inline.

@gsmet
Copy link
Member

gsmet commented Oct 28, 2024

Could we install a callback somewhere to clean the mocks after test execution?

@gian1200
Copy link
Contributor

Until then I think the testing guide should contain a similar hint as the 3.0 migration guide (see above).

+1 to the addition in the Testing Guide.

Although googling the error can lead you to #32952 (comment) (or the migration guide), new users aren't expected to read Migration guides when creating apps directly on Quarkus 3, right? 🤔

@geoand
Copy link
Contributor

geoand commented Feb 6, 2025

I opened #46119 to fix this, however I could not find a way to reproduce the original issue with OOM error so I don't know if the change reintroduces it or not...

@gsmet
Copy link
Member

gsmet commented Feb 6, 2025

Should we make sure we call Mockito.framework().clearInlineMocks(); after each test as was mentioned in one of the issues?

Not sure if it's still needed though. I think we should probably bite the bullet and adjust if we have reports. A lot of things changed since 3.0.0.

@geoand
Copy link
Contributor

geoand commented Feb 6, 2025

Should we make sure we call Mockito.framework().clearInlineMocks(); after each test as was mentioned in one of the issues?

That's essentially what the PR does.

I think we should probably bite the bullet and adjust if we have reports. A lot of things changed since 3.0.0.

Right. I'm like +0.5 on that. I still wish I could have reproduced the original issue though...

@gsmet
Copy link
Member

gsmet commented Feb 6, 2025

That's essentially what #46119 does.

Ah yes sorry. Not my best self right now.

@geoand
Copy link
Contributor

geoand commented Feb 6, 2025

No worries :)

@gsmet gsmet closed this as completed in b67eb22 Feb 6, 2025
gsmet added a commit that referenced this issue Feb 6, 2025
Use inline strategy for Mockito instead of subclass one
@quarkus-bot quarkus-bot bot added this to the 3.19 - main milestone Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants