stopAllGrids() used by default and further steps

[Mmuzaf]

Igniters,

Recenly, we've pached Ingite testing framework to stop all started
instances after all test-cases completion by default.
Details of impelemtaion can be viewed here [1]. This change leads us to a
lot of boilerplate code.


1) I've created issue [2] and prepared PR [3] which removes all this
boilerplate code. Most of these changes is about removing:
```
@Override protected void afterTestsStopped() throws Exception {
super.afterTestsStopped();
    stopAllGrids();
}
```
All tests looks good here. I've double cheched all requiremets and now I
need your help with review and futher steps up to merge.

Can anyone help me?


2) I've created issue [4] and planning to clean rarely used methods related
to stopAllGrids().
E.g. stopAllClients and stopAllServers methods from GridAbstactTest used
only once in whole project but they locates in the root class.
From my point of view, this will simplify for futher migration Ignite
project to JUnit 4/5 framework.

What else can be done here?
Please, share your thoughts.




[1] https://issues.apache.org/jira/browse/IGNITE-6842
[2] https://issues.apache.org/jira/browse/IGNITE-8266
[3] https://github.com/apache/ignite/pull/3844
[4] https://issues.apache.org/jira/browse/IGNITE-8157

[Dmitriy Pavlov]

Hi Maxim,

I think next step can be creation of Junit4/5 IgniteAbstractTest and/or
IgniteTestRunner. Would you like to contribute this prototype?

Regarding TC run there is a number of suspicious tests
  (e.g. IgnitePdsNativeIoTestSuite2:
IgniteUidAsConsistentIdMigrationTest.testNewStyleAlwaysSmallestNodeIndexIsCreatedMultithreaded
(fail rate 0,0%) & IgniteCacheTestSuite6:
TxRollbackAsyncNearCacheTest.testSynchronousRollback (fail rate 0,0%)  )
and a number of timeouts ocurred, so I've retriggered re-run for failed
tests -
https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&branch_IgniteTests24Java8=pull%2F3844%2Fhead

For IGNITE-8266 <https://issues.apache.org/jira/browse/IGNITE-8266>  could
you please create CR?

Sincerely,
Dmitriy Pavlov


пт, 4 мая 2018 г. в 12:27, Maxim Muzafarov <[hidden email]>:

> Igniters,
>
> Recenly, we've pached Ingite testing framework to stop all started
> instances after all test-cases completion by default.
> Details of impelemtaion can be viewed here [1]. This change leads us to a
> lot of boilerplate code.
>
>
> 1) I've created issue [2] and prepared PR [3] which removes all this
> boilerplate code. Most of these changes is about removing:
> ```
> @Override protected void afterTestsStopped() throws Exception {
> super.afterTestsStopped();
>     stopAllGrids();
> }
> ```
> All tests looks good here. I've double cheched all requiremets and now I
> need your help with review and futher steps up to merge.
>
> Can anyone help me?
>
>
> 2) I've created issue [4] and planning to clean rarely used methods related
> to stopAllGrids().
> E.g. stopAllClients and stopAllServers methods from GridAbstactTest used
> only once in whole project but they locates in the root class.
> From my point of view, this will simplify for futher migration Ignite
> project to JUnit 4/5 framework.
>
> What else can be done here?
> Please, share your thoughts.
>
>
>
>
> [1] https://issues.apache.org/jira/browse/IGNITE-6842
> [2] https://issues.apache.org/jira/browse/IGNITE-8266
> [3] https://github.com/apache/ignite/pull/3844
> [4] https://issues.apache.org/jira/browse/IGNITE-8157
>

[Mmuzaf]

Dmitry,

Task of migration to JUnit 4/5 sounds very interesting for me, but I'm not
sure that I will have time for it in the next few weeks. Anyway let's
create new task to it e.g. "providing design and analisys for migration to
JUnit 4/5". I'll try to help with it!


Test cases IgniteUidAsConsistentIdMigrationTest and
TxRollbackAsyncNearCacheTest
are not affected by my change.
Nevertheless, I've rerun Run::All for this PR.


All other preparations have already been done for this issue:

PR: https://github.com/apache/ignite/pull/3844
TC:
https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_RunAll&tab=buildTypeStatusDiv&branch_IgniteTests24Java8=pull%2F3844%2Fhead
Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-581
JIRA: https://issues.apache.org/jira/browse/IGNITE-8266

Will you or others have to to review it?



пт, 4 мая 2018 г. в 14:24, Dmitry Pavlov <[hidden email]>:

