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
|

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

Dmitriy Pavlov
Hi Igniters,

During MTCGA monitoring I’ve discovered a number of issues related to test
framework itself.

In addition it is not convinient to mute tests on TC, because in branches
(e.g. 2.5 and in master) we can’t mute it separately. So some tests, which
already fixed are shown as failed in our PRs and release branches. But
current JUnit3 test style does not allow to use @Ignore.

Is it time to start translating tests to Unit 4 style?

Are there any of us who would like to assist in the research and create a
new GridCommonAbstractTest and/or IgniteJUnitRunner?

Sincerely,
Dmitriy Pavlov
Reply | Threaded
Open this post in threaded view
|

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

Alexey Kuznetsov
+100500 for upgrading of JUnit

https://developer.ibm.com/dwblog/2017/top-five-reasons-to-use-junit-5-java/
https://stackify.com/junit-5/

On Wed, Apr 18, 2018 at 8:59 PM, Dmitry Pavlov <[hidden email]>
wrote:

> Hi Igniters,
>
> During MTCGA monitoring I’ve discovered a number of issues related to test
> framework itself.
>
> In addition it is not convinient to mute tests on TC, because in branches
> (e.g. 2.5 and in master) we can’t mute it separately. So some tests, which
> already fixed are shown as failed in our PRs and release branches. But
> current JUnit3 test style does not allow to use @Ignore.
>
> Is it time to start translating tests to Unit 4 style?
>
> Are there any of us who would like to assist in the research and create a
> new GridCommonAbstractTest and/or IgniteJUnitRunner?
>
> Sincerely,
> Dmitriy Pavlov
>



--
Alexey Kuznetsov
Reply | Threaded
Open this post in threaded view
|

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

daradurvs
I vote for upgrading to JUnit 5.

It will entail significant changes in the current testing framework.

Dmitriy, do you want to change API of GridCommonAbstractTest or
deprecate some methods?
Reply | Threaded
Open this post in threaded view
|

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

Mmuzaf
+1 for JUnit5

I suppose we should change\create new not only GridCommonAbstractTest, but
also rework GridSpiAbstractTest, GridAbstractTest.


ср, 18 апр. 2018 г. в 17:37, Vyacheslav Daradur <[hidden email]>:

> I vote for upgrading to JUnit 5.
>
> It will entail significant changes in the current testing framework.
>
> Dmitriy, do you want to change API of GridCommonAbstractTest or
> deprecate some methods?
>
Reply | Threaded
Open this post in threaded view
|

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

Dmitriy Pavlov
In reply to this post by daradurvs
First of all it should be new way for testing without modificaitions in
JUnit3-style tests and its base classes. Test would migrate one-by one.

If old test is quite stable on 3 and no need to mute it/apply config
variations (Parametrized annotation) we could keep it as is for some time.


ср, 18 апр. 2018 г. в 17:37, Vyacheslav Daradur <[hidden email]>:

> I vote for upgrading to JUnit 5.


>
> It will entail significant changes in the current testing framework.
>
> Dmitriy, do you want to change API of GridCommonAbstractTest or
> deprecate some methods?
>
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 Dmitriy Pavlov
Hello,

I created JIRA task for this move, with detailed explanation and
step-by-step subtasks, your comments are welcome:

- https://issues.apache.org/jira/browse/IGNITE-10173

In brief, the plan is to gradually migrate the part of tests that still uses
Junit 3 (hundreds if not thousands of those that depend on GridAbstractTest
and its subclasses) to newer version. The trick proposed to avoid doing all
this in one (big and risky) step is to introduce a Junit4-based "twin" of
GridAbstractTest (and respective twins of its subclasses) and use it to
gradually change tests to use that newer API instead.

After above is over, Junit 3 and all its related stuff (including
GridAbstractTest) could be safely removed from project.

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)?

daradurvs
Hi, thanks for pushing this activity.

You have mentioned only GridAbstractTest to be changed at the first
step, but how about GridCommonAbstractTest?

Tests in examples depend on GridCommonAbstractTest, should we clone
this class too or merge with GridAbstractTest?
On Wed, Nov 7, 2018 at 7:57 PM oignatenko <[hidden email]> wrote:

