Don't add MockitoExtension during migration when unsafe (#875)#1017
Conversation
`AddMockitoExtensionIfAnnotationsUsed` was adding `@ExtendWith(MockitoExtension.class)` even when Mockito support was already provided another way, breaking tests. - Remove `AddMockitoExtensionIfAnnotationsUsed` and its companion `AddMockitoJupiterDependency` from `Mockito1to3Migration`, keeping them as commented-out entries with an explanation so they are not re-added blindly. - Harden the standalone recipe: skip when any `@ExtendWith`/`@RunWith` is already present (e.g. `SpringExtension`), skip non-concrete classes (interfaces/abstract), and only annotate classes that themselves declare a Mockito annotation. - Preserve the PowerMock migration: `PowerMockRunnerDelegateToRunWith` now replaces `@RunWith(PowerMockRunner.class)` with `@RunWith(MockitoJUnitRunner.class)` when the class uses Mockito annotations, instead of relying on the removed auto-add. - Update affected tests and add standalone coverage for `AddMockitoJupiterDependency`.
| import org.mockito.Mock; | ||
| import org.mockito.junit.jupiter.MockitoExtension; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) |
There was a problem hiding this comment.
I think you’ll still need to add the extension when migrating from JMockit to Mockito, otherwise the migration won’t work.
(I’m not familiar with that migration though)
There was a problem hiding this comment.
Good catch — you're right for one of the two JMockit styles. I dug into it:
- The
JMockitToMockitomigration already carries the extension over for the annotation-based style viaChangeType:@ExtendWith(JMockitExtension.class)→@ExtendWith(MockitoExtension.class)and@RunWith(JMockit.class)→@RunWith(MockitoJUnitRunner.class). Almost all of the JMockit tests use this style, and they keep working. - The gap is the javaagent style —
@Mockedwith no@ExtendWith/@RunWith(which is exactly this test). There's no extension to convert, so the migrated@Mockfields would never be initialized.
I've pushed a fix: re-added the (now hardened) AddMockitoExtensionIfAnnotationsUsed scoped to JMockitToMockito only, plus a version-correct mockito-junit-jupiter:5.x dependency. Because the hardened recipe bails when any @ExtendWith/@RunWith is already present, the annotation-based tests (which already have MockitoExtension after ChangeType) are left untouched, while the agent-based case now gets the extension. Restored this test's expected output accordingly.
JMockit tests that use `@Mocked` without `@ExtendWith(JMockitExtension.class)` (the javaagent style) have no extension to carry over via ChangeType, so the converted `@Mock` fields would never be initialized. Re-add the hardened `AddMockitoExtensionIfAnnotationsUsed` scoped to the JMockit migration, plus a version-correct `mockito-junit-jupiter:5.x` dependency. Annotation-based JMockit tests already get `MockitoExtension` via ChangeType, so the hardened recipe leaves them untouched.
- Detect an already-present extension/runner via `AnnotationService.getAllAnnotations` instead of only the leading annotations (more robust, consistent with `PowerMockRunnerDelegateToRunWith`). - Collapse the four `FindAnnotations.find` calls into one stream over a shared list; keep the explicit annotation list rather than an `org.mockito.*` wildcard, which would also match non-injection annotations (`@DoNotMock`, `@Incubating`, ...). - Keep the subtree search so a `@Mock` in a `@Nested` inner class annotates the enclosing class (JUnit 5 propagates the extension); add a test for it.
Now that the recipe bails on any existing extension/runner (not just MockitoExtension), fold that check back into the precondition using the original pattern with the `.class` argument dropped from the matcher. This removes the per-class extension loop and the AnnotationService dependency; `shouldSkip` is left with just the concrete-class and Mockito-annotation checks.
…rom 3.37.0 to 3.38.0 [skip ci] Bumps [org.openrewrite.recipe:rewrite-testing-frameworks](https://github.com/openrewrite/rewrite-testing-frameworks) from 3.37.0 to 3.38.0. Release notes *Sourced from [org.openrewrite.recipe:rewrite-testing-frameworks's releases](https://github.com/openrewrite/rewrite-testing-frameworks/releases).* > 3.38.0 > ------ > > What's Changed > -------------- > > * Don't add MockitoExtension during migration when unsafe ([#875](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/875)) by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#1017](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/1017) > * Add junit-platform-launcher testRuntimeOnly dependency for Gradle during JUnit 6 migration by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#1018](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/1018) > * Migrate removed LocalStack Service enum and getEndpointOverride in Testcontainers 2.x migration by [`@MBoegers`](https://github.com/MBoegers) in [openrewrite/rewrite-testing-frameworks#1014](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/1014) > * Re-include CloseUnclosedStaticMocks in Mockito1to4Migration by [`@steve-aom-elliott`](https://github.com/steve-aom-elliott) in [openrewrite/rewrite-testing-frameworks#1019](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/1019) > * Update MockWebServer MockResponse test for nested-type render change by [`@steve-aom-elliott`](https://github.com/steve-aom-elliott) in [openrewrite/rewrite-testing-frameworks#1020](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/1020) > * Fix UsesType precondition pattern in KotlinTestMethodsShouldReturnUnit by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#1022](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/1022) > * Support 4-arg Whitebox.setInternalState(target, field, value, Class) by [`@MBoegers`](https://github.com/MBoegers) in [openrewrite/rewrite-testing-frameworks#1023](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/1023) > * Refactor `PowerMockWhiteboxToJavaReflection` into per-API recipes by [`@MBoegers`](https://github.com/MBoegers) in [openrewrite/rewrite-testing-frameworks#1021](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/1021) > * Box primitive casts in `Whitebox.getInternalState`/`invokeMethod` reflection by [`@MBoegers`](https://github.com/MBoegers) in [openrewrite/rewrite-testing-frameworks#1024](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/1024) > > **Full Changelog**: <openrewrite/rewrite-testing-frameworks@v3.37.0...v3.38.0> Commits * [`862bd49`](openrewrite/rewrite-testing-frameworks@862bd49) Box primitive casts in Whitebox getInternalState/invokeMethod reflection ([#1024](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/1024)) * [`9e86af9`](openrewrite/rewrite-testing-frameworks@9e86af9) Refactor `PowerMockWhiteboxToJavaReflection` into per-API recipes ([#1021](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/1021)) * [`30c392d`](openrewrite/rewrite-testing-frameworks@30c392d) Support 4-arg Whitebox.setInternalState(target, field, value, Class) ([#1023](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/1023)) * [`c9cd3bd`](openrewrite/rewrite-testing-frameworks@c9cd3bd) Fix UsesType precondition pattern in KotlinTestMethodsShouldReturnUnit ([#1022](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/1022)) * [`a602b44`](openrewrite/rewrite-testing-frameworks@a602b44) OpenRewrite recipe best practices * [`ff368ff`](openrewrite/rewrite-testing-frameworks@ff368ff) Update UpdateMockWebServerMockResponse test for nested-type render change ([#1](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/1)... * [`45342dc`](openrewrite/rewrite-testing-frameworks@45342dc) Re-include CloseUnclosedStaticMocks in Mockito1to4Migration ([#1019](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/1019)) * [`e1cb892`](openrewrite/rewrite-testing-frameworks@e1cb892) Migrate removed LocalStack `Service` enum and `getEndpointOverride` in Testco... * [`3934d1e`](openrewrite/rewrite-testing-frameworks@3934d1e) Add junit-platform-launcher testRuntimeOnly dependency for Gradle during JUni... * [`d5554a9`](openrewrite/rewrite-testing-frameworks@d5554a9) Don't add MockitoExtension during migration when unsafe ([#875](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/875)) ([#1017](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/1017)) * Additional commits viewable in [compare view](openrewrite/rewrite-testing-frameworks@v3.37.0...v3.38.0)
Problem
AddMockitoExtensionIfAnnotationsUsed(run as part ofMockito1to3Migration) added@ExtendWith(MockitoExtension.class)to any test using@Mock/@Spy/@Captor/@InjectMocks, even when Mockito support was already provided another way. This was at best redundant and at worst broke the test lifecycle — e.g. whenSpringExtensionis present (it already initializes@Mockfields), when a parent class/interface/meta-annotation provides the extension, or whenMockitoAnnotations.openMocks(this)is used explicitly.Changes
Removed from the migration (do no harm)
AddMockitoExtensionIfAnnotationsUsedand its companionAddMockitoJupiterDependencyare no longer applied as part ofMockito1to3Migration. They are kept as commented-out entries with an explanatory comment so the intent is captured and they aren't re-added without first teaching the recipes about these cases.Hardened the standalone recipe (
AddMockitoExtensionIfAnnotationsUsed)@ExtendWith/@RunWithis already present (coversSpringExtension).abstractbases).Preserved the PowerMock migration path
PowerMockRunnerDelegateToRunWithnow replaces@RunWith(PowerMockRunner.class)with@RunWith(MockitoJUnitRunner.class)when the class uses Mockito annotations (instead of relying on the now-removed generic auto-add), and still removes it entirely otherwise.Tests
AddMockitoJupiterDependencyTestto keep that recipe covered.initMocksis now preserved/migrated toopenMocks+AutoCloseableinstead of being replaced by the extension).Notes / not covered
The hardened recipe still does not detect the inheritance, meta-annotation (e.g.
@MockitoSettings), or explicit-initMockscases — these now only matter to someone invoking the standalone recipe directly, since it no longer runs in the migration.Full test suite passes locally.