You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hudi.apache.org by Shiyan Xu <xu...@gmail.com> on 2020/03/25 04:27:09 UTC

[DISCUSS] Upgrade unit test: Junit 5 & AssertJ

Hi all,

I'd like to gather some feedback about
1. upgrading Junit 4 to 5
2. adopt AssertJ as preferred assertion statement style

IMO 1) will give many benefits on writing better unit tests. A google
search of "junit 4 vs 5" could lead to many good points. And it is some
migration can be done piece by piece (keeping both 4 and 5 during upgrade
and enforce new test using 5)

2) is to spice things up and bring the test readability to another level,
though I'll treat it as nice-to-have.

Would you +1 or -1 on either or both?

Knowing that it'll be a long way to go due to the large number of tests,
this needs to be planned and tracked carefully.

Thank you.

Best,
Raymond

Re: [DISCUSS] Upgrade unit test: Junit 5 & AssertJ

Posted by Vinoth Chandar <vi...@apache.org>.
Thanks Raymond.. We can continue engaging on the ticket!

On Thu, Apr 9, 2020 at 4:54 PM Shiyan Xu <xu...@gmail.com>
wrote:

> Filed!
> https://issues.apache.org/jira/browse/HUDI-779
>
> On Wed, Apr 8, 2020 at 11:05 PM Vinoth Chandar <vi...@apache.org> wrote:
>
> > +1 on an umbrella task..
> >
> > We can do the RFC for overhaul of tests (mocking more tests, cleaning up
> > test data gen and so on)..
> > For adding junit5 itself and doing the initial work, we could just begin
> > with JIRA?
> >
> > On Wed, Apr 8, 2020 at 12:56 PM Shiyan Xu <xu...@gmail.com>
> > wrote:
> >
> > > Thank you all for the feedback.
> > >
> > > > This increases the scope to a overhaul of tests across the project..
> > > Wonder if we can do a RFC for this?
> > > Indeed it is overhaul type of change. IMO RFC is needed specifically
> for
> > > the test utility re-design part. Guess it can be created when it's good
> > to
> > > start? Since it'll be a long-running task, have an umbrella ticket for
> > this
> > > topic first? @vinoth
> > >
> > >
> > > On Sat, Apr 4, 2020 at 2:47 AM leesf <le...@gmail.com> wrote:
> > >
> > > > +1 to upgrade the unit test. junit5 combine better with java8, and
> > there
> > > > are some migration guides already, and we maybe could upgrade by
> > module.
> > > >
> > > > vino yang <vi...@apache.org> 于2020年4月2日周四 下午4:38写道:
> > > >
> > > > > Hi Shiyan,
> > > > >
> > > > > +1 from my side.
> > > > >
> > > > > Best,
> > > > > Vino
> > > > >
> > > > > Vinoth Chandar <vi...@apache.org> 于2020年3月30日周一 下午11:00写道:
> > > > >
> > > > > > Hi Raymond,
> > > > > >
> > > > > > Sounds good to me. This increases the scope to a overhaul of
> tests
> > > > across
> > > > > > the project.. Wonder if we can do a RFC for this? But overall +1
> > from
> > > > me.
> > > > > >
> > > > > > I would like to call upon the community to chime in more though
> :)
> > .
> > > > > let's
> > > > > > give it a few days..
> > > > > >
> > > > > >
> > > > > > Thanks
> > > > > > Vinoth
> > > > > >
> > > > > > On Fri, Mar 27, 2020 at 5:18 PM Shiyan Xu <
> > > xu.shiyan.raymond@gmail.com
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Understand Vinoth. To me AssertJ is nice-to-have. I agree with
> > the
> > > > > > learning
> > > > > > > overhead.
> > > > > > >
> > > > > > > The current CI time is too long and we do need to use more
> > mocking
> > > > and
> > > > > > > optimize spark jobs setup.
> > > > > > >
> > > > > > > Based on your points, I imagine the path forward can be planned
> > as
> > > > this
> > > > > > >
> > > > > > > 1. An initial PR to add Junit 5 to co-exist with 4 in the
> project
> > > > with
> > > > > a
> > > > > > > simple testcase converted to 5 as a working proof
> > > > > > > 2. A design task to refactor test utilities (create new
> utilities
> > > > with
> > > > > > > Junit 5 for easy switch-over of affected testcases)
> > > > > > > 3. Track all test improvement PRs (using Junit 5). Each PR
> should
> > > aim
> > > > > to
> > > > > > > solve 1 of the problems below
> > > > > > >   - test can be improved with mocking
> > > > > > >   - test can be optimized on spark job setup
> > > > > > > 4. Clean unused test utilities (from step 2)
> > > > > > >
> > > > > > > We should recognize these steps to be carried out in a
> > long-running
> > > > > > ongoing
> > > > > > > fashion.
> > > > > > >
> > > > > > > Any thoughts or feedback?
> > > > > > >
> > > > > > > On Wed, Mar 25, 2020 at 7:52 AM Vinoth Chandar <
> > vinoth@apache.org>
> > > > > > wrote:
> > > > > > >
> > > > > > > > +1 on Junit5.
> > > > > > > >  does seem nicer with support for lambdas. assuming we do a
> > > gradual
> > > > > > > > rollout. At any point, we cannot have any of the core tests
> > > > disabled
> > > > > :)
> > > > > > > > May be we can use the vintage framework for now, do minimal
> > > changes
> > > > > > > migrate
> > > > > > > > and then proceed to redoing the tests
> > > > > > > >
> > > > > > > > On AssertJ type frameworks, I wonder if there is a cost to
> this
> > > > type
> > > > > of
> > > > > > > > framework for new devs.
> > > > > > > > They already need to learn junit 5, mockito, all the
> TestUtils
> > > and
> > > > > like
> > > > > > > one
> > > > > > > > more framework for asserting
> > > > > > > >
> > > > > > > > Orthogonally, I will be thrilled if you also took upon a
> large
> > > > > > > > restructuring on tests cleanly into
> > > > > > > > - unit tests that test class functionality using mocks
> > > > > > > > - functional tests that bring up a spark context and actually
> > run
> > > > the
> > > > > > job
> > > > > > > > (we have a lot of these tests masquerading as unit tests)
> > > > > > > > - Clean redesign of the test utility classes
> > > > > > > >
> > > > > > > > Sorry to expand scope, but when someone is going to take a
> look
> > > at
> > > > > > every
> > > > > > > > test, I could not pass up an opportunity to sneak this in :)
> > > > > > > >
> > > > > > > > Love to hear others thoughts.. any one with experience
> working
> > > with
> > > > > > > > Junit5/Assertj-Hamcrest?
> > > > > > > >
> > > > > > > > On Tue, Mar 24, 2020 at 9:36 PM Shiyan Xu <
> > > > > xu.shiyan.raymond@gmail.com
> > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Some references
> > > > > > > > > https://junit.org/junit5/docs/current/user-guide/
> > > > > > > > > https://joel-costigliola.github.io/assertj/
> > > > > > > > >
> > > > > > > > > On Tue, Mar 24, 2020 at 9:27 PM Shiyan Xu <
> > > > > > xu.shiyan.raymond@gmail.com
> > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi all,
> > > > > > > > > >
> > > > > > > > > > I'd like to gather some feedback about
> > > > > > > > > > 1. upgrading Junit 4 to 5
> > > > > > > > > > 2. adopt AssertJ as preferred assertion statement style
> > > > > > > > > >
> > > > > > > > > > IMO 1) will give many benefits on writing better unit
> > tests.
> > > A
> > > > > > google
> > > > > > > > > > search of "junit 4 vs 5" could lead to many good points.
> > And
> > > it
> > > > > is
> > > > > > > some
> > > > > > > > > > migration can be done piece by piece (keeping both 4 and
> 5
> > > > during
> > > > > > > > upgrade
> > > > > > > > > > and enforce new test using 5)
> > > > > > > > > >
> > > > > > > > > > 2) is to spice things up and bring the test readability
> to
> > > > > another
> > > > > > > > level,
> > > > > > > > > > though I'll treat it as nice-to-have.
> > > > > > > > > >
> > > > > > > > > > Would you +1 or -1 on either or both?
> > > > > > > > > >
> > > > > > > > > > Knowing that it'll be a long way to go due to the large
> > > number
> > > > of
> > > > > > > > tests,
> > > > > > > > > > this needs to be planned and tracked carefully.
> > > > > > > > > >
> > > > > > > > > > Thank you.
> > > > > > > > > >
> > > > > > > > > > Best,
> > > > > > > > > > Raymond
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Upgrade unit test: Junit 5 & AssertJ

