Thanks Raymond.. We can continue engaging on the ticket! On Thu, Apr 9, 2020 at 4:54 PM Shiyan Xu <[email protected]> wrote:
> Filed! > https://issues.apache.org/jira/browse/HUDI-779 > > On Wed, Apr 8, 2020 at 11:05 PM Vinoth Chandar <[email protected]> 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 <[email protected]> > > 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 <[email protected]> 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 <[email protected]> 于2020年4月2日周四 下午4:38写道： > > > > > > > > > Hi Shiyan, > > > > > > > > > > +1 from my side. > > > > > > > > > > Best, > > > > > Vino > > > > > > > > > > Vinoth Chandar <[email protected]> 于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 < > > > [email protected] > > > > > > > > > > > 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 < > > [email protected]> > > > > > > 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 < > > > > > [email protected] > > > > > > > > > > > > > > > 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 < > > > > > > [email protected] > > > > > > > > > > > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
