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