Is it time to move forward to JUnit4 (5)?

classic Classic list List threaded Threaded
41 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

Vladimir Ozerov
Got it. Thank you.

пн, 10 дек. 2018 г. в 20:52, Dmitriy Pavlov <[hidden email]>:

> Vladimir, AFAIK there will be no single change for overall migration. All
> migration will be in a step-by-step manner and mostly transparent for
> contributors.
>
> Both approaches 3&4 will be available for some time.
>
> First of all, we need an opportunity to run existing integration tests in
> JUnit4.
>
> пн, 10 дек. 2018 г. в 20:38, Vladimir Ozerov <[hidden email]>:
>
> > 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 <
> > [hidden email]
> > >:
> >
> > > 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 Павлухин Иван <[hidden email]>
> > 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 <[hidden email]>:
> > > > >
> > > > > 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
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

oignatenko
In reply to this post by Eduard Shangareev
Hello,

I would like add few minor clarifications to what Ed wrote.

1. After transition to JUnit4 is fully over (ETA this week) there won't be a
need to use @RunWith(JUnit4.class) anymore, so you will be writing test
classes as a normal JUnit 4 user, without any special scaffolding.
1.1. Please keep in mind that tests that don't inherit from GridAbstractTest
can (and I think should) be written full Junit 4 and don't need anything
special at all - these are essentially out of scope of this discussion.

2. Note that "full JUnit 4 experience" doesn't apply to part of testsuites
that currently use suite() method because in these you would still have to
use JUnit4TestAdapter. Unless of course you will choose to rewrite
particular suite Junit 4 style (which I would encourage everyone to do,
after we complete migration).

3. As a bit of further perspective, we also considering next migration -
after all tests will be uniform Junit 4 (instead of current mix of Junit 3
and 4) we will look into whether it is worth migrating to most recent
version - JUnit5. Those interested can keep an eye on respective JIRA
ticket:
  https://issues.apache.org/jira/browse/IGNITE-10180
 
regards, Oleg


Eduard Shangareev wrote

> 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/
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

oignatenko
In reply to this post by Vladimir Ozerov
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/
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

Vladimir Ozerov
Hi Oleg,

Yes, makes perfect sense. Thank you.

On Mon, Dec 10, 2018 at 10:14 PM oignatenko <[hidden email]> 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/
>
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

Ivan Pavlukhin
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 <[hidden email]>:

>
> Hi Oleg,
>
> Yes, makes perfect sense. Thank you.
>
> On Mon, Dec 10, 2018 at 10:14 PM oignatenko <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

oignatenko
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/
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

Ivan Pavlukhin
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 <[hidden email]>:

>
> 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
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

Ivan Pavlukhin
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, Павлухин Иван <[hidden email]>:

>
> 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 <[hidden email]>:
> >
> > 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
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

oignatenko
Hi Ivan,

The issues you mentioned are real and these issues are expected to be
addressed per IGNITE-10177.

I am currently experimenting with various approaches to tackle that, will
post here when "early preview implementation" is ready to let others take a
look and test it.

Until this is completed the most reliable approach I've found is to fallback
to JUnit 3 way every time that you observe unexpected test behavior and see
no reasonable way to fix it. You can see how I followed this "transitional"
approach in IGNITE-10176 which is expected to be merged to master soon
(meanwhile the code can be seen in PR
https://github.com/apache/ignite/pull/5659).

-----

With regards to using Before* and After* annotations on overrides of before*
and after* methods of GridAbstractTest, this is pure evil and should be
always avoided (removed if noticed).

When test runs under JUnit 3 these annotations are simply ignored which
makes them totally useless (and misleading to the reader of the code) and
when it runs under JUnit 4 these start doing real harm by making annotated
method run twice.

If you are interested, more detailed explanation with code example is
available in comments under IGNITE-10176:
-
https://issues.apache.org/jira/browse/IGNITE-10176?focusedCommentId=16718914&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16718914

I am currently trying to figure what could be the best way to communicate
this to API users, possibly in respective method javadocs. Also I plan to
find and clean up all the inappropriate annotations that are already there
in the project in the IGNITE-10177.

If you have further questions, please let me know.

regards, Oleg



--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

oignatenko
In reply to this post by Ivan Pavlukhin
Hi Ivan,

As promised in my prior mail, here is the branch where I experimented to
address concerns you raised:
- https://github.com/gridgain/apache-ignite/tree/ignite-10177-experimental

I tested it locally and on Teamcity and it worked as intended.

I think I managed to exactly reproduce execution sequence of JUnit 3 test
case so that tests designed expecting it will run exactly as it was before.

As for troublesome APIs I used deprecation to warn developers agains using
these and recommend what they need to use instead.

If you are interested in closer studying the changes, class
GridAbstractTest1 is probably best as a starting point. This class is a
temporary copy of GridAbstractTest made to minimise amount of editing in
dependent classes while I was experimenting; in real implementation (per
IGNITE-10177) this code is expected to be in GridAbstractTest.

Also, I used ML testsuite to debug the changes I made, because it contains
convenient mix of usecases and runs fast.

Your feedback is much appreciated.

regards, Oleg


Павлухин Иван wrote

> 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, Павлухин Иван &lt;

> vololo100@

> &gt;:
>>
>> 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 &lt;

> oignatenko@

> &gt;:
>> >
>> > 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





--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

Ivan Pavlukhin
Hi Oleg,

The things become challenging. Truly I do not see any trivial solution
so far. Could you please outline main problems which we are facing
today? And how many tests are affected? Some clarifying questions:
1. I know that setup->test->teardown threading was changed for junit4
tests, but actually I thought that it might affect only small number
of tests. Am I right here?
2. Also I saw that in your experiment [1] some changes related to
TestCounters were made. What is wrong with them?
3. Why do we need wrap test execution into critical section
(runSerializer lock)? I thought that we always run tests serially.

I generally like an idea of having workaround falling back to old test
execution flow. But for me the most desired trait of things like
LegacySupport is being lightweight and enabled only on purpose. And
from the first glance current prototype looks a little bit
complicated. As an alternative we can keep junit3 for troublesome
tests, can't we?

Also is there any vision how many migration problems do we have so far
and how many tests was not migrated yet?
вс, 16 дек. 2018 г. в 17:39, oignatenko <[hidden email]>:

>
> Hi Ivan,
>
> As promised in my prior mail, here is the branch where I experimented to
> address concerns you raised:
> - https://github.com/gridgain/apache-ignite/tree/ignite-10177-experimental
>
> I tested it locally and on Teamcity and it worked as intended.
>
> I think I managed to exactly reproduce execution sequence of JUnit 3 test
> case so that tests designed expecting it will run exactly as it was before.
>
> As for troublesome APIs I used deprecation to warn developers agains using
> these and recommend what they need to use instead.
>
> If you are interested in closer studying the changes, class
> GridAbstractTest1 is probably best as a starting point. This class is a
> temporary copy of GridAbstractTest made to minimise amount of editing in
> dependent classes while I was experimenting; in real implementation (per
> IGNITE-10177) this code is expected to be in GridAbstractTest.
>
> Also, I used ML testsuite to debug the changes I made, because it contains
> convenient mix of usecases and runs fast.
>
> Your feedback is much appreciated.
>
> regards, Oleg
>
>
> Павлухин Иван wrote
> > 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, Павлухин Иван &lt;
>
> > vololo100@
>
> > &gt;:
> >>
> >> 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 &lt;
>
> > oignatenko@
>
> > &gt;:
> >> >
> >> > 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
>
>
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/



--
Best regards,
Ivan Pavlukhin
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

oignatenko
Hi Ivan,

Per my cursory check of the code currently in master, we have 239 test
classes that couldn't migrate (that's probably about 500 or something test
cases). For comparison, over 1600 classes migrated without problems. This is
to answer your questions about how many tests are affected by change and
how many tests were not migrated yet.
 
-----

I think we can say that only small percent of tests turned out sensitive to
JUnit framework change.

