I'm also +1 on option 2. If there are no other objections, then I would go ahead and merge https://github.com/apache/iceberg/pull/9161 at the end of the week. That should give people some time to add their feedback to the PR.
Thanks Eduard On Thu, Dec 7, 2023 at 6:06 PM Jack Ye <yezhao...@gmail.com> wrote: > Looking at the referenced open issue on Junit5 side, it seems like at > least the community is actively working on a @ParameterizedContainer > solution. In that case I would +1 for option 2, since we have an easy path > for moving to the official class-level annotation when that is ready. > > -Jack > > On Thu, Dec 7, 2023 at 8:27 AM Jean-Baptiste Onofré <j...@nanthrax.net> > wrote: > >> Hi Ed, >> >> I like option 2, it simplifies the parameterized support. >> >> Just my $0.01 >> >> Regards >> JB >> >> On Thu, Dec 7, 2023 at 5:16 PM Eduard Tudenhoefner < >> etudenhoef...@apache.org> wrote: >> >>> Hey everyone, >>> >>> a while ago we (thanks to many contributors that helped here) started >>> migrating tests from JUnit4 to JUnit5. While most things are >>> straightforward to migrate, parameterized tests are not. >>> >>> JUnit5 doesn't support parameterization at the class level yet >>> <https://github.com/junit-team/junit5/issues/878>, which makes it >>> difficult to migrate existing parameterized tests to JUnit5. >>> >>> There are a few possible options and I'd like to see what the community >>> thinks here: >>> >>> 1) When migrating to JUnit5, parameterized testing is moved to every >>> test method, meaning that every test method would have a >>> *@ParameterizedTest* and a *@ValueSource/@MethodSource *annotation. >>> >>> This is the official JUnit5 approach that is recommended. However, it is >>> quite repetitive, as every single test method now needs to define its own >>> parameters. >>> One example of this approach can be seen in #9185 >>> <https://github.com/apache/iceberg/pull/9185>, where *TestFlinkScan* is >>> migrated to JUnit5. >>> >>> 2) We could introduce a similar approach that the Flink community did >>> with https://issues.apache.org/jira/browse/FLINK-25315 / >>> apache/flink#20145 <https://github.com/apache/flink/pull/20145>. >>> This would introduce a custom *ParameterizedTestExtension* with two >>> additional *@Parameter / **@Parameters* annotations to mimic the same >>> behavior that JUnit4 provided. >>> One example of this approach can be seen in #9161 >>> <https://github.com/apache/iceberg/pull/9161>, where >>> *TestDictionaryRowGroupFilter* is migrated to use this way of >>> parameterized testing. >>> >>> I guess another option would be to wait until this is properly supported >>> by JUnit5, but it is unclear when that will be. >>> >>> >>> Eduard >>> >>> >>> >>>
