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 >
