Errors after upgrade of JUnit 4 to JUnit 5 in the Parsson project

[api-from-the-ion]

What version of OpenRewrite are you using?

I am using

• OpenRewrite v5.9.8 in Maven plugin
• Maven plugin v5.14.0

How are you running OpenRewrite?

I am using the Maven plugin, calling from the shell.

mvn -U org.openrewrite.maven:rewrite-maven-plugin:5.14.0:run \
  -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-testing-frameworks:RELEASE \
  -Drewrite.activeRecipes=org.openrewrite.java.testing.junit5.JUnit4to5Migration 

What is the smallest, simplest way to reproduce the problem?

I upgraded JUnit 4 to JUnit 5 in the Parsson project. Anybody can repeat this and see the following errors in result.

What did you expect to see?

Upgraded project without any errors.

What did you see instead?

1. JUnit version was changed in the POM of the "impl" module, but not in the project POM
2. JUnit version wasn't changed in the POM of the "customprovider-jdk9" module
3. Hamcrest is missing in POMs of modules, is present in the project POM
4. The JsonMergePatchDiffTest (JsonPointerRemoveOperationTest, JsonPointerAddOperationTest, JsonPointerReplaceOperationTest, JsonPointerTest,JsonPatchDiffTest, JsonPatchTest, JsonMergePatchTest) class was with @RunWith(Parameterized.class); static method with @Parameters(name = "{index}: ({0})={1}") returning List<Object[]>; the test class is POJO with fields and constructor initializing these fields; test is using fields values (constructor injection).
   1. After the rewrite, we have old annotations removed and @ParameterizedTest with @MethodSource correctly placed, the parameters now the parameters of the test method. But fields are still here, and constructor is renamed to some method, which is now called at the beginning of the test. And this method still has super() call in it, so we have a compile error.
   2. JsonPatchDiffTest test still has this.fieldName in the test (two times) after the rewrite JsonPatch diff = Json.createDiff(this.original, this.target);
   3. JsonPatchTest test originally having variable with the same name as this.fieldName, now also a parameter with same name - not easy to rewrite.
5. TestProviderTest wasn't changed - it's the only test in the "customprovider-jdk9" module; Assert.assertTrue and the wrong import is here; @Test isn't imported either
6. JsonParserTest, JsonObjectTest, JsonArrayTest, JsonReaderTest, JsonBigDecimalLengthLimitTest, JsonBigDecimalScaleLimitTest, JsonNumberTest, JsonBuilderTest, JsonGeneratorTest, JsonParserFactoryTest, JsonStringTest, JsonGeneratorFactoryTest, JsonWriterTest, aren't TestCase anymore - but constructor(String) still calling super(String)
7. Converting TestCase, private testXXX methods are also got @Test annotation
   But after this - 380 test were successfully converted! ;-)

What is the full stack trace of any errors you encountered?

No stacktrace

Are you interested in contributing a fix to OpenRewrite?

I would like to, but first I have some questions for every item in the list, which should be clarified before the start.

1-3, 5: this is maybe not a problem with recipe, but with framework? Or maybe with a recipe? I don't know where to look for in case of this missing dependency declarations.

4.I took a look on the ParameterizedRunnerToParameterized and ParameterizedRunnerToParameterizedTest because I thought that the bug is here, but it is how this recipe was developed and tested. And now I'm even more confused.

• So before in JUnit 4 we can inject the values through the constructor of the class or through the fields of the class. Now in JUnit5 we have shortcut this, so the values a directly the parameters of the test. There is no need for constructor or fields, no more.
• But the test of this recipe assures that this happens exact this way. So my first question is: what for?
• We can simply eradicate both the constructor and the fields, because there is no use for that. Or did I miss something?

6. This should be easy. Because we don't have any more inheritance, we don't need to call the super constructor, at least not the one with String parameter. But then I thought further: Do we need any kind of constructor here? In my tests I have no inheritance but from the Object, so I would suggest a general rule for the tests which erase all constructors and super calls? But maybe the last is a bridge too far?
   For the rest, there are some other recipes which assures that tests are not public, but somehow it doesn't work. Even worse - there is a code in the recipe (MigrateJUnitTestCase and MigrateJUnitTestCaseTest) which promotes a non-public method to public. There is no need for this.

7. This also should be easy: we should check that the method isn't private. The private method, even with the testat the beginning isn't a test, but some helpful method inside the class. They shouldn't get a @Test annotation.