ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From oignatenko <oignate...@gridgain.com>
Subject Re: Is it time to move forward to JUnit4 (5)?
Date Tue, 11 Dec 2018 17:20:11 GMT
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/

Mime
View raw message