>
> Hello,
>
> I created JIRA task for this move, with detailed explanation and
> step-by-step subtasks, your comments are welcome:
>
> - https://issues.apache.org/jira/browse/IGNITE-10173
>
> In brief, the plan is to gradually migrate the part of tests that still uses
> Junit 3 (hundreds if not thousands of those that depend on GridAbstractTest
> and its subclasses) to newer version. The trick proposed to avoid doing all
> this in one (big and risky) step is to introduce a Junit4-based "twin" of
> GridAbstractTest (and respective twins of its subclasses) and use it to
> gradually change tests to use that newer API instead.
>
> After above is over, Junit 3 and all its related stuff (including
> GridAbstractTest) could be safely removed from project.
>
> regards, Oleg
>
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/



--
Best Regards, Vyacheslav D.
Reply | Threaded
Open this post in threaded view
|

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

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

Migrating to Junit 4 sounds as great idea. I believe everybody
is quite surprised when finds Junit 3 in Ignite.
But for me personally it is good to understand what problem we
are trying to solve? What benefits will Junit 4 give us? What are
the most painful moments with current testing framework?
(I my mind I draw a proposal which contains sections "Motivation",
"Alternatives", etc.)

Also I must say that I made some experiments related to Junit4
migration. And from the first glance it seems that the most
important Ignite test framework classes contain a huge amount of
utility methods and a little amount of code related to a test lifecycle.
These classes are GridAbstractTest and GridCommonAbstractTest.
Both are quite thick, but as already said it is mostly utility code
which could be extracted. I would be great to come to very thin base
test classes (or even to baseclass-less tests at all) and include all
kind of utilities without inheritance.

But massive refactoring perhaps is more for future. The main point
here is that Ignite test framework is not complex at all and could be
easily adopted to Junit 4.

Regarding twin classes. How to keep them in sync? If the migration
process will be paused halfway it could make things complicated in
future.

Additionally, I have a wild idea how to marry junit 3 and 4 without
introducing twins. The idea looks ugly from OOP perspective, but
here it is. We can simply annotate overridden Junit 3 setUp and
tearDown methods with @Before and @After annotations. And
use runTest overridden method for Junit 3 and TestRule for Junit 4
in order to run test methods in newly started thread.

For all details you can see an experiment on github [1]. Among them
are ticks with @RunWith annotation making possible to run a test
inherited from TestCase as Junit 4 test. Still, I might have missed
something.

Does it sound sane anyhow?

[1] https://github.com/apache/ignite/pull/5354/files


ср, 7 нояб. 2018 г. в 19:57, oignatenko <[hidden email]>:

> Hello,
>
> I created JIRA task for this move, with detailed explanation and
> step-by-step subtasks, your comments are welcome:
>
> - https://issues.apache.org/jira/browse/IGNITE-10173
>
> In brief, the plan is to gradually migrate the part of tests that still
> uses
> Junit 3 (hundreds if not thousands of those that depend on GridAbstractTest
> and its subclasses) to newer version. The trick proposed to avoid doing all
> this in one (big and risky) step is to introduce a Junit4-based "twin" of
> GridAbstractTest (and respective twins of its subclasses) and use it to
> gradually change tests to use that newer API instead.
>
> After above is over, Junit 3 and all its related stuff (including
> GridAbstractTest) could be safely removed from project.
>
> 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 daradurvs
Hi Vyacheslav,

GridCommonAbstractTest is expected to change too.

As pointed in IGNITE-10173 our approach involves all the subclasses of
GridAbstractTest - including but not limited to one you mentioned
(GridCommonAbstractTest).

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,

That's a very good question. I would say the primary motivation in the task
we discuss is to make the change at the minimal cost and risk. Or, as stated
in description of IGNITE-10173 "move... gradually, smoothly and safely".

With this in mind I would say that main expected benefit is laid out at the
top of ticket description:
> Currently unit tests in the project are mix of two versions Junit 3 and 4.
> This makes it hard to develop and maintain. Making all tests use the same
> version Junit is intended to address this problem.

Above goal is probably modest but given that this change is expected to take
minimal effort it looks good enough for me.

On a more general note I don't expect strong technical benefits of this
change, at least not immediately. This is based on my studies of prior
discussions related to this matter and of the existing tests. I believe that
if there were any strong and obvious technical benefits we would be long
aware of these and we would migrate to newer Junit long time ago.