Posted by Shiyan Xu <xu...@gmail.com>.
Filed!
https://issues.apache.org/jira/browse/HUDI-779

On Wed, Apr 8, 2020 at 11:05 PM Vinoth Chandar <vi...@apache.org> wrote:

> +1 on an umbrella task..
>
> We can do the RFC for overhaul of tests (mocking more tests, cleaning up
> test data gen and so on)..
> For adding junit5 itself and doing the initial work, we could just begin
> with JIRA?
>
> On Wed, Apr 8, 2020 at 12:56 PM Shiyan Xu <xu...@gmail.com>
> wrote:
>
> > Thank you all for the feedback.
> >
> > > This increases the scope to a overhaul of tests across the project..
> > Wonder if we can do a RFC for this?
> > Indeed it is overhaul type of change. IMO RFC is needed specifically for
> > the test utility re-design part. Guess it can be created when it's good
> to
> > start? Since it'll be a long-running task, have an umbrella ticket for
> this
> > topic first? @vinoth
> >
> >
> > On Sat, Apr 4, 2020 at 2:47 AM leesf <le...@gmail.com> wrote:
> >
> > > +1 to upgrade the unit test. junit5 combine better with java8, and
> there
> > > are some migration guides already, and we maybe could upgrade by
> module.
> > >
> > > vino yang <vi...@apache.org> 于2020年4月2日周四 下午4:38写道:
> > >
> > > > Hi Shiyan,
> > > >
> > > > +1 from my side.
> > > >
> > > > Best,
> > > > Vino
> > > >
> > > > Vinoth Chandar <vi...@apache.org> 于2020年3月30日周一 下午11:00写道:
> > > >
> > > > > Hi Raymond,
> > > > >
> > > > > Sounds good to me. This increases the scope to a overhaul of tests
> > > across
> > > > > the project.. Wonder if we can do a RFC for this? But overall +1
> from
> > > me.
> > > > >
> > > > > I would like to call upon the community to chime in more though :)
> .
> > > > let's
> > > > > give it a few days..
> > > > >
> > > > >
> > > > > Thanks
> > > > > Vinoth
> > > > >
> > > > > On Fri, Mar 27, 2020 at 5:18 PM Shiyan Xu <
> > xu.shiyan.raymond@gmail.com
> > > >
> > > > > wrote:
> > > > >
> > > > > > Understand Vinoth. To me AssertJ is nice-to-have. I agree with
> the
> > > > > learning
> > > > > > overhead.
> > > > > >
> > > > > > The current CI time is too long and we do need to use more
> mocking
> > > and
> > > > > > optimize spark jobs setup.
> > > > > >
> > > > > > Based on your points, I imagine the path forward can be planned
> as
> > > this
> > > > > >
> > > > > > 1. An initial PR to add Junit 5 to co-exist with 4 in the project
> > > with
> > > > a
> > > > > > simple testcase converted to 5 as a working proof
> > > > > > 2. A design task to refactor test utilities (create new utilities
> > > with
> > > > > > Junit 5 for easy switch-over of affected testcases)
> > > > > > 3. Track all test improvement PRs (using Junit 5). Each PR should
> > aim
> > > > to
> > > > > > solve 1 of the problems below
> > > > > >   - test can be improved with mocking
> > > > > >   - test can be optimized on spark job setup
> > > > > > 4. Clean unused test utilities (from step 2)
> > > > > >
> > > > > > We should recognize these steps to be carried out in a
> long-running
> > > > > ongoing
> > > > > > fashion.
> > > > > >
> > > > > > Any thoughts or feedback?
> > > > > >
> > > > > > On Wed, Mar 25, 2020 at 7:52 AM Vinoth Chandar <
> vinoth@apache.org>
> > > > > wrote:
> > > > > >
> > > > > > > +1 on Junit5.
> > > > > > >  does seem nicer with support for lambdas. assuming we do a
> > gradual
> > > > > > > rollout. At any point, we cannot have any of the core tests
> > > disabled
> > > > :)
> > > > > > > May be we can use the vintage framework for now, do minimal
> > changes
> > > > > > migrate
> > > > > > > and then proceed to redoing the tests
> > > > > > >
> > > > > > > On AssertJ type frameworks, I wonder if there is a cost to this
> > > type
> > > > of
> > > > > > > framework for new devs.
> > > > > > > They already need to learn junit 5, mockito, all the TestUtils
> > and
> > > > like
> > > > > > one
> > > > > > > more framework for asserting
> > > > > > >
> > > > > > > Orthogonally, I will be thrilled if you also took upon a large
> > > > > > > restructuring on tests cleanly into
> > > > > > > - unit tests that test class functionality using mocks
> > > > > > > - functional tests that bring up a spark context and actually
> run
> > > the
> > > > > job
> > > > > > > (we have a lot of these tests masquerading as unit tests)
> > > > > > > - Clean redesign of the test utility classes
> > > > > > >
> > > > > > > Sorry to expand scope, but when someone is going to take a look
> > at
> > > > > every
> > > > > > > test, I could not pass up an opportunity to sneak this in :)
> > > > > > >
> > > > > > > Love to hear others thoughts.. any one with experience working
> > with
> > > > > > > Junit5/Assertj-Hamcrest?
> > > > > > >
> > > > > > > On Tue, Mar 24, 2020 at 9:36 PM Shiyan Xu <
> > > > xu.shiyan.raymond@gmail.com
> > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Some references
> > > > > > > > https://junit.org/junit5/docs/current/user-guide/
> > > > > > > > https://joel-costigliola.github.io/assertj/
> > > > > > > >
> > > > > > > > On Tue, Mar 24, 2020 at 9:27 PM Shiyan Xu <
> > > > > xu.shiyan.raymond@gmail.com
> > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi all,
> > > > > > > > >
> > > > > > > > > I'd like to gather some feedback about
> > > > > > > > > 1. upgrading Junit 4 to 5
> > > > > > > > > 2. adopt AssertJ as preferred assertion statement style
> > > > > > > > >
> > > > > > > > > IMO 1) will give many benefits on writing better unit
> tests.
> > A
> > > > > google
> > > > > > > > > search of "junit 4 vs 5" could lead to many good points.
> And
> > it
> > > > is
> > > > > > some
> > > > > > > > > migration can be done piece by piece (keeping both 4 and 5
> > > during
> > > > > > > upgrade
> > > > > > > > > and enforce new test using 5)
> > > > > > > > >
> > > > > > > > > 2) is to spice things up and bring the test readability to
> > > > another
> > > > > > > level,
> > > > > > > > > though I'll treat it as nice-to-have.
> > > > > > > > >
> > > > > > > > > Would you +1 or -1 on either or both?
> > > > > > > > >
> > > > > > > > > Knowing that it'll be a long way to go due to the large
> > number
> > > of
> > > > > > > tests,
> > > > > > > > > this needs to be planned and tracked carefully.
> > > > > > > > >
> > > > > > > > > Thank you.
> > > > > > > > >
> > > > > > > > > Best,
> > > > > > > > > Raymond
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Upgrade unit test: Junit 5 & AssertJ

