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

[640] - check for junit4 mockito runners to add mockito-junit-jupiter #670

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

promanenko
Copy link

What's changed?

Add mockito-junit-jupiterdependency

- org.openrewrite.java.dependencies.AddDependency:
groupId: org.mockito
artifactId: mockito-junit-jupiter
version: 3.x
onlyIfUsing: org.mockito.junit.jupiter.*
acceptTransitive: true
scope: test
expected to handle @MockitoExtension didn't work because AddDependency is a scanning recipe and scanning for org.mockito.junit.jupiter.* is performed before the package is added on editSources phase by RunnerToExtension
Now onlyIfUsing is looking for the initial runners that are present on the scanSources phase.
Note: if the old version of recipe is ran in two cycles then it adds mockito-junit-jupiter dependency in the second cycle.

What's your motivation?

Fixes #640

Have you considered any alternatives or workarounds?

Alternatives considered:

  1. Modify RunnerToExtension to handle addition of a dependency if needed. It contradicts the best practice If it can be declarative, it should be declarative
  2. Make RunnerToExtension recipe to cause another cycle. It contradicts the best practice Stay single cycle

Any additional context

There is a similar rewrite runners -> add dependency sequence in vertx migration but it is looking for org.vertx.testtools.VertxUnitRunner which doesn't exist and it seems to be a separate issue.

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@promanenko promanenko marked this pull request as draft January 28, 2025 18:16
@MBoegers
Copy link
Collaborator

Looks good so far, easy and small 👍
Do you think we still need the AddDependency Recipe the in the rewrite-testing-frameworks/src/main/resources/META-INF/rewrite/mockito.yml?

@promanenko
Copy link
Author

Looks good so far, easy and small 👍 Do you think we still need the AddDependency Recipe the in the rewrite-testing-frameworks/src/main/resources/META-INF/rewrite/mockito.yml?

Good catch, now I don't see a case when onlyIfUsing: org.mockito.junit.jupiter.* will catch what is not yet handled by onlyIfUsing: org.mockito..MockitoJUnit*Runner. Mockito1to3Migration could be called separately, but if org.mockito.junit.jupiter.* is already present then JUnit 5 is used and mockito-junit-jupiter dependency should be also present already.
I left the old AddDependency as a safety net and not to break any subtle edge case, but I would be happy to remove it if you think it's safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

JUnit 4 to 5 migration does not add the mockito-junit-jupiter dependency
2 participants