Observing that migration hasn't happened - despite JUnit 3 being obsolete
for over 10 years now, and despite many more fundamental changes that
successfully made it to Ignite in past years - it looks natural to assume
that there are just no technical reasons that would be strong enough to
justify investing much effort in the migration.

---

Now about your experiment at ignite/pull/5354, I briefly skimmed it and here
are some initial impressions.

First the bad news, some parts look indeed "insane" to me. I am talking
specifically about renames in CacheMvccDmlSimpleTest and changes to test
design in IgniteCacheCommitDelayTxRecoveryTest. I believe that these are
wildly out of scope of the discussed change because I firmly want to keep it
limited to absolute minimum of changes (ideally only annotations with
involved imports).

This is because there are just too many tests to (manually) change. My rough
estimate is there about 7,000 (seven thousands!) test cases to change in
core module alone; and I guess there are few thousands more in other parts
of the project. Doing any extra changes to these tests makes things much
more complicated, effort consuming and risky than was initially supposed.

On a brighter side, changes made to GridAbstractTest look worth further
studying. At this point I still prefer to keep this class unchanged (because
this gives us a bulletproof guarantee against regressions in all prior
tests) but this may change after we do "pilot" changes to examples tests
(IGNITE-10174).

These pilot changes will help us estimate with concrete code how much effort
could be involved in maintaining "twin" classes in sync (you are absolutely
right pointing that this is concerning). If it turns out too complicated to
sync we would probably switch to something like you have done in pull/5354

regards, Oleg


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

> Hi Oleg,
>
> Migrating to Junit 4 sounds as great idea. I believe everybody
> is quite surprised when finds Junit 3 in Ignite.
> But for me personally it is good to understand what problem we
> are trying to solve? What benefits will Junit 4 give us? What are
> the most painful moments with current testing framework?
> (I my mind I draw a proposal which contains sections "Motivation",
> "Alternatives", etc.)
>
> Also I must say that I made some experiments related to Junit4
> migration. And from the first glance it seems that the most
> important Ignite test framework classes contain a huge amount of
> utility methods and a little amount of code related to a test lifecycle.
> These classes are GridAbstractTest and GridCommonAbstractTest.
> Both are quite thick, but as already said it is mostly utility code
> which could be extracted. I would be great to come to very thin base
> test classes (or even to baseclass-less tests at all) and include all
> kind of utilities without inheritance.
>
> But massive refactoring perhaps is more for future. The main point
> here is that Ignite test framework is not complex at all and could be
> easily adopted to Junit 4.
>
> Regarding twin classes. How to keep them in sync? If the migration
> process will be paused halfway it could make things complicated in
> future.
>
> Additionally, I have a wild idea how to marry junit 3 and 4 without
> introducing twins. The idea looks ugly from OOP perspective, but
> here it is. We can simply annotate overridden Junit 3 setUp and
> tearDown methods with @Before and @After annotations. And
> use runTest overridden method for Junit 3 and TestRule for Junit 4
> in order to run test methods in newly started thread.
>
> For all details you can see an experiment on github [1]. Among them
> are ticks with @RunWith annotation making possible to run a test
> inherited from TestCase as Junit 4 test. Still, I might have missed
> something.
>
> Does it sound sane anyhow?
>
> [1] https://github.com/apache/ignite/pull/5354/files
>
>
> ср, 7 нояб. 2018 г. в 19:57, oignatenko &lt;

> oignatenko@

> &gt;:
>
>> Hello,
>>
>> I created JIRA task for this move, with detailed explanation and
>> step-by-step subtasks, your comments are welcome:
>>
>> - https://issues.apache.org/jira/browse/IGNITE-10173
>>
>> In brief, the plan is to gradually migrate the part of tests that still
>> uses
>> Junit 3 (hundreds if not thousands of those that depend on
>> GridAbstractTest
>> and its subclasses) to newer version. The trick proposed to avoid doing
>> all
>> this in one (big and risky) step is to introduce a Junit4-based "twin" of
>> GridAbstractTest (and respective twins of its subclasses) and use it to
>> gradually change tests to use that newer API instead.
>>
>> After above is over, Junit 3 and all its related stuff (including
>> GridAbstractTest) could be safely removed from project.
>>
>> 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,

