ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Павлухин Иван <vololo...@gmail.com>
Subject Re: Is it time to move forward to JUnit4 (5)?
Date Fri, 14 Dec 2018 12:10:52 GMT
Hi guys,

I started writing a junit4 test for a feature I am currently working
on. And immediately I faced a peculiarity with @Before test fixture. I
automatically named a method "setUp" and realized that I override a
method from a superclass. It can produce unwanted effects. First, it
looks reasonable to call a corresponding superclass method in an
overridden method, but in my understanding it is not a canonical way
in junit4. Second, in junit4 overriding method annotated with @Before
can lead changing order of this method execution among other @Before
method. Second point might lead to surprising effects in tests
hierarchies defining @Before methods on different levels.

Let's return to the test I mentioned in the beginning of current
email. I decided to use a different name for my @Before method
(actually it was unpleasant "setUp1"). I think that we can do the
following to make things straightforward:
1. Prohibit overriding base class setUp and tearDown methods by making
them final (at least in GridCommonAbstractTest and
GridSpiAbstractTest).
2. Use common default name for @Before and @After methods defined in
concrete test classes. I do not have really good candidates for names.
Simply we can use setUp1, tearDown1. Another option initTest,
cleanupTest. Your name candidates are quite appreciated.

WDYT?

P.S. Also I went through other tests and found really dangerous thing.
In IgniteJmsStreamerTest (not migrated to junit4 yet) we have
beforeTest method (overriding GridAbstractTest framework method)
annotated with @Before annotation (I believe accidentally)! It means
that migrating such test to junit4 will lead to calling beforeTest
twice. Ouch!
ср, 12 дек. 2018 г. в 13:21, Павлухин Иван <vololo100@gmail.com>:
>
> Hi Oleg,
>
> Thanks your for detailed answer. Using junit4 annotations where
> possible sounds good to me.
>
> Also I would like to thank everybody involved in migration to junit4
> process for your efforts. You were able to move a problem which looked
> mountain-heavy. I believe that is a significant milestone on Ignite
> stabilization road.
>
> Well done!
> вт, 11 дек. 2018 г. в 20:20, oignatenko <oignatenko@gridgain.com>:
> >
> > Hi Ivan,
> >
> > That's a very good question and I think you spotted a point where Ignite
> > test developer's preferences need to change when migrating from Junit 3 to
> > 4.
> >
> > In brief, when developing under JUnit 3 there were good reasons to prefer
> > our homebrew template methods (beforeTestsStarted, beforeTest, afterTest,
> > afterTestsStopped) over those provided by JUnit (setUp / tearDown). To
> > answer your other question, in order to reliably understand how our methods
> > work one would better go to (potentially changing) code of GridAbstractTest
> > and check where and how these are invoked.
> >
> > But under JUnit 4 these reasons don't apply anymore and preference should go
> > the other way, namely in favor of annotations @Before, @After, @BeforeClass,
> > @AfterClass.
> >
> > In other words, when these annotations allow you implement desired test
> > logic you better use them. These annotations have stable, well documented
> > and widely understood semantics (and don't require one to skim through 3K
> > lines of code for grasping details:).
> >
> > And it makes sense to "fallback" to our old-fashioned methods only when you
> > simply have no other choice. For example doing some quick and simple fix in
> > a legacy code that is filled with multiple usages of our old template
> > methods it may be okay to choose to keep old style - instead of doing
> > cumbersome, risky and lengthy refactoring or polluting test code with
> > inconsistent use of annotations intertwined with old style methods.
> >
> > That's what I think. If you're interested in why I think so, refer to boring
> > explanation in PS.
> >
> > regards, Oleg
> >
> > -----
> >
> > PS. With regards to old preferences I believe it is very important to keep
> > in mind that Junit 3 didn't provide means to do initialization and cleanup
> > once for all test methods in the class.
> >
> > So when one needed things to run once per test class they had to go
> > somewhere else. In Ignite the natural way to go was GridAbstractTest which
> > provided this functionality via beforeTestsStarted and afterTestsStopped
> > (with some glitches as I recently learned but still). And after one had to
> > use these methods there was no compelling reason to use JUnit's setUp and
> > tearDown, because GridAbstractTest also provided similar methods.
> >
> > It was just easier to consistently use full and cohesive set of methods
> > provided by GridAbstractTest than switch back and forth between it and JUnit
> > 3 methods.
> >
> > With JUnit 4 above reasoning doesn't apply anymore because its features are
> > just as full and cohesive and can be consistently used without having to
> > fallback to stuff from GridAbstractTest. So we have a real choice now and
> > can judge based on other criteria than functional completeness (since both
> > ways now satisfy that most important criteria).
> >
> > And this reveals us a full spectrum of JUnit advantages over
> > GridAbstractTest (which was obscured before, because JUnit 3 was
> > functionally incomplete).
> >
> > And on that newly opened spectrum JUnit 4 API wins hands down.
> >
> > JUnit 4 is much better documented and "googlable", it is stable and much
> > wider known. The last but not the least, as I already mentioned, it doesn't
> > require one to skim through 3K lines of code for grasping details and its
> > semantics doesn't depend on (possibly changing) implementation code.
> >
> > Also, initialization and cleanup methods of JUnit 4 make an organic part of
> > wider, more comprehensive API which smoothly integrates with IDE, Maven,
> > Teamcity and all imaginable Java tools over there.
> >
> > Every time one annotates @Test or @Ignore gives them a reason to prefer
> > other JUnit annotations over something else. It works about the same as I
> > described above for beforeTestsStarted in GridAbstractTest and how it
> > inhibited use of JUnit 3 methods: after you picked one part of some API it
> > naturally becomes more convenient to keep using other parts of that API.
> >
> > Summing up, I believe that every time you have a (sensible) choice between
> > JUnit 4 and GridAbstractTest methods you just pick JUnit 4 and don't look
> > back.
> >
> >
> > Павлухин Иван wrote
> > > Ed, Oleg,
> > >
> > > Could you please clarify on the following point:
> > >> 3. Add @Before, @After on methods which should run before, after a
> > >> test (setUp, tearDown in current approach).
> > > Beforehand we used to override beforeTestsStarted, beforeTest,
> > > afterTest, afterTestsStarted. What will happen with these methods
> > > after migration to junit4? I can see that order of execution can
> > > become tricky especially when both styles are used at the same time.
> > > And an immediate suggestion is to avoid using both styles (annotations
> > > and overridden methods) in the same test. What is your insight?
> > > вт, 11 дек. 2018 г. в 10:27, Vladimir Ozerov &lt;
> >
> > > vozerov@
> >
> > > &gt;:
> > >>
> > >> Hi Oleg,
> > >>
> > >> Yes, makes perfect sense. Thank you.
> > >>
> > >> On Mon, Dec 10, 2018 at 10:14 PM oignatenko &lt;
> >
> > > oignatenko@
> >
> > > &gt; wrote:
> > >>
> > >> > Hi Vovan,
> > >> >
> > >> > I just created JIRA ticket to address your concerns:
> > >> > - https://issues.apache.org/jira/browse/IGNITE-10629
> > >> >
> > >> > In brief, the plan is that a week or two after migration is over we
> > >> will
> > >> > run
> > >> > code inspection that detects JUnit 3 style tests that lack @Test
> > >> annotation
> > >> > and fix these tests if there are any.
> > >> >
> > >> > Does that answer your question?
> > >> >
> > >> > regards, Oleg
> > >> > Vladimir Ozerov wrote
> > >> > > Ed,
> > >> > >
> > >> > > Several questions from my side:
> > >> > > 1) When the change is expected to be merged?
> > >> > > 2) What contributors with opened PRs and new or updated JUnit3
tests
> > >> are
> > >> > > supposed to do? Rewrite to JUnit4?
> > >> > >
> > >> > > If yes, then we should give them time to have a chance to get
used to
> > >> new
> > >> > > approach and resolve possible conflicts.
> > >> > >
> > >> > > Vladimir.
> > >> > >
> > >> > > пн, 10 дек. 2018 г. в 20:32, Eduard Shangareev &lt;
> > >> >
> > >> > > eduard.shangareev@
> > >> >
> > >> > > &gt;:
> > >> > >
> > >> > >> Ivan,
> > >> > >>
> > >> > >> So, suggested actions with the new approach:
> > >> > >> 1. Add @Test annotation on test methods.
> > >> > >> 2. Add @RunWith(JUnit4.class) annotation on test class.
> > >> > >> 3. Add @Before, @After on methods which should run before,
after a
> > >> > >> test (setUp, tearDown in current approach).
> > >> > >> 4. Add your test-class to a suite using suite.addTest(new
> > >> > >> JUnit4TestAdapter(YourTestClass.class));
> > >> > >> 5. Use @Ignore instead fail(); for muting test.
> > >> > >> 6. You could start using @Parametrized instead of inheritance.
> > >> > >>
> > >> > >>
> > >> > >> On Mon, Dec 3, 2018 at 1:15 PM Павлухин Иван
&lt;
> > >> >
> > >> > > vololo100@
> > >> >
> > >> > > &gt; wrote:
> > >> > >>
> > >> > >> > Hi Oleg,
> > >> > >> >
> > >> > >> > I noticed that GridAbstractTest is now capable to run
junit4
> > >> tests.
> > >> > >> > What are the current recommendations for writing new
tests? Can we
> > >> use
> > >> > >> > junit4 annotation for new tests?
> > >> > >> > пн, 12 нояб. 2018 г. в 19:58, oignatenko &lt;
> > >> >
> > >> > > oignatenko@
> > >> >
> > >> > > &gt;:
> > >> > >> > >
> > >> > >> > > Hi Ivan,
> > >> > >> > >
> > >> > >> > > I am currently testing approach you used in pull/5354
in the
> > >> "pilot"
> > >> > >> > > sub-task with examples tests (IGNITE-10174).
> > >> > >> > >
> > >> > >> > > So far it looks more and more like the way to go.
The most
> > >> promising
> > >> > >> > thing I
> > >> > >> > > observed is that after I changed classes in our
test framework
> > >> the
> > >> > >> way
> > >> > >> > you
> > >> > >> > > did, execution of (unchanged) examples tests went
exactly the
> > >> same
> > >> > as
> > >> > >> it
> > >> > >> > was
> > >> > >> > > before changes.
> > >> > >> > >
> > >> > >> > > This indicates that existing tests won't be affected
making it
> > >> > indeed
> > >> > >> low
> > >> > >> > > risk.
> > >> > >> > >
> > >> > >> > > After that I converted examples tests to Junit
4 by adding
> > >> @RunWith
> > >> > >> and
> > >> > >> > > @Test annotations and tried a few, and these looked
okay.
> > >> > >> > >
> > >> > >> > > Currently I am running full examples test suite
and after it is
> > >> over
> > >> > >> I
> > >> > >> > will
> > >> > >> > > compare results to the reference list I made by
running it prior
> > >> to
> > >> > >> > > migration.
> > >> > >> > >
> > >> > >> > > regards, Oleg
> > >> > >> > >
> > >> > >> > >
> > >> > >> > >
> > >> > >> > > --
> > >> > >> > > Sent from:
> > >> http://apache-ignite-developers.2346864.n4.nabble.com/
> > >> > >> >
> > >> > >> >
> > >> > >> >
> > >> > >> > --
> > >> > >> > Best regards,
> > >> > >> > Ivan Pavlukhin
> > >> > >> >
> > >> > >>
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> > --
> > >> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > >> >
> > >
> > >
> > >
> > > --
> > > Best regards,
> > > Ivan Pavlukhin
> >
> >
> >
> >
> >
> > --
> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>
>
>
> --
> Best regards,
> Ivan Pavlukhin



-- 
Best regards,
Ivan Pavlukhin

Mime
View raw message