openrewrite / openrewrite/rewrite-testing-frameworks
MigrateJUnitTestCase rewrites super.run(TestResult) to Assertions.run(...), which does not exist
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 100
- Forks
- 105
- Avg merge
- 2h 25m
- Merged PRs (30d)
- 9
Description
Versions
rewrite-maven-plugin 6.46.1, rewrite-testing-frameworks 3.44.0.
Scope note
This is about org.openrewrite.java.testing.junit5.MigrateJUnitTestCase, whose stated job is
migrating junit.framework.TestCase, so JUnit 3 constructs are deliberately in its scope. I am
not suggesting JUnit4to5Migration ought to cover JUnit 3 in general - it opts into that by
including this sub-recipe. I isolated the behaviour to this one recipe rather than reporting it
against the composite.
Reproduction
A TestCase that overrides run(TestResult) in order to keep a reference to the result, so a
worker thread can report failures. A common shape in multi-threaded JUnit 3 tests; this one is
reduced from a 2009 codebase.
package com.example;
import junit.framework.AssertionFailedError;
import junit.framework.TestCase;
import junit.framework.TestResult;
public class MultiThreadedTest extends TestCase {
private TestResult testResult = null;
@Override
public void run(TestResult result) {
this.testResult = result;
super.run(result);
}
public void handleException(Throwable t) {
if (t instanceof AssertionFailedError) {
testResult.addFailure(this, (AssertionFailedError) t);
} else {
testResult.addError(this, t);
}
}
public void testSomething() {
assertEquals(2, 1 + 1);
}
}
with junit:junit:4.13.2 on the test classpath, then:
mvn -U org.openrewrite.maven:rewrite-maven-plugin:6.46.1:run \
-Drewrite.activeRecipes=org.openrewrite.java.testing.junit5.MigrateJUnitTestCase \
-Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-testing-frameworks:3.44.0
Result
-import junit.framework.TestCase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
-public class MultiThreadedTest extends TestCase {
+public class MultiThreadedTest {
@Override
public void run(TestResult result) {
this.testResult = result;
- super.run(result);
+ Assertions.run(result);
}
Two problems:
super.run(result)becomesAssertions.run(result).
org.junit.jupiter.api.Assertionshas zerorunoverloads - verified withjavap
against junit-jupiter-api 5.14.4 - so this cannot compile under any configuration. It looks
like ajunit.framework.Assert.*toAssertions.*mapping being applied to asuper.run(...)
call that happens to resolve toTestCase.extends TestCaseis removed whilerun(TestResult)and its@Overrideare kept, so
javac reportsmethod does not override or implement a method from a supertype.
MultiThreadedTest.java:[15,21] cannot find symbol
MultiThreadedTest.java:[14,5] method does not override or implement a method from a supertype
Expected
I would suggest skipping the class when it overrides run(TestResult). A TestCase that
overrides run(TestResult) is participating in the JUnit 3 execution model, which has no Jupiter
equivalent - Jupiter has no TestResult, and no run hook to override - so no partial rewrite
of it can be correct. A guard on "declares run(junit.framework.TestResult)" would be simple and
safe, and leaving the class untouched gives the user something that still compiles.
At minimum, super.run(result) should not be rewritten to a method that does not exist.
Secondary, in the same composite
Running MigrateAssertionFailedError on the same file retargets
junit.framework.AssertionFailedError to org.opentest4j.AssertionFailedError while
TestResult.addFailure(Test, junit.framework.AssertionFailedError) is left as-is, so the
argument type no longer matches. Consistent with the above: these classes are reachable from
JUnit4to5Migration but not safely convertible in isolation.
Contributor 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.
Research direction
Start with the org.openrewrite.java.testing.junit5.MigrateJUnitTestCase recipe and reproduce the transformation using the Maven command and reduced MultiThreadedTest example in the issue. Add coverage for a class declaring run(junit.framework.TestResult), then verify the recipe leaves that class unchanged and the result still compiles; consider the related MigrateAssertionFailedError mismatch separately.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100