Posted by Vinoth Chandar <vi...@apache.org>.
+1 on an umbrella task..

We can do the RFC for overhaul of tests (mocking more tests, cleaning up
test data gen and so on)..
For adding junit5 itself and doing the initial work, we could just begin
with JIRA?

On Wed, Apr 8, 2020 at 12:56 PM Shiyan Xu <xu...@gmail.com>
wrote:

> Thank you all for the feedback.
>
> > This increases the scope to a overhaul of tests across the project..
> Wonder if we can do a RFC for this?
> Indeed it is overhaul type of change. IMO RFC is needed specifically for
> the test utility re-design part. Guess it can be created when it's good to
> start? Since it'll be a long-running task, have an umbrella ticket for this
> topic first? @vinoth
>
>
> On Sat, Apr 4, 2020 at 2:47 AM leesf <le...@gmail.com> wrote:
>
> > +1 to upgrade the unit test. junit5 combine better with java8, and there
> > are some migration guides already, and we maybe could upgrade by module.
> >
> > vino yang <vi...@apache.org> 于2020年4月2日周四 下午4:38写道:
> >
> > > Hi Shiyan,
> > >
> > > +1 from my side.
> > >
> > > Best,
> > > Vino
> > >
> > > Vinoth Chandar <vi...@apache.org> 于2020年3月30日周一 下午11:00写道:
> > >
> > > > Hi Raymond,
> > > >
> > > > Sounds good to me. This increases the scope to a overhaul of tests
> > across
> > > > the project.. Wonder if we can do a RFC for this? But overall +1 from
> > me.
> > > >
> > > > I would like to call upon the community to chime in more though :) .
> > > let's
> > > > give it a few days..
> > > >
> > > >
> > > > Thanks
> > > > Vinoth
> > > >
> > > > On Fri, Mar 27, 2020 at 5:18 PM Shiyan Xu <
> xu.shiyan.raymond@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > Understand Vinoth. To me AssertJ is nice-to-have. I agree with the
> > > > learning
> > > > > overhead.
> > > > >
> > > > > The current CI time is too long and we do need to use more mocking
> > and
> > > > > optimize spark jobs setup.
> > > > >
> > > > > Based on your points, I imagine the path forward can be planned as
> > this
> > > > >
> > > > > 1. An initial PR to add Junit 5 to co-exist with 4 in the project
> > with
> > > a
> > > > > simple testcase converted to 5 as a working proof
> > > > > 2. A design task to refactor test utilities (create new utilities
> > with
> > > > > Junit 5 for easy switch-over of affected testcases)
> > > > > 3. Track all test improvement PRs (using Junit 5). Each PR should
> aim
> > > to
> > > > > solve 1 of the problems below
> > > > >   - test can be improved with mocking
> > > > >   - test can be optimized on spark job setup
> > > > > 4. Clean unused test utilities (from step 2)
> > > > >
> > > > > We should recognize these steps to be carried out in a long-running
> > > > ongoing
> > > > > fashion.
> > > > >
> > > > > Any thoughts or feedback?
> > > > >
> > > > > On Wed, Mar 25, 2020 at 7:52 AM Vinoth Chandar <vi...@apache.org>
> > > > wrote:
> > > > >
> > > > > > +1 on Junit5.
> > > > > >  does seem nicer with support for lambdas. assuming we do a
> gradual
> > > > > > rollout. At any point, we cannot have any of the core tests
> > disabled
> > > :)
> > > > > > May be we can use the vintage framework for now, do minimal
> changes
> > > > > migrate
> > > > > > and then proceed to redoing the tests
> > > > > >
> > > > > > On AssertJ type frameworks, I wonder if there is a cost to this
> > type
> > > of
> > > > > > framework for new devs.
> > > > > > They already need to learn junit 5, mockito, all the TestUtils
> and
> > > like
> > > > > one
> > > > > > more framework for asserting
> > > > > >
> > > > > > Orthogonally, I will be thrilled if you also took upon a large
> > > > > > restructuring on tests cleanly into
> > > > > > - unit tests that test class functionality using mocks
> > > > > > - functional tests that bring up a spark context and actually run
> > the
> > > > job
> > > > > > (we have a lot of these tests masquerading as unit tests)
> > > > > > - Clean redesign of the test utility classes
> > > > > >
> > > > > > Sorry to expand scope, but when someone is going to take a look
> at
> > > > every
> > > > > > test, I could not pass up an opportunity to sneak this in :)
> > > > > >
> > > > > > Love to hear others thoughts.. any one with experience working
> with
> > > > > > Junit5/Assertj-Hamcrest?
> > > > > >
> > > > > > On Tue, Mar 24, 2020 at 9:36 PM Shiyan Xu <
> > > xu.shiyan.raymond@gmail.com
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Some references
> > > > > > > https://junit.org/junit5/docs/current/user-guide/
> > > > > > > https://joel-costigliola.github.io/assertj/
> > > > > > >
> > > > > > > On Tue, Mar 24, 2020 at 9:27 PM Shiyan Xu <
> > > > xu.shiyan.raymond@gmail.com
> > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi all,
> > > > > > > >
> > > > > > > > I'd like to gather some feedback about
> > > > > > > > 1. upgrading Junit 4 to 5
> > > > > > > > 2. adopt AssertJ as preferred assertion statement style
> > > > > > > >
> > > > > > > > IMO 1) will give many benefits on writing better unit tests.
> A
> > > > google
> > > > > > > > search of "junit 4 vs 5" could lead to many good points. And
> it
> > > is
> > > > > some
> > > > > > > > migration can be done piece by piece (keeping both 4 and 5
> > during
> > > > > > upgrade
> > > > > > > > and enforce new test using 5)
> > > > > > > >
> > > > > > > > 2) is to spice things up and bring the test readability to
> > > another
> > > > > > level,
> > > > > > > > though I'll treat it as nice-to-have.
> > > > > > > >
> > > > > > > > Would you +1 or -1 on either or both?
> > > > > > > >
> > > > > > > > Knowing that it'll be a long way to go due to the large
> number
> > of
> > > > > > tests,
> > > > > > > > this needs to be planned and tracked carefully.
> > > > > > > >
> > > > > > > > Thank you.
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > Raymond
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Upgrade unit test: Junit 5 & AssertJ

