Recipe quality triage: remaining fixes, wiring and polish (A1–O8)

Open
#1,211 2 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

@tashen247 is already working on this.

Since Oct 26, 2026.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
48/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
java, yaml

Research direction

Choose one item and check its status and prerequisites first; items A require a maintainer decision, and B1 is already closed. For a selected item, start with the named Java recipe under src/main/java/org/openrewrite/java/testing/, its YAML wiring under src/main/resources/META-INF/rewrite/, and the corresponding tests under src/test/java/org/openrewrite/java/testing/. Write or run a regression test using the stated probe where provided, then verify the behavior and expected output match the item's acceptance criteria.

Written by the indexing model from the issue text.

Description

good first issue

We triaged all recipes in this repo against our recipe review checklist: 218 Java recipe classes and 23 YAML files. This issue tracks what we found. We reproduced 27 of the behavioural bugs with a throwaway RewriteTest, and marked those PROBED.

Each item is self-contained, so you can pick one up independently or point an agent at it ("work on item D3 of this issue"). End your PR description with Fixes item **D3** of #1211. As soon as the PR is open, tick the item's box, strike it through and link the PR, for example - [x] ~~**D3** <title>~~ (#1234), so nobody else picks it up.

How to pick up an item
  • Line numbers are against main at 6c313e2e and may have drifted, so search by symbol if needed.
  • Paths are abbreviated:
    • M/ = src/main/java/org/openrewrite/java/testing/
    • T/ = src/test/java/org/openrewrite/java/testing/
    • Y/ = src/main/resources/META-INF/rewrite/
  • PROBED means a throwaway test reproduced the behaviour; the probe input is given and can become the regression test. Confirmed means someone traced the code path, or checked the dependency's bytecode, and the behaviour is certain. Suspected means it was inferred from reading. Write the probe test first; if it does not reproduce, close the item with a note.
  • Prefer bailing out (making no change) over guessing when a recipe can't preserve semantics.
  • Every fix comes with a test. Tests use realistic classpaths and avoid TypeValidation.none(). Expected output must be correct, not just a record of today's behaviour.
  • Use one concern per commit, with an imperative headline that names APIs in backticks.
  • Items in A need a maintainer decision first. Comment on the issue before starting.
  • Already tracked elsewhere, so left out:
    • Parameterized setup: #1125, #1126, #1139, #1169, #1190.
    • TestCase(String) constructors: #1107, #1108, #1119, #1144, #1145.
    • TestSetup: #1110, #1146.
    • ExpectedException matcher type: #1124, #1129.
    • MockitoWhenOnStaticToMockStatic: #1123, #1128.
    • MockUtilsToStatic: #1084, #1086.
    • Half-migrated classes in general: #1167.
    • Kotlin across the JUnit 4 migration: #1189.

