Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-05-02 Thread Jing Ge
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 
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
>  -- with some effort we
> could minimize the need for suppressions altogether.
>
> Cheers,
> Panagiotis
>
> On Sun, Apr 30, 2023 at 6:16 AM Jing Ge 
> 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 
> > 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 
> > > > 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  于2023年4月25日周二 23:02写道:
> > > > > >
> > > > > > > Ok, thanks for the clarification.
> > > > > > >
> > > > > > > Piotrek
> > > > > > >
> > > > > > > wt., 25 kwi 2023 o 16:38 Chesnay Schepler 
> > > > > > 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 
> > > > > 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]
> > > > > > > > >>
> > > > > > > > >>
> > > > > > > >
> > > > > > 

Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-30 Thread Panagiotis Garefalakis
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
 -- with some effort we
could minimize the need for suppressions altogether.

Cheers,
Panagiotis

On Sun, Apr 30, 2023 at 6:16 AM Jing Ge  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 
> 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 
> 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 
> > > 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  >
> > > > 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  于2023年4月25日周二 23:02写道:
> > > > >
> > > > > > Ok, thanks for the clarification.
> > > > > >
> > > > > > Piotrek
> > > > > >
> > > > > > wt., 25 kwi 2023 o 16:38 Chesnay Schepler 
> > > > > 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 
> > > > 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.
> > > > > > > >>
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>


Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-30 Thread Jing Ge
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 
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  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 
> > 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 
> > > 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  于2023年4月25日周二 23:02写道:
> > > >
> > > > > Ok, thanks for the clarification.
> > > > >
> > > > > Piotrek
> > > > >
> > > > > wt., 25 kwi 2023 o 16:38 Chesnay Schepler 
> > > > 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 
> > > 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.
> > > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>


Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-27 Thread David Morávek
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 
wrote:

> +1 for introducing this rule for junit4 and mockito.
>
> Best regards,
>
> Weijie
>
>
> Alexander Fedulov  于2023年4月26日周三 23:50写道:
>
> > +1 for the proposal,
> >
> > Best,
> > Alex
> >
> > On Wed, 26 Apr 2023 at 15:50, Chesnay Schepler 
> 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
> > > >  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 
> > > 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  >
> > > >>> 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  于2023年4月25日周二 23:02写道:
> > > >
> > > >> Ok, thanks for the clarification.
> > > >>
> > > >> Piotrek
> > > >>
> > > >> wt., 25 kwi 2023 o 16:38 Chesnay Schepler 
> > > > 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 
> > >  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.
> > > >
> > > >>>

Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-27 Thread weijie guo
+1 for introducing this rule for junit4 and mockito.

Best regards,

Weijie


Alexander Fedulov  于2023年4月26日周三 23:50写道:

> +1 for the proposal,
>
> Best,
> Alex
>
> On Wed, 26 Apr 2023 at 15:50, Chesnay Schepler  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
> > >  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 
> > 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 
> > >>> 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  于2023年4月25日周二 23:02写道:
> > >
> > >> Ok, thanks for the clarification.
> > >>
> > >> Piotrek
> > >>
> > >> wt., 25 kwi 2023 o 16:38 Chesnay Schepler 
> > > 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 
> >  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.
> > >
> > >>>
> >
> >
>


Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-26 Thread Alexander Fedulov
+1 for the proposal,

Best,
Alex

On Wed, 26 Apr 2023 at 15:50, Chesnay Schepler  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
> >  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 
> 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 
> >>> 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 
>  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  于2023年4月25日周二 23:02写道:
> >
> >> Ok, thanks for the clarification.
> >>
> >> Piotrek
> >>
> >> wt., 25 kwi 2023 o 16:38 Chesnay Schepler 
> > 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 
>  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.
> >
> >>>
>
>


Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-26 Thread Chesnay Schepler

* 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
 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  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 
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 
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  于2023年4月25日周二 23:02写道:


Ok, thanks for the clarification.

Piotrek

wt., 25 kwi 2023 o 16:38 Chesnay Schepler 

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 

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.







Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-26 Thread Maximilian Michels
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
 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  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 
> > 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 
> > > 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  于2023年4月25日周二 23:02写道:
> > > >
> > > > > Ok, thanks for the clarification.
> > > > >
> > > > > Piotrek
> > > > >
> > > > > wt., 25 kwi 2023 o 16:38 Chesnay Schepler 
> > > > 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 
> > > 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.
> > > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >


Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-26 Thread Panagiotis Garefalakis
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  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 
> 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 
> > 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  于2023年4月25日周二 23:02写道:
> > >
> > > > Ok, thanks for the clarification.
> > > >
> > > > Piotrek
> > > >
> > > > wt., 25 kwi 2023 o 16:38 Chesnay Schepler 
> > > 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 
> > 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.
> > > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>


Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-26 Thread Weihua Hu
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  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 
> 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  于2023年4月25日周二 23:02写道:
> >
> > > Ok, thanks for the clarification.
> > >
> > > Piotrek
> > >
> > > wt., 25 kwi 2023 o 16:38 Chesnay Schepler 
> > 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 
> 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.
> > > > >>
> > > >
> > > >
> > >
> >
>


Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-25 Thread Jing Ge
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  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  于2023年4月25日周二 23:02写道:
>
> > Ok, thanks for the clarification.
> >
> > Piotrek
> >
> > wt., 25 kwi 2023 o 16:38 Chesnay Schepler 
> 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  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.
> > > >>
> > >
> > >
> >
>


Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-25 Thread Lijie Wang
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  于2023年4月25日周二 23:02写道:

> Ok, thanks for the clarification.
>
> Piotrek
>
> wt., 25 kwi 2023 o 16:38 Chesnay Schepler  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  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.
> > >>
> >
> >
>


Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-25 Thread Piotr Nowojski
Ok, thanks for the clarification.

Piotrek

wt., 25 kwi 2023 o 16:38 Chesnay Schepler  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  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.
> >>
>
>


Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-25 Thread Chesnay Schepler

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  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.





Re: [DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-25 Thread Piotr Nowojski
+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  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.
>


[DISCUSS] Preventing Mockito usage for the new code with Checkstyle

2023-04-25 Thread David Morávek
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.