Posted by Shiyan Xu <xu...@gmail.com>.
Thank you all for the feedback.

> This increases the scope to a overhaul of tests across the project..
Wonder if we can do a RFC for this?
Indeed it is overhaul type of change. IMO RFC is needed specifically for
the test utility re-design part. Guess it can be created when it's good to
start? Since it'll be a long-running task, have an umbrella ticket for this
topic first? @vinoth


On Sat, Apr 4, 2020 at 2:47 AM leesf <le...@gmail.com> wrote:

> +1 to upgrade the unit test. junit5 combine better with java8, and there
> are some migration guides already, and we maybe could upgrade by module.
>
> vino yang <vi...@apache.org> 于2020年4月2日周四 下午4:38写道:
>
> > Hi Shiyan,
> >
> > +1 from my side.
> >
> > Best,
> > Vino
> >
> > Vinoth Chandar <vi...@apache.org> 于2020年3月30日周一 下午11:00写道:
> >
> > > Hi Raymond,
> > >
> > > Sounds good to me. This increases the scope to a overhaul of tests
> across
> > > the project.. Wonder if we can do a RFC for this? But overall +1 from
> me.
> > >
> > > I would like to call upon the community to chime in more though :) .
> > let's
> > > give it a few days..
> > >
> > >
> > > Thanks
> > > Vinoth
> > >
> > > On Fri, Mar 27, 2020 at 5:18 PM Shiyan Xu <xu.shiyan.raymond@gmail.com
> >
> > > wrote:
> > >
> > > > Understand Vinoth. To me AssertJ is nice-to-have. I agree with the
> > > learning
> > > > overhead.
> > > >
> > > > The current CI time is too long and we do need to use more mocking
> and
> > > > optimize spark jobs setup.
> > > >
> > > > Based on your points, I imagine the path forward can be planned as
> this
> > > >
> > > > 1. An initial PR to add Junit 5 to co-exist with 4 in the project
> with
> > a
> > > > simple testcase converted to 5 as a working proof
> > > > 2. A design task to refactor test utilities (create new utilities
> with
> > > > Junit 5 for easy switch-over of affected testcases)
> > > > 3. Track all test improvement PRs (using Junit 5). Each PR should aim
> > to
> > > > solve 1 of the problems below
> > > >   - test can be improved with mocking
> > > >   - test can be optimized on spark job setup
> > > > 4. Clean unused test utilities (from step 2)
> > > >
> > > > We should recognize these steps to be carried out in a long-running
> > > ongoing
> > > > fashion.
> > > >
> > > > Any thoughts or feedback?
> > > >
> > > > On Wed, Mar 25, 2020 at 7:52 AM Vinoth Chandar <vi...@apache.org>
> > > wrote:
> > > >
> > > > > +1 on Junit5.
> > > > >  does seem nicer with support for lambdas. assuming we do a gradual
> > > > > rollout. At any point, we cannot have any of the core tests
> disabled
> > :)
> > > > > May be we can use the vintage framework for now, do minimal changes
> > > > migrate
> > > > > and then proceed to redoing the tests
> > > > >
> > > > > On AssertJ type frameworks, I wonder if there is a cost to this
> type
> > of
> > > > > framework for new devs.
> > > > > They already need to learn junit 5, mockito, all the TestUtils and
> > like
> > > > one
> > > > > more framework for asserting
> > > > >
> > > > > Orthogonally, I will be thrilled if you also took upon a large
> > > > > restructuring on tests cleanly into
> > > > > - unit tests that test class functionality using mocks
> > > > > - functional tests that bring up a spark context and actually run
> the
> > > job
> > > > > (we have a lot of these tests masquerading as unit tests)
> > > > > - Clean redesign of the test utility classes
> > > > >
> > > > > Sorry to expand scope, but when someone is going to take a look at
> > > every
> > > > > test, I could not pass up an opportunity to sneak this in :)
> > > > >
> > > > > Love to hear others thoughts.. any one with experience working with
> > > > > Junit5/Assertj-Hamcrest?
> > > > >
> > > > > On Tue, Mar 24, 2020 at 9:36 PM Shiyan Xu <
> > xu.shiyan.raymond@gmail.com
> > > >
> > > > > wrote:
> > > > >
> > > > > > Some references
> > > > > > https://junit.org/junit5/docs/current/user-guide/
> > > > > > https://joel-costigliola.github.io/assertj/
> > > > > >
> > > > > > On Tue, Mar 24, 2020 at 9:27 PM Shiyan Xu <
> > > xu.shiyan.raymond@gmail.com
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi all,
> > > > > > >
> > > > > > > I'd like to gather some feedback about
> > > > > > > 1. upgrading Junit 4 to 5
> > > > > > > 2. adopt AssertJ as preferred assertion statement style
> > > > > > >
> > > > > > > IMO 1) will give many benefits on writing better unit tests. A
> > > google
> > > > > > > search of "junit 4 vs 5" could lead to many good points. And it
> > is
> > > > some
> > > > > > > migration can be done piece by piece (keeping both 4 and 5
> during
> > > > > upgrade
> > > > > > > and enforce new test using 5)
> > > > > > >
> > > > > > > 2) is to spice things up and bring the test readability to
> > another
> > > > > level,
> > > > > > > though I'll treat it as nice-to-have.
> > > > > > >
> > > > > > > Would you +1 or -1 on either or both?
> > > > > > >
> > > > > > > Knowing that it'll be a long way to go due to the large number
> of
> > > > > tests,
> > > > > > > this needs to be planned and tracked carefully.
> > > > > > >
> > > > > > > Thank you.
> > > > > > >
> > > > > > > Best,
> > > > > > > Raymond
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Upgrade unit test: Junit 5 & AssertJ