I can outline some motivating ideas. There is no agreed solutions yet,
but junit4 might help us in reusing existing tests for improving MVCC
coverage. Another area is managing RunAll execution time with help
of some kind of tests parameterization.

Regarding concrete tests changes from my experiment [1]. They were
changed in order to demonstrate certain capabilities in action.
1. CacheMvccDmlSimpleTest simply demonstrates that it is a valid
junit4 test. Test names without "test" prefix make it evident that tests
are run by junit4 engine.
2. IgniteCacheCommitDelayTxRecoveryTest shows how a pattern
common for our junit3 tests could be improved by junit4.

I did not have each test method renaming in mind (but I suppose we
cannot avoid marking each test with @Test annotation).

And I also have run RunAll after introducing changes in GridAbstractTest
[2].
There were several rebuilds because of problems found along the way. I will
return to it in order to receive good enough single RunAll result eligible
for
analysis by human.

[1] https://github.com/apache/ignite/pull/5354/files
[2]
https://mtcga.gridgain.com/pr.html?serverId=apache&suiteId=IgniteTests24Java8_RunAll&branchForTc=pull/5354/head&action=Latest


пт, 9 нояб. 2018 г. в 16:48, oignatenko <[hidden email]>:

> Hi Ivan,
>
> That's a very good question. I would say the primary motivation in the task
> we discuss is to make the change at the minimal cost and risk. Or, as
> stated
> in description of IGNITE-10173 "move... gradually, smoothly and safely".
>
> With this in mind I would say that main expected benefit is laid out at the
> top of ticket description:
> > Currently unit tests in the project are mix of two versions Junit 3 and
> 4.
> > This makes it hard to develop and maintain. Making all tests use the same
> > version Junit is intended to address this problem.
>
> Above goal is probably modest but given that this change is expected to
> take
> minimal effort it looks good enough for me.
>
> On a more general note I don't expect strong technical benefits of this
> change, at least not immediately. This is based on my studies of prior
> discussions related to this matter and of the existing tests. I believe
> that
> if there were any strong and obvious technical benefits we would be long
> aware of these and we would migrate to newer Junit long time ago.
>
> Observing that migration hasn't happened - despite JUnit 3 being obsolete
> for over 10 years now, and despite many more fundamental changes that
> successfully made it to Ignite in past years - it looks natural to assume
> that there are just no technical reasons that would be strong enough to
> justify investing much effort in the migration.
>
> ---
>
> Now about your experiment at ignite/pull/5354, I briefly skimmed it and
> here
> are some initial impressions.
>
> First the bad news, some parts look indeed "insane" to me. I am talking
> specifically about renames in CacheMvccDmlSimpleTest and changes to test
> design in IgniteCacheCommitDelayTxRecoveryTest. I believe that these are
> wildly out of scope of the discussed change because I firmly want to keep
> it
> limited to absolute minimum of changes (ideally only annotations with
> involved imports).
>
> This is because there are just too many tests to (manually) change. My
> rough
> estimate is there about 7,000 (seven thousands!) test cases to change in
> core module alone; and I guess there are few thousands more in other parts
> of the project. Doing any extra changes to these tests makes things much
> more complicated, effort consuming and risky than was initially supposed.
>
> On a brighter side, changes made to GridAbstractTest look worth further
> studying. At this point I still prefer to keep this class unchanged
> (because
> this gives us a bulletproof guarantee against regressions in all prior
> tests) but this may change after we do "pilot" changes to examples tests
> (IGNITE-10174).
>
> These pilot changes will help us estimate with concrete code how much
> effort
> could be involved in maintaining "twin" classes in sync (you are absolutely
> right pointing that this is concerning). If it turns out too complicated to
> sync we would probably switch to something like you have done in pull/5354
>
> regards, Oleg
>
>
> Павлухин Иван wrote
> > Hi Oleg,
> >
> > Migrating to Junit 4 sounds as great idea. I believe everybody
> > is quite surprised when finds Junit 3 in Ignite.
> > But for me personally it is good to understand what problem we
> > are trying to solve? What benefits will Junit 4 give us? What are
> > the most painful moments with current testing framework?
> > (I my mind I draw a proposal which contains sections "Motivation",
> > "Alternatives", etc.)
> >
> > Also I must say that I made some experiments related to Junit4
> > migration. And from the first glance it seems that the most
> > important Ignite test framework classes contain a huge amount of
> > utility methods and a little amount of code related to a test lifecycle.
> > These classes are GridAbstractTest and GridCommonAbstractTest.
> > Both are quite thick, but as already said it is mostly utility code
> > which could be extracted. I would be great to come to very thin base
> > test classes (or even to baseclass-less tests at all) and include all
> > kind of utilities without inheritance.
> >
> > But massive refactoring perhaps is more for future. The main point
> > here is that Ignite test framework is not complex at all and could be
> > easily adopted to Junit 4.
> >
> > Regarding twin classes. How to keep them in sync? If the migration
> > process will be paused halfway it could make things complicated in
> > future.
> >
> > Additionally, I have a wild idea how to marry junit 3 and 4 without
> > introducing twins. The idea looks ugly from OOP perspective, but
> > here it is. We can simply annotate overridden Junit 3 setUp and
> > tearDown methods with @Before and @After annotations. And
> > use runTest overridden method for Junit 3 and TestRule for Junit 4
> > in order to run test methods in newly started thread.
> >
> > For all details you can see an experiment on github [1]. Among them
> > are ticks with @RunWith annotation making possible to run a test
> > inherited from TestCase as Junit 4 test. Still, I might have missed
> > something.
> >
> > Does it sound sane anyhow?
> >
> > [1] https://github.com/apache/ignite/pull/5354/files
> >
> >
> > ср, 7 нояб. 2018 г. в 19:57, oignatenko &lt;
>
> > oignatenko@
>
> > &gt;:
> >
> >> Hello,
> >>
> >> I created JIRA task for this move, with detailed explanation and
> >> step-by-step subtasks, your comments are welcome:
> >>
> >> - https://issues.apache.org/jira/browse/IGNITE-10173
> >>
> >> In brief, the plan is to gradually migrate the part of tests that still
> >> uses
> >> Junit 3 (hundreds if not thousands of those that depend on
> >> GridAbstractTest
> >> and its subclasses) to newer version. The trick proposed to avoid doing
> >> all
> >> this in one (big and risky) step is to introduce a Junit4-based "twin"
> of
> >> GridAbstractTest (and respective twins of its subclasses) and use it to
> >> gradually change tests to use that newer API instead.
> >>
> >> After above is over, Junit 3 and all its related stuff (including
> >> GridAbstractTest) could be safely removed from project.
> >>
> >> 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)?