> Hi Maxim,
>
> I think next step can be creation of Junit4/5 IgniteAbstractTest and/or
> IgniteTestRunner. Would you like to contribute this prototype?
>
> Regarding TC run there is a number of suspicious tests
>   (e.g. IgnitePdsNativeIoTestSuite2:
>
> IgniteUidAsConsistentIdMigrationTest.testNewStyleAlwaysSmallestNodeIndexIsCreatedMultithreaded
> (fail rate 0,0%) & IgniteCacheTestSuite6:
> TxRollbackAsyncNearCacheTest.testSynchronousRollback (fail rate 0,0%)  )
> and a number of timeouts ocurred, so I've retriggered re-run for failed
> tests -
>
> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&branch_IgniteTests24Java8=pull%2F3844%2Fhead
>
> For IGNITE-8266 <https://issues.apache.org/jira/browse/IGNITE-8266>  could
> you please create CR?
>
> Sincerely,
> Dmitriy Pavlov
>
>
> пт, 4 мая 2018 г. в 12:27, Maxim Muzafarov <[hidden email]>:
>
> > Igniters,
> >
> > Recenly, we've pached Ingite testing framework to stop all started
> > instances after all test-cases completion by default.
> > Details of impelemtaion can be viewed here [1]. This change leads us to a
> > lot of boilerplate code.
> >
> >
> > 1) I've created issue [2] and prepared PR [3] which removes all this
> > boilerplate code. Most of these changes is about removing:
> > ```
> > @Override protected void afterTestsStopped() throws Exception {
> > super.afterTestsStopped();
> >     stopAllGrids();
> > }
> > ```
> > All tests looks good here. I've double cheched all requiremets and now I
> > need your help with review and futher steps up to merge.
> >
> > Can anyone help me?
> >
> >
> > 2) I've created issue [4] and planning to clean rarely used methods
> related
> > to stopAllGrids().
> > E.g. stopAllClients and stopAllServers methods from GridAbstactTest used
> > only once in whole project but they locates in the root class.
> > From my point of view, this will simplify for futher migration Ignite
> > project to JUnit 4/5 framework.
> >
> > What else can be done here?
> > Please, share your thoughts.
> >
> >
> >
> >
> > [1] https://issues.apache.org/jira/browse/IGNITE-6842
> > [2] https://issues.apache.org/jira/browse/IGNITE-8266
> > [3] https://github.com/apache/ignite/pull/3844
> > [4] https://issues.apache.org/jira/browse/IGNITE-8157
> >
>

[Dmitriy Pavlov]

Hi Nikolay,

Would you have a minute to finalize this review?

Sincerely,
Dmitriy Pavlov

пт, 4 мая 2018 г. в 16:05, Maxim Muzafarov <[hidden email]>:

> Dmitry,
>
> Task of migration to JUnit 4/5 sounds very interesting for me, but I'm not
> sure that I will have time for it in the next few weeks. Anyway let's
> create new task to it e.g. "providing design and analisys for migration to
> JUnit 4/5". I'll try to help with it!
>
>
> Test cases IgniteUidAsConsistentIdMigrationTest and
> TxRollbackAsyncNearCacheTest
> are not affected by my change.
> Nevertheless, I've rerun Run::All for this PR.
>
>
> All other preparations have already been done for this issue:
>
> PR: https://github.com/apache/ignite/pull/3844
> TC:
>
> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_RunAll&tab=buildTypeStatusDiv&branch_IgniteTests24Java8=pull%2F3844%2Fhead
> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-581
> JIRA: https://issues.apache.org/jira/browse/IGNITE-8266
>
> Will you or others have to to review it?
>
>
>
> пт, 4 мая 2018 г. в 14:24, Dmitry Pavlov <[hidden email]>:
>
> > Hi Maxim,
> >
> > I think next step can be creation of Junit4/5 IgniteAbstractTest and/or
> > IgniteTestRunner. Would you like to contribute this prototype?
> >
> > Regarding TC run there is a number of suspicious tests
> >   (e.g. IgnitePdsNativeIoTestSuite2:
> >
> >
> IgniteUidAsConsistentIdMigrationTest.testNewStyleAlwaysSmallestNodeIndexIsCreatedMultithreaded
> > (fail rate 0,0%) & IgniteCacheTestSuite6:
> > TxRollbackAsyncNearCacheTest.testSynchronousRollback (fail rate 0,0%)  )
> > and a number of timeouts ocurred, so I've retriggered re-run for failed
> > tests -
> >
> >
> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&branch_IgniteTests24Java8=pull%2F3844%2Fhead
> >
> > For IGNITE-8266 <https://issues.apache.org/jira/browse/IGNITE-8266>
> could
> > you please create CR?
> >
> > Sincerely,
> > Dmitriy Pavlov
> >
> >
> > пт, 4 мая 2018 г. в 12:27, Maxim Muzafarov <[hidden email]>:
> >
> > > Igniters,
> > >
> > > Recenly, we've pached Ingite testing framework to stop all started
> > > instances after all test-cases completion by default.
> > > Details of impelemtaion can be viewed here [1]. This change leads us
> to a
> > > lot of boilerplate code.
> > >
> > >
> > > 1) I've created issue [2] and prepared PR [3] which removes all this
> > > boilerplate code. Most of these changes is about removing:
> > > ```
> > > @Override protected void afterTestsStopped() throws Exception {
> > > super.afterTestsStopped();
> > >     stopAllGrids();
> > > }
> > > ```
> > > All tests looks good here. I've double cheched all requiremets and now
> I
> > > need your help with review and futher steps up to merge.
> > >
> > > Can anyone help me?
> > >
> > >
> > > 2) I've created issue [4] and planning to clean rarely used methods
> > related
> > > to stopAllGrids().
> > > E.g. stopAllClients and stopAllServers methods from GridAbstactTest
> used
> > > only once in whole project but they locates in the root class.
> > > From my point of view, this will simplify for futher migration Ignite
> > > project to JUnit 4/5 framework.
> > >
> > > What else can be done here?
> > > Please, share your thoughts.
> > >
> > >
> > >
> > >
> > > [1] https://issues.apache.org/jira/browse/IGNITE-6842
> > > [2] https://issues.apache.org/jira/browse/IGNITE-8266
> > > [3] https://github.com/apache/ignite/pull/3844
> > > [4] https://issues.apache.org/jira/browse/IGNITE-8157
> > >
> >
>