Posted by leesf <le...@gmail.com>.
+1 to upgrade the unit test. junit5 combine better with java8, and there
are some migration guides already, and we maybe could upgrade by module.

vino yang <vi...@apache.org> 于2020年4月2日周四 下午4:38写道:

> Hi Shiyan,
>
> +1 from my side.
>
> Best,
> Vino
>
> Vinoth Chandar <vi...@apache.org> 于2020年3月30日周一 下午11:00写道:
>
> > Hi Raymond,
> >
> > Sounds good to me. This increases the scope to a overhaul of tests across
> > the project.. Wonder if we can do a RFC for this? But overall +1 from me.
> >
> > I would like to call upon the community to chime in more though :) .
> let's
> > give it a few days..
> >
> >
> > Thanks
> > Vinoth
> >
> > On Fri, Mar 27, 2020 at 5:18 PM Shiyan Xu <xu...@gmail.com>
> > wrote:
> >
> > > Understand Vinoth. To me AssertJ is nice-to-have. I agree with the
> > learning
> > > overhead.
> > >
> > > The current CI time is too long and we do need to use more mocking and
> > > optimize spark jobs setup.
> > >
> > > Based on your points, I imagine the path forward can be planned as this
> > >
> > > 1. An initial PR to add Junit 5 to co-exist with 4 in the project with
> a
> > > simple testcase converted to 5 as a working proof
> > > 2. A design task to refactor test utilities (create new utilities with
> > > Junit 5 for easy switch-over of affected testcases)
> > > 3. Track all test improvement PRs (using Junit 5). Each PR should aim
> to
> > > solve 1 of the problems below
> > >   - test can be improved with mocking
> > >   - test can be optimized on spark job setup
> > > 4. Clean unused test utilities (from step 2)
> > >
> > > We should recognize these steps to be carried out in a long-running
> > ongoing
> > > fashion.
> > >
> > > Any thoughts or feedback?
> > >
> > > On Wed, Mar 25, 2020 at 7:52 AM Vinoth Chandar <vi...@apache.org>
> > wrote:
> > >
> > > > +1 on Junit5.
> > > >  does seem nicer with support for lambdas. assuming we do a gradual
> > > > rollout. At any point, we cannot have any of the core tests disabled
> :)
> > > > May be we can use the vintage framework for now, do minimal changes
> > > migrate
> > > > and then proceed to redoing the tests
> > > >
> > > > On AssertJ type frameworks, I wonder if there is a cost to this type
> of
> > > > framework for new devs.
> > > > They already need to learn junit 5, mockito, all the TestUtils and
> like
> > > one
> > > > more framework for asserting
> > > >
> > > > Orthogonally, I will be thrilled if you also took upon a large
> > > > restructuring on tests cleanly into
> > > > - unit tests that test class functionality using mocks
> > > > - functional tests that bring up a spark context and actually run the
> > job
> > > > (we have a lot of these tests masquerading as unit tests)
> > > > - Clean redesign of the test utility classes
> > > >
> > > > Sorry to expand scope, but when someone is going to take a look at
> > every
> > > > test, I could not pass up an opportunity to sneak this in :)
> > > >
> > > > Love to hear others thoughts.. any one with experience working with
> > > > Junit5/Assertj-Hamcrest?
> > > >
> > > > On Tue, Mar 24, 2020 at 9:36 PM Shiyan Xu <
> xu.shiyan.raymond@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > Some references
> > > > > https://junit.org/junit5/docs/current/user-guide/
> > > > > https://joel-costigliola.github.io/assertj/
> > > > >
> > > > > On Tue, Mar 24, 2020 at 9:27 PM Shiyan Xu <
> > xu.shiyan.raymond@gmail.com
> > > >
> > > > > wrote:
> > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > I'd like to gather some feedback about
> > > > > > 1. upgrading Junit 4 to 5
> > > > > > 2. adopt AssertJ as preferred assertion statement style
> > > > > >
> > > > > > IMO 1) will give many benefits on writing better unit tests. A
> > google
> > > > > > search of "junit 4 vs 5" could lead to many good points. And it
> is
> > > some
> > > > > > migration can be done piece by piece (keeping both 4 and 5 during
> > > > upgrade
> > > > > > and enforce new test using 5)
> > > > > >
> > > > > > 2) is to spice things up and bring the test readability to
> another
> > > > level,
> > > > > > though I'll treat it as nice-to-have.
> > > > > >
> > > > > > Would you +1 or -1 on either or both?
> > > > > >
> > > > > > Knowing that it'll be a long way to go due to the large number of
> > > > tests,
> > > > > > this needs to be planned and tracked carefully.
> > > > > >
> > > > > > Thank you.
> > > > > >
> > > > > > Best,
> > > > > > Raymond
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Upgrade unit test: Junit 5 & AssertJ