Ivan Pavlukhin
Hi,

I have rerun RunAll for the aforementioned PR [1]. Results [2] look
very similar to ones which we currently have for master.

[1] https://github.com/apache/ignite/pull/5354/files
[2] https://ci.ignite.apache.org/viewLog.html?buildId=2282588&tab=buildResultsDiv&buildTypeId=IgniteTests24Java8_RunAll
пт, 9 нояб. 2018 г. в 17:30, Павлухин Иван <[hidden email]>:

>
> Hi Oleg,
>
> I can outline some motivating ideas. There is no agreed solutions yet,
> but junit4 might help us in reusing existing tests for improving MVCC
> coverage. Another area is managing RunAll execution time with help
> of some kind of tests parameterization.
>
> Regarding concrete tests changes from my experiment [1]. They were
> changed in order to demonstrate certain capabilities in action.
> 1. CacheMvccDmlSimpleTest simply demonstrates that it is a valid
> junit4 test. Test names without "test" prefix make it evident that tests
> are run by junit4 engine.
> 2. IgniteCacheCommitDelayTxRecoveryTest shows how a pattern
> common for our junit3 tests could be improved by junit4.
>
> I did not have each test method renaming in mind (but I suppose we
> cannot avoid marking each test with @Test annotation).
>
> And I also have run RunAll after introducing changes in GridAbstractTest [2].
> There were several rebuilds because of problems found along the way. I will
> return to it in order to receive good enough single RunAll result eligible for
> analysis by human.
>
> [1] https://github.com/apache/ignite/pull/5354/files
> [2] https://mtcga.gridgain.com/pr.html?serverId=apache&suiteId=IgniteTests24Java8_RunAll&branchForTc=pull/5354/head&action=Latest
>
>
> пт, 9 нояб. 2018 г. в 16:48, oignatenko <[hidden email]>:
>>
>> Hi Ivan,
>>
>> That's a very good question. I would say the primary motivation in the task
>> we discuss is to make the change at the minimal cost and risk. Or, as stated
>> in description of IGNITE-10173 "move... gradually, smoothly and safely".
>>
>> With this in mind I would say that main expected benefit is laid out at the
>> top of ticket description:
>> > Currently unit tests in the project are mix of two versions Junit 3 and 4.
>> > This makes it hard to develop and maintain. Making all tests use the same
>> > version Junit is intended to address this problem.
>>
>> Above goal is probably modest but given that this change is expected to take
>> minimal effort it looks good enough for me.
>>
>> On a more general note I don't expect strong technical benefits of this
>> change, at least not immediately. This is based on my studies of prior
>> discussions related to this matter and of the existing tests. I believe that
>> if there were any strong and obvious technical benefits we would be long
>> aware of these and we would migrate to newer Junit long time ago.
>>
>> Observing that migration hasn't happened - despite JUnit 3 being obsolete
>> for over 10 years now, and despite many more fundamental changes that
>> successfully made it to Ignite in past years - it looks natural to assume
>> that there are just no technical reasons that would be strong enough to
>> justify investing much effort in the migration.
>>
>> ---
>>
>> Now about your experiment at ignite/pull/5354, I briefly skimmed it and here
>> are some initial impressions.
>>
>> First the bad news, some parts look indeed "insane" to me. I am talking
>> specifically about renames in CacheMvccDmlSimpleTest and changes to test
>> design in IgniteCacheCommitDelayTxRecoveryTest. I believe that these are
>> wildly out of scope of the discussed change because I firmly want to keep it
>> limited to absolute minimum of changes (ideally only annotations with
>> involved imports).
>>
>> This is because there are just too many tests to (manually) change. My rough
>> estimate is there about 7,000 (seven thousands!) test cases to change in
>> core module alone; and I guess there are few thousands more in other parts
>> of the project. Doing any extra changes to these tests makes things much
>> more complicated, effort consuming and risky than was initially supposed.
>>
>> On a brighter side, changes made to GridAbstractTest look worth further
>> studying. At this point I still prefer to keep this class unchanged (because
>> this gives us a bulletproof guarantee against regressions in all prior
>> tests) but this may change after we do "pilot" changes to examples tests
>> (IGNITE-10174).
>>
>> These pilot changes will help us estimate with concrete code how much effort
>> could be involved in maintaining "twin" classes in sync (you are absolutely
>> right pointing that this is concerning). If it turns out too complicated to
>> sync we would probably switch to something like you have done in pull/5354
>>
>> regards, Oleg
>>
>>
>> Павлухин Иван wrote
>> > Hi Oleg,
>> >
>> > Migrating to Junit 4 sounds as great idea. I believe everybody
>> > is quite surprised when finds Junit 3 in Ignite.
>> > But for me personally it is good to understand what problem we
>> > are trying to solve? What benefits will Junit 4 give us? What are
>> > the most painful moments with current testing framework?
>> > (I my mind I draw a proposal which contains sections "Motivation",
>> > "Alternatives", etc.)
>> >
>> > Also I must say that I made some experiments related to Junit4
>> > migration. And from the first glance it seems that the most
>> > important Ignite test framework classes contain a huge amount of
>> > utility methods and a little amount of code related to a test lifecycle.
>> > These classes are GridAbstractTest and GridCommonAbstractTest.
>> > Both are quite thick, but as already said it is mostly utility code
>> > which could be extracted. I would be great to come to very thin base
>> > test classes (or even to baseclass-less tests at all) and include all
>> > kind of utilities without inheritance.
>> >
>> > But massive refactoring perhaps is more for future. The main point
>> > here is that Ignite test framework is not complex at all and could be
>> > easily adopted to Junit 4.
>> >
>> > Regarding twin classes. How to keep them in sync? If the migration
>> > process will be paused halfway it could make things complicated in
>> > future.
>> >
>> > Additionally, I have a wild idea how to marry junit 3 and 4 without
>> > introducing twins. The idea looks ugly from OOP perspective, but
>> > here it is. We can simply annotate overridden Junit 3 setUp and
>> > tearDown methods with @Before and @After annotations. And
>> > use runTest overridden method for Junit 3 and TestRule for Junit 4
>> > in order to run test methods in newly started thread.
>> >
>> > For all details you can see an experiment on github [1]. Among them
>> > are ticks with @RunWith annotation making possible to run a test
>> > inherited from TestCase as Junit 4 test. Still, I might have missed
>> > something.
>> >
>> > Does it sound sane anyhow?
>> >
>> > [1] https://github.com/apache/ignite/pull/5354/files
>> >
>> >
>> > ср, 7 нояб. 2018 г. в 19:57, oignatenko &lt;
>>
>> > oignatenko@
>>
>> > &gt;:
>> >
>> >> Hello,
>> >>
>> >> I created JIRA task for this move, with detailed explanation and
>> >> step-by-step subtasks, your comments are welcome:
>> >>
>> >> - https://issues.apache.org/jira/browse/IGNITE-10173
>> >>
>> >> In brief, the plan is to gradually migrate the part of tests that still
>> >> uses
>> >> Junit 3 (hundreds if not thousands of those that depend on
>> >> GridAbstractTest
>> >> and its subclasses) to newer version. The trick proposed to avoid doing
>> >> all
>> >> this in one (big and risky) step is to introduce a Junit4-based "twin" of
>> >> GridAbstractTest (and respective twins of its subclasses) and use it to
>> >> gradually change tests to use that newer API instead.
>> >>
>> >> After above is over, Junit 3 and all its related stuff (including
>> >> GridAbstractTest) could be safely removed from project.
>> >>
>> >> 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



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

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

