Recipe quality triage: remaining fixes, wiring and polish (A1–O8)
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
- Domain
- devtools, testing-qa
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
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
mainat6c313e2eand 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.ExpectedExceptionmatcher 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
Mockito1to4MigrationfromMockito4to5OnlyDetails
- Where:
Y/mockito.yml:50-59. - Problem: ef62c319 created
Mockito4to5Onlyso the Spring Boot 3.1 upgrade could include Mockito 5 without re-running the 1→4 migration. The commit moved the oldMockito1to5MigrationrecipeList across wholesale, including its first entryMockito1to4Migration, so the split never took effect. The description ("Does not include 1.x to 4.x migration") has been wrong ever since, andrecipes.csvcounts 101 sub-recipes against 102 for the full migration. - Impact: rewrite-spring's Boot 3.x upgrades reach
ReplacePowerMockitothis 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;
Mockito1to5Migrationalready chains both halves. Then decide whether Boot upgrades should run the PowerMock replacement at all.
- Where:
-
A2 Stop adding(#1239)ExternalResourceSupportwithout its dependency, then deleting the dependencyDetails
- Where:
- The usage is added by
M/junit5/HandleExternalResourceRules.java:74-83, wired atY/junit5.yml:163. - The dependency is removed by
RemoveJupiterMigrationSupportinY/junit-jupiter.yml:50,53-68(insideJupiterBestPractices), and by a copy inY/junit6.yml:62-64.
- The usage is added by
- Problem:
- The JUnit 4→5 migration emits
@ExtendWith(ExternalResourceSupport.class), but nothing addsjunit-jupiter-migrationsupport. JUnit5BestPractices,JUnit6BestPracticesandJUnit5to6Migrationthen remove that artifact with no usage guard. migrationsupport 6.1.3 still shipsExternalResourceSupportand@EnableRuleMigrationSupport, so any project that uses them breaks.
- The JUnit 4→5 migration emits
- Fix:
- Add the dependency with
onlyIfUsing: org.junit.jupiter.migrationsupport..*. - Guard both removals with
unlessUsingon the same pattern. - Chain
RemoveJupiterMigrationSupportinjunit6.ymlinstead of copying it.
- Add the dependency with
- Also (Suspected): migrationsupport depends on
junit:junit, whichExcludeJUnit4UnlessUsingTestcontainers(Y/junit5.yml:166-181) would exclude.
- Where:
-
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 intoJUnit5to6Migration.RemoveEmptyTests(M/cleanup/RemoveEmptyTests.java) was never wired. It deletes tests, including an empty@Overridethat deliberately disables an inherited test (:75-78, Suspected). Decide whether it belongs incleanup.BestPractices.MockitoJUnitRunnerSilentToExtensionis@Deprecatedand was superseded in #711. Delete it.AddMissingNestedwas unwired in #1160 and is now only used throughEnclosedToNested. Add it toJupiterBestPractices(RSPEC-S5790), or document that it is standalone.AddMockitoJupiterDependencyonly appears as a comment (Y/mockito.yml:195-201, #875).AnyStringToNullableis reachable only throughdoAfterVisit(M/mockito/AnyToNullable.java:65); see F11.
-
A4 Decide how much the JUnit 4→5 migration and
Assertjshould pull inDetails
- What
JUnit4to5Migrationpulls in:- Mockito 4 and PowerMock replacement (via
UseMockitoExtension, thenMockito1to4Migration, thenReplacePowerMockito). The PowerMock replacement disables tests. - OkHttp
mockwebserver35.x (Y/junit5.yml:118). MigrateHamcrestToJUnit5throughJupiterBestPractices(Y/junit-jupiter.yml:29). That recipe has the worst bugs in this issue (E1, E2).
- Mockito 4 and PowerMock replacement (via
- What
Assertjpulls in: about 1048 recipes, including all ofJUnit5BestPracticesthroughJUnitToAssertj. - 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.
- What
-
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.KafkaContainerbecomesorg.testcontainers.kafka.KafkaContainer. Bytecode shows the new class assertsapache/kafkaimages and has no no-arg constructor,withEmbeddedZookeeperorwithKraft. Confluent users fail at runtime or at compile time. ExplicitContainerImagesinjectsconfluentinc/cp-kafka:5.4.3, soTestcontainers2Migrationrun on its own guarantees that failure.- The old class still exists in Testcontainers 2.0.5, so the
ChangeTypeisn't needed to compile.
- Every
- Options: map Confluent usage to
ConfluentKafkaContainer, or retarget only after the image has actually been switched (insideMigrateToKafkaNative). - Also (Suspected): in
TestContainersBestPractices(:63-65), theChangeTypes inMigrateToKafkaNativeandMigrateToOracleFreerun beforeExplicitContainerImages. That recipe's<constructor>()matcher targets the old FQN, sonew KafkaContainer()ends up as a constructor that doesn't exist. Probe:new KafkaContainer()underTestContainersBestPractices.
- Where:
-
A6
AddMockitoExtensionIfAnnotationsUsed: disabled inmockito.yml, still active injmockit.ymlDetails
- Where:
Y/mockito.yml:195-201(disabled with the #875 rationale) versusY/jmockit.yml:50. - Also:
M/mockito/AddMockitoExtensionIfAnnotationsUsed.java:59-61,73-74adds@ExtendWith(MockitoExtension::class)to Kotlin JUnit 4 tests. This is Confirmed.
- Where:
-
A7 Remove duplicate and copied entries
Details
RemoveDoNothingForDefaultMocksis atY/mockito.yml:32and is reached again via:206.- The
junit-jupiter-paramsscanners inY/junit5.yml:139-145duplicateM/junit5/AddJupiterDependencies.java:58-62. Y/junit6.yml:62-64copiesRemoveJupiterMigrationSupport(see A2).M/mockito/PowerMockitoMockStaticToMockito.java:92-93queues the sameChangeMethodTargetToStaticthat runs next inY/powermockito.yml:188-191.
-
A8 Decide on AssertJ's
actual()floor (3.27)Details
- Problem:
AbstractAssert#actual()is@since 3.27.0.ReturnActualand the value path ofJUnitAssertThrowsToAssertExceptionType(:106-108,117-119) emit it. - The
AddDependency 3.xwithacceptTransitive: trueinJUnitToAssertj(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.
- Problem:
B. JUnit 4 → 5 migration bugs
-
B1(#1218)UseWiremockExtensionturns every@Rulein the file into@RegisterExtension(PROBED)Details
- Where:
M/junit5/UseWiremockExtension.java:44-49,68-69. - Problem: a file-wide
ChangeType(org.junit.Rule → RegisterExtension)is queued whenever anew WireMockRule(..)exists. InJUnit4to5Migrationit runs atY/junit5.yml:61, beforeTemporaryFolderToTempDir(:105) andTestRuleToTestInfo(:107). Those no longer recognise their fields. - Probe:
This gives@Rule public WireMockRule wm = new WireMockRule(); @Rule public TemporaryFolder tmp = new TemporaryFolder();@RegisterExtension public TemporaryFolder tmp, which Jupiter rejects at runtime. - Fix: change only the annotation on the
WireMockRuledeclaration. Coordinate with #1191, which adds the same file-wide pattern for@ClassRule.
- Where:
-
B2
MigrateJUnitTestCase:@Teston any method namedtest*(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 withParameterResolutionException. - Fix: apply JUnit 3's own discovery rule: public, non-static, void, no parameters.
- Also (Suspected,
:84): a file-wideChangeType(TestCase → org.junit.Assert)retypes non-assertion uses. Probe:static void run(junit.framework.TestCase t) { t.run(); }.
- Where:
-
B3(#1237)TemporaryFolderToTempDir: two harmful mappingsDetails
newFolder()with no arguments (PROBED,:279-284with:338-345): it becomesnewFolder(root, "junit"), so a second call in the same test failsmkdirs()withIOException. 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
TimeoutRuleToClassAnnotationremovesTimeoutfields that are still referenced (PROBED)Details
- Where:
M/junit5/TimeoutRuleToClassAnnotation.java:58-75. - Problem: any
Timeoutfield is removed, with no@Rulecheck and no reference check. - Probe:
private final Timeout t = Timeout.seconds(5); @Rule public RuleChain c = RuleChain.outerRule(t);leaves a danglingt.T/junit5/TimeoutRuleToClassAnnotationTest.java:52even uses a field without@Ruleas its input.
- Where:
-
B5
TestRuleToTestInfo: file-wide rename, lost parameters, forced spacingDetails
- Confirmed (
:71-72): an unconditional file-wideChangeType(org.junit.Before → BeforeEach). Run on its own, it leaves a JUnit 4 class with a@BeforeEachthat never runs. - Suspected (
:174-182):replaceParameters()drops parameters that an existing@BeforeEachalready has. - Confirmed (
:66): the prefix is forced to" ", solog(name.getMethodName())becomeslog( name). Usemi.getPrefix().
- Confirmed (
-
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
mainsources count as "may be used elsewhere". A concreteBaseTestwhoseoutrule is read bySubTestin another file is retyped (System Stubs) or removed (Pioneer), andSubTeststops compiling. - Decision: scan across files for
owner#name, or add the existing TODO comment.
- Where:
-
B7(#1233)AssumeNotNullToAssumptions: the varargs form produces a method that doesn't existDetails
- Where:
M/junit5/AssumeNotNullToAssumptions.java:52-63, then theChangeType Assume → AssumptionsatY/junit5.yml:223-226. - Problem (Confirmed):
Assume.assumeNotNull(a, b)doesn't match, so it ends up asAssumptions.assumeNotNull(a, b). - Test:
T/junit5/AssumeNotNullToAssumptionsTest.java:65is@Disabledagainst #886, which is closed. Its expected output is also wrong: the input passesnew Object(), new Object(), but the expected output usesobject1, object2. - Fix: a Java visitor for the varargs form, or a TODO plus bail-out. Fix and enable the test.
- Where:
-
B8
ExpectedExceptionToAssertThrows: name clashes and lostthrows(related: #1124)Details
- Suspected (
:203,229): the variable nameexceptionis hard-coded. Probe:RuntimeException exception = …; thrown.expectMessage("boom"); throw exception;. - Suspected (
:133-179): the checked-exception scan ignores the implicitclose()of try-with-resources and calls with a null method type, sothrows IOExceptionis dropped wrongly. - Suspected:
expect*inside@BeforegivesassertThrows(X.class, () -> {})in setUp. - This is separate from #1124 and #1129.
- Suspected (
-
B9
JUnitParamsRunnerToParameterized: static init methods that read instance stateDetails
- Confirmed (
:342-386):MakeMethodStaticonly 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.
- Confirmed (
-
B10
UpdateMockWebServer*recipes: non-compiling output and wrong mappingsDetails
UpdateMockWebServerMockResponse(Confirmed)::45-51appends.build()to everyMockResponseargument in any call, so helper parameters retyped toBuilderbreak. Fix: only wrap arguments ofMockWebServer#enqueueandQueueDispatcher#enqueueResponse.:88mapssetHttp2ErrorCode(int)tocode(int), which sets the HTTP status. The test atT/junit5/UpdateMockWebServerMockResponseTest.java:244,270locks this in.:85,91:setErrorCodeandsetThrottleBodydon't exist in 3.14.9 or 4.10.0, so these entries are dead.SocketPolicyandsetSocketPolicyare unmapped, andChangePackagesends them to a class that doesn't exist.
UpdateMockWebServer(Confirmed,:73,157): with two@Rule MockWebServerfields, only the last getsclose().:178uses4.X(uppercase).UpdateMockWebServerDispatcher(Suspected,:91-107):returnstatements inside lambdas indispatch()get wrapped.
-
B11
AddHamcrestJUnitDependency: repo-wide flag and over-broad matcherDetails
- Where:
M/junit5/AddHamcrestJUnitDependency.java:39-85. - Problem (Confirmed):
- One
assertThatanywhere addshamcrest-junitto every module. Assert.assertThatmoves toMatcherAssert, which is in plain Hamcrest; onlyAssume.assumeThatneedshamcrest-junit.
- One
- Fix:
onlyIfUsing: org.hamcrest.junit..*after the migration, and drop the scanner.
- Where:
-
B12 Smaller JUnit 4 migration bugs
Details
ParameterizedRunnerToParameterized:118-124:ClassCastExceptionon@Parameter(INDEX)with a constant. Confirmed. Related: #91, #1125, #1126, #743.CategoryToTag:93-94:@Category({})throws aClassCastExceptiononJ.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: emitsthreadMode, which needs Jupiter 5.9, while a Spring Boot 2.5 parent manages 5.7. Suspected.MockitoJUnitToMockitoExtension:82: checksorg.mockito.runners.MockitoJUnitRunner, whichY/mockito.yml:187-189has already renamed, so the guard never fires. Confirmed.EnvironmentVariables:118-128,150-158:env.clear(names)becomesenv.remove(names), which doesn't compile. Suspected.JUnitSoftAssertionsToSoftAssertionsExtension:103-106: a file-wideChangeTypebreaks helpers in other files that takeJUnitSoftAssertions. Suspected.RunnerToExtension:101-120: handles@RunWithon methods, which can't happen. Dead code.
C. JUnit 5/6 best practices and cleanup bugs
-
C1(#1212)AssertEqualsIntegralDeltaToAssertEqualsdrops non-zero deltas (PROBED)Details
- Where:
M/cleanup/AssertEqualsIntegralDeltaToAssertEquals.java:59,83, reached fromJupiterBestPractices(Y/junit-jupiter.yml:34). - Problem:
assertEquals(100, elapsed, 50)becomesassertEquals(100, elapsed), so a tolerance test starts failing. The description (:43-45) wrongly says the delta is meaningless, and every test uses0. - Fix: only act when the delta is a literal zero.
- Where:
-
C2
CleanupKotlinJUnit5AssertionImportsremovesAssertions.*(PROBED)Details
- Where:
M/junit5/CleanupKotlinJUnit5AssertionImports.java:53,67-90, reached fromJupiterBestPractices(Y/junit-jupiter.yml:31). - Problem: with
import org.junit.jupiter.api.*, everyAssertionsimport is dropped. The KotlinAssertionsKtfunctions only coverassertAll,assertDoesNotThrow,assertInstanceOf,assertNotNull,assertNull,assertThrows,assertTimeout*andfail, soassertEquals(1, 1)no longer resolves. The tests only useassertNullandassertNotNull. - Fix: only drop explicit imports of names that have a Kotlin counterpart. Leave
Assertions.*alone, and add a precondition.
- Where:
-
C3(#1213)TestsShouldIncludeAssertionsturns skipped tests into failures (PROBED)Details
- Problem 1 (
M/cleanup/TestsShouldIncludeAssertions.java:44-61,126-131): assumptions don't count as assertions.assumeTrue(isLinux()); service.start();is wrapped inassertDoesNotThrow, which reports theTestAbortedExceptionas 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.
- Problem 1 (
-
C4(#1217)TestMethodsShouldBeVoidandKotlinTestMethodsShouldReturnUnitdelete early returns (PROBED)Details
- Early returns:
RemoveDirectReturns(M/cleanup/TestMethodsShouldBeVoid.java:93-98,M/cleanup/KotlinTestMethodsShouldReturnUnit.java:120-128) deletes everyreturn.if (skip) { return 0; } fail();then always fails.- Fix: turn
return expr;intoexpr;followed byreturn;. - Fix: bail out when the method is invoked anywhere, or is an override.
- Fix: turn
- Kotlin expression bodies (Confirmed):
KotlinTestMethodsShouldReturnUnit:82-91adds: Unitto expression bodies of any type, sofun t() = assertThat(x).isTrue()no longer compiles. The test inputs are not valid Kotlin. - Suspected:
TestMethodsShouldBeVoidhas no Kotlin guard and installs aJ.Primitivevoid, which the Kotlin printer may print asvoid. - Suspected:
:76'srequireNonNull(m.getBody())NPEs on abstract test methods.
- Early returns:
-
C5
RemoveTestPrefixbreaks overrides and implied@MethodSource(PROBED)Details
- Overrides:
ChangeMethodName(.., matchOverrides=false)(M/cleanup/RemoveTestPrefix.java:180) renamesBTest.testFoobut not the@Overridein a subclass, so the override no longer compiles. - Confirmed (
:150vs:160,175): the source-method rename is scheduled before the bail-outs. After a bail-out, the factory is calledfoo()while the test is still calledtestFoo.
- Overrides:
-
C6(#1247)TestsShouldNotBePublicnarrows methods of extended classes (related: #458)Details
- Where:
M/cleanup/TestsShouldNotBePublic.java:102vs:150-163. - Problem (Confirmed): the
extendedClassesguard 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.
- Where:
-
C7 Cleanup assertion rewrites without type checks (probed part: #1220)
Details
PROBED:(#1220)AssertNotEqualsBooleanToAssertBoolean:47doesn't check that the second argument is boolean (its sibling does,AssertEqualsBooleanToAssertBoolean:48).assertNotEquals(false, map.get("k"))becomesassertTrue(Object).- Confirmed:
AssertionsArgumentOrder:53-54,128-130matches JUnit 4(String, Object, Object)overloads by index, so it swaps the message.assertEquals(..)is listed twice (:50-51), while JUnit 4assertNotEqualsis missing. - Confirmed (rare):
SimplifyTestThrows:95-97turnsthrows IOException, Throwableintothrows Exception. - Suspected:
AssertTrueComparisonToAssertEquals:112-113turnsInteger == longinto an ambiguousassertEquals. - Suspected:
AssertTrueEquals:93andAssertFalseEquals:90matchequalsby 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, asAssertTrueNegationToAssertFalsedoes, fixes this.
-
C8(#1244)CsvSourceToValueSourcechanges values on the non-text-block path (PROBED)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"}), sosis"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[...].
- Where:
-
C9
ImplausibleTimeoutToMinutesdropsthreadMode(PROBED) and roundsDetails
- Where:
M/junit5/ImplausibleTimeoutToMinutes.java:71-96. - Problem:
@Timeout(value = 3600, threadMode = SEPARATE_THREAD)becomes@Timeout(value = 60, unit = MINUTES), losingthreadMode. - Also:
:89rounds (1000 s becomes 17 min), although the description says "equivalent".
- Where:
-
C10(#1242)MinimumJreConditionsrewrites every JRE range in the file (PROBED)Details
- Where:
M/junit6/MinimumJreConditions.java:180-181,217-231. - Problem: collapsing one method's range queues a file-wide
ChangeTypeand attribute edits. WithjavaVersion17,@EnabledForJreRange(min = JAVA_21)on another method becomes@EnabledOnJre(JAVA_17). - Suspected (
:195-200): JRE-constant arrays write the intversionsattribute. - Suspected (
:321-325):Integer.parseInton a constant reference throws. - No precondition (
:72-73).
- Where:
-
C11(#1221)RemoveInterceptDynamicTestdeletes an overload that JUnit 6 still has (PROBED)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.
- Where:
-
C12(#1223)UseAssertSameon Kotlin turns equality into identity (PROBED)Details
- Where:
M/junit5/UseAssertSame.java:59-76. - Problem: Kotlin
assertTrue(listOf(1) == listOf(1))becomesassertSame(listOf(1), listOf(1)), because==callsequalsin Kotlin. - Fix: skip
K.CompilationUnit. - Types (
:94-106): the fallback "stubbed" method type is the normal path.
- Where:
-
C13
RemoveJreOtherempties lone braced arrays and changes behaviourDetails
- 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
OTHERfrom{JAVA_21, OTHER}disables the test on future JREs.
- The recipe is currently unwired (A3).
- Where:
-
C14
AssertThrowsOnLastStatement: side effects outside the targetDetails
- Confirmed (
:72):doAfterVisit(new LambdaBlockToExpression())is scheduled for every method body, so it rewrites unrelated lambdas. - Confirmed (
:123): an empty block lambda() -> {}deletes the wholeassertThrowsstatement. - Suspected (
:123-125,158,290): hoisted locals from siblingassertThrowscalls 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")atT/junit5/AssertThrowsOnLastStatementTest.java:1287.
- Confirmed (
-
C15 Smaller JUnit 5 best-practice items
Details
AddMissingNested:114mutates the modifier list in place withremoveIf. UseListUtils.filter.AddParameterizedTestAnnotation:36-46misses@FieldSource,@ArgumentsSourcesand custom sources. UseAnnotationMatcher("@…ArgumentsSource", true). Coordinate with #1093.AssertTrueInstanceofToAssertInstanceOf:77-96: its JUnit 4 branch emits Jupiter'sassertInstanceOfinto JUnit 4 modules whenJupiterBestPracticesruns on its own. Needs a decision; coordinate with #1083 and #1088.
D. AssertJ bugs
-
D1(#1215)AdoptAssertJDurationAssertions: comparisons become equality, and units are lossy (PROBED)Details
- Comparisons (PROBED,
M/assertj/AdoptAssertJDurationAssertions.java:105-106,142-150):assertThat(d.getSeconds()).isGreaterThan(5)becomesassertThat(d).hasSeconds(5). getNano(Confirmed by running it,:67-68):getNano()is the nanosecond-of-second part, buthasNanoschecks the whole duration.- Units (Confirmed by running it,
:163-200):hasMillis(5000)becomeshasSeconds(5), which also passes for 5999 ms.- A zero argument recurses all the way to
hasDays(0). Math.toIntExactthrows for large values.
- Tests lock in the bugs:
getNanoToHasNanos,isEqualToVariableand thesimplifyDurationAssertionsCSV. - Fix: only map from
isEqualTo, drop thegetNanomapping, and remove the unit up-conversion.
- Comparisons (PROBED,
-
D2 JUnit-to-AssertJ recipes mishandle deltas (PROBED)(#1216)Details
- PROBED (
JUnitAssertEqualsToAssertThat:88-94,105-112):assertEquals(100, elapsed, 10)becomesassertThat(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 toassertEquals.assertNotEquals(0.0, d, 0)becomes.as(0), andassertNotEquals(5, count, 1.0)calls a method that doesn't exist. - Fix: share
floatingPointDeltaType/isIntegralType. Keep the delta viaisCloseTo(.., within(..)), or bail out unless it is a literal0.
- PROBED (
-
D3
ReturnActualemits code that doesn't compile for collectionsDetails
- Where:
M/assertj/ReturnActual.java:81. - Problem (Confirmed with javac): for
List,IterableandCollection,actual()returnsList<? extends E>, so the@DocumentExampleoutput (return assertThat(list).isNotNull().actual();in a method returningList<String>) doesn't compile. - Fix: bail out when the
assertThatoverload'sACTUALis a wildcard. Version floor: A8.
- Where:
-
D4(#1243)SimplifyHasSizeAssertionproduces ahasSameSizeAsthat fails at runtime (PROBED)Details
- Where:
M/assertj/SimplifyHasSizeAssertion.java:59-70. - Problem:
assertThat(list).hasSize(str.length())becomeshasSameSizeAs(str), which binds tohasSameSizeAs(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.
- Where:
-
D5(#1241)SimplifyRedundantAssertJChainsremoves guards that aren't redundant (PROBED)Details
- Where:
M/assertj/SimplifyRedundantAssertJChains.java:56,88,114-127. isNotEmpty()beforecontainsAll(..):assertThat(ids).isNotEmpty().containsAll(expected)loses theisNotEmpty(), althoughcontainsAll(emptyList)passes on an empty actual.isNotNull()beforematches(..):..* matches(..)also matchesAbstractAssert.matches(Predicate), which does not null-check.- Fix: type the matcher (
AbstractCharSequenceAssert matches(..)), and limit theisNotEmptyremoval.
- Where:
-
D6(#1222)JUnitAssertInstanceOfToAssertThatbreaks code that uses the return value (PROBED)Details
- Where:
M/assertj/JUnitAssertInstanceOfToAssertThat.java:54-69. - Problem:
Integer i = assertInstanceOf(Integer.class, o);becomesInteger i = assertThat(o).isInstanceOf(Integer.class);. - Fix: bail out unless the call is a statement, as
JUnitAssertThrows#hasReturnTypedoes.
- Where:
-
D7
JUnitAssertThrowsToAssertExceptionTyperetypes signaturesDetails
- Where:
M/assertj/JUnitAssertThrowsToAssertExceptionType.java:82-92,164-192,209-224. - Confirmed:
RetypeExecutableVariableretypes method parameters and non-private fields, and only checks the current file. A helperassertFails(Executable e)therefore becomesThrowingCallable, and its callers in other files break. - Confirmed:
assertThrows(X.class, this.executable)orexecutable()emitsisThrownBy(<Executable>), which doesn't compile. - Fix: only retype locals, and bail out on any other
Executableexpression.
- Where:
-
D8 Other AssertJ semantics changes
Details
SimplifyAssertJInstanceOfAssertion:93(Confirmed by running it):assertThat(x instanceof Foo).isFalse()passes for null, butisNotInstanceOffails on null.JUnitFailToAssertJFail(Confirmed)::63-74:fail(Supplier)becomesfail("", supplier), so the message is silently"".:84-85: a whole-fileRemoveUnusedImports.:89-110: unqualifies everyAssertions.failin the file.
JUnitTryFailToAssertThatThrownBy:- Confirmed (
:145,152): messages are inserted raw, so a quote or backslash breaks the template. UsegetValueSource(). - 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).
- Confirmed (
MigrateAssertionsForClassTypes:- Confirmed (
:47-63,92-95): run on its own, it emitsAssertionsForClassTypes.assertThatObject, which doesn't exist. - Suspected:
COLLISION_TYPESmissesAtomic*,LongAdder,BigInteger,CharSequence,CompletionStage,FutureandSpliterator.
- Confirmed (
SimplifyChainedAssertJAssertion:222-227(Confirmed): generatesPath.of(..), which is Java 11+. UsePaths.get.IsEqualToIgnoringMillisToIsCloseTo(Confirmed by running it): weaker semantics than the original, not mentioned in the description. Its display name says "byby isCloseTo".- Refaster (
AssertJDoubleRules:114-116,137-139,160-162,AssertJFloatRules:96-97,117-118,138-139):isSameAs(0)can never pass, so rewriting it toisZero()changes behaviour. Low impact. - Suspected: an unqualified
size(),stream()orgetFirst()inside a collection subclass passes a null select (SimplifyHasSizeAssertion:64,SimplifyStreamMapToExtracting:79,SimplifySequencedCollectionAssertions:151). - Suspected: Kotlin
.as(..)from theJUnit*ToAssertThattemplates;asis a Kotlin keyword. There are no Kotlin tests inT/assertj/.
-
D9 Dead YAML entries and null-able options in
Simplify*AssertJAssertionDetails
- 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 fourSimplifyChainedAssertJAssertionoptions (:47-73) arerequired = false, but they are spliced straight into matcher strings. Make them required.
- Entries that can never match (Confirmed):
E. Hamcrest & Truth bugs
-
E1(#1214)RemoveNotMatcherVisitor: a negation leaks into later assertions and flips them (PROBED)Details
- Where:
M/hamcrest/RemoveNotMatcherVisitor.java:31-40,47-50,67,71-72, used byHamcrestMatcherToJUnit5.java:155-177andHamcrestInstanceOfToJUnit5.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:
becomesassertThat(a, not(nullValue())); assertThat(b, nullValue());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 withimmutableExecutionContext(false):HamcrestMatcherToJUnit5Test:131,170,209,552,1066HamcrestInstanceOfToJUnit5Test:38RemoveNotMatcherTest:36MigrateHamcrestToJUnitTest:42
- Fix: count the
not(..)layers locally, drop the context messages, and remove the relaxations.
- Where:
-
E2
HamcrestMatcherToJUnit5picks templates by simple name, ignoring typesDetails
- Where:
M/hamcrest/HamcrestMatcherToJUnit5.java:60,65,70-75,172(Replacement.valueOf(getSimpleName().toUpperCase())). - Problem (Confirmed):
greaterThanonString,BigDecimalorLocalDateemitsassertTrue("b" > "a").closeTo(BigDecimal, ..)emitsMath.abs(bd - bd).closeTouses<where Hamcrest uses<=.equalToon arrays becomes a reference comparison.- Arguments come out as (actual, expected).
- Fix: one typed
MethodMatcherper replacement, a numeric guard,<=, andassertArrayEqualsfor arrays.
- Where:
-
E3(#1236)HamcrestMatcherToAssertJ/HamcrestNotMatcherToAssertJpass Hamcrest matchers into AssertJ (PROBED)Details
- Where:
M/hamcrest/HamcrestMatcherToAssertJ.java:74,93,M/hamcrest/HamcrestNotMatcherToAssertJ.java:71,94,127. - Problem: the
*(org.hamcrest.Matcher)exclusion doesn't match theMatcher[]varargs overloads.contains(equalTo("a"), startsWith("b"))becomescontainsExactly(equalTo("a"), startsWith("b")), and the same happens forcontainsInAnyOrder,hasItems,arrayContainingandhasEntry(Matcher, Matcher). - Test locks in non-compiling output:
T/hamcrest/HamcrestMatcherToAssertJTest.java:386-428, hidden withmethodInvocations(false). - Fix: bail out if any argument is (or contains) a
Matcher. - Also: the 3-argument path of
HamcrestNotMatcherToAssertJ(:143-147) leaves thenotand matcher imports unused.
- Where:
-
E4 Hamcrest 1.x— #1250is(Class)is migrated as equalityDetails
- 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 forinstanceOf, soassertThat(s, is(String.class))becomesisEqualTo(String.class). - Fix: exclude
is(java.lang.Class)or map it toisInstanceOf. The test needshamcrest-core-1.3inclasspath.tsv.gz.
- Where:
-
E5 Hamcrest YAML mappings that change behaviour
Details
equalToIgnoringWhiteSpace(Y/hamcrest.yml:230-232,329-331): it maps toisEqualToIgnoringWhitespace, which removes all whitespace in AssertJ 3.9+. The equivalent isisEqualToNormalizingWhitespace.not(hasItems(a, b))(:360-362): it becomesdoesNotContain(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):
:72casts the matcher toJ.MethodInvocation, soMatcher<Object> m = instanceOf(String.class); assertThat(o, m);throws aClassCastException. - Null (Confirmed):
:84turnsnot(instanceOf(X))intoX.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
asListDetails
containsMatch(Confirmed): Truth'scontainsMatchis a find, but it maps to full-match assertions:M/truth/TruthThrowableAssertions.java:83-95useshasMessageMatching.Y/truth.yml:53-58maps it tomatches, anddoesNotContainMatchtodoesNotMatch.- The repo's own expectation at
T/truth/TruthThrowableAssertionsTest.java:127-152would fail at runtime. - Fix: use
hasMessageFindingMatch,containsPatternanddoesNotContainPattern.
asList(Confirmed,Y/truth.yml:128-130):ObjectArraySubject asList()becomescontainsExactly(), which asserts that the array is empty.- Suspected:
Y/truth.yml:224-226removes Truth even whenExpectorCorrespondenceremain. - Dead entries:
:120-122(AbstractMapAssert containsEntries) never matches.:199-204targetsAbstract*ArrayAssert, but AssertJ returnsListAssert.
F. Mockito & PowerMock bugs
-
F1(#1224)RemovePowerMockClassExtensionsremoves the user's own base class (PROBED)Details
- Where:
M/mockito/RemovePowerMockClassExtensions.java:58-59. - Problem:
isAssignableTo(PowerMockTestCase, extends)also stripsclass FooTest extends MyBaseTest, whereMyBaseTest extends PowerMockTestCase.FooTestthen loses its inherited helpers. - Fix: use
TypeUtils.isOfClassType.
- Where:
-
F2
DisableUnsupportedPowerMockTestsleaves some classes on PowerMock, but the dependency is removed anywayDetails
- 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,@RepeatedTestand@Theoryaren't recognised as tests. - Also:
FlagUnsupportedPowerMockUsage.java:47-49still says such repositories "are left on PowerMock", which is no longer true.
- Where:
-
F3
CloseUnclosedStaticMockskeeps a static mock active past an explicitclose()(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 theclose()is removed, so the assertion now runs against the mock. - Fix: skip declarations that are already closed, or end the try at the close.
- Where:
-
F4
MockConstructionToTryWithResources: variable scope and early closesDetails
- 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()})withcontextSensitive().
- A local declared before
- Probe:
MockedConstruction<A> mc = mockConstruction(A.class); A a = new A(); mc.close(); a.method();.
- Where:
-
F5 Whitebox → reflection recipes don't compile for generic, array and lambda shapes
Details
- Erased types (Confirmed):
WhiteboxToReflectionVisitor.java:571-575,719-727usesgetClassName().List<String> l = Whitebox.getInternalState(..)becomesList l = (List) ...String[]becomesObject.- The same code path is used by the
GetInternalState,InvokeMethodandInvokeConstructorrecipes.
- Lambda blocks (Confirmed,
:362-387vs:522-531): a statement-level call inside a lambda block emitsgetDeclaredField, which throws a checked exception inside aConsumer. - Fix: print the declared type expression or bail out, and apply the lambda guard to every path.
- Erased types (Confirmed):
-
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(), givingmockTest.Files1. - Wrong class (Confirmed):
when(Outer.Inner.value())mocksOuter. - Also: the mutable
varCounteron 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.
- Where:
-
F7(#1249)RemoveInitMocksIfRunnersSpecifiedremoves initialisation the extension doesn't replace (PROBED)Details
- Where:
M/mockito/RemoveInitMocksIfRunnersSpecified.java:61-62,109-114,303-309. - Problem:
MockitoAnnotations.openMocks(this)in the constructor of a static, non-@Nestedfixture class is removed, so its@Mockfields stay null.openMocks(helper)is removed too.
- Fix: require a
thisargument, and only treat@Nestedinner classes as covered by the extension.
- Where:
-
F8
RemoveDoNothingForDefaultMocksremoves adoNothing()that overrides an earlier stubDetails
- Where:
M/mockito/RemoveDoNothingForDefaultMocks.java:154-193. - Problem (Confirmed): a
doThrow(..).when(foo).bar()in@Before, followed bydoNothing().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).
- Where:
-
F9
PowerMockitoDoStubbingToMockito: ambiguous private overloadsDetails
- Where:
M/mockito/PowerMockitoDoStubbingToMockito.java:85-87,125-136,154-155. - Problem (Confirmed): the private bail-out only fires when
resolveTargetMethodfinds a unique match. With two privatehelper(..)overloads, or missing types, it emitsdoNothing().when(s).helper("a")against a private method. - Also: the description (
:47-49) over-claims, and it uses a hand-builtTextComment(:88-92). - Test: turns off identifier validation in
defaults(T/mockito/PowerMockitoDoStubbingToMockitoTest.java:40-42).
- Where:
-
F10 Smaller Mockito items
Details
ReplaceMockitoTestExecutionListener:205(Confirmed):firstEnclosingOrThrow(J.CompilationUnit.class)throws on Kotlin. The wrapper recipes…ForJupiter,…ForJUnit4and…ForTestNGare never loaded in a test.ArgumentMatcherToLambda:180-184(Confirmed):ArgumentMatcher<T>and<String[]>are cast toArgumentMatcher<Object>.PowerMockRunnerDelegateToRunWith:77-86(Suspected): with a delegate,@Mockinitialisation is dropped. Add aMockitoJUnit.rule().PowerMockWhiteboxSetInternalStateToInjectMocks:89-120(Suspected):@InjectMocksinjects more than the one field the call set.ThenThrowCheckedExceptionToRuntimeException:110-116(Suspected, low):throws Ecounts as undeclared. AShallowClassis used at:75-77.MockitoBestPracticesremoves every@MockitoSettings(strictness = WARN), including the onesMockito1to3Migrationadds in the same run. Intended?
-
F11 Make(#1292)AnyToNullable/AnyStringToNullabledeclarativeDetails
- Where:
M/mockito/AnyToNullable.java:41-65,M/mockito/AnyStringToNullable.java:36,55. - Problem (Confirmed):
- The matchers use
org.mockito.Mockito, so calls throughimport static org.mockito.Matchers.*are missed.Y/mockito.yml:115-116already usesorg.mockito.*for this reason. - A global
AtomicBooleanconverts every module once any module usesmockito-all.
- The matchers use
- Fix: a YAML recipe with a
ModuleHasDependencyprecondition andorg.mockito.*patterns. This also replaces thedoAfterVisit. - Related:
VerifyZeroToNoMoreInteractionsuses the same scanner pattern, a misnamedASSERT_INSTANCE_OF_MATCHER(:33), and a garbled description.
- Where:
G. JMockit & EasyMock bugs
-
G1
JMockitBlockRewritersilently 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. @Capturingfields 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.
- Where:
-
G2 JMockit
with*argument matchers are not migratedDetails
- Where:
M/jmockit/ArgumentMatchersRewriter.java:110,118,140-144,270-279. - Problem (Confirmed):
withNotNull(),withCapture(list),withInstanceOf(..)andwithSameInstance(..)are emitted unchanged insidewhen(..)andverify(..), where they no longer resolve. When mixed withanyStringthey becomeeq(withNotNull()). - Also: matchers are checked by simple name, so a local called
anycounts as one. - Fix: bail out, or map them (
notNull(),isA(),same(), …).
- Where:
-
G3
JMockitMockUpToMockito: several shapes break or are silently lostDetails
- Where:
M/jmockit/JMockitMockUpToMockito.java. All Confirmed unless marked. - JUnit 5 setup: a MockUp in
@BeforeEachis closed when setup returns, because onlyorg.junit.Before/Afterare matched (:482-494). The comment at:71claims 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
@Mockmethod maps (:268-270,506-517). - MockUp fields and helper methods are inlined into the method body (
:187-199,281).
- A MockUp is removed with nothing in its place when no
- Unreachable code:
return null;is appended after a trailingthrow(:370-378). - NPEs:
:335on type-variable parameters. It is reached for any generic static method on the class.:361on 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).
- A whole-file
- Where:
-
G4 Smaller JMockit items
Details
JMockitAnnotatedArgumentToMockito:82-95(Confirmed):@Mocked List<String> lbecomesmock(List<String>.class). Use the erasure.SetupStatementsRewriter:59,74,153(Confirmed code path):ClassCastExceptionon fields or methods insidenew Expectations(){..}, and onOuter.this.mock.- Tests: every JMockit test runs the whole
JMockitToMockitocomposite (T/jmockit/JMockitTestUtils.java:39-46), on mockito-core 3.12, while the composite adds 5.x.
-
G5
EasyMockVerifyToMockitoVerify: stubs treated as expectations, bareverifyleft behindDetails
- Where:
M/easymock/EasyMockVerifyToMockitoVerify.java:61,67-70,121,125-129. - Problem (Confirmed):
andStubReturn,andStubThrowandandStubAnswerbecomeverify(dep).foo()checks that require exactly one call.- Some chains are never collected:
.times(2),.anyTimes(),.once(), expectations set in@Before, and multi-mockverify(a, b). These leaveverify(dep), which becomes a bareMockito.verify(dep);. That throwsUnfinishedVerificationException, or doesn't compile with two mocks. - The single-static
import org.mockito.Mockito.verifyshadowsEasyMock.*. The test atT/easymock/EasyMockVerifyToMockitoVerifyTest.java:271-327expects exactly this.
- Decision: what should happen to a
verify(dep)that can't be translated?
- Where:
-
G6 EasyMock leftovers that don't compile (related: #738)
Details
- PROBED:
RemoveExtendsEasyMockSupport(M/easymock/RemoveExtendsEasyMockSupport.java:45), run throughEasyMockToMockito, leavesRunnable r = createMock(Runnable.class);unresolved. InheritedcreateMock,mock,niceMock,strictMock,resetAllandcreateMockBuilderhave no mapping.isAssignableToalso strips a user base class that extendsEasyMockSupport. - Confirmed,
Y/easymock.yml::81-86:IMocksControl createMock(..)becomesMockito.createMock(..), which doesn't exist, and the later rename can't match.:93-95:mock(String, Class)becomes aMockito.mock(String, Class)that doesn't exist.capture(..)becomesargThat, althoughCaptureisn't anArgumentMatcher.- The
RemoveDependencyis unconditional.
- None of these is tested.
- PROBED:
H. TestNG bugs
-
H1(#1240)TestNgAssertThrowsToAssertThatgarbles the message overloads (PROBED)Details
- Where:
M/testng/TestNgAssertThrowsToAssertThat.java:87-95. - Problem:
assertThrows("msg", ISE.class, () -> ..)becomesassertThatExceptionOfType("msg").isThrownBy(ISE.class), and the lambda is lost. - Fix: handle 3 arguments by mapping the message to
.as(..), or bail out.
- Where:
-
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, soassertEquals(d, 1.0, 0)becomes.as(0). The same pattern is in:TestNgAssertEqualsToAssertThat:91,152TestNgAssertNotEqualsToAssertThat:75TestNgAssertionToAssertJ:131TestNgSoftAssertToAssertJ: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 anOffset<Double>to a float assertion. - Fix: read
getMethodType().getParameterTypes().get(2). There are no delta tests.
- Integer delta read as a message (Confirmed):
-
H3 TestNG
SoftAssert/Assertionrecipes skip nested calls and aren't self-containedDetails
- Where:
M/testng/TestNgSoftAssertToAssertJ.java:51-54,77,83andTestNgAssertionToAssertJ.java:54-57. - Problem (Confirmed): these return without calling
super, soxs.forEach(x -> softly.assertTrue(x > 0))is never migrated. The YAMLChangeType(Y/testng.yml:53-55) then retypessoftlytoSoftAssertions, which has noassertTrue. - Not self-contained: run on its own, the recipe emits
.assertThat(..)on a TestNGSoftAssert. - Fix: move the
ChangeTypeintogetRecipeList(). - Tests: there are no dedicated tests for
TestNgAssertNotEqualsToAssertThatorTestNgSoftAssertToAssertJ, and every TestNG test uses an unversionedclasspath("testng").
- Where:
I. Testcontainers, WireMock, Arquillian & DBRider bugs
-
I1
ReplaceContainerImageName: invalid Vault image, and an over-broad prefixDetails
- Vault (Confirmed):
M/testcontainers/ReplaceContainerImageName.java:88-89appendsvalue.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 matchescp-kafka-rest.- Any
DockerImageName.parsein a file that constructs aKafkaContaineris 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
dependsOnstubs even though the Testcontainers jars are on the classpath (alsoMigrateToKafkaNativeTest:34,MigrateToOracleFreeTest:35).
- Vault (Confirmed):
-
I2
AddTestcontainersAnnotationshalf-migrates JUnit 4 testsDetails
- Where:
M/testcontainers/AddTestcontainersAnnotations.java:53-58,62. - Problem (Confirmed):
@Testcontainersis added next toorg.junit.Test, and JUnit 4 ignores the extension. Notestcontainers-junit-jupiterdependency is added either. - Also: the outer class is annotated when only a nested class changed.
- Fix: gate on Jupiter usage, and add an
AddDependencywithonlyIfUsing.
- Where:
-
I3 Smaller Testcontainers items
Details
ConvertToRawType:73(Confirmed):(J.Identifier) pt.getClazz()throws on fully qualified types. Suspected:visitTypedoesn't recurse into type arguments (:45-53).- Missing relocations (Confirmed against the 2.0.5 jars):
containers.TrinoContainer(onlyConvertToRawTypeexists,Y/testcontainers.yml:761-762) andcontainers.ClickHouseContainer. - Suspected:
dynalitehas no 2.x artifact (:617).MigrateToKafkaNativeandMigrateToOracleFreerun on their own need 1.19.x but bump nothing.
-
I4
ReplaceSettersWithTransform: lost mutations and captured localsDetails
- 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.
- Where:
-
I5 Smaller WireMock, Arquillian and DBRider items
Details
ReplaceArquillianInSequenceAnnotation(Confirmed,:51): a bare@InSequencebecomes@Order, which has no default value. Also,:51queues theChangeTypeonce per class, and:61queues one visitor per method.ExecutionListenerToDbRiderAnnotation(Confirmed,:119,139-141): skips a non-arrayvalue = X.classand 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): aJavaTemplateis 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 nopackage-info.javawith@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
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from openrewrite/rewrite-testing-frameworks
-
JUnit 4 to 5 Migration: Tests with Multiple @Before Methods Fail Due to Arbitrary Execution OrderPossibly taken @ApekshaAmbala claimed this 19 days ago. Openjunit recipe
Difficulty 4/5 3-5 days Newbie friendliness 55/100
openrewrite/rewrite-testing-frameworks#1207 · 1 comment · 1 assignee ·
Maintainers usually reply within 1 day
-
JUnit4to5Migration - undiscovered or invalid Kotlin testsPossibly taken @hugoncosta claimed this 28 days ago. Openbug
Difficulty 4/5 3-5 days Newbie friendliness 42/100
openrewrite/rewrite-testing-frameworks#1188 ·
Maintainers usually reply within 1 day
-
Difficulty 5/5 Over a week Newbie friendliness 35/100
openrewrite/rewrite-testing-frameworks#1167 ·
Maintainers usually reply within 1 day
-
`ParameterizedRunnerToParameterized` silently skips classes whose `@Parameters` method is inherited, leaving a partially migrated class thatPossibly taken @mohitduhan19 claimed this 43 days ago. Openbug
Difficulty 4/5 3-5 days Newbie friendliness 68/100
openrewrite/rewrite-testing-frameworks#1126 ·
Maintainers usually reply within 1 day
-
`JUnit4to5Migration` moves `Parameterized` parameter assignment after `@BeforeEach`, so setup methods read nullPossibly taken @mohitduhan19 claimed this 37 days ago. Openbug
Difficulty 4/5 3-5 days Newbie friendliness 70/100
openrewrite/rewrite-testing-frameworks#1125 ·
Maintainers usually reply within 1 day
All issues in openrewrite/rewrite-testing-frameworks
Similar issues
-
Four TestConstraintBasedAlgorithm tests pass when nothing is thrownPossibly taken @sharanggupta claimed this today. Open
Difficulty 1/5 1-3 hours Newbie friendliness 85/100
-
bug documentation good first issue help wanted priority low size XS
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
martin-francois/symphony-trello#1351 ·
Maintainers usually reply within 1 day
-
Add test coverage for ConvertUtil.parseDate() with NULL inputPossibly taken @mirajul-mollah7 claimed this today. Open
Difficulty 1/5 Under an hour Newbie friendliness 78/100
eclipse-sw360/sw360#4792 · 1 comment ·
Maintainers usually reply within 3 days
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
aws/aws-emulator#30 · 1 comment ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 4 days