Posted by vino yang <vi...@apache.org>.
Hi Shiyan,

+1 from my side.

Best,
Vino

Vinoth Chandar <vi...@apache.org> 于2020年3月30日周一 下午11:00写道:

> Hi Raymond,
>
> Sounds good to me. This increases the scope to a overhaul of tests across
> the project.. Wonder if we can do a RFC for this? But overall +1 from me.
>
> I would like to call upon the community to chime in more though :) . let's
> give it a few days..
>
>
> Thanks
> Vinoth
>
> On Fri, Mar 27, 2020 at 5:18 PM Shiyan Xu <xu...@gmail.com>
> wrote:
>
> > Understand Vinoth. To me AssertJ is nice-to-have. I agree with the
> learning
> > overhead.
> >
> > The current CI time is too long and we do need to use more mocking and
> > optimize spark jobs setup.
> >
> > Based on your points, I imagine the path forward can be planned as this
> >
> > 1. An initial PR to add Junit 5 to co-exist with 4 in the project with a
> > simple testcase converted to 5 as a working proof
> > 2. A design task to refactor test utilities (create new utilities with
> > Junit 5 for easy switch-over of affected testcases)
> > 3. Track all test improvement PRs (using Junit 5). Each PR should aim to
> > solve 1 of the problems below
> >   - test can be improved with mocking
> >   - test can be optimized on spark job setup
> > 4. Clean unused test utilities (from step 2)
> >
> > We should recognize these steps to be carried out in a long-running
> ongoing
> > fashion.
> >
> > Any thoughts or feedback?
> >
> > On Wed, Mar 25, 2020 at 7:52 AM Vinoth Chandar <vi...@apache.org>
> wrote:
> >
> > > +1 on Junit5.
> > >  does seem nicer with support for lambdas. assuming we do a gradual
> > > rollout. At any point, we cannot have any of the core tests disabled :)
> > > May be we can use the vintage framework for now, do minimal changes
> > migrate
> > > and then proceed to redoing the tests
> > >
> > > On AssertJ type frameworks, I wonder if there is a cost to this type of
> > > framework for new devs.
> > > They already need to learn junit 5, mockito, all the TestUtils and like
> > one
> > > more framework for asserting
> > >
> > > Orthogonally, I will be thrilled if you also took upon a large
> > > restructuring on tests cleanly into
> > > - unit tests that test class functionality using mocks
> > > - functional tests that bring up a spark context and actually run the
> job
> > > (we have a lot of these tests masquerading as unit tests)
> > > - Clean redesign of the test utility classes
> > >
> > > Sorry to expand scope, but when someone is going to take a look at
> every
> > > test, I could not pass up an opportunity to sneak this in :)
> > >
> > > Love to hear others thoughts.. any one with experience working with
> > > Junit5/Assertj-Hamcrest?
> > >
> > > On Tue, Mar 24, 2020 at 9:36 PM Shiyan Xu <xu.shiyan.raymond@gmail.com
> >
> > > wrote:
> > >
> > > > Some references
> > > > https://junit.org/junit5/docs/current/user-guide/
> > > > https://joel-costigliola.github.io/assertj/
> > > >
> > > > On Tue, Mar 24, 2020 at 9:27 PM Shiyan Xu <
> xu.shiyan.raymond@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > I'd like to gather some feedback about
> > > > > 1. upgrading Junit 4 to 5
> > > > > 2. adopt AssertJ as preferred assertion statement style
> > > > >
> > > > > IMO 1) will give many benefits on writing better unit tests. A
> google
> > > > > search of "junit 4 vs 5" could lead to many good points. And it is
> > some
> > > > > migration can be done piece by piece (keeping both 4 and 5 during
> > > upgrade
> > > > > and enforce new test using 5)
> > > > >
> > > > > 2) is to spice things up and bring the test readability to another
> > > level,
> > > > > though I'll treat it as nice-to-have.
> > > > >
> > > > > Would you +1 or -1 on either or both?
> > > > >
> > > > > Knowing that it'll be a long way to go due to the large number of
> > > tests,
> > > > > this needs to be planned and tracked carefully.
> > > > >
> > > > > Thank you.
> > > > >
> > > > > Best,
> > > > > Raymond
> > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Upgrade unit test: Junit 5 & AssertJ

