Hello! I can help out with the effort -- I've got a bit of experience with JUnit 4 and 5 migration, and it looks like even with the AssertJ scripts there's going to be a lot of mechanical and manual work to be done. The migration document looks pretty comprehensive!
For the remaining topics to be discussed: I don't have a strong opinion on what to do about parameterized tests that use inheritance, although Jing Ge's proposal sounds reasonable and easy to follow. I wouldn't be worried about temporarily redundant test code too much if it simplifies getting us into a good final state, especially since redundant code would be easy to spot and remove when we get rid of JUnit 4 artifacts. Getting rid of PowerMock sounds fine to me. I don't think it's necessary to have a common author for commits, given that commits will have the [JUnit5 migration] tag. I guess my preference would be to have "one or a few" commits per module, merged progressively. Is there an existing branch on a repo with some of the modules already migrated? All my best, Ryan On Fri, Dec 17, 2021 at 5:19 PM Jing Ge <j...@ververica.com> wrote: > Thanks Hang and Qingsheng for your effort and starting this discussion. As > additional information, I've created an umbrella ticket( > https://issues.apache.org/jira/browse/FLINK-25325). It is recommended to > create all JUnit5 migration related tasks under it, So we could track the > whole migration easily. > > I think, for the parameterized test issue, the major problem is that, on > one hand, JUnit 5 has its own approach to make parameterized tests and it > does not allow to use parameterized fixtures at class level. This is a huge > difference compared to JUnit4. On the other hand, currently, there are many > cross module test class inheritances, which means that the migration could > not be done in one shot. It must be allowed to run JUnit4 and JUnit5 tests > simultaneously during the migration process. As long as there are sub > parameterized test classes in JUnit4, it will be risky to migrate the > parent class to JUnit5. And if the parent class has to stick with JUnit4 > during the migration, any migrated JUnit5 subclass might need to duplicate > the test methods defined in the parent class. In this case, I would prefer > to duplicate the test methods with different names in the parent class for > both JUnit4 and JUnit5 only during the migration process as temporary > solution and remove the test methods for JUnit4 once the migration process > is finished, i.e. when all subclasses are JUnit5 tests. It is a trade-off > solution. Hopefully we could find another better solution during the > discussion. > > Speaking of replacing @Test with @TestTemplate, since I did read all tests, > do we really need to replace all of them with @TestTemplate w.r.t. the > parameterized tests? > > For the PowrMock tests, it is a good opportunity to remove them. > > best regards > Jing > > On Fri, Dec 17, 2021 at 2:14 PM Hang Ruan <ruanhang1...@gmail.com> wrote: > > > Hi, all, > > > > Apache Flink is using JUnit for unit tests and integration tests widely > in > > the project, however, it binds to the legacy JUnit 4 deeply. It is > > important to migrate existing cases to JUnit 5 in order to avoid > splitting > > the project into different JUnit versions. > > > > Qingsheng Ren and I have conducted some trials about the JUnit 5 > migration, > > but there are too many modules that need to migrate. We would like to get > > more help from the community. It is planned to migrate module by module, > > and a JUnit 5 migration guide > > < > > > https://docs.google.com/document/d/1514Wa_aNB9bJUen4xm5uiuXOooOJTtXqS_Jqk9KJitU/edit?usp=sharing > > >[1] > > has been provided to new helpers on the cooperation method and how to > > migrate. > > > > There are still some problem to discuss: > > > > 1. About parameterized test: > > > > Some test classes inherit from other base test classes. We have discussed > > different situations in the guide, but the situation where a > parameterized > > test subclass inherits from a non parameterized parent class has not been > > resolved. > > > > In JUnit 4, the parent test class always has some test cases annotated by > > @Test. And the parameterized subclass will run these test cases in the > > parent class in a parameterized way. > > > > In JUnit 5, if we want a test case to be invoked multiple times, the test > > case must be annotated by @TestTemplate. A test case can not be annotated > > by both @Test and @TestTemplate, which means a test case can not be > invoked > > as both a parameterized test and a non parameterized test. > > > > We thought of two ways to migrate this situation, but not good enough. > Both > > two ways will introduce redundant codes, and make it hard to maintain. > > > > The first way is to change the parent class to a parameterized test and > > replace @Test tests to @TestTemplate tests. For its non parameterized > > subclasses, we provide them a fake parameter method, which will provide > > nothing. > > > > The second way is to change the parameterized subclasses. We should > > override all @Test tests in the parent class and annotate them with > > @TestTemplate, these methods in the parameterized subclass should invoke > > the same method in its parent class. > > > > > > 2. About PowerMock: > > > > According to the code style rules[2] of Flink project, it’s discouraged > to > > use mockito for test cases, as well as Powermock, an extension of > mockito. > > Considering the situation that PowerMock lacks JUnit 5 support [3], we > > suggest rewriting Powermock-based test class (~10 classes) with reusable > > test implementations, and finally deprecate Powermock from the project. > > > > > > 3. Commit Author: > > > > JUnit 5 migration will cause a lot of changed codes. Maybe we should use > a > > common author for the JUnit 5 migration commits. > > > > Looking forward to your suggestions, thanks! > > > > Best, > > > > Hang > > > > [1] > > > > > https://docs.google.com/document/d/1514Wa_aNB9bJUen4xm5uiuXOooOJTtXqS_Jqk9KJitU/edit?usp=sharing > > > > [2] > > > > > https://flink.apache.org/contributing/code-style-and-quality-common.html#avoid-mockito---use-reusable-test-implementations > > > > > > [3] https://github.com/powermock/powermock/issues/929 > > >
