Adding dependency guards to the Junit 4 -> 5 migration for when the POM or build.gradle have a dependency on `org.testng:testng`. by steve-aom-elliott · Pull Request #760 · openrewrite/rewrite-testing-frameworks

steve-aom-elliott · 2025-07-07T21:31:19Z

Relies on Adding recipe for DoesNotIncludeDependency for Gradle (Maven had one already), to allow for checking whether a dependency is not included. rewrite#5725

What's changed?

There was already a code guard for usage of org.testng..*, but not a guard for if there was a dependency on it in pom.xml or build.gradle. This meant it was able to make changes to some of those files regarding exclusions that left it in a half-way state.

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

steve-aom-elliott · 2025-07-14T19:15:32Z

Requires openrewrite/rewrite@2065d3e to make it into a release

sambsnyd

When there are both gradle and maven variations of a recipe we often create one which bundles the two together in rewrite-java-dependencies

steve-aom-elliott · 2025-07-14T20:14:30Z

I'll see if I can round out a PR for that as well and then switch this over to that. It's worth noting that given scope and configuration don't overlap in terms of allowable values, the existing shared DependencyInsight in the rewrite-java-dependencies is a bit of an odd one, given it only takes Maven scopes and passes null for configuration.

Laurens-W · 2025-07-15T08:22:21Z

+  - org.openrewrite.gradle.search.DoesNotIncludeDependency:
+      groupId: org.testng
+      artifactId: testng
+  - org.openrewrite.maven.search.DoesNotIncludeDependency:
+      groupId: org.testng
+      artifactId: testng


Could we introduce an overarching recipe?
org.openrewrite.java.dependencies.DoesNotIncludeDependency?

steve-aom-elliott · 2025-07-16T15:31:47Z

Added recipe in rewrite-java-dependencies as org.openrewrite.java.dependencies.search.DoesNotIncludeDependency

Relies on Adding DoesNotIncludeDependency that will call both the Maven and Gradle versions rewrite-java-dependencies#144 which in turn relies on Fixing the application of DoesNotIncludeDependency against files of the incorrect type. rewrite#5758

timtebeek

The single test you had here originally included a class with Test from TestNG, as well as both Maven and Gradle build files; that's a bit odd, because we wouldn't even need the Maven/Gradle build files for the test to pass as the presence of the class should already disqualify the DoesNotUseType precondition. I've separated that out into three distinct tests, with two merely using JUnit + testng dependency, which I'd have expected to pass. They're failing though and making changes, which if I understood to intention of this PR correctly should not happen. Is there anything wrong with the test setup or preconditions?

steve-aom-elliott · 2025-07-29T20:10:00Z

Working on fixes for higher level tests. I imagine they're situations where I need to wrap the test in a mavenProject, though maybe this is an indicator that I need to make the guard more flexible for situations where for some reason there's no surrounding project for a Java file.

github-actions

Some suggestions could not be made:

src/test/java/org/openrewrite/java/testing/testng/TestNgToAssertJTest.java
- lines 70-71

github-actions

Some suggestions could not be made:

src/test/java/org/openrewrite/java/testing/testng/TestNgToAssertJTest.java
- lines 70-71

timtebeek · 2025-07-29T22:30:47Z

-  - org.openrewrite.java.search.DoesNotUseType:
-      fullyQualifiedTypeName: org.testng..*
-      includeImplicit: true
+  - org.openrewrite.java.testing.junit5.TestNgGuard


The likeliness of this precondition now matching in the wild is quite a bit larger. I'm wondering if we should ask whoever reported this issue to take on this precondition in their recipes, as opposed to including it here.

In practical terms I'd still hope it to be rare that TestNG is present on the classpath in anyway, so we could still adopt this PR as is, but figured point this out before a merge.

