Hi Panagiotis, afaiu, those modules left are big and complex modules that still need a lot of effort to migrate to junit5. Overall, agree with you to enable suppressions.
Best regards, Jing On Mon, May 1, 2023 at 7:57 AM Panagiotis Garefalakis <pga...@apache.org> wrote: > Hey Jing, > > That's basically up to us to decide -- maybe a continuous approach is > safer? (tests with junit4 behaviour won't be mergeable if we enable rules > immediately) > That said, looks like the migration work is almost there > <https://issues.apache.org/jira/browse/FLINK-25325> -- with some effort we > could minimize the need for suppressions altogether. > > Cheers, > Panagiotis > > On Sun, Apr 30, 2023 at 6:16 AM Jing Ge <j...@ververica.com.invalid> > wrote: > > > Thanks @Panagiotis for the hint! Does it mean that those suppressions > need > > to be cleaned up continuously while we move forward with Junit5 > > migration(extra guideline is required), even if regex has been used? Or > we > > just leave them as they are and clean them up in one shot after every > > junit4 has been migrated to junit5. > > > > Best regards, > > Jing > > > > On Wed, Apr 26, 2023 at 9:02 AM Panagiotis Garefalakis < > pga...@apache.org> > > wrote: > > > > > Thanks for bringing this up! +1 for the proposal > > > > > > @Jing Ge -- we don't necessarily need to completely migrate to Junit5 > > (even > > > though it would be ideal). > > > We could introduce the checkstyle rule and add suppressions for the > > > existing problematic paths (as we do today for other rules e.g., > > > AvoidStarImport) > > > > > > Cheers, > > > Panagiotis > > > > > > On Tue, Apr 25, 2023 at 11:48 PM Weihua Hu <huweihua....@gmail.com> > > wrote: > > > > > > > Thanks for driving this. > > > > > > > > +1 for Mockito and Junit4. > > > > > > > > A clarity checkstyle will be of great help to new developers. > > > > > > > > Best, > > > > Weihua > > > > > > > > > > > > On Wed, Apr 26, 2023 at 1:47 PM Jing Ge <j...@ververica.com.invalid> > > > > wrote: > > > > > > > > > This is a great idea, thanks for bringing this up. +1 > > > > > > > > > > Also +1 for Junit4. If I am not mistaken, it could only be done > after > > > the > > > > > Junit5 migration is done. > > > > > > > > > > @Chesnay thanks for the hint. Do we have any doc about it? If not, > it > > > > might > > > > > deserve one. WDYT? > > > > > > > > > > Best regards, > > > > > Jing > > > > > > > > > > On Wed, Apr 26, 2023 at 5:13 AM Lijie Wang < > wangdachui9...@gmail.com > > > > > > > > wrote: > > > > > > > > > > > Thanks for driving this. +1 for the proposal. > > > > > > > > > > > > Can we also prevent Junit4 usage in new code by this way？Because > > > > > currently > > > > > > we are aiming to migrate our codebase to JUnit 5. > > > > > > > > > > > > Best, > > > > > > Lijie > > > > > > > > > > > > Piotr Nowojski <pnowoj...@apache.org> 于2023年4月25日周二 23:02写道： > > > > > > > > > > > > > Ok, thanks for the clarification. > > > > > > > > > > > > > > Piotrek > > > > > > > > > > > > > > wt., 25 kwi 2023 o 16:38 Chesnay Schepler <ches...@apache.org> > > > > > > napisał(a): > > > > > > > > > > > > > > > The checkstyle rule would just ban certain imports. > > > > > > > > We'd add exclusions for all existing usages as we did when > > > > > introducing > > > > > > > > other rules. > > > > > > > > So far we usually disabled checkstyle rules for a specific > > files. > > > > > > > > > > > > > > > > On 25/04/2023 16:34, Piotr Nowojski wrote: > > > > > > > > > +1 to the idea. > > > > > > > > > > > > > > > > > > How would this checkstyle rule work? Are you suggesting to > > > start > > > > > > with a > > > > > > > > > number of exclusions? On what level will those exclusions > be? > > > Per > > > > > > file? > > > > > > > > Per > > > > > > > > > line? > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > Piotrek > > > > > > > > > > > > > > > > > > wt., 25 kwi 2023 o 13:18 David Morávek <d...@apache.org> > > > > > napisał(a): > > > > > > > > > > > > > > > > > >> Hi Everyone, > > > > > > > > >> > > > > > > > > >> A long time ago, the community decided not to use > > > Mockito-based > > > > > > tests > > > > > > > > >> because those are hard to maintain. This is already baked > in > > > our > > > > > > Code > > > > > > > > Style > > > > > > > > >> and Quality Guide [1]. > > > > > > > > >> > > > > > > > > >> Because we still have Mockito imported into the code base, > > > it's > > > > > very > > > > > > > > easy > > > > > > > > >> for newcomers to unconsciously introduce new tests > violating > > > the > > > > > > code > > > > > > > > style > > > > > > > > >> because they're unaware of the decision. > > > > > > > > >> > > > > > > > > >> I propose to prevent Mockito usage with a Checkstyle rule > > for > > > a > > > > > new > > > > > > > > code, > > > > > > > > >> which would eventually allow us to eliminate it. This > could > > > also > > > > > > > prevent > > > > > > > > >> some wasted work and unnecessary feedback cycles during > > > reviews. > > > > > > > > >> > > > > > > > > >> WDYT? > > > > > > > > >> > > > > > > > > >> [1] > > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://flink.apache.org/how-to-contribute/code-style-and-quality-common/#avoid-mockito---use-reusable-test-implementations > > > > > > > > >> > > > > > > > > >> Best, > > > > > > > > >> D. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