A. Wiring & placement (decide first)

  • A1 Drop Mockito1to4Migration from Mockito4to5Only

    Details
    • Where: Y/mockito.yml:50-59.
    • Problem: ef62c319 created Mockito4to5Only so the Spring Boot 3.1 upgrade could include Mockito 5 without re-running the 1→4 migration. The commit moved the old Mockito1to5Migration recipeList across wholesale, including its first entry Mockito1to4Migration, so the split never took effect. The description ("Does not include 1.x to 4.x migration") has been wrong ever since, and recipes.csv counts 101 sub-recipes against 102 for the full migration.
    • Impact: rewrite-spring's Boot 3.x upgrades reach ReplacePowerMockito this way. That comments out test bodies and removes PowerMock (see #1127), which is probably not what a Boot upgrade should do.
    • Fix: remove the entry; Mockito1to5Migration already chains both halves. Then decide whether Boot upgrades should run the PowerMock replacement at all.
  • A2 Stop adding ExternalResourceSupport without its dependency, then deleting the dependency (#1239)

    Details
    • Where:
      • The usage is added by M/junit5/HandleExternalResourceRules.java:74-83, wired at Y/junit5.yml:163.
      • The dependency is removed by RemoveJupiterMigrationSupport in Y/junit-jupiter.yml:50,53-68 (inside JupiterBestPractices), and by a copy in Y/junit6.yml:62-64.
    • Problem:
      • The JUnit 4→5 migration emits @ExtendWith(ExternalResourceSupport.class), but nothing adds junit-jupiter-migrationsupport.
      • JUnit5BestPractices, JUnit6BestPractices and JUnit5to6Migration then remove that artifact with no usage guard. migrationsupport 6.1.3 still ships ExternalResourceSupport and @EnableRuleMigrationSupport, so any project that uses them breaks.
    • Fix:
      • Add the dependency with onlyIfUsing: org.junit.jupiter.migrationsupport..*.
      • Guard both removals with unlessUsing on the same pattern.
      • Chain RemoveJupiterMigrationSupport in junit6.yml instead of copying it.
    • Also (Suspected): migrationsupport depends on junit:junit, which ExcludeJUnit4UnlessUsingTestcontainers (Y/junit5.yml:166-181) would exclude.
  • A3 Wire, delete or mark opt-in the unreachable recipes

    Details
    • RemoveJreOther (M/junit6/RemoveJreOther.java) was added in #1002 next to the other 6.1 steps but never wired. It also has bugs; see C13. Fix those before wiring it into JUnit5to6Migration.
    • RemoveEmptyTests (M/cleanup/RemoveEmptyTests.java) was never wired. It deletes tests, including an empty @Override that deliberately disables an inherited test (:75-78, Suspected). Decide whether it belongs in cleanup.BestPractices.
    • MockitoJUnitRunnerSilentToExtension is @Deprecated and was superseded in #711. Delete it.
    • AddMissingNested was unwired in #1160 and is now only used through EnclosedToNested. Add it to JupiterBestPractices (RSPEC-S5790), or document that it is standalone.
    • AddMockitoJupiterDependency only appears as a comment (Y/mockito.yml:195-201, #875).
    • AnyStringToNullable is reachable only through doAfterVisit (M/mockito/AnyToNullable.java:65); see F11.
  • A4 Decide how much the JUnit 4→5 migration and Assertj should pull in

    Details
    • What JUnit4to5Migration pulls in:
      • Mockito 4 and PowerMock replacement (via UseMockitoExtension, then Mockito1to4Migration, then ReplacePowerMockito). The PowerMock replacement disables tests.
      • OkHttp mockwebserver3 5.x (Y/junit5.yml:118).
      • MigrateHamcrestToJUnit5 through JupiterBestPractices (Y/junit-jupiter.yml:29). That recipe has the worst bugs in this issue (E1, E2).
    • What Assertj pulls in: about 1048 recipes, including all of JUnit5BestPractices through JUnitToAssertj.
    • Decision: keep these as they are, or make the cross-library steps opt-in. At the very least, fix E1 before more users reach it.
  • A5 Testcontainers 2: decide the Kafka target

    Details
    • Where: Y/testcontainers.yml:653-655 (Testcontainers2ContainerClasses), :52-54 (MigrateToKafkaNative), :287-290 (ExplicitContainerImages).
    • Problem:
      • Every org.testcontainers.containers.KafkaContainer becomes org.testcontainers.kafka.KafkaContainer. Bytecode shows the new class asserts apache/kafka images and has no no-arg constructor, withEmbeddedZookeeper or withKraft. Confluent users fail at runtime or at compile time.
      • ExplicitContainerImages injects confluentinc/cp-kafka:5.4.3, so Testcontainers2Migration run on its own guarantees that failure.
      • The old class still exists in Testcontainers 2.0.5, so the ChangeType isn't needed to compile.
    • Options: map Confluent usage to ConfluentKafkaContainer, or retarget only after the image has actually been switched (inside MigrateToKafkaNative).
    • Also (Suspected): in TestContainersBestPractices (:63-65), the ChangeTypes in MigrateToKafkaNative and MigrateToOracleFree run before ExplicitContainerImages. That recipe's <constructor>() matcher targets the old FQN, so new KafkaContainer() ends up as a constructor that doesn't exist. Probe: new KafkaContainer() under TestContainersBestPractices.
  • A6 AddMockitoExtensionIfAnnotationsUsed: disabled in mockito.yml, still active in jmockit.yml

    Details
    • Where: Y/mockito.yml:195-201 (disabled with the #875 rationale) versus Y/jmockit.yml:50.
    • Also: M/mockito/AddMockitoExtensionIfAnnotationsUsed.java:59-61,73-74 adds @ExtendWith(MockitoExtension::class) to Kotlin JUnit 4 tests. This is Confirmed.
  • A7 Remove duplicate and copied entries

    Details
    • RemoveDoNothingForDefaultMocks is at Y/mockito.yml:32 and is reached again via :206.
    • The junit-jupiter-params scanners in Y/junit5.yml:139-145 duplicate M/junit5/AddJupiterDependencies.java:58-62.
    • Y/junit6.yml:62-64 copies RemoveJupiterMigrationSupport (see A2).
    • M/mockito/PowerMockitoMockStaticToMockito.java:92-93 queues the same ChangeMethodTargetToStatic that runs next in Y/powermockito.yml:188-191.
  • A8 Decide on AssertJ's actual() floor (3.27)

    Details
    • Problem: AbstractAssert#actual() is @since 3.27.0. ReturnActual and the value path of JUnitAssertThrowsToAssertExceptionType (:106-108,117-119) emit it.
    • The AddDependency 3.x with acceptTransitive: true in JUnitToAssertj (Y/assertj.yml:549-554) doesn't raise an existing older AssertJ, such as Spring Boot ≤3.4's 3.26.x.
    • Options: bump AssertJ to 3.27.x in Assertj, or bail out. See D3 for the separate compile error on collections.

B. JUnit 4 → 5 migration bugs

  • B1 UseWiremockExtension turns every @Rule in the file into @RegisterExtension (PROBED) (#1218)

    Details
    • Where: M/junit5/UseWiremockExtension.java:44-49,68-69.
    • Problem: a file-wide ChangeType(org.junit.Rule → RegisterExtension) is queued whenever a new WireMockRule(..) exists. In JUnit4to5Migration it runs at Y/junit5.yml:61, before TemporaryFolderToTempDir (:105) and TestRuleToTestInfo (:107). Those no longer recognise their fields.
    • Probe:
      @Rule public WireMockRule wm = new WireMockRule();
      @Rule public TemporaryFolder tmp = new TemporaryFolder();
      
      This gives @RegisterExtension public TemporaryFolder tmp, which Jupiter rejects at runtime.
    • Fix: change only the annotation on the WireMockRule declaration. Coordinate with #1191, which adds the same file-wide pattern for @ClassRule.
  • B2 MigrateJUnitTestCase: @Test on any method named test* (related: #1107, #1108)

    Details
    • Where: M/junit5/MigrateJUnitTestCase.java:184-185.
    • Problem (PROBED): private void testSum(int a, int b, int expected) gets @Test, and Jupiter then fails with ParameterResolutionException.
    • Fix: apply JUnit 3's own discovery rule: public, non-static, void, no parameters.
    • Also (Suspected, :84): a file-wide ChangeType(TestCase → org.junit.Assert) retypes non-assertion uses. Probe: static void run(junit.framework.TestCase t) { t.run(); }.
  • B3 TemporaryFolderToTempDir: two harmful mappings (#1237)

    Details
    • newFolder() with no arguments (PROBED, :279-284 with :338-345): it becomes newFolder(root, "junit"), so a second call in the same test fails mkdirs() with IOException. JUnit 4 created a unique folder on each call. Fix: Files.createTempDirectory(root.toPath(), "junit").toFile().
    • Builder initializers (Confirmed, :104-135): TemporaryFolder.builder().assureDeletion().build() is kept as the initializer of an @TempDir File. Fix: drop the initializer.
    • Types (:271): the class type is put on a method-name identifier.
  • B4 TimeoutRuleToClassAnnotation removes Timeout fields that are still referenced (PROBED)

    Details
    • Where: M/junit5/TimeoutRuleToClassAnnotation.java:58-75.
    • Problem: any Timeout field is removed, with no @Rule check and no reference check.
    • Probe: private final Timeout t = Timeout.seconds(5); @Rule public RuleChain c = RuleChain.outerRule(t); leaves a dangling t. T/junit5/TimeoutRuleToClassAnnotationTest.java:52 even uses a field without @Rule as its input.
  • B5 TestRuleToTestInfo: file-wide rename, lost parameters, forced spacing

    Details
    • Confirmed (:71-72): an unconditional file-wide ChangeType(org.junit.Before → BeforeEach). Run on its own, it leaves a JUnit 4 class with a @BeforeEach that never runs.
    • Suspected (:174-182): replaceParameters() drops parameters that an existing @BeforeEach already has.
    • Confirmed (:66): the prefix is forced to " ", so log(name.getMethodName()) becomes log( name). Use mi.getPrefix().
  • B6 System Rules recipes don't see subclasses in other files

    Details
    • Where: M/junit5/SystemRules.java:46-58 (mayBeUsedElsewhere), M/junit5/SystemRulesToSystemStubs.java:141-147, M/junit5/SystemPropertyRulesToPioneer.java:83-84,133-136.
    • Problem (Confirmed): only abstract owners or main sources count as "may be used elsewhere". A concrete BaseTest whose out rule is read by SubTest in another file is retyped (System Stubs) or removed (Pioneer), and SubTest stops compiling.
    • Decision: scan across files for owner#name, or add the existing TODO comment.
  • B7 AssumeNotNullToAssumptions: the varargs form produces a method that doesn't exist (#1233)

    Details
    • Where: M/junit5/AssumeNotNullToAssumptions.java:52-63, then the ChangeType Assume → Assumptions at Y/junit5.yml:223-226.
    • Problem (Confirmed): Assume.assumeNotNull(a, b) doesn't match, so it ends up as Assumptions.assumeNotNull(a, b).
    • Test: T/junit5/AssumeNotNullToAssumptionsTest.java:65 is @Disabled against #886, which is closed. Its expected output is also wrong: the input passes new Object(), new Object(), but the expected output uses object1, object2.
    • Fix: a Java visitor for the varargs form, or a TODO plus bail-out. Fix and enable the test.
  • B8 ExpectedExceptionToAssertThrows: name clashes and lost throws (related: #1124)

    Details
    • Suspected (:203,229): the variable name exception is hard-coded. Probe: RuntimeException exception = …; thrown.expectMessage("boom"); throw exception;.
    • Suspected (:133-179): the checked-exception scan ignores the implicit close() of try-with-resources and calls with a null method type, so throws IOException is dropped wrongly.
    • Suspected: expect* inside @Before gives assertThrows(X.class, () -> {}) in setUp.
    • This is separate from #1124 and #1129.
  • B9 JUnitParamsRunnerToParameterized: static init methods that read instance state

    Details
    • Confirmed (:342-386): MakeMethodStatic only follows method invocations, so an init method that reads an instance field no longer compiles. Probe: private final Foo foo = new Foo(); private Object[] parametersForTest() { return new Object[]{ foo.a() }; }.
    • Suspected (:114,328-333): method = "first, second" is split without trimming.
    • Suspected (:118): the | column separator is passed verbatim to @CsvSource.
  • B10 UpdateMockWebServer* recipes: non-compiling output and wrong mappings

    Details
    • UpdateMockWebServerMockResponse (Confirmed):
      • :45-51 appends .build() to every MockResponse argument in any call, so helper parameters retyped to Builder break. Fix: only wrap arguments of MockWebServer#enqueue and QueueDispatcher#enqueueResponse.
      • :88 maps setHttp2ErrorCode(int) to code(int), which sets the HTTP status. The test at T/junit5/UpdateMockWebServerMockResponseTest.java:244,270 locks this in.
      • :85,91: setErrorCode and setThrottleBody don't exist in 3.14.9 or 4.10.0, so these entries are dead.
      • SocketPolicy and setSocketPolicy are unmapped, and ChangePackage sends them to a class that doesn't exist.
    • UpdateMockWebServer (Confirmed, :73,157): with two @Rule MockWebServer fields, only the last gets close(). :178 uses 4.X (uppercase).
    • UpdateMockWebServerDispatcher (Suspected, :91-107): return statements inside lambdas in dispatch() get wrapped.
  • B11 AddHamcrestJUnitDependency: repo-wide flag and over-broad matcher

    Details
    • Where: M/junit5/AddHamcrestJUnitDependency.java:39-85.
    • Problem (Confirmed):
      • One assertThat anywhere adds hamcrest-junit to every module.
      • Assert.assertThat moves to MatcherAssert, which is in plain Hamcrest; only Assume.assumeThat needs hamcrest-junit.
    • Fix: onlyIfUsing: org.hamcrest.junit..* after the migration, and drop the scanner.
  • B12 Smaller JUnit 4 migration bugs

    Details
    • ParameterizedRunnerToParameterized:118-124: ClassCastException on @Parameter(INDEX) with a constant. Confirmed. Related: #91, #1125, #1126, #743.
    • CategoryToTag:93-94: @Category({}) throws a ClassCastException on J.Empty. Suspected.
    • AddMissingTestBeforeAfterAnnotations:76-84: an override inherits the super's @Test(expected/timeout) in JUnit 4, but gets a bare @Test. Suspected.
    • UpdateTestAnnotation:162: emits threadMode, which needs Jupiter 5.9, while a Spring Boot 2.5 parent manages 5.7. Suspected.
    • MockitoJUnitToMockitoExtension:82: checks org.mockito.runners.MockitoJUnitRunner, which Y/mockito.yml:187-189 has already renamed, so the guard never fires. Confirmed.
    • EnvironmentVariables:118-128,150-158: env.clear(names) becomes env.remove(names), which doesn't compile. Suspected.
    • JUnitSoftAssertionsToSoftAssertionsExtension:103-106: a file-wide ChangeType breaks helpers in other files that take JUnitSoftAssertions. Suspected.
    • RunnerToExtension:101-120: handles @RunWith on methods, which can't happen. Dead code.

C. JUnit 5/6 best practices and cleanup bugs

  • C1 AssertEqualsIntegralDeltaToAssertEquals drops non-zero deltas (PROBED) (#1212)

    Details
    • Where: M/cleanup/AssertEqualsIntegralDeltaToAssertEquals.java:59,83, reached from JupiterBestPractices (Y/junit-jupiter.yml:34).
    • Problem: assertEquals(100, elapsed, 50) becomes assertEquals(100, elapsed), so a tolerance test starts failing. The description (:43-45) wrongly says the delta is meaningless, and every test uses 0.
    • Fix: only act when the delta is a literal zero.
  • C2 CleanupKotlinJUnit5AssertionImports removes Assertions.* (PROBED)

    Details
    • Where: M/junit5/CleanupKotlinJUnit5AssertionImports.java:53,67-90, reached from JupiterBestPractices (Y/junit-jupiter.yml:31).
    • Problem: with import org.junit.jupiter.api.*, every Assertions import is dropped. The Kotlin AssertionsKt functions only cover assertAll, assertDoesNotThrow, assertInstanceOf, assertNotNull, assertNull, assertThrows, assertTimeout* and fail, so assertEquals(1, 1) no longer resolves. The tests only use assertNull and assertNotNull.
    • Fix: only drop explicit imports of names that have a Kotlin counterpart. Leave Assertions.* alone, and add a precondition.
  • C3 TestsShouldIncludeAssertions turns skipped tests into failures (PROBED) (#1213)

    Details
    • Problem 1 (M/cleanup/TestsShouldIncludeAssertions.java:44-61,126-131): assumptions don't count as assertions. assumeTrue(isLinux()); service.start(); is wrapped in assertDoesNotThrow, which reports the TestAbortedException as a failure.
    • Problem 2 (:172-198, Confirmed): the check reduces over the whole class, so one asserting helper called anywhere exempts every test.
    • Also: validate() (:77-88) can never fail.
  • C4 TestMethodsShouldBeVoid and KotlinTestMethodsShouldReturnUnit delete early returns (PROBED) (#1217)

    Details
    • Early returns: RemoveDirectReturns (M/cleanup/TestMethodsShouldBeVoid.java:93-98, M/cleanup/KotlinTestMethodsShouldReturnUnit.java:120-128) deletes every return. if (skip) { return 0; } fail(); then always fails.
      • Fix: turn return expr; into expr; followed by return;.
      • Fix: bail out when the method is invoked anywhere, or is an override.
    • Kotlin expression bodies (Confirmed): KotlinTestMethodsShouldReturnUnit:82-91 adds : Unit to expression bodies of any type, so fun t() = assertThat(x).isTrue() no longer compiles. The test inputs are not valid Kotlin.
    • Suspected: TestMethodsShouldBeVoid has no Kotlin guard and installs a J.Primitive void, which the Kotlin printer may print as void.
    • Suspected: :76's requireNonNull(m.getBody()) NPEs on abstract test methods.
  • C5 RemoveTestPrefix breaks overrides and implied @MethodSource (PROBED)

    Details
    • Overrides: ChangeMethodName(.., matchOverrides=false) (M/cleanup/RemoveTestPrefix.java:180) renames BTest.testFoo but not the @Override in a subclass, so the override no longer compiles.
    • Confirmed (:150 vs :160,175): the source-method rename is scheduled before the bail-outs. After a bail-out, the factory is called foo() while the test is still called testFoo.
  • C6 TestsShouldNotBePublic narrows methods of extended classes (related: #458) (#1247)

    Details
    • Where: M/cleanup/TestsShouldNotBePublic.java:102 vs :150-163.
    • Problem (Confirmed): the extendedClasses guard only protects the class modifier. A public base's @BeforeEach public void setUp() becomes package-private, which breaks subclasses in other packages.
    • This is different from #458.
  • C7 Cleanup assertion rewrites without type checks (probed part: #1220)

    Details
    • PROBED: AssertNotEqualsBooleanToAssertBoolean:47 doesn't check that the second argument is boolean (its sibling does, AssertEqualsBooleanToAssertBoolean:48). assertNotEquals(false, map.get("k")) becomes assertTrue(Object). (#1220)
    • Confirmed: AssertionsArgumentOrder:53-54,128-130 matches JUnit 4 (String, Object, Object) overloads by index, so it swaps the message. assertEquals(..) is listed twice (:50-51), while JUnit 4 assertNotEquals is missing.
    • Confirmed (rare): SimplifyTestThrows:95-97 turns throws IOException, Throwable into throws Exception.
    • Suspected: AssertTrueComparisonToAssertEquals:112-113 turns Integer == long into an ambiguous assertEquals.
    • Suspected: AssertTrueEquals:93 and AssertFalseEquals:90 match equals by name, and pass a null select for an implicit receiver.
    • Suspected: seven templates hard-code Assertions. for any non-null select: AssertEqualsBoolean:57, AssertNotEqualsBoolean:56, AssertFalseEquals:54, AssertFalseNegation:52, AssertFalseNull:56, AssertTrueComparison:55, AssertEqualsIntegralDelta:63,87. Renaming in place, as AssertTrueNegationToAssertFalse does, fixes this.
  • C8 CsvSourceToValueSource changes values on the non-text-block path (PROBED) (#1244)

    Details
    • Where: M/junit5/CsvSourceToValueSource.java:111-135. It skips the checks the text-block path makes at :100-110.
    • Probe: @CsvSource({"x, y", "z, w"}) void t(String s) becomes @ValueSource(strings = {"x, y", "z, w"}), so s is "x, y" instead of "x".
    • Other values that change: quotes ("'a'"), surrounding whitespace, and "" (which CSV reads as null).
    • Suspected: Kotlin @ValueSource(strings = "apple") needs [...].
  • C9 ImplausibleTimeoutToMinutes drops threadMode (PROBED) and rounds

    Details
    • Where: M/junit5/ImplausibleTimeoutToMinutes.java:71-96.
    • Problem: @Timeout(value = 3600, threadMode = SEPARATE_THREAD) becomes @Timeout(value = 60, unit = MINUTES), losing threadMode.
    • Also: :89 rounds (1000 s becomes 17 min), although the description says "equivalent".
  • C10 MinimumJreConditions rewrites every JRE range in the file (PROBED) (#1242)

    Details
    • Where: M/junit6/MinimumJreConditions.java:180-181,217-231.
    • Problem: collapsing one method's range queues a file-wide ChangeType and attribute edits. With javaVersion 17, @EnabledForJreRange(min = JAVA_21) on another method becomes @EnabledOnJre(JAVA_17).
    • Suspected (:195-200): JRE-constant arrays write the int versions attribute.
    • Suspected (:321-325): Integer.parseInt on a constant reference throws.
    • No precondition (:72-73).
  • C11 RemoveInterceptDynamicTest deletes an overload that JUnit 6 still has (PROBED) (#1221)

    Details
    • Where: M/junit6/RemoveInterceptDynamicTest.java:31-32,47-48.
    • Problem: interceptDynamicTest(..) also matches the 3-argument (Invocation, DynamicTestInvocationContext, ExtensionContext), which is still in 6.1.3, so user interceptors are deleted.
    • Also: deleting the 2-argument override discards its logic.
    • Fix: match the 2-argument signature exactly, and migrate it to the 3-argument form by adding the parameter.
  • C12 UseAssertSame on Kotlin turns equality into identity (PROBED) (#1223)

    Details
    • Where: M/junit5/UseAssertSame.java:59-76.
    • Problem: Kotlin assertTrue(listOf(1) == listOf(1)) becomes assertSame(listOf(1), listOf(1)), because == calls equals in Kotlin.
    • Fix: skip K.CompilationUnit.
    • Types (:94-106): the fallback "stubbed" method type is the normal path.
  • C13 RemoveJreOther empties lone braced arrays and changes behaviour

    Details
    • Where: M/junit6/RemoveJreOther.java:78-84.
    • Problem (Confirmed):
      • @EnabledOnJre({JRE.OTHER}) becomes @EnabledOnJre({}), which JUnit rejects. That contradicts the recipe's own description (:50-53).
      • Dropping OTHER from {JAVA_21, OTHER} disables the test on future JREs.
    • The recipe is currently unwired (A3).
  • C14 AssertThrowsOnLastStatement: side effects outside the target

    Details
    • Confirmed (:72): doAfterVisit(new LambdaBlockToExpression()) is scheduled for every method body, so it rewrites unrelated lambdas.
    • Confirmed (:123): an empty block lambda () -> {} deletes the whole assertThrows statement.
    • Suspected (:123-125,158,290): hoisted locals from sibling assertThrows calls can get duplicate names.
    • Not reproduced: hoisting a throwing argument out of a single-statement lambda. A System.out.println(Integer.parseInt("x")) probe stayed inside the lambda.
    • Test: a bogus @Issue(".../issues/2861") at T/junit5/AssertThrowsOnLastStatementTest.java:1287.
  • C15 Smaller JUnit 5 best-practice items

    Details
    • AddMissingNested:114 mutates the modifier list in place with removeIf. Use ListUtils.filter.
    • AddParameterizedTestAnnotation:36-46 misses @FieldSource, @ArgumentsSources and custom sources. Use AnnotationMatcher("@…ArgumentsSource", true). Coordinate with #1093.
    • AssertTrueInstanceofToAssertInstanceOf:77-96: its JUnit 4 branch emits Jupiter's assertInstanceOf into JUnit 4 modules when JupiterBestPractices runs on its own. Needs a decision; coordinate with #1083 and #1088.

D. AssertJ bugs

  • D1 AdoptAssertJDurationAssertions: comparisons become equality, and units are lossy (PROBED) (#1215)

    Details
    • Comparisons (PROBED, M/assertj/AdoptAssertJDurationAssertions.java:105-106,142-150): assertThat(d.getSeconds()).isGreaterThan(5) becomes assertThat(d).hasSeconds(5).
    • getNano (Confirmed by running it, :67-68): getNano() is the nanosecond-of-second part, but hasNanos checks the whole duration.
    • Units (Confirmed by running it, :163-200):
      • hasMillis(5000) becomes hasSeconds(5), which also passes for 5999 ms.
      • A zero argument recurses all the way to hasDays(0).
      • Math.toIntExact throws for large values.
    • Tests lock in the bugs: getNanoToHasNanos, isEqualToVariable and the simplifyDurationAssertions CSV.
    • Fix: only map from isEqualTo, drop the getNano mapping, and remove the unit up-conversion.
  • D2 JUnit-to-AssertJ recipes mishandle deltas (PROBED) (#1216)

    Details
    • PROBED (JUnitAssertEqualsToAssertThat:88-94,105-112): assertEquals(100, elapsed, 10) becomes assertThat(elapsed).isEqualTo(100).
    • Confirmed with javac (JUnitAssertNotEqualsToAssertThat:75-90,106-115, JUnitAssertArrayEqualsToAssertThat:70-86,105-114): these decide "delta or message" from the argument's own type. The #911 fix was only applied to assertEquals. assertNotEquals(0.0, d, 0) becomes .as(0), and assertNotEquals(5, count, 1.0) calls a method that doesn't exist.
    • Fix: share floatingPointDeltaType/isIntegralType. Keep the delta via isCloseTo(.., within(..)), or bail out unless it is a literal 0.
  • D3 ReturnActual emits code that doesn't compile for collections

    Details
    • Where: M/assertj/ReturnActual.java:81.
    • Problem (Confirmed with javac): for List, Iterable and Collection, actual() returns List<? extends E>, so the @DocumentExample output (return assertThat(list).isNotNull().actual(); in a method returning List<String>) doesn't compile.
    • Fix: bail out when the assertThat overload's ACTUAL is a wildcard. Version floor: A8.
  • D4 SimplifyHasSizeAssertion produces a hasSameSizeAs that fails at runtime (PROBED) (#1243)

    Details
    • Where: M/assertj/SimplifyHasSizeAssertion.java:59-70.
    • Problem: assertThat(list).hasSize(str.length()) becomes hasSameSizeAs(str), which binds to hasSameSizeAs(Object array) and fails. The same happens for a list against a map, and for an array against a string.
    • Fix: only rewrite when the asserted type and the size source are compatible.
  • D5 SimplifyRedundantAssertJChains removes guards that aren't redundant (PROBED) (#1241)

    Details
    • Where: M/assertj/SimplifyRedundantAssertJChains.java:56,88,114-127.
    • isNotEmpty() before containsAll(..): assertThat(ids).isNotEmpty().containsAll(expected) loses the isNotEmpty(), although containsAll(emptyList) passes on an empty actual.
    • isNotNull() before matches(..): ..* matches(..) also matches AbstractAssert.matches(Predicate), which does not null-check.
    • Fix: type the matcher (AbstractCharSequenceAssert matches(..)), and limit the isNotEmpty removal.
  • D6 JUnitAssertInstanceOfToAssertThat breaks code that uses the return value (PROBED) (#1222)

    Details
    • Where: M/assertj/JUnitAssertInstanceOfToAssertThat.java:54-69.
    • Problem: Integer i = assertInstanceOf(Integer.class, o); becomes Integer i = assertThat(o).isInstanceOf(Integer.class);.
    • Fix: bail out unless the call is a statement, as JUnitAssertThrows#hasReturnType does.
  • D7 JUnitAssertThrowsToAssertExceptionType retypes signatures

    Details
    • Where: M/assertj/JUnitAssertThrowsToAssertExceptionType.java:82-92,164-192,209-224.
    • Confirmed: RetypeExecutableVariable retypes method parameters and non-private fields, and only checks the current file. A helper assertFails(Executable e) therefore becomes ThrowingCallable, and its callers in other files break.
    • Confirmed: assertThrows(X.class, this.executable) or executable() emits isThrownBy(<Executable>), which doesn't compile.
    • Fix: only retype locals, and bail out on any other Executable expression.
  • D8 Other AssertJ semantics changes

    Details
    • SimplifyAssertJInstanceOfAssertion:93 (Confirmed by running it): assertThat(x instanceof Foo).isFalse() passes for null, but isNotInstanceOf fails on null.
    • JUnitFailToAssertJFail (Confirmed):
      • :63-74: fail(Supplier) becomes fail("", supplier), so the message is silently "".
      • :84-85: a whole-file RemoveUnusedImports.
      • :89-110: unqualifies every Assertions.fail in the file.
    • JUnitTryFailToAssertThatThrownBy:
      • Confirmed (:145,152): messages are inserted raw, so a quote or backslash breaks the template. Use getValueSource().
      • Confirmed (:141,148): getMessage() is matched on any receiver.
      • Suspected (:167-169): a single-statement body that is a declaration or an outer assignment.
      • No precondition (M1).
    • MigrateAssertionsForClassTypes:
      • Confirmed (:47-63,92-95): run on its own, it emits AssertionsForClassTypes.assertThatObject, which doesn't exist.
      • Suspected: COLLISION_TYPES misses Atomic*, LongAdder, BigInteger, CharSequence, CompletionStage, Future and Spliterator.
    • SimplifyChainedAssertJAssertion:222-227 (Confirmed): generates Path.of(..), which is Java 11+. Use Paths.get.
    • IsEqualToIgnoringMillisToIsCloseTo (Confirmed by running it): weaker semantics than the original, not mentioned in the description. Its display name says "by by isCloseTo".
    • Refaster (AssertJDoubleRules:114-116,137-139,160-162, AssertJFloatRules:96-97,117-118,138-139): isSameAs(0) can never pass, so rewriting it to isZero() changes behaviour. Low impact.
    • Suspected: an unqualified size(), stream() or getFirst() inside a collection subclass passes a null select (SimplifyHasSizeAssertion:64, SimplifyStreamMapToExtracting:79, SimplifySequencedCollectionAssertions:151).
    • Suspected: Kotlin .as(..) from the JUnit*ToAssertThat templates; as is a Kotlin keyword. There are no Kotlin tests in T/assertj/.
  • D9 Dead YAML entries and null-able options in Simplify*AssertJAssertion

    Details
    • Entries that can never match (Confirmed): Y/assertj.yml:164-168 (String.getString), :234-238 (assertThat(int).hasSize), :239-243 (File.getFile).
    • Options: SimplifyAssertJAssertion.assertToReplace (:39-44) and all four SimplifyChainedAssertJAssertion options (:47-73) are required = false, but they are spliced straight into matcher strings. Make them required.

E. Hamcrest & Truth bugs

  • E1 RemoveNotMatcherVisitor: a negation leaks into later assertions and flips them (PROBED) (#1214)

    Details
    • Where: M/hamcrest/RemoveNotMatcherVisitor.java:31-40,47-50,67,71-72, used by HamcrestMatcherToJUnit5.java:155-177 and HamcrestInstanceOfToJUnit5.java:73-82.
    • Problem: whether an assertion is negated is stored on the ExecutionContext, keyed by the printed matcher. The probe shows it flips assertions even within one file:
      assertThat(a, not(nullValue()));
      assertThat(b, nullValue());
      
      becomes assertNotNull(a); assertNotNull(b);.
    • Also: not(not(x)) NPEs (:47-48).
    • Reach: JupiterBestPractices → JUnit5BestPractices, JUnit6BestPractices, JUnitToAssertj.
    • Why the tests miss it: RewriteTest's immutable-context check catches this, but eight tests turn it off with immutableExecutionContext(false):
      • HamcrestMatcherToJUnit5Test:131,170,209,552,1066
      • HamcrestInstanceOfToJUnit5Test:38
      • RemoveNotMatcherTest:36
      • MigrateHamcrestToJUnitTest:42
    • Fix: count the not(..) layers locally, drop the context messages, and remove the relaxations.
  • E2 HamcrestMatcherToJUnit5 picks templates by simple name, ignoring types

    Details
    • Where: M/hamcrest/HamcrestMatcherToJUnit5.java:60,65,70-75,172 (Replacement.valueOf(getSimpleName().toUpperCase())).
    • Problem (Confirmed):
      • greaterThan on String, BigDecimal or LocalDate emits assertTrue("b" > "a").
      • closeTo(BigDecimal, ..) emits Math.abs(bd - bd).
      • closeTo uses < where Hamcrest uses <=.
      • equalTo on arrays becomes a reference comparison.
      • Arguments come out as (actual, expected).
    • Fix: one typed MethodMatcher per replacement, a numeric guard, <=, and assertArrayEquals for arrays.
  • E3 HamcrestMatcherToAssertJ/HamcrestNotMatcherToAssertJ pass Hamcrest matchers into AssertJ (PROBED) (#1236)

    Details
    • Where: M/hamcrest/HamcrestMatcherToAssertJ.java:74,93, M/hamcrest/HamcrestNotMatcherToAssertJ.java:71,94,127.
    • Problem: the *(org.hamcrest.Matcher) exclusion doesn't match the Matcher[] varargs overloads. contains(equalTo("a"), startsWith("b")) becomes containsExactly(equalTo("a"), startsWith("b")), and the same happens for containsInAnyOrder, hasItems, arrayContaining and hasEntry(Matcher, Matcher).
    • Test locks in non-compiling output: T/hamcrest/HamcrestMatcherToAssertJTest.java:386-428, hidden with methodInvocations(false).
    • Fix: bail out if any argument is (or contains) a Matcher.
    • Also: the 3-argument path of HamcrestNotMatcherToAssertJ (:143-147) leaves the not and matcher imports unused.
  • E4 Hamcrest 1.x is(Class) is migrated as equality — #1250

    Details
    • Where: M/hamcrest/HamcrestIsMatcherToAssertJ.java:42-43,61, M/hamcrest/HamcrestHasPropertyToAssertJ.java:45-46,103.
    • Problem (Confirmed with javap): in hamcrest-core 1.3, is(Class<T>) is the deprecated alias for instanceOf, so assertThat(s, is(String.class)) becomes isEqualTo(String.class).
    • Fix: exclude is(java.lang.Class) or map it to isInstanceOf. The test needs hamcrest-core-1.3 in classpath.tsv.gz.
  • E5 Hamcrest YAML mappings that change behaviour

    Details
    • equalToIgnoringWhiteSpace (Y/hamcrest.yml:230-232,329-331): it maps to isEqualToIgnoringWhitespace, which removes all whitespace in AssertJ 3.9+. The equivalent is isEqualToNormalizingWhitespace.
    • not(hasItems(a, b)) (:360-362): it becomes doesNotContain(a, b), which is stricter.
  • E6 HamcrestInstanceOfToJUnit5: crash and null handling

    • Opaque/non-invocation matcher crashes addressed in #1251. Direct-negative null and primitive/dynamic-Class handling remain open.
    Details
    • Crash (Confirmed): :72 casts the matcher to J.MethodInvocation, so Matcher<Object> m = instanceOf(String.class); assertThat(o, m); throws a ClassCastException.
    • Null (Confirmed): :84 turns not(instanceOf(X)) into X.class.isAssignableFrom(o.getClass()), which NPEs on null.
    • Suspected: the Java-only template runs inside Kotlin files from MigrateHamcrestToJUnit5 (Y/hamcrest.yml:64).
  • E7 Truth → AssertJ: find vs match, and asList

    Details
    • containsMatch (Confirmed): Truth's containsMatch is a find, but it maps to full-match assertions:
      • M/truth/TruthThrowableAssertions.java:83-95 uses hasMessageMatching.
      • Y/truth.yml:53-58 maps it to matches, and doesNotContainMatch to doesNotMatch.
      • The repo's own expectation at T/truth/TruthThrowableAssertionsTest.java:127-152 would fail at runtime.
      • Fix: use hasMessageFindingMatch, containsPattern and doesNotContainPattern.
    • asList (Confirmed, Y/truth.yml:128-130): ObjectArraySubject asList() becomes containsExactly(), which asserts that the array is empty.
    • Suspected: Y/truth.yml:224-226 removes Truth even when Expect or Correspondence remain.
    • Dead entries: :120-122 (AbstractMapAssert containsEntries) never matches. :199-204 targets Abstract*ArrayAssert, but AssertJ returns ListAssert.

F. Mockito & PowerMock bugs

  • F1 RemovePowerMockClassExtensions removes the user's own base class (PROBED) (#1224)

    Details
    • Where: M/mockito/RemovePowerMockClassExtensions.java:58-59.
    • Problem: isAssignableTo(PowerMockTestCase, extends) also strips class FooTest extends MyBaseTest, where MyBaseTest extends PowerMockTestCase. FooTest then loses its inherited helpers.
    • Fix: use TypeUtils.isOfClassType.
  • F2 DisableUnsupportedPowerMockTests leaves some classes on PowerMock, but the dependency is removed anyway

    Details
    • Where: M/mockito/DisableUnsupportedPowerMockTests.java:98-101,254-267.
    • Problem (Confirmed): when frameworkOf(cd) is null, the class is returned unchanged. That covers abstract base classes with an unsupported @Before, and TestNG classes that use a class-level @Test. RemovePowerMockDependencies (Y/powermockito.yml:45) still removes PowerMock, so one base class breaks the whole module.
    • Also: @ParameterizedTest, @RepeatedTest and @Theory aren't recognised as tests.
    • Also: FlagUnsupportedPowerMockUsage.java:47-49 still says such repositories "are left on PowerMock", which is no longer true.
  • F3 CloseUnclosedStaticMocks keeps a static mock active past an explicit close() (PROBED)

    Details
    • Where: M/mockito/CloseUnclosedStaticMocks.java:183-191,232-255,275-284.
    • Problem: m.close(); assertEquals(42, Num.getNumber()); is wrapped in try-with-resources and the close() is removed, so the assertion now runs against the mock.
    • Fix: skip declarations that are already closed, or end the try at the close.
  • F4 MockConstructionToTryWithResources: variable scope and early closes

    Details
    • Where: M/mockito/MockConstructionToTryWithResources.java:84-91,133-136,163-165.
    • Problem (Confirmed):
      • A local declared before close() and used after it moves into the try body and goes out of scope.
      • A mid-body close() is removed.
      • There is no precondition (UsesMethod<>(MOCK_CONSTRUCTION_MATCHER) fits).
      • The template uses an untyped try (#{any()}) with contextSensitive().
    • Probe: MockedConstruction<A> mc = mockConstruction(A.class); A a = new A(); mc.close(); a.method();.
  • F5 Whitebox → reflection recipes don't compile for generic, array and lambda shapes

    Details
    • Erased types (Confirmed): WhiteboxToReflectionVisitor.java:571-575,719-727 uses getClassName().
      • List<String> l = Whitebox.getInternalState(..) becomes List l = (List) ...
      • String[] becomes Object.
      • The same code path is used by the GetInternalState, InvokeMethod and InvokeConstructor recipes.
    • Lambda blocks (Confirmed, :362-387 vs :522-531): a statement-level call inside a lambda block emits getDeclaredField, which throws a checked exception inside a Consumer.
    • Fix: print the declared type expression or bail out, and apply the lambda guard to every path.
  • F6 MockitoWhenOnStaticToMockStatic: nested classes (related: #1123)

    Details
    • Where: M/mockito/MockitoWhenOnStaticToMockStatic.java:174,202,223,387-394.
    • Invalid name (Confirmed): variable names come from an unsanitised getClassName(), giving mockTest.Files1.
    • Wrong class (Confirmed): when(Outer.Inner.value()) mocks Outer.
    • Also: the mutable varCounter on the Recipe (:57) isn't thread-safe and depends on run order.
    • Fix: use getMethodType().getDeclaringType() and .replace(".", "_"). Coordinate with #1128, which touches these lines.
  • F7 RemoveInitMocksIfRunnersSpecified removes initialisation the extension doesn't replace (PROBED) (#1249)

    Details
    • Where: M/mockito/RemoveInitMocksIfRunnersSpecified.java:61-62,109-114,303-309.
    • Problem:
      • MockitoAnnotations.openMocks(this) in the constructor of a static, non-@Nested fixture class is removed, so its @Mock fields stay null.
      • openMocks(helper) is removed too.
    • Fix: require a this argument, and only treat @Nested inner classes as covered by the extension.
  • F8 RemoveDoNothingForDefaultMocks removes a doNothing() that overrides an earlier stub

    Details
    • Where: M/mockito/RemoveDoNothingForDefaultMocks.java:154-193.
    • Problem (Confirmed): a doThrow(..).when(foo).bar() in @Before, followed by doNothing().when(foo).bar() in a test, turns the test into one that throws.
    • Also: a HashSet<JavaType.Variable> (:63); see L1.
    • Also: a whole-file RemoveUnusedImports (:162).
  • F9 PowerMockitoDoStubbingToMockito: ambiguous private overloads

    Details
    • Where: M/mockito/PowerMockitoDoStubbingToMockito.java:85-87,125-136,154-155.
    • Problem (Confirmed): the private bail-out only fires when resolveTargetMethod finds a unique match. With two private helper(..) overloads, or missing types, it emits doNothing().when(s).helper("a") against a private method.
    • Also: the description (:47-49) over-claims, and it uses a hand-built TextComment (:88-92).
    • Test: turns off identifier validation in defaults (T/mockito/PowerMockitoDoStubbingToMockitoTest.java:40-42).
  • F10 Smaller Mockito items

    Details
    • ReplaceMockitoTestExecutionListener:205 (Confirmed): firstEnclosingOrThrow(J.CompilationUnit.class) throws on Kotlin. The wrapper recipes …ForJupiter, …ForJUnit4 and …ForTestNG are never loaded in a test.
    • ArgumentMatcherToLambda:180-184 (Confirmed): ArgumentMatcher<T> and <String[]> are cast to ArgumentMatcher<Object>.
    • PowerMockRunnerDelegateToRunWith:77-86 (Suspected): with a delegate, @Mock initialisation is dropped. Add a MockitoJUnit.rule().
    • PowerMockWhiteboxSetInternalStateToInjectMocks:89-120 (Suspected): @InjectMocks injects more than the one field the call set.
    • ThenThrowCheckedExceptionToRuntimeException:110-116 (Suspected, low): throws E counts as undeclared. A ShallowClass is used at :75-77.
    • MockitoBestPractices removes every @MockitoSettings(strictness = WARN), including the ones Mockito1to3Migration adds in the same run. Intended?
  • F11 Make AnyToNullable/AnyStringToNullable declarative (#1292)

    Details
    • Where: M/mockito/AnyToNullable.java:41-65, M/mockito/AnyStringToNullable.java:36,55.
    • Problem (Confirmed):
      • The matchers use org.mockito.Mockito, so calls through import static org.mockito.Matchers.* are missed. Y/mockito.yml:115-116 already uses org.mockito.* for this reason.
      • A global AtomicBoolean converts every module once any module uses mockito-all.
    • Fix: a YAML recipe with a ModuleHasDependency precondition and org.mockito.* patterns. This also replaces the doAfterVisit.
    • Related: VerifyZeroToNoMoreInteractions uses the same scanner pattern, a misnamed ASSERT_INSTANCE_OF_MATCHER (:33), and a garbled description.

G. JMockit & EasyMock bugs

  • G1 JMockitBlockRewriter silently drops expectations and verifications (related: #508)

    Details
    • Where: M/jmockit/JMockitBlockRewriter.java:121-147,233-236,303-306,499,502-531, M/jmockit/JMockitBlockToMockito.java:82-87.
    • Problem (Confirmed): the block is removed before rewriting, and a failure is never rolled back. Code is lost in four ways:
      • A result type the template can't handle (array, type variable) drops that stubbing.
      • rewriteFailed = instead of |= lets a later success erase an earlier failure.
      • Invocations whose select isn't an identifier (cascades, Outer.this.foo.x(), statics) are discarded.
      • @Capturing fields aren't recognised as mocks (SetupStatementsRewriter.java:195-201).
    • Probes:
      • new Expectations(){{ foo.names(); result = new String[]{"a"}; foo.count(); result = 1; }};
      • new Verifications(){{ foo.getBar().run(); times = 1; }};
    • Fix: rewrite into a copy, and keep the original block when any part fails.
  • G2 JMockit with* argument matchers are not migrated

    Details
    • Where: M/jmockit/ArgumentMatchersRewriter.java:110,118,140-144,270-279.
    • Problem (Confirmed): withNotNull(), withCapture(list), withInstanceOf(..) and withSameInstance(..) are emitted unchanged inside when(..) and verify(..), where they no longer resolve. When mixed with anyString they become eq(withNotNull()).
    • Also: matchers are checked by simple name, so a local called any counts as one.
    • Fix: bail out, or map them (notNull(), isA(), same(), …).
  • G3 JMockitMockUpToMockito: several shapes break or are silently lost

    Details
    • Where: M/jmockit/JMockitMockUpToMockito.java. All Confirmed unless marked.
    • JUnit 5 setup: a MockUp in @BeforeEach is closed when setup returns, because only org.junit.Before/After are matched (:482-494). The comment at :71 claims otherwise.
    • Unwanted tearDown: an empty @After tearDown() is added to any JUnit 4 class with an @Before (:89-91,140-145).
    • Lost code:
      • A MockUp is removed with nothing in its place when no @Mock method maps (:268-270,506-517).
      • MockUp fields and helper methods are inlined into the method body (:187-199,281).
    • Unreachable code: return null; is appended after a trailing throw (:370-378).
    • NPEs:
      • :335 on type-variable parameters. It is reached for any generic static method on the class.
      • :361 on array parameters.
    • Side effects:
      • A whole-file LambdaBlockToExpression (:305).
      • A forced import static org.mockito.Mockito.* even when nothing was migrated (:303).
      • A duplicate static import (:286).
  • G4 Smaller JMockit items

    Details
    • JMockitAnnotatedArgumentToMockito:82-95 (Confirmed): @Mocked List<String> l becomes mock(List<String>.class). Use the erasure.
    • SetupStatementsRewriter:59,74,153 (Confirmed code path): ClassCastException on fields or methods inside new Expectations(){..}, and on Outer.this.mock.
    • Tests: every JMockit test runs the whole JMockitToMockito composite (T/jmockit/JMockitTestUtils.java:39-46), on mockito-core 3.12, while the composite adds 5.x.
  • G5 EasyMockVerifyToMockitoVerify: stubs treated as expectations, bare verify left behind

    Details
    • Where: M/easymock/EasyMockVerifyToMockitoVerify.java:61,67-70,121,125-129.
    • Problem (Confirmed):
      • andStubReturn, andStubThrow and andStubAnswer become verify(dep).foo() checks that require exactly one call.
      • Some chains are never collected: .times(2), .anyTimes(), .once(), expectations set in @Before, and multi-mock verify(a, b). These leave verify(dep), which becomes a bare Mockito.verify(dep);. That throws UnfinishedVerificationException, or doesn't compile with two mocks.
      • The single-static import org.mockito.Mockito.verify shadows EasyMock.*. The test at T/easymock/EasyMockVerifyToMockitoVerifyTest.java:271-327 expects exactly this.
    • Decision: what should happen to a verify(dep) that can't be translated?
  • G6 EasyMock leftovers that don't compile (related: #738)

    Details
    • PROBED: RemoveExtendsEasyMockSupport (M/easymock/RemoveExtendsEasyMockSupport.java:45), run through EasyMockToMockito, leaves Runnable r = createMock(Runnable.class); unresolved. Inherited createMock, mock, niceMock, strictMock, resetAll and createMockBuilder have no mapping. isAssignableTo also strips a user base class that extends EasyMockSupport.
    • Confirmed, Y/easymock.yml:
      • :81-86: IMocksControl createMock(..) becomes Mockito.createMock(..), which doesn't exist, and the later rename can't match.
      • :93-95: mock(String, Class) becomes a Mockito.mock(String, Class) that doesn't exist.
      • capture(..) becomes argThat, although Capture isn't an ArgumentMatcher.
      • The RemoveDependency is unconditional.
    • None of these is tested.

H. TestNG bugs

  • H1 TestNgAssertThrowsToAssertThat garbles the message overloads (PROBED) (#1240)

    Details
    • Where: M/testng/TestNgAssertThrowsToAssertThat.java:87-95.
    • Problem: assertThrows("msg", ISE.class, () -> ..) becomes assertThatExceptionOfType("msg").isThrownBy(ISE.class), and the lambda is lost.
    • Fix: handle 3 arguments by mapping the message to .as(..), or bail out.
  • H2 TestNG deltas read as messages, dropped, or of the wrong type

    Details
    • Integer delta read as a message (Confirmed): TestNgAsserts.isFloatingPointType (M/testng/TestNgAsserts.java:31-40) inspects the argument, not the parameter, so assertEquals(d, 1.0, 0) becomes .as(0). The same pattern is in:
      • TestNgAssertEqualsToAssertThat:91,152
      • TestNgAssertNotEqualsToAssertThat:75
      • TestNgAssertionToAssertJ:131
      • TestNgSoftAssertToAssertJ:124
    • Integral delta dropped (Confirmed): TestNgAssertEqualsToAssertThat:163-169,180-186, TestNgAssertNotEqualsToAssertThat:83-88,99-104. This is the same class of bug as C1.
    • Float (Suspected): isCloseTo(.., within(0.001)) passes an Offset<Double> to a float assertion.
    • Fix: read getMethodType().getParameterTypes().get(2). There are no delta tests.
  • H3 TestNG SoftAssert/Assertion recipes skip nested calls and aren't self-contained

    Details
    • Where: M/testng/TestNgSoftAssertToAssertJ.java:51-54,77,83 and TestNgAssertionToAssertJ.java:54-57.
    • Problem (Confirmed): these return without calling super, so xs.forEach(x -> softly.assertTrue(x > 0)) is never migrated. The YAML ChangeType (Y/testng.yml:53-55) then retypes softly to SoftAssertions, which has no assertTrue.
    • Not self-contained: run on its own, the recipe emits .assertThat(..) on a TestNG SoftAssert.
    • Fix: move the ChangeType into getRecipeList().
    • Tests: there are no dedicated tests for TestNgAssertNotEqualsToAssertThat or TestNgSoftAssertToAssertJ, and every TestNG test uses an unversioned classpath("testng").

I. Testcontainers, WireMock, Arquillian & DBRider bugs

  • I1 ReplaceContainerImageName: invalid Vault image, and an over-broad prefix

    Details
    • Vault (Confirmed): M/testcontainers/ReplaceContainerImageName.java:88-89 appends value.substring(imagePrefix.length()), and the Vault entry's prefix (Y/testcontainers.yml:282-285) already contains the colon. So "vault:1.13.3" becomes "hashicorp/vault1.13.3". There is no Vault test.
    • Prefix (Confirmed, :69,73-86):
      • startsWith("confluentinc/cp-kafka") also matches cp-kafka-rest.
      • Any DockerImageName.parse in a file that constructs a KafkaContainer is rewritten, even when it feeds another container.
    • Fix: require a :, @ or end boundary after the prefix, put back the separator, and check the enclosing constructor.
    • Tests: they use dependsOn stubs even though the Testcontainers jars are on the classpath (also MigrateToKafkaNativeTest:34, MigrateToOracleFreeTest:35).
  • I2 AddTestcontainersAnnotations half-migrates JUnit 4 tests

    Details
    • Where: M/testcontainers/AddTestcontainersAnnotations.java:53-58,62.
    • Problem (Confirmed): @Testcontainers is added next to org.junit.Test, and JUnit 4 ignores the extension. No testcontainers-junit-jupiter dependency is added either.
    • Also: the outer class is annotated when only a nested class changed.
    • Fix: gate on Jupiter usage, and add an AddDependency with onlyIfUsing.
  • I3 Smaller Testcontainers items

    Details
    • ConvertToRawType:73 (Confirmed): (J.Identifier) pt.getClazz() throws on fully qualified types. Suspected: visitType doesn't recurse into type arguments (:45-53).
    • Missing relocations (Confirmed against the 2.0.5 jars): containers.TrinoContainer (only ConvertToRawType exists, Y/testcontainers.yml:761-762) and containers.ClickHouseContainer.
    • Suspected: dynalite has no 2.x artifact (:617). MigrateToKafkaNative and MigrateToOracleFree run on their own need 1.19.x but bump nothing.
  • I4 ReplaceSettersWithTransform: lost mutations and captured locals

    Details
    • Where: M/wiremock/ReplaceSettersWithTransform.java:199-207,233-244.
    • Dead stores (Confirmed): parameters and for-each variables get m = m.transform(..), so the caller's stub is unchanged. The tests enshrine this (T/wiremock/ReplaceSettersWithTransformTest.java:69-130,161-185,223-245).
    • Captured locals (Confirmed): a local captured by a lambda elsewhere in the method loses effective finality, so the code no longer compiles.
    • Decision: bail out with a TODO, or keep today's silent rewrite.
  • I5 Smaller WireMock, Arquillian and DBRider items

    Details
    • ReplaceArquillianInSequenceAnnotation (Confirmed, :51): a bare @InSequence becomes @Order, which has no default value. Also, :51 queues the ChangeType once per class, and :61 queues one visitor per method.
    • ExecutionListenerToDbRiderAnnotation (Confirmed, :119,139-141): skips a non-array value = X.class and classes that already have @DBRider, but the composite still swaps the dependency, so the listener reference breaks. shouldAddDbRiderAnnotation() is dead code.
    • ReplaceRemovedConstructors (Confirmed, :240,250-258): a JavaTemplate is cached in a static field, which races and pins the first run's classpath.
    • RemoveDuplicateContentTypeStubHeader:81-89 (Suspected): also edits webhook headers.
    • RemoveDuplicateContentTypeHeader:86-98 (Suspected): misses chained duplicates.
    • M/wiremock/ has no package-info.java with @NullMarked.

Sections J–O (dependencies & build files, crash-proofing, types, preconditions, mechanical polish, open questions) continue in the first comment, because the full list exceeds GitHub's issue size limit.

Dominant language
Java
Stars
100
Forks
109
Avg merge
3h 43m
Merged PRs (30d)
82

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from openrewrite/rewrite-testing-frameworks

All issues in openrewrite/rewrite-testing-frameworks

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.