Posted by Vinoth Chandar <vi...@apache.org>.
Hi Raymond,

Sounds good to me. This increases the scope to a overhaul of tests across
the project.. Wonder if we can do a RFC for this? But overall +1 from me.

I would like to call upon the community to chime in more though :) . let's
give it a few days..


Thanks
Vinoth

On Fri, Mar 27, 2020 at 5:18 PM Shiyan Xu <xu...@gmail.com>
wrote:

> Understand Vinoth. To me AssertJ is nice-to-have. I agree with the learning
> overhead.
>
> The current CI time is too long and we do need to use more mocking and
> optimize spark jobs setup.
>
> Based on your points, I imagine the path forward can be planned as this
>
> 1. An initial PR to add Junit 5 to co-exist with 4 in the project with a
> simple testcase converted to 5 as a working proof
> 2. A design task to refactor test utilities (create new utilities with
> Junit 5 for easy switch-over of affected testcases)
> 3. Track all test improvement PRs (using Junit 5). Each PR should aim to
> solve 1 of the problems below
>   - test can be improved with mocking
>   - test can be optimized on spark job setup
> 4. Clean unused test utilities (from step 2)
>
> We should recognize these steps to be carried out in a long-running ongoing
> fashion.
>
> Any thoughts or feedback?
>
> On Wed, Mar 25, 2020 at 7:52 AM Vinoth Chandar <vi...@apache.org> wrote:
>
> > +1 on Junit5.
> >  does seem nicer with support for lambdas. assuming we do a gradual
> > rollout. At any point, we cannot have any of the core tests disabled :)
> > May be we can use the vintage framework for now, do minimal changes
> migrate
> > and then proceed to redoing the tests
> >
> > On AssertJ type frameworks, I wonder if there is a cost to this type of
> > framework for new devs.
> > They already need to learn junit 5, mockito, all the TestUtils and like
> one
> > more framework for asserting
> >
> > Orthogonally, I will be thrilled if you also took upon a large
> > restructuring on tests cleanly into
> > - unit tests that test class functionality using mocks
> > - functional tests that bring up a spark context and actually run the job
> > (we have a lot of these tests masquerading as unit tests)
> > - Clean redesign of the test utility classes
> >
> > Sorry to expand scope, but when someone is going to take a look at every
> > test, I could not pass up an opportunity to sneak this in :)
> >
> > Love to hear others thoughts.. any one with experience working with
> > Junit5/Assertj-Hamcrest?
> >
> > On Tue, Mar 24, 2020 at 9:36 PM Shiyan Xu <xu...@gmail.com>
> > wrote:
> >
> > > Some references
> > > https://junit.org/junit5/docs/current/user-guide/
> > > https://joel-costigliola.github.io/assertj/
> > >
> > > On Tue, Mar 24, 2020 at 9:27 PM Shiyan Xu <xu.shiyan.raymond@gmail.com
> >
> > > wrote:
> > >
> > > > Hi all,
> > > >
> > > > I'd like to gather some feedback about
> > > > 1. upgrading Junit 4 to 5
> > > > 2. adopt AssertJ as preferred assertion statement style
> > > >
> > > > IMO 1) will give many benefits on writing better unit tests. A google
> > > > search of "junit 4 vs 5" could lead to many good points. And it is
> some
> > > > migration can be done piece by piece (keeping both 4 and 5 during
> > upgrade
> > > > and enforce new test using 5)
> > > >
> > > > 2) is to spice things up and bring the test readability to another
> > level,
> > > > though I'll treat it as nice-to-have.
> > > >
> > > > Would you +1 or -1 on either or both?
> > > >
> > > > Knowing that it'll be a long way to go due to the large number of
> > tests,
> > > > this needs to be planned and tracked carefully.
> > > >
> > > > Thank you.
> > > >
> > > > Best,
> > > > Raymond
> > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Upgrade unit test: Junit 5 & AssertJ

Posted by Shiyan Xu <xu...@gmail.com>.
Understand Vinoth. To me AssertJ is nice-to-have. I agree with the learning
overhead.

The current CI time is too long and we do need to use more mocking and
optimize spark jobs setup.

Based on your points, I imagine the path forward can be planned as this

1. An initial PR to add Junit 5 to co-exist with 4 in the project with a
simple testcase converted to 5 as a working proof
2. A design task to refactor test utilities (create new utilities with
Junit 5 for easy switch-over of affected testcases)
3. Track all test improvement PRs (using Junit 5). Each PR should aim to
solve 1 of the problems below
  - test can be improved with mocking
  - test can be optimized on spark job setup
