Thanks, everyone, for participating. There seems to be a broad consensus, so I'll move forward. I've created [1] and [2] to track this.
[1] https://issues.apache.org/jira/browse/FLINK-31954 [2] https://issues.apache.org/jira/browse/FLINK-31955 Best, D. On Thu, Apr 27, 2023 at 9:25 AM weijie guo <guoweijieres...@gmail.com> wrote: > +1 for introducing this rule for junit4 and mockito. > > Best regards, > > Weijie > > > Alexander Fedulov <alexander.fedu...@gmail.com> 于2023年4月26日周三 23:50写道： > > > +1 for the proposal, > > > > Best, > > Alex > > > > On Wed, 26 Apr 2023 at 15:50, Chesnay Schepler <ches...@apache.org> > wrote: > > > > > * adds a note to not include "import " in the regex" * > > > > > > On 26/04/2023 11:22, Maximilian Michels wrote: > > > > If we ban Mockito imports, I can still write tests using the full > > > > qualifiers, right? > > > > > > > > For example: > > > > > > > > > > org.mockito.Mockito.when(somethingThatShouldHappen).thenReturn(somethingThatNeverActuallyHappens) > > > > > > > > Just kidding, +1 on the proposal. > > > > > > > > -Max > > > > > > > > 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. > > > >>>>>>>>> > > > >>>>>>> > > > > > > > > >