Jörn Franke
In reply to this post by Dmitriy Pavlov
A transition to junit5 should also be easy - even for the real junit5 (called Jupiter) and not the junit5 compatibility  layer (called vintage).
For Jupiter the major changes are the order of arguments for assertions (I came over this for 1000 test case with probably 10000 assertions in another project) - you can use regular expression to reorder them in the source code (E.g. using Eclipse replace).
Another thing is the verification that an Exception is thrown - this is also more elegantly handled in Junit5 jupiter.


> Am 18.04.2018 um 15:59 schrieb Dmitry Pavlov <[hidden email]>:
>
> Hi Igniters,
>
> During MTCGA monitoring I’ve discovered a number of issues related to test
> framework itself.
>
> In addition it is not convinient to mute tests on TC, because in branches
> (e.g. 2.5 and in master) we can’t mute it separately. So some tests, which
> already fixed are shown as failed in our PRs and release branches. But
> current JUnit3 test style does not allow to use @Ignore.
>
> Is it time to start translating tests to Unit 4 style?
>
> Are there any of us who would like to assist in the research and create a
> new GridCommonAbstractTest and/or IgniteJUnitRunner?
>
> Sincerely,
> Dmitriy Pavlov
Reply | Threaded
Open this post in threaded view
|

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

oignatenko
Hi Jörn,

Agree, it is also my understanding that after we migrate part of the tests
that still use JUnit 3 (https://issues.apache.org/jira/browse/IGNITE-10173),
upgrade to next version is going to be easy and low risk.

With regards to asserting exceptions I think you have a good point here, I
used JUnit 5 in one of recent projects and liked the way how it handles
this. It felt much more convenient than in JUnit 4.

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,

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

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

Ivan Pavlukhin
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)?

Eduard Shangareev
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)?

Vladimir Ozerov
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)?

Eduard Shangareev
Vovan,

1) Tomorrow in core-module.
2) I believe that nothing, old JUnit3 would work if they are a new one.
Otherwise, they need to fix merge conflict and add @Test on their new test.
And I think that it's unavoidable, we couldn't ask all people to finish
their current activity and not start a new one. So, it's no matter when we
would merge it.

On Mon, Dec 10, 2018 at 8:38 PM Vladimir Ozerov <[hidden email]>
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 <
> [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)?

Dmitry Pavlov
In reply to this post by Vladimir Ozerov
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
> > >
> >
>
123