4. Clean unused test utilities (from step 2)

We should recognize these steps to be carried out in a long-running ongoing
fashion.

Any thoughts or feedback?

On Wed, Mar 25, 2020 at 7:52 AM Vinoth Chandar <vi...@apache.org> wrote:

> +1 on Junit5.
>  does seem nicer with support for lambdas. assuming we do a gradual
> rollout. At any point, we cannot have any of the core tests disabled :)
> May be we can use the vintage framework for now, do minimal changes migrate
> and then proceed to redoing the tests
>
> On AssertJ type frameworks, I wonder if there is a cost to this type of
> framework for new devs.
> They already need to learn junit 5, mockito, all the TestUtils and like one
> more framework for asserting
>
> Orthogonally, I will be thrilled if you also took upon a large
> restructuring on tests cleanly into
> - unit tests that test class functionality using mocks
> - functional tests that bring up a spark context and actually run the job
> (we have a lot of these tests masquerading as unit tests)
> - Clean redesign of the test utility classes
>
> Sorry to expand scope, but when someone is going to take a look at every
> test, I could not pass up an opportunity to sneak this in :)
>
> Love to hear others thoughts.. any one with experience working with
> Junit5/Assertj-Hamcrest?
>
> On Tue, Mar 24, 2020 at 9:36 PM Shiyan Xu <xu...@gmail.com>
> wrote:
>
> > Some references
> > https://junit.org/junit5/docs/current/user-guide/
> > https://joel-costigliola.github.io/assertj/
> >
> > On Tue, Mar 24, 2020 at 9:27 PM Shiyan Xu <xu...@gmail.com>
> > wrote:
> >
> > > Hi all,
> > >
> > > I'd like to gather some feedback about
> > > 1. upgrading Junit 4 to 5
> > > 2. adopt AssertJ as preferred assertion statement style
> > >
> > > IMO 1) will give many benefits on writing better unit tests. A google
> > > search of "junit 4 vs 5" could lead to many good points. And it is some
> > > migration can be done piece by piece (keeping both 4 and 5 during
> upgrade
> > > and enforce new test using 5)
> > >
> > > 2) is to spice things up and bring the test readability to another
> level,
> > > though I'll treat it as nice-to-have.
> > >
> > > Would you +1 or -1 on either or both?
> > >
> > > Knowing that it'll be a long way to go due to the large number of
> tests,
> > > this needs to be planned and tracked carefully.
> > >
> > > Thank you.
> > >
> > > Best,
> > > Raymond
> > >
> > >
> >
>

Re: [DISCUSS] Upgrade unit test: Junit 5 & AssertJ

Posted by Vinoth Chandar <vi...@apache.org>.
+1 on Junit5.
 does seem nicer with support for lambdas. assuming we do a gradual
rollout. At any point, we cannot have any of the core tests disabled :)
May be we can use the vintage framework for now, do minimal changes migrate
and then proceed to redoing the tests

On AssertJ type frameworks, I wonder if there is a cost to this type of
framework for new devs.
They already need to learn junit 5, mockito, all the TestUtils and like one
more framework for asserting

Orthogonally, I will be thrilled if you also took upon a large
restructuring on tests cleanly into
- unit tests that test class functionality using mocks
- functional tests that bring up a spark context and actually run the job
(we have a lot of these tests masquerading as unit tests)
- Clean redesign of the test utility classes

Sorry to expand scope, but when someone is going to take a look at every
test, I could not pass up an opportunity to sneak this in :)

Love to hear others thoughts.. any one with experience working with
Junit5/Assertj-Hamcrest?

On Tue, Mar 24, 2020 at 9:36 PM Shiyan Xu <xu...@gmail.com>
wrote:

> Some references
> https://junit.org/junit5/docs/current/user-guide/
> https://joel-costigliola.github.io/assertj/
>
> On Tue, Mar 24, 2020 at 9:27 PM Shiyan Xu <xu...@gmail.com>
> wrote:
>
> > Hi all,
> >
> > I'd like to gather some feedback about
> > 1. upgrading Junit 4 to 5
> > 2. adopt AssertJ as preferred assertion statement style
> >
> > IMO 1) will give many benefits on writing better unit tests. A google
> > search of "junit 4 vs 5" could lead to many good points. And it is some
> > migration can be done piece by piece (keeping both 4 and 5 during upgrade
> > and enforce new test using 5)
> >
> > 2) is to spice things up and bring the test readability to another level,
> > though I'll treat it as nice-to-have.
> >
> > Would you +1 or -1 on either or both?
> >
> > Knowing that it'll be a long way to go due to the large number of tests,
> > this needs to be planned and tracked carefully.
> >
> > Thank you.
> >
> > Best,
> > Raymond
> >
> >
>

Re: [DISCUSS] Upgrade unit test: Junit 5 & AssertJ

Posted by Shiyan Xu <xu...@gmail.com>.
Some references
https://junit.org/junit5/docs/current/user-guide/
https://joel-costigliola.github.io/assertj/

On Tue, Mar 24, 2020 at 9:27 PM Shiyan Xu <xu...@gmail.com>
wrote:

> Hi all,
>
> I'd like to gather some feedback about
> 1. upgrading Junit 4 to 5
> 2. adopt AssertJ as preferred assertion statement style
>
> IMO 1) will give many benefits on writing better unit tests. A google
> search of "junit 4 vs 5" could lead to many good points. And it is some
> migration can be done piece by piece (keeping both 4 and 5 during upgrade
> and enforce new test using 5)
>
> 2) is to spice things up and bring the test readability to another level,
> though I'll treat it as nice-to-have.
>
> Would you +1 or -1 on either or both?
>
> Knowing that it'll be a long way to go due to the large number of tests,
> this needs to be planned and tracked carefully.
>
> Thank you.
>
> Best,
> Raymond
>
>