[Nikolay Izhikov-2]

Yes.

В Пн, 14/05/2018 в 20:48 +0300, Dmitry Pavlov пишет:

> Hi Nikolay,
>
> Would you have a minute to finalize this review?
>
> Sincerely,
> Dmitriy Pavlov
>
> пт, 4 мая 2018 г. в 16:05, Maxim Muzafarov <[hidden email]>:
> > Dmitry,
> >
> > Task of migration to JUnit 4/5 sounds very interesting for me, but I'm not
> > sure that I will have time for it in the next few weeks. Anyway let's
> > create new task to it e.g. "providing design and analisys for migration to
> > JUnit 4/5". I'll try to help with it!
> >
> >
> > Test cases IgniteUidAsConsistentIdMigrationTest and
> > TxRollbackAsyncNearCacheTest
> > are not affected by my change.
> > Nevertheless, I've rerun Run::All for this PR.
> >
> >
> > All other preparations have already been done for this issue:
> >
> > PR: https://github.com/apache/ignite/pull/3844
> > TC:
> > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_RunAll&tab=buildTypeStatusDiv&branch_IgniteTests24Java8=pull%2F3844%2Fhead
> > Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-581
> > JIRA: https://issues.apache.org/jira/browse/IGNITE-8266
> >
> > Will you or others have to to review it?
> >
> >
> >
> > пт, 4 мая 2018 г. в 14:24, Dmitry Pavlov <[hidden email]>:
> >
> > > Hi Maxim,
> > >
> > > I think next step can be creation of Junit4/5 IgniteAbstractTest and/or
> > > IgniteTestRunner. Would you like to contribute this prototype?
> > >
> > > Regarding TC run there is a number of suspicious tests
> > >   (e.g. IgnitePdsNativeIoTestSuite2:
> > >
> > > IgniteUidAsConsistentIdMigrationTest.testNewStyleAlwaysSmallestNodeIndexIsCreatedMultithreaded
> > > (fail rate 0,0%) & IgniteCacheTestSuite6:
> > > TxRollbackAsyncNearCacheTest.testSynchronousRollback (fail rate 0,0%)  )
> > > and a number of timeouts ocurred, so I've retriggered re-run for failed
> > > tests -
> > >
> > > https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&branch_IgniteTests24Java8=pull%2F3844%2Fhead
> > >
> > > For IGNITE-8266 <https://issues.apache.org/jira/browse/IGNITE-8266>  could
> > > you please create CR?
> > >
> > > Sincerely,
> > > Dmitriy Pavlov
> > >
> > >
> > > пт, 4 мая 2018 г. в 12:27, Maxim Muzafarov <[hidden email]>:
> > >
> > > > Igniters,
> > > >
> > > > Recenly, we've pached Ingite testing framework to stop all started
> > > > instances after all test-cases completion by default.
> > > > Details of impelemtaion can be viewed here [1]. This change leads us to a
> > > > lot of boilerplate code.
> > > >
> > > >
> > > > 1) I've created issue [2] and prepared PR [3] which removes all this
> > > > boilerplate code. Most of these changes is about removing:
> > > > ```
> > > > @Override protected void afterTestsStopped() throws Exception {
> > > > super.afterTestsStopped();
> > > >     stopAllGrids();
> > > > }
> > > > ```
> > > > All tests looks good here. I've double cheched all requiremets and now I
> > > > need your help with review and futher steps up to merge.
> > > >
> > > > Can anyone help me?
> > > >
> > > >
> > > > 2) I've created issue [4] and planning to clean rarely used methods
> > > related
> > > > to stopAllGrids().
> > > > E.g. stopAllClients and stopAllServers methods from GridAbstactTest used
> > > > only once in whole project but they locates in the root class.
> > > > From my point of view, this will simplify for futher migration Ignite
> > > > project to JUnit 4/5 framework.
> > > >
> > > > What else can be done here?
> > > > Please, share your thoughts.
> > > >
> > > >
> > > >
> > > >
> > > > [1] https://issues.apache.org/jira/browse/IGNITE-6842
> > > > [2] https://issues.apache.org/jira/browse/IGNITE-8266
> > > > [3] https://github.com/apache/ignite/pull/3844
> > > > [4] https://issues.apache.org/jira/browse/IGNITE-8157
> > > >
> > >