In the same time, due to very large overall amount of our tests we have
quite a big number as an absolute value. I point this because if amount of
troublesome tests was smaller (say, order of magnitude smaller) I would
consider delaying migration until all tests reworked to blend totally
seamlessly into new version JUnit. This is doable - as sort of "pilot
exercise" me and Ed adjusted one test case this way - but the sheer amount
of work on 200+ classes raises a question, whether it is worth it (I think
it isn't).

With regards to question 2, changes to TestCounters are farly trivial (and
necessary) if you look at the code diff. Prior code counted amount of test
cases in the class by JUnit 3 convention: it picked public void methods
without parameters starting with "test". Changed code counts test cases as
JUnit 4 does: by checking if method is annotated with @Test. Change is
necessary because it will allow test developers fully use JUnit 4 features
by adding test cases that don't follow old naming requirement. I
experimented with it a little and as far as I could see the old TestCounters
indeed break when there are methods following new convention, it triggered
afterTestsStopped too early.

The answer to your question 3 lies in javadocs I added to runSerializer
declaration, or, more precisely, in TestCounters javadoc referred from
there. As you can see, current plan is to get rid of TestCounters fairly
soon (per IGNITE-10179) and this will also get rid of runSerializer so this
is merely a temporary band aid to be removed soon.

For the sake of completeness, my initial plan was to thoroughly investigate
and test whether change from JUnit 3 to JUnit 4 would keep accessing
counters safe or not and only introduce runSerializer if it really does but
after realising that this whole thing is likely to go away in a few days
from now I decided that it isn't worth wasting time and just temporarily
made it the way that is waterproof guaranteed to be safe.

-----

Now, to answer your question whether it is an option for us to keep part of
tests under JUnit 3, my answer is most definitely no.
 
Main reason is that having part of tests under JUnit 3 will deprive us
ability to consistently use Ignore annotation and force us fallback to old
way to fail the tests we want to ignore. This would kind of trash the whole
purpose of migration because we won't be able to simplify and improve
maintenance of ignored tests.
 
Another important reason is that keeping JUnit 3 will much complicate our
test framework code. We will have to implement and maintain two versions of
TestCounters (see answer to your question #2 above). We will also have to
keep the code that currently determines first/last test in the class and
possibly even complicate it to account for two versions of the framework -
compare that to current plan to simply get rid of all that code per
IGNITE-10179.
 
The last but not the least, this makes it much more complicated to later
migrate to JUnit 5. Although this is currently not in the nearest plans
(IGNITE-10180) we eventually will want to (especially taking into account
that migration from JUnit 4 is said to be easy). Having JUnit 3 tests would
much complicate this because we have no idea if JUnit 5 could interoperate
with such an old version (and I see no reason why we would want to waste our
time and efforts investigating and testing this).
 
Summing up, I believe it is very well worth it for us to get rid of JUnit 3
completely.
 
 -----
 
With regards to making LegacySupport enabled only on purpose, at this point
I see no reason to enforce this in code because I expect that deprecation
notices will do that job.

If a developer writing new test or reworking an old one follows the
deprecation recommendations they will use JUnit 4 way instead of deprecated
methods and you can see from the code that in this case LegacySupport will
just transparently pass-through the test code without introducing anything
else beyond. (Note we can reconsider and rework this later in case if it
turns out that my expectation doesn't hold).

Does that answer your questions?
 
regards, Oleg


Павлухин Иван wrote

> Hi Oleg,
>
> The things become challenging. Truly I do not see any trivial solution
> so far. Could you please outline main problems which we are facing
> today? And how many tests are affected? Some clarifying questions:
> 1. I know that setup->test->teardown threading was changed for junit4
> tests, but actually I thought that it might affect only small number
> of tests. Am I right here?
> 2. Also I saw that in your experiment [1] some changes related to
> TestCounters were made. What is wrong with them?
> 3. Why do we need wrap test execution into critical section
> (runSerializer lock)? I thought that we always run tests serially.
>
> I generally like an idea of having workaround falling back to old test
> execution flow. But for me the most desired trait of things like
> LegacySupport is being lightweight and enabled only on purpose. And
> from the first glance current prototype looks a little bit
> complicated. As an alternative we can keep junit3 for troublesome
> tests, can't we?
>
> Also is there any vision how many migration problems do we have so far
> and how many tests was not migrated yet?
> вс, 16 дек. 2018 г. в 17:39, oignatenko &lt;

> oignatenko@

> &gt;:
>>
>> Hi Ivan,
>>
>> As promised in my prior mail, here is the branch where I experimented to
>> address concerns you raised:
>> -
>> https://github.com/gridgain/apache-ignite/tree/ignite-10177-experimental
>>
>> I tested it locally and on Teamcity and it worked as intended.
>>
>> I think I managed to exactly reproduce execution sequence of JUnit 3 test
>> case so that tests designed expecting it will run exactly as it was
>> before.
>>
>> As for troublesome APIs I used deprecation to warn developers agains
>> using
>> these and recommend what they need to use instead.
>>
>> If you are interested in closer studying the changes, class
>> GridAbstractTest1 is probably best as a starting point. This class is a
>> temporary copy of GridAbstractTest made to minimise amount of editing in
>> dependent classes while I was experimenting; in real implementation (per
>> IGNITE-10177) this code is expected to be in GridAbstractTest.
>>
>> Also, I used ML testsuite to debug the changes I made, because it
>> contains
>> convenient mix of usecases and runs fast.
>>
>> Your feedback is much appreciated.
>>
>> 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/
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

Ivan Pavlukhin
Hi Oleg,

Now concerns about junit3 elimination are clear for me. And I agree
that it is worth to do it. By the way is it possible to access tests
which have problems running under junit4? I would like to take a look.

Also a clarifying bit regarding next migration steps is needed. Sorry
if it was described but I missed it. Currently we have tons of tests
which rely on our home brewed beforeTestsStarted, beforeTest,
afterTest, afterTestsStarted. Are you going to mark them all with
corresponding junit4 annotations?
пн, 17 дек. 2018 г. в 19:13, oignatenko <[hidden email]>:

>
> Hi Ivan,
>
> Per my cursory check of the code currently in master, we have 239 test
> classes that couldn't migrate (that's probably about 500 or something test
> cases). For comparison, over 1600 classes migrated without problems. This is
> to answer your questions about how many tests are affected by change and
> how many tests were not migrated yet.
>
> -----
>
> I think we can say that only small percent of tests turned out sensitive to
> JUnit framework change.
>
> In the same time, due to very large overall amount of our tests we have
> quite a big number as an absolute value. I point this because if amount of
> troublesome tests was smaller (say, order of magnitude smaller) I would
> consider delaying migration until all tests reworked to blend totally
> seamlessly into new version JUnit. This is doable - as sort of "pilot
> exercise" me and Ed adjusted one test case this way - but the sheer amount
> of work on 200+ classes raises a question, whether it is worth it (I think
> it isn't).
>
> With regards to question 2, changes to TestCounters are farly trivial (and
> necessary) if you look at the code diff. Prior code counted amount of test
> cases in the class by JUnit 3 convention: it picked public void methods
> without parameters starting with "test". Changed code counts test cases as
> JUnit 4 does: by checking if method is annotated with @Test. Change is
> necessary because it will allow test developers fully use JUnit 4 features
> by adding test cases that don't follow old naming requirement. I
> experimented with it a little and as far as I could see the old TestCounters
> indeed break when there are methods following new convention, it triggered
> afterTestsStopped too early.
>
> The answer to your question 3 lies in javadocs I added to runSerializer
> declaration, or, more precisely, in TestCounters javadoc referred from
> there. As you can see, current plan is to get rid of TestCounters fairly
> soon (per IGNITE-10179) and this will also get rid of runSerializer so this
> is merely a temporary band aid to be removed soon.
>
> For the sake of completeness, my initial plan was to thoroughly investigate
> and test whether change from JUnit 3 to JUnit 4 would keep accessing
> counters safe or not and only introduce runSerializer if it really does but
> after realising that this whole thing is likely to go away in a few days
> from now I decided that it isn't worth wasting time and just temporarily
> made it the way that is waterproof guaranteed to be safe.
>
> -----
>
> Now, to answer your question whether it is an option for us to keep part of
> tests under JUnit 3, my answer is most definitely no.
>
> Main reason is that having part of tests under JUnit 3 will deprive us
> ability to consistently use Ignore annotation and force us fallback to old
> way to fail the tests we want to ignore. This would kind of trash the whole
> purpose of migration because we won't be able to simplify and improve
> maintenance of ignored tests.
>
> Another important reason is that keeping JUnit 3 will much complicate our
> test framework code. We will have to implement and maintain two versions of
> TestCounters (see answer to your question #2 above). We will also have to
> keep the code that currently determines first/last test in the class and
> possibly even complicate it to account for two versions of the framework -
> compare that to current plan to simply get rid of all that code per
> IGNITE-10179.
>
> The last but not the least, this makes it much more complicated to later
> migrate to JUnit 5. Although this is currently not in the nearest plans
> (IGNITE-10180) we eventually will want to (especially taking into account
> that migration from JUnit 4 is said to be easy). Having JUnit 3 tests would
> much complicate this because we have no idea if JUnit 5 could interoperate
> with such an old version (and I see no reason why we would want to waste our
> time and efforts investigating and testing this).
>
> Summing up, I believe it is very well worth it for us to get rid of JUnit 3
> completely.
>
>  -----
>
> With regards to making LegacySupport enabled only on purpose, at this point
> I see no reason to enforce this in code because I expect that deprecation
> notices will do that job.
>
> If a developer writing new test or reworking an old one follows the
> deprecation recommendations they will use JUnit 4 way instead of deprecated
> methods and you can see from the code that in this case LegacySupport will
> just transparently pass-through the test code without introducing anything
> else beyond. (Note we can reconsider and rework this later in case if it
> turns out that my expectation doesn't hold).
>
> Does that answer your questions?
>
> regards, Oleg
>
>
> Павлухин Иван wrote
> > Hi Oleg,
> >
> > The things become challenging. Truly I do not see any trivial solution
> > so far. Could you please outline main problems which we are facing
> > today? And how many tests are affected? Some clarifying questions:
> > 1. I know that setup->test->teardown threading was changed for junit4
> > tests, but actually I thought that it might affect only small number
> > of tests. Am I right here?
> > 2. Also I saw that in your experiment [1] some changes related to
> > TestCounters were made. What is wrong with them?
> > 3. Why do we need wrap test execution into critical section
> > (runSerializer lock)? I thought that we always run tests serially.
> >
> > I generally like an idea of having workaround falling back to old test
> > execution flow. But for me the most desired trait of things like
> > LegacySupport is being lightweight and enabled only on purpose. And
> > from the first glance current prototype looks a little bit
> > complicated. As an alternative we can keep junit3 for troublesome
> > tests, can't we?
> >
> > Also is there any vision how many migration problems do we have so far
> > and how many tests was not migrated yet?
> > вс, 16 дек. 2018 г. в 17:39, oignatenko &lt;
>
> > oignatenko@
>
> > &gt;:
> >>
> >> Hi Ivan,
> >>
> >> As promised in my prior mail, here is the branch where I experimented to
> >> address concerns you raised:
> >> -
> >> https://github.com/gridgain/apache-ignite/tree/ignite-10177-experimental
> >>
> >> I tested it locally and on Teamcity and it worked as intended.
> >>
> >> I think I managed to exactly reproduce execution sequence of JUnit 3 test
> >> case so that tests designed expecting it will run exactly as it was
> >> before.
> >>
> >> As for troublesome APIs I used deprecation to warn developers agains
> >> using
> >> these and recommend what they need to use instead.
> >>
> >> If you are interested in closer studying the changes, class
> >> GridAbstractTest1 is probably best as a starting point. This class is a
> >> temporary copy of GridAbstractTest made to minimise amount of editing in
> >> dependent classes while I was experimenting; in real implementation (per
> >> IGNITE-10177) this code is expected to be in GridAbstractTest.
> >>
> >> Also, I used ML testsuite to debug the changes I made, because it
> >> contains
> >> convenient mix of usecases and runs fast.
> >>
> >> Your feedback is much appreciated.
> >>
> >> 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
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

oignatenko
Hi Ivan,

To answer your last question, yes, all the tests are to be marked with @Test
annotations, and those that are meant to be ignored are going to be marked
with @Ignore annotations. There is no exceptions to that, and these
annotations will work just as well on tests using our home brewed
beforeTestsStarted, beforeTest, afterTest, afterTestsStarted.

For the sake of completeness, developers will also be able to use Before* /
After* annotations on their own methods in tests. The only exception
(clarified in respective javadocs) is that these annotations shouldn't be
used on overrides of our home brewed methods - and these methods, in
addition, will be recommended (not mandated) to avoid wia deprecation
notices.

-----

As for accessing tests which have problems running under junit4, the way how
I search for these in current master is regex search in IDEA for
"addTestSuite.*class", that is I search in testsuites for entries that are
added using method "addTestSuite" with parameter class.

Probably worth noting that some of the problems that were previously
blocking addition of particular tests have been resolved in the course of
working on IGNITE-10177 (https://github.com/apache/ignite/pull/5615). One
riddle that currently looks particularly difficult to crack is Teamcity
failures in "Queries 1", I even haven't yet figured how to reproduce these
locally.

regards, Oleg


Павлухин Иван wrote

> Hi Oleg,
>
> Now concerns about junit3 elimination are clear for me. And I agree
> that it is worth to do it. By the way is it possible to access tests
> which have problems running under junit4? I would like to take a look.
>
> Also a clarifying bit regarding next migration steps is needed. Sorry
> if it was described but I missed it. Currently we have tons of tests
> which rely on our home brewed beforeTestsStarted, beforeTest,
> afterTest, afterTestsStarted. Are you going to mark them all with
> corresponding junit4 annotations?
> пн, 17 дек. 2018 г. в 19:13, oignatenko &lt;

> oignatenko@

> &gt;:
>>
>> Hi Ivan,
>>
>> Per my cursory check of the code currently in master, we have 239 test
>> classes that couldn't migrate (that's probably about 500 or something
>> test
>> cases). For comparison, over 1600 classes migrated without problems. This
>> is
>> to answer your questions about how many tests are affected by change and
>> how many tests were not migrated yet.
>>
>> -----
>>
>> I think we can say that only small percent of tests turned out sensitive
>> to
>> JUnit framework change.
>>
>> In the same time, due to very large overall amount of our tests we have
>> quite a big number as an absolute value. I point this because if amount
>> of
>> troublesome tests was smaller (say, order of magnitude smaller) I would
>> consider delaying migration until all tests reworked to blend totally
>> seamlessly into new version JUnit. This is doable - as sort of "pilot
>> exercise" me and Ed adjusted one test case this way - but the sheer
>> amount
>> of work on 200+ classes raises a question, whether it is worth it (I
>> think
>> it isn't).
>>
>> With regards to question 2, changes to TestCounters are farly trivial
>> (and
>> necessary) if you look at the code diff. Prior code counted amount of
>> test
>> cases in the class by JUnit 3 convention: it picked public void methods
>> without parameters starting with "test". Changed code counts test cases
>> as
>> JUnit 4 does: by checking if method is annotated with @Test. Change is
>> necessary because it will allow test developers fully use JUnit 4
>> features
>> by adding test cases that don't follow old naming requirement. I
>> experimented with it a little and as far as I could see the old
>> TestCounters
>> indeed break when there are methods following new convention, it
>> triggered
>> afterTestsStopped too early.
>>
>> The answer to your question 3 lies in javadocs I added to runSerializer
>> declaration, or, more precisely, in TestCounters javadoc referred from
>> there. As you can see, current plan is to get rid of TestCounters fairly
>> soon (per IGNITE-10179) and this will also get rid of runSerializer so
>> this
>> is merely a temporary band aid to be removed soon.
>>
>> For the sake of completeness, my initial plan was to thoroughly
>> investigate
>> and test whether change from JUnit 3 to JUnit 4 would keep accessing
>> counters safe or not and only introduce runSerializer if it really does
>> but
>> after realising that this whole thing is likely to go away in a few days
>> from now I decided that it isn't worth wasting time and just temporarily
>> made it the way that is waterproof guaranteed to be safe.
>>
>> -----
>>
>> Now, to answer your question whether it is an option for us to keep part
>> of
>> tests under JUnit 3, my answer is most definitely no.
>>
>> Main reason is that having part of tests under JUnit 3 will deprive us
>> ability to consistently use Ignore annotation and force us fallback to
>> old
>> way to fail the tests we want to ignore. This would kind of trash the
>> whole
>> purpose of migration because we won't be able to simplify and improve
>> maintenance of ignored tests.
>>
>> Another important reason is that keeping JUnit 3 will much complicate our
>> test framework code. We will have to implement and maintain two versions
>> of
>> TestCounters (see answer to your question #2 above). We will also have to
>> keep the code that currently determines first/last test in the class and
>> possibly even complicate it to account for two versions of the framework
>> -
>> compare that to current plan to simply get rid of all that code per
>> IGNITE-10179.
>>
>> The last but not the least, this makes it much more complicated to later
>> migrate to JUnit 5. Although this is currently not in the nearest plans
>> (IGNITE-10180) we eventually will want to (especially taking into account
>> that migration from JUnit 4 is said to be easy). Having JUnit 3 tests
>> would
>> much complicate this because we have no idea if JUnit 5 could
>> interoperate
>> with such an old version (and I see no reason why we would want to waste
>> our
>> time and efforts investigating and testing this).
>>
>> Summing up, I believe it is very well worth it for us to get rid of JUnit
>> 3
>> completely.
>>
>>  -----
>>
>> With regards to making LegacySupport enabled only on purpose, at this
>> point
>> I see no reason to enforce this in code because I expect that deprecation
>> notices will do that job.
>>
>> If a developer writing new test or reworking an old one follows the
>> deprecation recommendations they will use JUnit 4 way instead of
>> deprecated
>> methods and you can see from the code that in this case LegacySupport
>> will
>> just transparently pass-through the test code without introducing
>> anything
>> else beyond. (Note we can reconsider and rework this later in case if it
>> turns out that my expectation doesn't hold).
>>
>> Does that answer your questions?
>>
>> regards, Oleg
>>
>>
>> Павлухин Иван wrote
>> > Hi Oleg,
>> >
>> > The things become challenging. Truly I do not see any trivial solution
>> > so far. Could you please outline main problems which we are facing
>> > today? And how many tests are affected? Some clarifying questions:
>> > 1. I know that setup->test->teardown threading was changed for junit4
>> > tests, but actually I thought that it might affect only small number
>> > of tests. Am I right here?
>> > 2. Also I saw that in your experiment [1] some changes related to
>> > TestCounters were made. What is wrong with them?
>> > 3. Why do we need wrap test execution into critical section
>> > (runSerializer lock)? I thought that we always run tests serially.
>> >
>> > I generally like an idea of having workaround falling back to old test
>> > execution flow. But for me the most desired trait of things like
>> > LegacySupport is being lightweight and enabled only on purpose. And
>> > from the first glance current prototype looks a little bit
>> > complicated. As an alternative we can keep junit3 for troublesome
>> > tests, can't we?
>> >
>> > Also is there any vision how many migration problems do we have so far
>> > and how many tests was not migrated yet?
>> > вс, 16 дек. 2018 г. в 17:39, oignatenko &lt;
>>
>> > oignatenko@
>>
>> > &gt;:
>> >>
>> >> Hi Ivan,
>> >>
>> >> As promised in my prior mail, here is the branch where I experimented
>> to
>> >> address concerns you raised:
>> >> -
>> >>
>> https://github.com/gridgain/apache-ignite/tree/ignite-10177-experimental
>> >>
>> >> I tested it locally and on Teamcity and it worked as intended.
>> >>
>> >> I think I managed to exactly reproduce execution sequence of JUnit 3
>> test
>> >> case so that tests designed expecting it will run exactly as it was
>> >> before.
>> >>
>> >> As for troublesome APIs I used deprecation to warn developers agains
>> >> using
>> >> these and recommend what they need to use instead.
>> >>
>> >> If you are interested in closer studying the changes, class
>> >> GridAbstractTest1 is probably best as a starting point. This class is
>> a
>> >> temporary copy of GridAbstractTest made to minimise amount of editing
>> in
>> >> dependent classes while I was experimenting; in real implementation
>> (per
>> >> IGNITE-10177) this code is expected to be in GridAbstractTest.
>> >>
>> >> Also, I used ML testsuite to debug the changes I made, because it
>> >> contains
>> >> convenient mix of usecases and runs fast.
>> >>
>> >> Your feedback is much appreciated.
>> >>
>> >> 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/
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

Ivan Pavlukhin
Hi Oleg,

I have not quite understood who is going to call (from standpoint of
test framework) beforeTestsStarted, beforeTest, afterTest,
afterTestsStarted methods?
вт, 18 дек. 2018 г. в 23:31, oignatenko <[hidden email]>:

>
> Hi Ivan,
>
> To answer your last question, yes, all the tests are to be marked with @Test
> annotations, and those that are meant to be ignored are going to be marked
> with @Ignore annotations. There is no exceptions to that, and these
> annotations will work just as well on tests using our home brewed
> beforeTestsStarted, beforeTest, afterTest, afterTestsStarted.
>
> For the sake of completeness, developers will also be able to use Before* /
> After* annotations on their own methods in tests. The only exception
> (clarified in respective javadocs) is that these annotations shouldn't be
> used on overrides of our home brewed methods - and these methods, in
> addition, will be recommended (not mandated) to avoid wia deprecation
> notices.
>
> -----
>
> As for accessing tests which have problems running under junit4, the way how
> I search for these in current master is regex search in IDEA for
> "addTestSuite.*class", that is I search in testsuites for entries that are
> added using method "addTestSuite" with parameter class.
>
> Probably worth noting that some of the problems that were previously
> blocking addition of particular tests have been resolved in the course of
> working on IGNITE-10177 (https://github.com/apache/ignite/pull/5615). One
> riddle that currently looks particularly difficult to crack is Teamcity
> failures in "Queries 1", I even haven't yet figured how to reproduce these
> locally.
>
> regards, Oleg
>
>
> Павлухин Иван wrote
> > Hi Oleg,
> >
> > Now concerns about junit3 elimination are clear for me. And I agree
> > that it is worth to do it. By the way is it possible to access tests
> > which have problems running under junit4? I would like to take a look.
> >
> > Also a clarifying bit regarding next migration steps is needed. Sorry
> > if it was described but I missed it. Currently we have tons of tests
> > which rely on our home brewed beforeTestsStarted, beforeTest,
> > afterTest, afterTestsStarted. Are you going to mark them all with
> > corresponding junit4 annotations?
> > пн, 17 дек. 2018 г. в 19:13, oignatenko &lt;
>
> > oignatenko@
>
> > &gt;:
> >>
> >> Hi Ivan,
> >>
> >> Per my cursory check of the code currently in master, we have 239 test
> >> classes that couldn't migrate (that's probably about 500 or something
> >> test
> >> cases). For comparison, over 1600 classes migrated without problems. This
> >> is
> >> to answer your questions about how many tests are affected by change and
> >> how many tests were not migrated yet.
> >>
> >> -----
> >>
> >> I think we can say that only small percent of tests turned out sensitive
> >> to
> >> JUnit framework change.
> >>
> >> In the same time, due to very large overall amount of our tests we have
> >> quite a big number as an absolute value. I point this because if amount
> >> of
> >> troublesome tests was smaller (say, order of magnitude smaller) I would
> >> consider delaying migration until all tests reworked to blend totally
> >> seamlessly into new version JUnit. This is doable - as sort of "pilot
> >> exercise" me and Ed adjusted one test case this way - but the sheer
> >> amount
> >> of work on 200+ classes raises a question, whether it is worth it (I
> >> think
> >> it isn't).
> >>
> >> With regards to question 2, changes to TestCounters are farly trivial
> >> (and
> >> necessary) if you look at the code diff. Prior code counted amount of
> >> test
> >> cases in the class by JUnit 3 convention: it picked public void methods
> >> without parameters starting with "test". Changed code counts test cases
> >> as
> >> JUnit 4 does: by checking if method is annotated with @Test. Change is
> >> necessary because it will allow test developers fully use JUnit 4
> >> features
> >> by adding test cases that don't follow old naming requirement. I
> >> experimented with it a little and as far as I could see the old
> >> TestCounters
> >> indeed break when there are methods following new convention, it
> >> triggered
> >> afterTestsStopped too early.
> >>
> >> The answer to your question 3 lies in javadocs I added to runSerializer
> >> declaration, or, more precisely, in TestCounters javadoc referred from
> >> there. As you can see, current plan is to get rid of TestCounters fairly
> >> soon (per IGNITE-10179) and this will also get rid of runSerializer so
> >> this
> >> is merely a temporary band aid to be removed soon.
> >>
> >> For the sake of completeness, my initial plan was to thoroughly
> >> investigate
> >> and test whether change from JUnit 3 to JUnit 4 would keep accessing
> >> counters safe or not and only introduce runSerializer if it really does
> >> but
> >> after realising that this whole thing is likely to go away in a few days
> >> from now I decided that it isn't worth wasting time and just temporarily
> >> made it the way that is waterproof guaranteed to be safe.
> >>
> >> -----
> >>
> >> Now, to answer your question whether it is an option for us to keep part
> >> of
> >> tests under JUnit 3, my answer is most definitely no.
> >>
> >> Main reason is that having part of tests under JUnit 3 will deprive us
> >> ability to consistently use Ignore annotation and force us fallback to
> >> old
> >> way to fail the tests we want to ignore. This would kind of trash the
> >> whole
> >> purpose of migration because we won't be able to simplify and improve
> >> maintenance of ignored tests.
> >>
> >> Another important reason is that keeping JUnit 3 will much complicate our
> >> test framework code. We will have to implement and maintain two versions
> >> of
> >> TestCounters (see answer to your question #2 above). We will also have to
> >> keep the code that currently determines first/last test in the class and
> >> possibly even complicate it to account for two versions of the framework
> >> -
> >> compare that to current plan to simply get rid of all that code per
> >> IGNITE-10179.
> >>
> >> The last but not the least, this makes it much more complicated to later
> >> migrate to JUnit 5. Although this is currently not in the nearest plans
> >> (IGNITE-10180) we eventually will want to (especially taking into account
> >> that migration from JUnit 4 is said to be easy). Having JUnit 3 tests
> >> would
> >> much complicate this because we have no idea if JUnit 5 could
> >> interoperate
> >> with such an old version (and I see no reason why we would want to waste
> >> our
> >> time and efforts investigating and testing this).
> >>
> >> Summing up, I believe it is very well worth it for us to get rid of JUnit
> >> 3
> >> completely.
> >>
> >>  -----
> >>
> >> With regards to making LegacySupport enabled only on purpose, at this
> >> point
> >> I see no reason to enforce this in code because I expect that deprecation
> >> notices will do that job.
> >>
> >> If a developer writing new test or reworking an old one follows the
> >> deprecation recommendations they will use JUnit 4 way instead of
> >> deprecated
> >> methods and you can see from the code that in this case LegacySupport
> >> will
> >> just transparently pass-through the test code without introducing
> >> anything
> >> else beyond. (Note we can reconsider and rework this later in case if it
> >> turns out that my expectation doesn't hold).
> >>
> >> Does that answer your questions?
> >>
> >> regards, Oleg
> >>
> >>
> >> Павлухин Иван wrote
> >> > Hi Oleg,
> >> >
> >> > The things become challenging. Truly I do not see any trivial solution
> >> > so far. Could you please outline main problems which we are facing
> >> > today? And how many tests are affected? Some clarifying questions:
> >> > 1. I know that setup->test->teardown threading was changed for junit4
> >> > tests, but actually I thought that it might affect only small number
> >> > of tests. Am I right here?
> >> > 2. Also I saw that in your experiment [1] some changes related to
> >> > TestCounters were made. What is wrong with them?
> >> > 3. Why do we need wrap test execution into critical section
> >> > (runSerializer lock)? I thought that we always run tests serially.
> >> >
> >> > I generally like an idea of having workaround falling back to old test
> >> > execution flow. But for me the most desired trait of things like
> >> > LegacySupport is being lightweight and enabled only on purpose. And
> >> > from the first glance current prototype looks a little bit
> >> > complicated. As an alternative we can keep junit3 for troublesome
> >> > tests, can't we?
> >> >
> >> > Also is there any vision how many migration problems do we have so far
> >> > and how many tests was not migrated yet?
> >> > вс, 16 дек. 2018 г. в 17:39, oignatenko &lt;
> >>
> >> > oignatenko@
> >>
> >> > &gt;:
> >> >>
> >> >> Hi Ivan,
> >> >>
> >> >> As promised in my prior mail, here is the branch where I experimented
> >> to
> >> >> address concerns you raised:
> >> >> -
> >> >>
> >> https://github.com/gridgain/apache-ignite/tree/ignite-10177-experimental
> >> >>
> >> >> I tested it locally and on Teamcity and it worked as intended.
> >> >>
> >> >> I think I managed to exactly reproduce execution sequence of JUnit 3
> >> test
> >> >> case so that tests designed expecting it will run exactly as it was
> >> >> before.
> >> >>
> >> >> As for troublesome APIs I used deprecation to warn developers agains
> >> >> using
> >> >> these and recommend what they need to use instead.
> >> >>
> >> >> If you are interested in closer studying the changes, class
> >> >> GridAbstractTest1 is probably best as a starting point. This class is
> >> a
> >> >> temporary copy of GridAbstractTest made to minimise amount of editing
> >> in
> >> >> dependent classes while I was experimenting; in real implementation
> >> (per
> >> >> IGNITE-10177) this code is expected to be in GridAbstractTest.
> >> >>
> >> >> Also, I used ML testsuite to debug the changes I made, because it
> >> >> contains
> >> >> convenient mix of usecases and runs fast.
> >> >>
> >> >> Your feedback is much appreciated.
> >> >>
> >> >> 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
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

Dmitry Pavlov
Hi Igniters,

I've tried yesterday to find any of @Ignore d tests with a link to the
issue in its value on TeamCity, but I did not manage to.

Do you know if JUnit Ignored tests are completely invisible in the TC and
its REST? I've also tried to google it, but not found anything yet.

I remember that .NET Ignores are at least visible in the UI and in a REST
request (see, for example,
https://ci.ignite.apache.org/viewLog.html?buildId=2590524&tab=buildResultsDiv&buildTypeId=IgniteTests24Java8_PlatformNet&branch_IgniteTests24Java8=%3Cdefault%3E
).

Please advice me or share a link to a build with Java JUnit Ignores, which
is visible in TC.

Sincerely,
Dmitriy Pavlov

ср, 19 дек. 2018 г. в 11:59, Павлухин Иван <[hidden email]>:

> Hi Oleg,
>
> I have not quite understood who is going to call (from standpoint of
> test framework) beforeTestsStarted, beforeTest, afterTest,
> afterTestsStarted methods?
> вт, 18 дек. 2018 г. в 23:31, oignatenko <[hidden email]>:
> >
> > Hi Ivan,
> >
> > To answer your last question, yes, all the tests are to be marked with
> @Test
> > annotations, and those that are meant to be ignored are going to be
> marked
> > with @Ignore annotations. There is no exceptions to that, and these
> > annotations will work just as well on tests using our home brewed
> > beforeTestsStarted, beforeTest, afterTest, afterTestsStarted.
> >
> > For the sake of completeness, developers will also be able to use
> Before* /
> > After* annotations on their own methods in tests. The only exception
> > (clarified in respective javadocs) is that these annotations shouldn't be
> > used on overrides of our home brewed methods - and these methods, in
> > addition, will be recommended (not mandated) to avoid wia deprecation
> > notices.
> >
> > -----
> >
> > As for accessing tests which have problems running under junit4, the way
> how
> > I search for these in current master is regex search in IDEA for
> > "addTestSuite.*class", that is I search in testsuites for entries that
> are
> > added using method "addTestSuite" with parameter class.
> >
> > Probably worth noting that some of the problems that were previously
> > blocking addition of particular tests have been resolved in the course of
> > working on IGNITE-10177 (https://github.com/apache/ignite/pull/5615).
> One
> > riddle that currently looks particularly difficult to crack is Teamcity
> > failures in "Queries 1", I even haven't yet figured how to reproduce
> these
> > locally.
> >
> > regards, Oleg
> >
> >
> > Павлухин Иван wrote
> > > Hi Oleg,
> > >
> > > Now concerns about junit3 elimination are clear for me. And I agree
> > > that it is worth to do it. By the way is it possible to access tests
> > > which have problems running under junit4? I would like to take a look.
> > >
> > > Also a clarifying bit regarding next migration steps is needed. Sorry
> > > if it was described but I missed it. Currently we have tons of tests
> > > which rely on our home brewed beforeTestsStarted, beforeTest,
> > > afterTest, afterTestsStarted. Are you going to mark them all with
> > > corresponding junit4 annotations?
> > > пн, 17 дек. 2018 г. в 19:13, oignatenko &lt;
> >
> > > oignatenko@
> >
> > > &gt;:
> > >>
> > >> Hi Ivan,
> > >>
> > >> Per my cursory check of the code currently in master, we have 239 test
> > >> classes that couldn't migrate (that's probably about 500 or something
> > >> test
> > >> cases). For comparison, over 1600 classes migrated without problems.
> This
> > >> is
> > >> to answer your questions about how many tests are affected by change
> and
> > >> how many tests were not migrated yet.
> > >>
> > >> -----
> > >>
> > >> I think we can say that only small percent of tests turned out
> sensitive
> > >> to
> > >> JUnit framework change.
> > >>
> > >> In the same time, due to very large overall amount of our tests we
> have
> > >> quite a big number as an absolute value. I point this because if
> amount
> > >> of
> > >> troublesome tests was smaller (say, order of magnitude smaller) I
> would
> > >> consider delaying migration until all tests reworked to blend totally
> > >> seamlessly into new version JUnit. This is doable - as sort of "pilot
> > >> exercise" me and Ed adjusted one test case this way - but the sheer
> > >> amount
> > >> of work on 200+ classes raises a question, whether it is worth it (I
> > >> think
> > >> it isn't).
> > >>
> > >> With regards to question 2, changes to TestCounters are farly trivial
> > >> (and
> > >> necessary) if you look at the code diff. Prior code counted amount of
> > >> test
> > >> cases in the class by JUnit 3 convention: it picked public void
> methods
> > >> without parameters starting with "test". Changed code counts test
> cases
> > >> as
> > >> JUnit 4 does: by checking if method is annotated with @Test. Change is
> > >> necessary because it will allow test developers fully use JUnit 4
> > >> features
> > >> by adding test cases that don't follow old naming requirement. I
> > >> experimented with it a little and as far as I could see the old
> > >> TestCounters
> > >> indeed break when there are methods following new convention, it
> > >> triggered
> > >> afterTestsStopped too early.
> > >>
> > >> The answer to your question 3 lies in javadocs I added to
> runSerializer
> > >> declaration, or, more precisely, in TestCounters javadoc referred from
> > >> there. As you can see, current plan is to get rid of TestCounters
> fairly
> > >> soon (per IGNITE-10179) and this will also get rid of runSerializer so
> > >> this
> > >> is merely a temporary band aid to be removed soon.
> > >>
> > >> For the sake of completeness, my initial plan was to thoroughly
> > >> investigate
> > >> and test whether change from JUnit 3 to JUnit 4 would keep accessing
> > >> counters safe or not and only introduce runSerializer if it really
> does
> > >> but
> > >> after realising that this whole thing is likely to go away in a few
> days
> > >> from now I decided that it isn't worth wasting time and just
> temporarily
> > >> made it the way that is waterproof guaranteed to be safe.
> > >>
> > >> -----
> > >>
> > >> Now, to answer your question whether it is an option for us to keep
> part
> > >> of
> > >> tests under JUnit 3, my answer is most definitely no.
> > >>
> > >> Main reason is that having part of tests under JUnit 3 will deprive us
> > >> ability to consistently use Ignore annotation and force us fallback to
> > >> old
> > >> way to fail the tests we want to ignore. This would kind of trash the
> > >> whole
> > >> purpose of migration because we won't be able to simplify and improve
> > >> maintenance of ignored tests.
> > >>
> > >> Another important reason is that keeping JUnit 3 will much complicate
> our
> > >> test framework code. We will have to implement and maintain two
> versions
> > >> of
> > >> TestCounters (see answer to your question #2 above). We will also
> have to
> > >> keep the code that currently determines first/last test in the class
> and
> > >> possibly even complicate it to account for two versions of the
> framework
> > >> -
> > >> compare that to current plan to simply get rid of all that code per
> > >> IGNITE-10179.
> > >>
> > >> The last but not the least, this makes it much more complicated to
> later
> > >> migrate to JUnit 5. Although this is currently not in the nearest
> plans
> > >> (IGNITE-10180) we eventually will want to (especially taking into
> account
> > >> that migration from JUnit 4 is said to be easy). Having JUnit 3 tests
> > >> would
> > >> much complicate this because we have no idea if JUnit 5 could
> > >> interoperate
> > >> with such an old version (and I see no reason why we would want to
> waste
> > >> our
> > >> time and efforts investigating and testing this).
> > >>
> > >> Summing up, I believe it is very well worth it for us to get rid of
> JUnit
> > >> 3
> > >> completely.
> > >>
> > >>  -----
> > >>
> > >> With regards to making LegacySupport enabled only on purpose, at this
> > >> point
> > >> I see no reason to enforce this in code because I expect that
> deprecation
> > >> notices will do that job.
> > >>
> > >> If a developer writing new test or reworking an old one follows the
> > >> deprecation recommendations they will use JUnit 4 way instead of
> > >> deprecated
> > >> methods and you can see from the code that in this case LegacySupport
> > >> will
> > >> just transparently pass-through the test code without introducing
> > >> anything
> > >> else beyond. (Note we can reconsider and rework this later in case if
> it
> > >> turns out that my expectation doesn't hold).
> > >>
> > >> Does that answer your questions?
> > >>
> > >> regards, Oleg
> > >>
> > >>
> > >> Павлухин Иван wrote
> > >> > Hi Oleg,
> > >> >
> > >> > The things become challenging. Truly I do not see any trivial
> solution
> > >> > so far. Could you please outline main problems which we are facing
> > >> > today? And how many tests are affected? Some clarifying questions:
> > >> > 1. I know that setup->test->teardown threading was changed for
> junit4
> > >> > tests, but actually I thought that it might affect only small number
> > >> > of tests. Am I right here?
> > >> > 2. Also I saw that in your experiment [1] some changes related to
> > >> > TestCounters were made. What is wrong with them?
> > >> > 3. Why do we need wrap test execution into critical section
> > >> > (runSerializer lock)? I thought that we always run tests serially.
> > >> >
> > >> > I generally like an idea of having workaround falling back to old
> test
> > >> > execution flow. But for me the most desired trait of things like
> > >> > LegacySupport is being lightweight and enabled only on purpose. And
> > >> > from the first glance current prototype looks a little bit
> > >> > complicated. As an alternative we can keep junit3 for troublesome
> > >> > tests, can't we?
> > >> >
> > >> > Also is there any vision how many migration problems do we have so
> far
> > >> > and how many tests was not migrated yet?
> > >> > вс, 16 дек. 2018 г. в 17:39, oignatenko &lt;
> > >>
> > >> > oignatenko@
> > >>
> > >> > &gt;:
> > >> >>
> > >> >> Hi Ivan,
> > >> >>
> > >> >> As promised in my prior mail, here is the branch where I
> experimented
> > >> to
> > >> >> address concerns you raised:
> > >> >> -
> > >> >>
> > >>
> https://github.com/gridgain/apache-ignite/tree/ignite-10177-experimental
> > >> >>
> > >> >> I tested it locally and on Teamcity and it worked as intended.
> > >> >>
> > >> >> I think I managed to exactly reproduce execution sequence of JUnit
> 3
> > >> test
> > >> >> case so that tests designed expecting it will run exactly as it was
> > >> >> before.
> > >> >>
> > >> >> As for troublesome APIs I used deprecation to warn developers
> agains
> > >> >> using
> > >> >> these and recommend what they need to use instead.
> > >> >>
> > >> >> If you are interested in closer studying the changes, class
> > >> >> GridAbstractTest1 is probably best as a starting point. This class
> is
> > >> a
> > >> >> temporary copy of GridAbstractTest made to minimise amount of
> editing
> > >> in
> > >> >> dependent classes while I was experimenting; in real implementation
> > >> (per
> > >> >> IGNITE-10177) this code is expected to be in GridAbstractTest.
> > >> >>
> > >> >> Also, I used ML testsuite to debug the changes I made, because it
> > >> >> contains
> > >> >> convenient mix of usecases and runs fast.
> > >> >>
> > >> >> Your feedback is much appreciated.
> > >> >>
> > >> >> 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
>
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

oignatenko
In reply to this post by Ivan Pavlukhin
Hi Ivan,

These methods are called from within GridAbstractTest in exactly same
sequence as they were called in the past from JUnit 3 TestCase class.

This is by the way the reason why these methods should not be annotated with
Before* / After* - because if someone does that then method will be called
twice, once from JUnit 4 framework and one more time from GridAbstractTest.

regards, Oleg


Павлухин Иван wrote
> Hi Oleg,
>
> I have not quite understood who is going to call (from standpoint of
> test framework) beforeTestsStarted, beforeTest, afterTest,
> afterTestsStarted methods?
> вт, 18 дек. 2018 г. в 23:31, oignatenko &lt;

> oignatenko@

> &gt;:
>>
>> Hi Ivan,
>>
>> To answer your last question, yes, all the tests are to be marked with
>> @Test
>> annotations, and those that are meant to be ignored are going to be
>> marked
>> with @Ignore annotations. There is no exceptions to that, and these
>> annotations will work just as well on tests using our home brewed
>> beforeTestsStarted, beforeTest, afterTest, afterTestsStarted.
>>
>> For the sake of completeness, developers will also be able to use Before*
>> /
>> After* annotations on their own methods in tests. The only exception
>> (clarified in respective javadocs) is that these annotations shouldn't be
>> used on overrides of our home brewed methods - and these methods, in
>> addition, will be recommended (not mandated) to avoid wia deprecation
>> notices.
>>
>> -----
>>
>> As for accessing tests which have problems running under junit4, the way
>> how
>> I search for these in current master is regex search in IDEA for
>> "addTestSuite.*class", that is I search in testsuites for entries that
>> are
>> added using method "addTestSuite" with parameter class.
>>
>> Probably worth noting that some of the problems that were previously
>> blocking addition of particular tests have been resolved in the course of
>> working on IGNITE-10177 (https://github.com/apache/ignite/pull/5615). One
>> riddle that currently looks particularly difficult to crack is Teamcity
>> failures in "Queries 1", I even haven't yet figured how to reproduce
>> these
>> locally.
>>
>> regards, Oleg
>>
>>
>> Павлухин Иван wrote
>> > Hi Oleg,
>> >
>> > Now concerns about junit3 elimination are clear for me. And I agree
>> > that it is worth to do it. By the way is it possible to access tests
>> > which have problems running under junit4? I would like to take a look.
>> >
>> > Also a clarifying bit regarding next migration steps is needed. Sorry
>> > if it was described but I missed it. Currently we have tons of tests
>> > which rely on our home brewed beforeTestsStarted, beforeTest,
>> > afterTest, afterTestsStarted. Are you going to mark them all with
>> > corresponding junit4 annotations?
>> > пн, 17 дек. 2018 г. в 19:13, oignatenko &lt;
>>
>> > oignatenko@
>>
>> > &gt;:
>> >>
>> >> Hi Ivan,
>> >>
>> >> Per my cursory check of the code currently in master, we have 239 test
>> >> classes that couldn't migrate (that's probably about 500 or something
>> >> test
>> >> cases). For comparison, over 1600 classes migrated without problems.
>> This
>> >> is
>> >> to answer your questions about how many tests are affected by change
>> and
>> >> how many tests were not migrated yet.
>> >>
>> >> -----
>> >>
>> >> I think we can say that only small percent of tests turned out
>> sensitive
>> >> to
>> >> JUnit framework change.
>> >>
>> >> In the same time, due to very large overall amount of our tests we
>> have
>> >> quite a big number as an absolute value. I point this because if
>> amount
>> >> of
>> >> troublesome tests was smaller (say, order of magnitude smaller) I
>> would
>> >> consider delaying migration until all tests reworked to blend totally
>> >> seamlessly into new version JUnit. This is doable - as sort of "pilot
>> >> exercise" me and Ed adjusted one test case this way - but the sheer
>> >> amount
>> >> of work on 200+ classes raises a question, whether it is worth it (I
>> >> think
>> >> it isn't).
>> >>
>> >> With regards to question 2, changes to TestCounters are farly trivial
>> >> (and
>> >> necessary) if you look at the code diff. Prior code counted amount of
>> >> test
>> >> cases in the class by JUnit 3 convention: it picked public void
>> methods
>> >> without parameters starting with "test". Changed code counts test
>> cases
>> >> as
>> >> JUnit 4 does: by checking if method is annotated with @Test. Change is
>> >> necessary because it will allow test developers fully use JUnit 4
>> >> features
>> >> by adding test cases that don't follow old naming requirement. I
>> >> experimented with it a little and as far as I could see the old
>> >> TestCounters
>> >> indeed break when there are methods following new convention, it
>> >> triggered
>> >> afterTestsStopped too early.
>> >>
>> >> The answer to your question 3 lies in javadocs I added to
>> runSerializer
>> >> declaration, or, more precisely, in TestCounters javadoc referred from
>> >> there. As you can see, current plan is to get rid of TestCounters
>> fairly
>> >> soon (per IGNITE-10179) and this will also get rid of runSerializer so
>> >> this
>> >> is merely a temporary band aid to be removed soon.
>> >>
>> >> For the sake of completeness, my initial plan was to thoroughly
>> >> investigate
>> >> and test whether change from JUnit 3 to JUnit 4 would keep accessing
>> >> counters safe or not and only introduce runSerializer if it really
>> does
>> >> but
>> >> after realising that this whole thing is likely to go away in a few
>> days
>> >> from now I decided that it isn't worth wasting time and just
>> temporarily
>> >> made it the way that is waterproof guaranteed to be safe.
>> >>
>> >> -----
>> >>
>> >> Now, to answer your question whether it is an option for us to keep
>> part
>> >> of
>> >> tests under JUnit 3, my answer is most definitely no.
>> >>
>> >> Main reason is that having part of tests under JUnit 3 will deprive us
>> >> ability to consistently use Ignore annotation and force us fallback to
>> >> old
>> >> way to fail the tests we want to ignore. This would kind of trash the
>> >> whole
>> >> purpose of migration because we won't be able to simplify and improve
>> >> maintenance of ignored tests.
>> >>
>> >> Another important reason is that keeping JUnit 3 will much complicate
>> our
>> >> test framework code. We will have to implement and maintain two
>> versions
>> >> of
>> >> TestCounters (see answer to your question #2 above). We will also have
>> to
>> >> keep the code that currently determines first/last test in the class
>> and
>> >> possibly even complicate it to account for two versions of the
>> framework
>> >> -
>> >> compare that to current plan to simply get rid of all that code per
>> >> IGNITE-10179.
>> >>
>> >> The last but not the least, this makes it much more complicated to
>> later
>> >> migrate to JUnit 5. Although this is currently not in the nearest
>> plans
>> >> (IGNITE-10180) we eventually will want to (especially taking into
>> account
>> >> that migration from JUnit 4 is said to be easy). Having JUnit 3 tests
>> >> would
>> >> much complicate this because we have no idea if JUnit 5 could
>> >> interoperate
>> >> with such an old version (and I see no reason why we would want to
>> waste
>> >> our
>> >> time and efforts investigating and testing this).
>> >>
>> >> Summing up, I believe it is very well worth it for us to get rid of
>> JUnit
>> >> 3
>> >> completely.
>> >>
>> >>  -----
>> >>
>> >> With regards to making LegacySupport enabled only on purpose, at this
>> >> point
>> >> I see no reason to enforce this in code because I expect that
>> deprecation
>> >> notices will do that job.
>> >>
>> >> If a developer writing new test or reworking an old one follows the
>> >> deprecation recommendations they will use JUnit 4 way instead of
>> >> deprecated
>> >> methods and you can see from the code that in this case LegacySupport
>> >> will
>> >> just transparently pass-through the test code without introducing
>> >> anything
>> >> else beyond. (Note we can reconsider and rework this later in case if
>> it
>> >> turns out that my expectation doesn't hold).
>> >>
>> >> Does that answer your questions?
>> >>
>> >> regards, Oleg
>> >>
>> >>
>> >> Павлухин Иван wrote
>> >> > Hi Oleg,
>> >> >
>> >> > The things become challenging. Truly I do not see any trivial
>> solution
>> >> > so far. Could you please outline main problems which we are facing
>> >> > today? And how many tests are affected? Some clarifying questions:
>> >> > 1. I know that setup->test->teardown threading was changed for
>> junit4
>> >> > tests, but actually I thought that it might affect only small number
>> >> > of tests. Am I right here?
>> >> > 2. Also I saw that in your experiment [1] some changes related to
>> >> > TestCounters were made. What is wrong with them?
>> >> > 3. Why do we need wrap test execution into critical section
>> >> > (runSerializer lock)? I thought that we always run tests serially.
>> >> >
>> >> > I generally like an idea of having workaround falling back to old
>> test
>> >> > execution flow. But for me the most desired trait of things like
>> >> > LegacySupport is being lightweight and enabled only on purpose. And
>> >> > from the first glance current prototype looks a little bit
>> >> > complicated. As an alternative we can keep junit3 for troublesome
>> >> > tests, can't we?
>> >> >
>> >> > Also is there any vision how many migration problems do we have so
>> far
>> >> > and how many tests was not migrated yet?
>> >> > вс, 16 дек. 2018 г. в 17:39, oignatenko &lt;
>> >>
>> >> > oignatenko@
>> >>
>> >> > &gt;:
>> >> >>
>> >> >> Hi Ivan,
>> >> >>
>> >> >> As promised in my prior mail, here is the branch where I
>> experimented
>> >> to
>> >> >> address concerns you raised:
>> >> >> -
>> >> >>
>> >>
>> https://github.com/gridgain/apache-ignite/tree/ignite-10177-experimental
>> >> >>
>> >> >> I tested it locally and on Teamcity and it worked as intended.
>> >> >>
>> >> >> I think I managed to exactly reproduce execution sequence of JUnit
>> 3
>> >> test
>> >> >> case so that tests designed expecting it will run exactly as it was
>> >> >> before.
>> >> >>
>> >> >> As for troublesome APIs I used deprecation to warn developers
>> agains
>> >> >> using
>> >> >> these and recommend what they need to use instead.
>> >> >>
>> >> >> If you are interested in closer studying the changes, class
>> >> >> GridAbstractTest1 is probably best as a starting point. This class
>> is
>> >> a
>> >> >> temporary copy of GridAbstractTest made to minimise amount of
>> editing
>> >> in
>> >> >> dependent classes while I was experimenting; in real implementation
>> >> (per
>> >> >> IGNITE-10177) this code is expected to be in GridAbstractTest.
>> >> >>
>> >> >> Also, I used ML testsuite to debug the changes I made, because it
>> >> >> contains
>> >> >> convenient mix of usecases and runs fast.
>> >> >>
>> >> >> Your feedback is much appreciated.
>> >> >>
>> >> >> 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





--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

Ivan Pavlukhin
Dmitriy,

Thank you for noticing! It seems we have a problem here. When junit4
test is called from junit3 suite (with help of JUnit4TestAdapter) such
tests is skipped silently. It seems that we cannot use @Ignore
everywhere yet.
ср, 19 дек. 2018 г. в 15:09, oignatenko <[hidden email]>:

>
> Hi Ivan,
>
> These methods are called from within GridAbstractTest in exactly same
> sequence as they were called in the past from JUnit 3 TestCase class.
>
> This is by the way the reason why these methods should not be annotated with
> Before* / After* - because if someone does that then method will be called
> twice, once from JUnit 4 framework and one more time from GridAbstractTest.
>
> regards, Oleg
>
>
> Павлухин Иван wrote
> > Hi Oleg,
> >
> > I have not quite understood who is going to call (from standpoint of
> > test framework) beforeTestsStarted, beforeTest, afterTest,
> > afterTestsStarted methods?
> > вт, 18 дек. 2018 г. в 23:31, oignatenko &lt;
>
> > oignatenko@
>
> > &gt;:
> >>
> >> Hi Ivan,
> >>
> >> To answer your last question, yes, all the tests are to be marked with
> >> @Test
> >> annotations, and those that are meant to be ignored are going to be
> >> marked
> >> with @Ignore annotations. There is no exceptions to that, and these
> >> annotations will work just as well on tests using our home brewed
> >> beforeTestsStarted, beforeTest, afterTest, afterTestsStarted.
> >>
> >> For the sake of completeness, developers will also be able to use Before*
> >> /
> >> After* annotations on their own methods in tests. The only exception
> >> (clarified in respective javadocs) is that these annotations shouldn't be
> >> used on overrides of our home brewed methods - and these methods, in
> >> addition, will be recommended (not mandated) to avoid wia deprecation
> >> notices.
> >>
> >> -----
> >>
> >> As for accessing tests which have problems running under junit4, the way
> >> how
> >> I search for these in current master is regex search in IDEA for
> >> "addTestSuite.*class", that is I search in testsuites for entries that
> >> are
> >> added using method "addTestSuite" with parameter class.
> >>
> >> Probably worth noting that some of the problems that were previously
> >> blocking addition of particular tests have been resolved in the course of
> >> working on IGNITE-10177 (https://github.com/apache/ignite/pull/5615). One
> >> riddle that currently looks particularly difficult to crack is Teamcity
> >> failures in "Queries 1", I even haven't yet figured how to reproduce
> >> these
> >> locally.
> >>
> >> regards, Oleg
> >>
> >>
> >> Павлухин Иван wrote
> >> > Hi Oleg,
> >> >
> >> > Now concerns about junit3 elimination are clear for me. And I agree
> >> > that it is worth to do it. By the way is it possible to access tests
> >> > which have problems running under junit4? I would like to take a look.
> >> >
> >> > Also a clarifying bit regarding next migration steps is needed. Sorry
> >> > if it was described but I missed it. Currently we have tons of tests
> >> > which rely on our home brewed beforeTestsStarted, beforeTest,
> >> > afterTest, afterTestsStarted. Are you going to mark them all with
> >> > corresponding junit4 annotations?
> >> > пн, 17 дек. 2018 г. в 19:13, oignatenko &lt;
> >>
> >> > oignatenko@
> >>
> >> > &gt;:
> >> >>
> >> >> Hi Ivan,
> >> >>
> >> >> Per my cursory check of the code currently in master, we have 239 test
> >> >> classes that couldn't migrate (that's probably about 500 or something
> >> >> test
> >> >> cases). For comparison, over 1600 classes migrated without problems.
> >> This
> >> >> is
> >> >> to answer your questions about how many tests are affected by change
> >> and
> >> >> how many tests were not migrated yet.
> >> >>
> >> >> -----
> >> >>
> >> >> I think we can say that only small percent of tests turned out
> >> sensitive
> >> >> to
> >> >> JUnit framework change.
> >> >>
> >> >> In the same time, due to very large overall amount of our tests we
> >> have
> >> >> quite a big number as an absolute value. I point this because if
> >> amount
> >> >> of
> >> >> troublesome tests was smaller (say, order of magnitude smaller) I
> >> would
> >> >> consider delaying migration until all tests reworked to blend totally
> >> >> seamlessly into new version JUnit. This is doable - as sort of "pilot
> >> >> exercise" me and Ed adjusted one test case this way - but the sheer
> >> >> amount
> >> >> of work on 200+ classes raises a question, whether it is worth it (I
> >> >> think
> >> >> it isn't).
> >> >>
> >> >> With regards to question 2, changes to TestCounters are farly trivial
> >> >> (and
> >> >> necessary) if you look at the code diff. Prior code counted amount of
> >> >> test
> >> >> cases in the class by JUnit 3 convention: it picked public void
> >> methods
> >> >> without parameters starting with "test". Changed code counts test
> >> cases
> >> >> as
> >> >> JUnit 4 does: by checking if method is annotated with @Test. Change is
> >> >> necessary because it will allow test developers fully use JUnit 4
> >> >> features
> >> >> by adding test cases that don't follow old naming requirement. I
> >> >> experimented with it a little and as far as I could see the old
> >> >> TestCounters
> >> >> indeed break when there are methods following new convention, it
> >> >> triggered
> >> >> afterTestsStopped too early.
> >> >>
> >> >> The answer to your question 3 lies in javadocs I added to
> >> runSerializer
> >> >> declaration, or, more precisely, in TestCounters javadoc referred from
> >> >> there. As you can see, current plan is to get rid of TestCounters
> >> fairly
> >> >> soon (per IGNITE-10179) and this will also get rid of runSerializer so
> >> >> this
> >> >> is merely a temporary band aid to be removed soon.
> >> >>
> >> >> For the sake of completeness, my initial plan was to thoroughly
> >> >> investigate
> >> >> and test whether change from JUnit 3 to JUnit 4 would keep accessing
> >> >> counters safe or not and only introduce runSerializer if it really
> >> does
> >> >> but
> >> >> after realising that this whole thing is likely to go away in a few
> >> days
> >> >> from now I decided that it isn't worth wasting time and just
> >> temporarily
> >> >> made it the way that is waterproof guaranteed to be safe.
> >> >>
> >> >> -----
> >> >>
> >> >> Now, to answer your question whether it is an option for us to keep
> >> part
> >> >> of
> >> >> tests under JUnit 3, my answer is most definitely no.
> >> >>
> >> >> Main reason is that having part of tests under JUnit 3 will deprive us
> >> >> ability to consistently use Ignore annotation and force us fallback to
> >> >> old
> >> >> way to fail the tests we want to ignore. This would kind of trash the
> >> >> whole
> >> >> purpose of migration because we won't be able to simplify and improve
> >> >> maintenance of ignored tests.
> >> >>
> >> >> Another important reason is that keeping JUnit 3 will much complicate
> >> our
> >> >> test framework code. We will have to implement and maintain two
> >> versions
> >> >> of
> >> >> TestCounters (see answer to your question #2 above). We will also have
> >> to
> >> >> keep the code that currently determines first/last test in the class
> >> and
> >> >> possibly even complicate it to account for two versions of the
> >> framework
> >> >> -
> >> >> compare that to current plan to simply get rid of all that code per
> >> >> IGNITE-10179.
> >> >>
> >> >> The last but not the least, this makes it much more complicated to
> >> later
> >> >> migrate to JUnit 5. Although this is currently not in the nearest
> >> plans
> >> >> (IGNITE-10180) we eventually will want to (especially taking into
> >> account
> >> >> that migration from JUnit 4 is said to be easy). Having JUnit 3 tests
> >> >> would
> >> >> much complicate this because we have no idea if JUnit 5 could
> >> >> interoperate
> >> >> with such an old version (and I see no reason why we would want to
> >> waste
> >> >> our
> >> >> time and efforts investigating and testing this).
> >> >>
> >> >> Summing up, I believe it is very well worth it for us to get rid of
> >> JUnit
> >> >> 3
> >> >> completely.
> >> >>
> >> >>  -----
> >> >>
> >> >> With regards to making LegacySupport enabled only on purpose, at this
> >> >> point
> >> >> I see no reason to enforce this in code because I expect that
> >> deprecation
> >> >> notices will do that job.
> >> >>
> >> >> If a developer writing new test or reworking an old one follows the
> >> >> deprecation recommendations they will use JUnit 4 way instead of
> >> >> deprecated
> >> >> methods and you can see from the code that in this case LegacySupport
> >> >> will
> >> >> just transparently pass-through the test code without introducing
> >> >> anything
> >> >> else beyond. (Note we can reconsider and rework this later in case if
> >> it
> >> >> turns out that my expectation doesn't hold).
> >> >>
> >> >> Does that answer your questions?
> >> >>
> >> >> regards, Oleg
> >> >>
> >> >>
> >> >> Павлухин Иван wrote
> >> >> > Hi Oleg,
> >> >> >
> >> >> > The things become challenging. Truly I do not see any trivial
> >> solution
> >> >> > so far. Could you please outline main problems which we are facing
> >> >> > today? And how many tests are affected? Some clarifying questions:
> >> >> > 1. I know that setup->test->teardown threading was changed for
> >> junit4
> >> >> > tests, but actually I thought that it might affect only small number
> >> >> > of tests. Am I right here?
> >> >> > 2. Also I saw that in your experiment [1] some changes related to
> >> >> > TestCounters were made. What is wrong with them?
> >> >> > 3. Why do we need wrap test execution into critical section
> >> >> > (runSerializer lock)? I thought that we always run tests serially.
> >> >> >
> >> >> > I generally like an idea of having workaround falling back to old
> >> test
> >> >> > execution flow. But for me the most desired trait of things like
> >> >> > LegacySupport is being lightweight and enabled only on purpose. And
> >> >> > from the first glance current prototype looks a little bit
> >> >> > complicated. As an alternative we can keep junit3 for troublesome
> >> >> > tests, can't we?
> >> >> >
> >> >> > Also is there any vision how many migration problems do we have so
> >> far
> >> >> > and how many tests was not migrated yet?
> >> >> > вс, 16 дек. 2018 г. в 17:39, oignatenko &lt;
> >> >>
> >> >> > oignatenko@
> >> >>
> >> >> > &gt;:
> >> >> >>
> >> >> >> Hi Ivan,
> >> >> >>
> >> >> >> As promised in my prior mail, here is the branch where I
> >> experimented
> >> >> to
> >> >> >> address concerns you raised:
> >> >> >> -
> >> >> >>
> >> >>
> >> https://github.com/gridgain/apache-ignite/tree/ignite-10177-experimental
> >> >> >>
> >> >> >> I tested it locally and on Teamcity and it worked as intended.
> >> >> >>
> >> >> >> I think I managed to exactly reproduce execution sequence of JUnit
> >> 3
> >> >> test
> >> >> >> case so that tests designed expecting it will run exactly as it was
> >> >> >> before.
> >> >> >>
> >> >> >> As for troublesome APIs I used deprecation to warn developers
> >> agains
> >> >> >> using
> >> >> >> these and recommend what they need to use instead.
> >> >> >>
> >> >> >> If you are interested in closer studying the changes, class
> >> >> >> GridAbstractTest1 is probably best as a starting point. This class
> >> is
> >> >> a
> >> >> >> temporary copy of GridAbstractTest made to minimise amount of
> >> editing
> >> >> in
> >> >> >> dependent classes while I was experimenting; in real implementation
> >> >> (per
> >> >> >> IGNITE-10177) this code is expected to be in GridAbstractTest.
> >> >> >>
> >> >> >> Also, I used ML testsuite to debug the changes I made, because it
> >> >> >> contains
> >> >> >> convenient mix of usecases and runs fast.
> >> >> >>
> >> >> >> Your feedback is much appreciated.
> >> >> >>
> >> >> >> 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
>
>
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/



--
Best regards,
Ivan Pavlukhin
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

oignatenko
In reply to this post by Vladimir Ozerov
...sort of a follow-up. I gave folks a bit more leeway than originally
planned week or two.

It's been about a month since master was changed in a way that wouldn't
allow JUnit 3 tests execute anymore (per IGNITE-10177) and I just started
progress on IGNITE-10629 and made a promised inspection for tests that could
slip through without required annotation while we were migrating the project
to JUnit 4.

Results of this check look pretty good, IDEA reported only 6 (six)
troublesome test cases (4 of which were by the way annotated @Ignore,
meaning that even with added @Test these won't run anyway). Compared to over
11,600 properly annotated test cases this looks really minor. We probably
can say that tests in master migrated to JUnit 4 quite smoothly.

 regards, Oleg


Vladimir Ozerov wrote
> 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.
>> >
>> [cut]
>>
>> --
>> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>>





--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Reply | Threaded
Open this post in threaded view
|

Re: Is it time to move forward to JUnit4 (5)?

Ivan Pavlukhin
Cheers!

вт, 22 янв. 2019 г. в 20:19, oignatenko <[hidden email]>:

>
> ...sort of a follow-up. I gave folks a bit more leeway than originally
> planned week or two.
>
> It's been about a month since master was changed in a way that wouldn't
> allow JUnit 3 tests execute anymore (per IGNITE-10177) and I just started
> progress on IGNITE-10629 and made a promised inspection for tests that could
> slip through without required annotation while we were migrating the project
> to JUnit 4.
>
> Results of this check look pretty good, IDEA reported only 6 (six)
> troublesome test cases (4 of which were by the way annotated @Ignore,
> meaning that even with added @Test these won't run anyway). Compared to over
> 11,600 properly annotated test cases this looks really minor. We probably
> can say that tests in master migrated to JUnit 4 quite smoothly.
>
>  regards, Oleg
>
>
> Vladimir Ozerov wrote
> > 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.
> >> >
> >> [cut]
> >>
> >> --
> >> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> >>
>
>
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/



--
Best regards,
Ivan Pavlukhin
123