It looks like entry point for them is org.openrewrite.java.spring.boot3.UpgradeSpringBoot_3_1 which is fairly high up the chain (about 10 levels above the eventual org.openrewrite.java.testing.junit5.JUnit4to5Migration composite recipe), hmm. The composite recipe that we have the condition on here itself calls 47 other recipes, so it's quite the expansion to alter that uniquely. I know that the composite recipe also leads back up to org.openrewrite.java.spring.boot3.SpringBoot33BestPractices, org.openrewrite.java.testing.assertj.Assertj and org.openrewrite.java.testing.junit5.JUnit5BestPractices.

timtebeek

Approved save for the small caveat pointed out in

https://github.com/openrewrite/rewrite-testing-frameworks/pull/760/files#r2241141235

if @sambsnyd agrees then we can merge, to have some shared responsibility.

sambsnyd · 2025-07-30T18:49:06Z

+
+    @Override
+    public TreeVisitor<?, ExecutionContext> getScanner(Set<JavaProject> acc) {
+        return new ModuleHasDependency("org.testng", "testng*", null, null).getScanner(acc);


Seems like we could add a flag to ModuleHasDependency to invert its own marking behavior, then we wouldn't need specialized "module doesn't have dependency" recipes like this one

@sambsnyd Adding invertMarking flag to ModuleHasDependency for flipping what gets marked rewrite-java-dependencies#146

Switched to using the new option on ModuleHasDependency instead of a separate class in the latest push.

…OM or build.gradle have a dependency on `org.testng:testng`. Relies on openrewrite/rewrite#5725

…in `rewrite-java-dependencies`

…irst pass

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

… an enclosing project for a source file, which was a majority of our existing tests. - If it figures out it's a loose file not in a `JavaProject`, it can't ensure project scope anyway, so falls back to solely whether the file itself has a TestNG dependency or uses TestNG types instead for those

Assuming classes can not be present without those dependencies

…ency` for the TestNG guard for JUnit

steve-aom-elliott · 2025-07-31T21:44:32Z

Failing at the moment because it seems openrewrite/rewrite-java-dependencies isn't seeing the new version of Gradle ChangeDependency when it's trying to make a build: https://github.com/openrewrite/rewrite-java-dependencies/actions/runs/16660315340/job/47155660481#step:6:350

steve-aom-elliott · 2025-08-01T14:51:35Z

So the reason it's failing again turns out to actually be because ModuleHasDependency won't mark files at all if they aren't part of modules, which to a degree makes sense based on the recipe itself, but a number of our tests don't wrap their java blocks in a mavenProject, meaning they won't have a JavaProject marker on them when it reaches the recipe. Trying to figure out the best approach here, because while in the real world, you're basically always going to have a module present, our tests are different situation and having to decide to wrap certain tests, especially those many levels up for composite recipes, feels like it's going to be onerous.

steve-aom-elliott · 2025-08-01T20:49:36Z

Added PR Marking files not containing JavaProject marker if invertMarking is also true, as they vacuously would not have a particular dependency in real-world scenarios rewrite-java-dependencies#147 for allowing to mark non-project-wrapped loose Java files so that it doesn't break tests upstream that aren't caring about having TestNG in the first place.

steve-aom-elliott · 2025-08-01T21:30:11Z

@sambsnyd Looks like this is back to green if you'd be able to give a final look over with it using ModuleHasDependency instead of a separate recipe for the guard.

github-actions

Some suggestions could not be made:

src/test/java/org/openrewrite/java/testing/mockito/MockitoJUnitRunnerToExtensionTest.java
- lines 21-20
- lines 38-58

…rom 3.11.0 to 3.15.0 [skip ci] Bumps [org.openrewrite.recipe:rewrite-testing-frameworks](https://github.com/openrewrite/rewrite-testing-frameworks) from 3.11.0 to 3.15.0. Release notes *Sourced from [org.openrewrite.recipe:rewrite-testing-frameworks's releases](https://github.com/openrewrite/rewrite-testing-frameworks/releases).* > 3.15.0 > ------ > > What's Changed > -------------- > > * Include additional AssertJ migrations by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#773](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/773) > * Include recipes in larger composites where possible by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#774](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/774) > * Retain comments when chaining assertions by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#775](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/775) > * refactor: Static imports for Collections and Collectors by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#779](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/779) > * Added implementation to extract any expression arguments from last statement in assertThrows by [`@JohannisK`](https://github.com/JohannisK) in [openrewrite/rewrite-testing-frameworks#724](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/724) > * Don't clean up mockito imports for Kotlin by [`@jevanlingen`](https://github.com/jevanlingen) in [openrewrite/rewrite-testing-frameworks#782](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/782) > * Adding dependency guards to the Junit 4 -> 5 migration for when the POM or build.gradle have a dependency on `org.testng:testng`. by [`@steve-aom-elliott`](https://github.com/steve-aom-elliott) in [openrewrite/rewrite-testing-frameworks#760](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/760) > * rename settings.local.json to settings.json by [`@zieka`](https://github.com/zieka) in [openrewrite/rewrite-testing-frameworks#783](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/783) > * Fix ClassCastException in PowerMockitoWhenNewToMockito by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#786](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/786) > * MockitoJUnitRunnerToExtension: Handle existing MockitoExtension by [`@amishra-u`](https://github.com/amishra-u) in [openrewrite/rewrite-testing-frameworks#788](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/788) > > **Full Changelog**: <openrewrite/rewrite-testing-frameworks@v3.14.0...v3.15.0> > > 3.14.1 > ------ > > What's Changed > -------------- > > * OpenRewrite v8.59.1: <https://github.com/openrewrite/rewrite> > > **Full Changelog**: <https://github.com/openrewrite/rewrite-testing-frameworks> > > 3.14.0 > ------ > > What's Changed > -------------- > > * Collapse more consecutive assertions by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#769](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/769) > * refactor: Unwrap else block after return or throw statement by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#771](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/771) > * Remove all PowerMock imports after migration by [`@JohannisK`](https://github.com/JohannisK) in [openrewrite/rewrite-testing-frameworks#770](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/770) > * Remove `assertTrue(true)` and `assertFalse(false)` by [`@aet2505`](https://github.com/aet2505) in [openrewrite/rewrite-testing-frameworks#767](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/767) > > New Contributors > ---------------- > > * [`@aet2505`](https://github.com/aet2505) made their first contribution in [openrewrite/rewrite-testing-frameworks#767](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/767) > > **Full Changelog**: <openrewrite/rewrite-testing-frameworks@v3.13.0...v3.14.0> > > 3.13.0 > ------ > > What's Changed > -------------- > > * Removing `NoInitializationForInjectMock` recipe due to side effects when using recipe by [`@steve-aom-elliott`](https://github.com/steve-aom-elliott) in [openrewrite/rewrite-testing-frameworks#765](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/765) > * Adding functionality to `MockitoWhenOnStaticToMockStatic` for recognizing an existing try-with-resources `MockedStatic<T>` can be reused instead of creating a new one. by [`@steve-aom-elliott`](https://github.com/steve-aom-elliott) in [openrewrite/rewrite-testing-frameworks#766](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/766) > * Inconvertible Mockito `when` situation involving lambda and static method. by [`@steve-aom-elliott`](https://github.com/steve-aom-elliott) in [openrewrite/rewrite-testing-frameworks#763](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/763) > * Changing order of `mockito-all` -> `mockito-core` change vs `org.mockito:*` upgrade, and made the former upgrade to `3.x` at the same time, as there aren't aligned versions between the two. by [`@steve-aom-elliott`](https://github.com/steve-aom-elliott) in [openrewrite/rewrite-testing-frameworks#768](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/768) > > New Contributors > ---------------- > > * [`@steve-aom-elliott`](https://github.com/steve-aom-elliott) made their first contribution in [openrewrite/rewrite-testing-frameworks#765](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/765) > > **Full Changelog**: <openrewrite/rewrite-testing-frameworks@v3.12.0...v3.13.0> > > 3.12.0 > ------ > > What's Changed > -------------- > > * Fix for JUnit test method throws clause preservation by [`@knutwannheden`](https://github.com/knutwannheden) in [openrewrite/rewrite-testing-frameworks#755](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/755) > * refactor: org.openrewrite.mavencentral by [`@Laurens-W`](https://github.com/Laurens-W) in [openrewrite/rewrite-testing-frameworks#756](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/pull/756) ... (truncated) Commits * [`76c6888`](openrewrite/rewrite-testing-frameworks@76c6888) MockitoJUnitRunnerToExtension: Handle existing MockitoExtension ([#788](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/788)) * [`75d2019`](openrewrite/rewrite-testing-frameworks@75d2019) Retain `Throwable` in `SimplifyTestThrows` * [`0ea47e9`](openrewrite/rewrite-testing-frameworks@0ea47e9) Include `SimplifyTestThrows` with `JUnit5BestPractices` * [`efb0a4e`](openrewrite/rewrite-testing-frameworks@efb0a4e) Add AssertJ change type recipes for classes deprecated in v3.27.4 * [`f203a20`](openrewrite/rewrite-testing-frameworks@f203a20) Update documentation examples * [`69d7716`](openrewrite/rewrite-testing-frameworks@69d7716) Fix ClassCastException in PowerMockitoWhenNewToMockito ([#786](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/786)) * [`dc6f272`](openrewrite/rewrite-testing-frameworks@dc6f272) rename settings.local.json to settings.json ([#783](https://redirect.github.com/openrewrite/rewrite-testing-frameworks/issues/783)) * [`96fca52`](openrewrite/rewrite-testing-frameworks@96fca52) Documentation example for MockitoJUnitRunnerToExtensionTest * [`9a2082a`](openrewrite/rewrite-testing-frameworks@9a2082a) Adding dependency guards to the Junit 4 -> 5 migration for when the POM or bu... * [`74366cf`](openrewrite/rewrite-testing-frameworks@74366cf) OpenRewrite recipe best practices * Additional commits viewable in [compare view](openrewrite/rewrite-testing-frameworks@v3.11.0...v3.15.0) [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility\_score?dependency-name=org.openrewrite.recipe:rewrite-testing-frameworks&package-manager=maven&previous-version=3.11.0&new-version=3.15.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

steve-aom-elliott self-assigned this Jul 7, 2025

steve-aom-elliott added bug Something isn't working recipe Recipe request labels Jul 7, 2025

steve-aom-elliott added this to OpenRewrite Jul 7, 2025

steve-aom-elliott added the test provided label Jul 7, 2025

github-project-automation Bot moved this to In Progress in OpenRewrite Jul 7, 2025

steve-aom-elliott force-pushed the extend-testng-guard-for-junit4-to-junit5 branch from f4645f4 to e5ee3de Compare July 14, 2025 19:15

steve-aom-elliott moved this from In Progress to Ready to Review in OpenRewrite Jul 14, 2025

sambsnyd approved these changes Jul 14, 2025

View reviewed changes

Laurens-W reviewed Jul 15, 2025

View reviewed changes

steve-aom-elliott force-pushed the extend-testng-guard-for-junit4-to-junit5 branch from e5ee3de to c869d70 Compare July 16, 2025 15:30

steve-aom-elliott force-pushed the extend-testng-guard-for-junit4-to-junit5 branch from c869d70 to 6db5765 Compare July 24, 2025 16:07

steve-aom-elliott marked this pull request as ready for review July 24, 2025 16:24

timtebeek self-requested a review July 25, 2025 13:31

timtebeek reviewed Jul 25, 2025

View reviewed changes

steve-aom-elliott force-pushed the extend-testng-guard-for-junit4-to-junit5 branch from d7d91b4 to 941d017 Compare July 28, 2025 21:46

github-actions Bot reviewed Jul 28, 2025

View reviewed changes

Comment thread src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java Outdated

Comment thread src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java Outdated

timtebeek reviewed Jul 28, 2025

View reviewed changes

Comment thread src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java Outdated

github-actions Bot reviewed Jul 29, 2025

View reviewed changes

Comment thread src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java Outdated

github-actions Bot reviewed Jul 29, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

github-actions Bot reviewed Jul 29, 2025

View reviewed changes

timtebeek reviewed Jul 29, 2025

View reviewed changes

timtebeek approved these changes Jul 29, 2025

View reviewed changes

sambsnyd reviewed Jul 30, 2025

View reviewed changes

steve-aom-elliott mentioned this pull request Jul 31, 2025

Adding invertMarking flag to ModuleHasDependency for flipping what gets marked openrewrite/rewrite-java-dependencies#146

Merged

3 tasks

steve-aom-elliott and others added 15 commits July 31, 2025 17:17

Adding dependency guards to the Junit 4 -> 5 migration for when the P…

3c56cf7

…OM or build.gradle have a dependency on `org.testng:testng`. Relies on openrewrite/rewrite#5725

Updating DoesNotIncludeDependency recipe call to be the shared one …

f800288

…in `rewrite-java-dependencies`

Avoid tests passing for the wrong reason (class present)

a059fa8

Only pass testng dependency into the single test that needs it

c307680

Match any testng dependency

57ad02e

Adding TestNgGuard scanning recipe. Needs refinement, but this is a f…

199cc3e

…irst pass

Apply suggestions from code review

172b6f0

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Making separate TestNgGuard tests and cleaning up other code

6d580da

Cleanup of import

cca0096

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Do not use getters internally

c69b79d

Adopt ModuleHasDependency and drop odd test: only look at dependencies

fd977e2

Assuming classes can not be present without those dependencies

Revert change to TestNgToAssertJTest

8bc8281

Drop tests that were showing improbable use of types

a0aa295

Switching to using the new invertMarking option on `ModuleHasDepend…

b504c95

…ency` for the TestNG guard for JUnit

steve-aom-elliott force-pushed the extend-testng-guard-for-junit4-to-junit5 branch from f8fbc37 to b504c95 Compare July 31, 2025 21:25

Merge branch 'main' into extend-testng-guard-for-junit4-to-junit5

be8db0e

timtebeek approved these changes Aug 5, 2025

View reviewed changes

github-actions Bot reviewed Aug 5, 2025

View reviewed changes

timtebeek merged commit 9a2082a into main Aug 5, 2025
2 checks passed

timtebeek deleted the extend-testng-guard-for-junit4-to-junit5 branch August 5, 2025 13:25

github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite Aug 5, 2025

Conversation

steve-aom-elliott commented Jul 7, 2025

What's changed?

Checklist

Uh oh!

steve-aom-elliott commented Jul 14, 2025

Uh oh!

sambsnyd left a comment

Choose a reason for hiding this comment

Uh oh!

steve-aom-elliott commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Laurens-W Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

steve-aom-elliott commented Jul 16, 2025

Uh oh!

timtebeek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

steve-aom-elliott commented Jul 29, 2025

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

timtebeek Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

steve-aom-elliott Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

timtebeek left a comment

Choose a reason for hiding this comment

Uh oh!

sambsnyd Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

steve-aom-elliott Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

steve-aom-elliott Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

steve-aom-elliott commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steve-aom-elliott commented Aug 1, 2025

Uh oh!

steve-aom-elliott commented Aug 1, 2025

Uh oh!

steve-aom-elliott commented Aug 1, 2025

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

steve-aom-elliott commented Jul 14, 2025 •

edited

Loading

steve-aom-elliott commented Jul 31, 2025 •

edited

Loading