subject:"Re\: Stop nodes after test by default \- IGNITE\-6842"

Re: Stop nodes after test by default - IGNITE-6842

2018-03-21 Thread Dmitry Pavlov

Hi Nikolai, will you have time to merge this change?

вт, 20 мар. 2018 г. в 11:52, Dmitry Pavlov :

> Dmitriy, thank you for review. This fix should do our tests more stable.
>
> Nikolay, could you please merge?
>
> вт, 20 мар. 2018 г. в 11:49, Dmitriy Govorukhin <
> dmitriy.govoruk...@gmail.com>:
>
>> Looks good for me, please merge.
>>
>> 19 мар. 2018 г. 3:22 ПП пользователь "Dmitry Pavlov" <
>> dpavlov@gmail.com> написал:
>>
>> I agree it is important, I'm going to do a review, but do not have time
>>> slot to do.
>>>
>>> Who could pick up this review?
>>>
>>> Dmitriy G., could I ask you?
>>>
>>> пн, 19 мар. 2018 г. в 15:13, Maxim Muzafarov :
>>>
 Dmitry and other igniters,

 Will you have time to review this issue?
 I've preperated PR and TC for this, also I've fixed all comments made
 by Andrey Kuznetsov and Vyacheslav Daradur.

 I think this is important issue and will make test framework more
 stable and clear.


 TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151
 JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
 Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
 PR: https://github.com/apache/ignite/pull/3542

 чт, 15 мар. 2018 г. в 13:31, Maxim Muzafarov :

> Dmtry,
>
> Can we proceed with this change?
> I've done with fixing review comments and tests that you mentioned
> before.
>
> TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151
> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
> PR: https://github.com/apache/ignite/pull/3542
>
>
>
> вт, 6 мар. 2018 г. в 20:42, Dmitry Pavlov :
>
>> Ok, thank you.
>>
>> Please let me know when we can proceed with review
>> https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>>
>>
>> вт, 6 мар. 2018 г. в 20:17, Maxim Muzafarov :
>>
>> > Hello Dmitry,
>> >
>> > Yes, I've updated test classes as you metioned before.
>> > Now i'm fixing review comments. Within next few days I'll prepare
>> final
>> > version of this PR.
>> >
>> > вт, 6 мар. 2018 г. в 20:12, Dmitry Pavlov :
>> >
>> > > Hi Maxim,
>> > >
>> > > are there any news on these test fails?
>> > >
>> > > Is issue ready for review?
>> > >
>> > > Sincerely,
>> > > Dmitiry Pavlov
>> > >
>> > > вт, 27 февр. 2018 г. в 17:12, Dmitry Pavlov <
>> dpavlov@gmail.com>:
>> > >
>> > > > Hi, thank you!
>> > > >
>> > > > I've found several suspicious fails: such test fails have rate
>> less
>> > than
>> > > > 1%, it is probably new failures.
>> > > >
>> > > > It would be great if we can fix it before merge. Could you
>> address this
>> > > > fails?
>> > > >
>> > > > Sincerely,
>> > > > Dmitriy Pavlov
>> > > >
>> > > > IgniteCacheTestSuite5:
>> IgniteCacheStoreCollectionTest.testStoreMap
>> > (fail
>> > > > rate 0,0%)
>> > > > IgniteCacheTestSuite5:
>> > > > CacheLateAffinityAssignmentTest.testDelayAssignmentClientJoin
>> (fail
>> > rate
>> > > > 0,0%)
>> > > > IgniteCacheWithIndexingTestSuite:
>> > > >
>> CacheRandomOperationsMultithreadedTest.testAtomicOffheapEviction (fail
>> > > rate
>> > > > 0,0%)
>> > > > IgniteCacheWithIndexingTestSuite:
>> > > >
>> >
>> CacheRandomOperationsMultithreadedTest.testAtomicOffheapEvictionIndexing
>> > > > (fail rate 0,0%)
>> > > > IgniteCacheWithIndexingTestSuite:
>> > > > CacheRandomOperationsMultithreadedTest.testTxOffheapEviction
>> (fail rate
>> > > > 0,0%)
>> > > > IgniteCacheWithIndexingTestSuite:
>> > > >
>> CacheRandomOperationsMultithreadedTest.testTxOffheapEvictionIndexing
>> > > (fail
>> > > > rate 0,0%)
>> > > >
>> > > > IgniteBinarySimpleNameMapperCacheFullApiTestSuite:
>> > > >
>> > >
>> >
>> GridCachePartitionedNearDisabledMultiNodeWithGroupFullApiSelfTest.testWithSkipStoreTx
>> > > > (fail rate 0,0%)
>> > > >
>> > > > вт, 27 февр. 2018 г. в 17:04, Maxim Muzafarov <
>> maxmu...@gmail.com>:
>> > > >
>> > > >> Yep, link may not be correct.
>> > > >>
>> > > >> Here is correct version:
>> > > >> TC: *
>> > > >>
>> > >
>> >
>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
>> > > >> <
>> > > >>
>> > >
>> >
>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
>> > > >> >*
>> > > >>
>> > > >>
>> > > >> вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov <
>>

Re: Stop nodes after test by default - IGNITE-6842

2018-03-20 Thread Dmitry Pavlov

Dmitriy, thank you for review. This fix should do our tests more stable.

Nikolay, could you please merge?

вт, 20 мар. 2018 г. в 11:49, Dmitriy Govorukhin <
dmitriy.govoruk...@gmail.com>:

> Looks good for me, please merge.
>
> 19 мар. 2018 г. 3:22 ПП пользователь "Dmitry Pavlov" <
> dpavlov@gmail.com> написал:
>
> I agree it is important, I'm going to do a review, but do not have time
>> slot to do.
>>
>> Who could pick up this review?
>>
>> Dmitriy G., could I ask you?
>>
>> пн, 19 мар. 2018 г. в 15:13, Maxim Muzafarov :
>>
>>> Dmitry and other igniters,
>>>
>>> Will you have time to review this issue?
>>> I've preperated PR and TC for this, also I've fixed all comments made by
>>> Andrey Kuznetsov and Vyacheslav Daradur.
>>>
>>> I think this is important issue and will make test framework more stable
>>> and clear.
>>>
>>>
>>> TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151
>>> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
>>> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>>> PR: https://github.com/apache/ignite/pull/3542
>>>
>>> чт, 15 мар. 2018 г. в 13:31, Maxim Muzafarov :
>>>
 Dmtry,

 Can we proceed with this change?
 I've done with fixing review comments and tests that you mentioned
 before.

 TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151
 JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
 Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
 PR: https://github.com/apache/ignite/pull/3542



 вт, 6 мар. 2018 г. в 20:42, Dmitry Pavlov :

> Ok, thank you.
>
> Please let me know when we can proceed with review
> https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>
>
> вт, 6 мар. 2018 г. в 20:17, Maxim Muzafarov :
>
> > Hello Dmitry,
> >
> > Yes, I've updated test classes as you metioned before.
> > Now i'm fixing review comments. Within next few days I'll prepare
> final
> > version of this PR.
> >
> > вт, 6 мар. 2018 г. в 20:12, Dmitry Pavlov :
> >
> > > Hi Maxim,
> > >
> > > are there any news on these test fails?
> > >
> > > Is issue ready for review?
> > >
> > > Sincerely,
> > > Dmitiry Pavlov
> > >
> > > вт, 27 февр. 2018 г. в 17:12, Dmitry Pavlov  >:
> > >
> > > > Hi, thank you!
> > > >
> > > > I've found several suspicious fails: such test fails have rate
> less
> > than
> > > > 1%, it is probably new failures.
> > > >
> > > > It would be great if we can fix it before merge. Could you
> address this
> > > > fails?
> > > >
> > > > Sincerely,
> > > > Dmitriy Pavlov
> > > >
> > > > IgniteCacheTestSuite5:
> IgniteCacheStoreCollectionTest.testStoreMap
> > (fail
> > > > rate 0,0%)
> > > > IgniteCacheTestSuite5:
> > > > CacheLateAffinityAssignmentTest.testDelayAssignmentClientJoin
> (fail
> > rate
> > > > 0,0%)
> > > > IgniteCacheWithIndexingTestSuite:
> > > > CacheRandomOperationsMultithreadedTest.testAtomicOffheapEviction
> (fail
> > > rate
> > > > 0,0%)
> > > > IgniteCacheWithIndexingTestSuite:
> > > >
> >
> CacheRandomOperationsMultithreadedTest.testAtomicOffheapEvictionIndexing
> > > > (fail rate 0,0%)
> > > > IgniteCacheWithIndexingTestSuite:
> > > > CacheRandomOperationsMultithreadedTest.testTxOffheapEviction
> (fail rate
> > > > 0,0%)
> > > > IgniteCacheWithIndexingTestSuite:
> > > >
> CacheRandomOperationsMultithreadedTest.testTxOffheapEvictionIndexing
> > > (fail
> > > > rate 0,0%)
> > > >
> > > > IgniteBinarySimpleNameMapperCacheFullApiTestSuite:
> > > >
> > >
> >
> GridCachePartitionedNearDisabledMultiNodeWithGroupFullApiSelfTest.testWithSkipStoreTx
> > > > (fail rate 0,0%)
> > > >
> > > > вт, 27 февр. 2018 г. в 17:04, Maxim Muzafarov <
> maxmu...@gmail.com>:
> > > >
> > > >> Yep, link may not be correct.
> > > >>
> > > >> Here is correct version:
> > > >> TC: *
> > > >>
> > >
> >
> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
> > > >> <
> > > >>
> > >
> >
> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
> > > >> >*
> > > >>
> > > >>
> > > >> вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov <
> dpavlov@gmail.com>:
> > > >>
> > > >> > Hi Maxim,
> > > >> >
> > > >> > could you please provide link to TC run on your PR? It seems
> link
> > > >> provided
> > > >> > points to run of master. In changes field you may select
> > > pull/3542/head
> > > >> >

Re: Stop nodes after test by default - IGNITE-6842

2018-03-20 Thread Dmitriy Govorukhin

Looks good for me, please merge.

19 мар. 2018 г. 3:22 ПП пользователь "Dmitry Pavlov" 
написал:

> I agree it is important, I'm going to do a review, but do not have time
> slot to do.
>
> Who could pick up this review?
>
> Dmitriy G., could I ask you?
>
> пн, 19 мар. 2018 г. в 15:13, Maxim Muzafarov :
>
>> Dmitry and other igniters,
>>
>> Will you have time to review this issue?
>> I've preperated PR and TC for this, also I've fixed all comments made by
>> Andrey Kuznetsov and Vyacheslav Daradur.
>>
>> I think this is important issue and will make test framework more stable
>> and clear.
>>
>>
>> TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151
>> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
>> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>> PR: https://github.com/apache/ignite/pull/3542
>>
>> чт, 15 мар. 2018 г. в 13:31, Maxim Muzafarov :
>>
>>> Dmtry,
>>>
>>> Can we proceed with this change?
>>> I've done with fixing review comments and tests that you mentioned
>>> before.
>>>
>>> TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151
>>> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
>>> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>>> PR: https://github.com/apache/ignite/pull/3542
>>>
>>>
>>>
>>> вт, 6 мар. 2018 г. в 20:42, Dmitry Pavlov :
>>>
 Ok, thank you.

 Please let me know when we can proceed with review
 https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502


 вт, 6 мар. 2018 г. в 20:17, Maxim Muzafarov :

 > Hello Dmitry,
 >
 > Yes, I've updated test classes as you metioned before.
 > Now i'm fixing review comments. Within next few days I'll prepare
 final
 > version of this PR.
 >
 > вт, 6 мар. 2018 г. в 20:12, Dmitry Pavlov :
 >
 > > Hi Maxim,
 > >
 > > are there any news on these test fails?
 > >
 > > Is issue ready for review?
 > >
 > > Sincerely,
 > > Dmitiry Pavlov
 > >
 > > вт, 27 февр. 2018 г. в 17:12, Dmitry Pavlov :
 > >
 > > > Hi, thank you!
 > > >
 > > > I've found several suspicious fails: such test fails have rate
 less
 > than
 > > > 1%, it is probably new failures.
 > > >
 > > > It would be great if we can fix it before merge. Could you
 address this
 > > > fails?
 > > >
 > > > Sincerely,
 > > > Dmitriy Pavlov
 > > >
 > > > IgniteCacheTestSuite5: IgniteCacheStoreCollectionTest
 .testStoreMap
 > (fail
 > > > rate 0,0%)
 > > > IgniteCacheTestSuite5:
 > > > CacheLateAffinityAssignmentTest.testDelayAssignmentClientJoin
 (fail
 > rate
 > > > 0,0%)
 > > > IgniteCacheWithIndexingTestSuite:
 > > > CacheRandomOperationsMultithreadedTest.testAtomicOffheapEviction
 (fail
 > > rate
 > > > 0,0%)
 > > > IgniteCacheWithIndexingTestSuite:
 > > >
 > CacheRandomOperationsMultithreadedTest.testAtomicOffheapEvictionIndex
 ing
 > > > (fail rate 0,0%)
 > > > IgniteCacheWithIndexingTestSuite:
 > > > CacheRandomOperationsMultithreadedTest.testTxOffheapEviction
 (fail rate
 > > > 0,0%)
 > > > IgniteCacheWithIndexingTestSuite:
 > > > CacheRandomOperationsMultithreadedTest.
 testTxOffheapEvictionIndexing
 > > (fail
 > > > rate 0,0%)
 > > >
 > > > IgniteBinarySimpleNameMapperCacheFullApiTestSuite:
 > > >
 > >
 > GridCachePartitionedNearDisabledMultiNodeWithGroupFullApiSel
 fTest.testWithSkipStoreTx
 > > > (fail rate 0,0%)
 > > >
 > > > вт, 27 февр. 2018 г. в 17:04, Maxim Muzafarov :
 > > >
 > > >> Yep, link may not be correct.
 > > >>
 > > >> Here is correct version:
 > > >> TC: *
 > > >>
 > >
 > https://ci.ignite.apache.org/project.html?projectId=
 IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
 > > >> <
 > > >>
 > >
 > https://ci.ignite.apache.org/project.html?projectId=
 IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
 > > >> >*
 > > >>
 > > >>
 > > >> вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov <
 dpavlov@gmail.com>:
 > > >>
 > > >> > Hi Maxim,
 > > >> >
 > > >> > could you please provide link to TC run on your PR? It seems
 link
 > > >> provided
 > > >> > points to run of master. In changes field you may select
 > > pull/3542/head
 > > >> > before starting RunAll.
 > > >> >
 > > >> > Igniters,
 > > >> >
 > > >> > this change is related to our test framework, so change may
 affect
 > > your
 > > >> > tests. Please join to review
 > > >> > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
 > > >> >
 > > >> > Sincerely,
 > > >> >

Re: Stop nodes after test by default - IGNITE-6842

2018-03-19 Thread Dmitry Pavlov

I agree it is important, I'm going to do a review, but do not have time
slot to do.

Who could pick up this review?

Dmitriy G., could I ask you?

пн, 19 мар. 2018 г. в 15:13, Maxim Muzafarov :

> Dmitry and other igniters,
>
> Will you have time to review this issue?
> I've preperated PR and TC for this, also I've fixed all comments made by
> Andrey Kuznetsov and Vyacheslav Daradur.
>
> I think this is important issue and will make test framework more stable
> and clear.
>
>
> TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151
> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
> PR: https://github.com/apache/ignite/pull/3542
>
> чт, 15 мар. 2018 г. в 13:31, Maxim Muzafarov :
>
>> Dmtry,
>>
>> Can we proceed with this change?
>> I've done with fixing review comments and tests that you mentioned before.
>>
>> TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151
>> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
>> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>> PR: https://github.com/apache/ignite/pull/3542
>>
>>
>>
>> вт, 6 мар. 2018 г. в 20:42, Dmitry Pavlov :
>>
>>> Ok, thank you.
>>>
>>> Please let me know when we can proceed with review
>>> https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>>>
>>>
>>> вт, 6 мар. 2018 г. в 20:17, Maxim Muzafarov :
>>>
>>> > Hello Dmitry,
>>> >
>>> > Yes, I've updated test classes as you metioned before.
>>> > Now i'm fixing review comments. Within next few days I'll prepare final
>>> > version of this PR.
>>> >
>>> > вт, 6 мар. 2018 г. в 20:12, Dmitry Pavlov :
>>> >
>>> > > Hi Maxim,
>>> > >
>>> > > are there any news on these test fails?
>>> > >
>>> > > Is issue ready for review?
>>> > >
>>> > > Sincerely,
>>> > > Dmitiry Pavlov
>>> > >
>>> > > вт, 27 февр. 2018 г. в 17:12, Dmitry Pavlov :
>>> > >
>>> > > > Hi, thank you!
>>> > > >
>>> > > > I've found several suspicious fails: such test fails have rate less
>>> > than
>>> > > > 1%, it is probably new failures.
>>> > > >
>>> > > > It would be great if we can fix it before merge. Could you address
>>> this
>>> > > > fails?
>>> > > >
>>> > > > Sincerely,
>>> > > > Dmitriy Pavlov
>>> > > >
>>> > > > IgniteCacheTestSuite5: IgniteCacheStoreCollectionTest.testStoreMap
>>> > (fail
>>> > > > rate 0,0%)
>>> > > > IgniteCacheTestSuite5:
>>> > > > CacheLateAffinityAssignmentTest.testDelayAssignmentClientJoin (fail
>>> > rate
>>> > > > 0,0%)
>>> > > > IgniteCacheWithIndexingTestSuite:
>>> > > > CacheRandomOperationsMultithreadedTest.testAtomicOffheapEviction
>>> (fail
>>> > > rate
>>> > > > 0,0%)
>>> > > > IgniteCacheWithIndexingTestSuite:
>>> > > >
>>> >
>>> CacheRandomOperationsMultithreadedTest.testAtomicOffheapEvictionIndexing
>>> > > > (fail rate 0,0%)
>>> > > > IgniteCacheWithIndexingTestSuite:
>>> > > > CacheRandomOperationsMultithreadedTest.testTxOffheapEviction (fail
>>> rate
>>> > > > 0,0%)
>>> > > > IgniteCacheWithIndexingTestSuite:
>>> > > >
>>> CacheRandomOperationsMultithreadedTest.testTxOffheapEvictionIndexing
>>> > > (fail
>>> > > > rate 0,0%)
>>> > > >
>>> > > > IgniteBinarySimpleNameMapperCacheFullApiTestSuite:
>>> > > >
>>> > >
>>> >
>>> GridCachePartitionedNearDisabledMultiNodeWithGroupFullApiSelfTest.testWithSkipStoreTx
>>> > > > (fail rate 0,0%)
>>> > > >
>>> > > > вт, 27 февр. 2018 г. в 17:04, Maxim Muzafarov >> >:
>>> > > >
>>> > > >> Yep, link may not be correct.
>>> > > >>
>>> > > >> Here is correct version:
>>> > > >> TC: *
>>> > > >>
>>> > >
>>> >
>>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
>>> > > >> <
>>> > > >>
>>> > >
>>> >
>>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
>>> > > >> >*
>>> > > >>
>>> > > >>
>>> > > >> вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov <
>>> dpavlov@gmail.com>:
>>> > > >>
>>> > > >> > Hi Maxim,
>>> > > >> >
>>> > > >> > could you please provide link to TC run on your PR? It seems
>>> link
>>> > > >> provided
>>> > > >> > points to run of master. In changes field you may select
>>> > > pull/3542/head
>>> > > >> > before starting RunAll.
>>> > > >> >
>>> > > >> > Igniters,
>>> > > >> >
>>> > > >> > this change is related to our test framework, so change may
>>> affect
>>> > > your
>>> > > >> > tests. Please join to review
>>> > > >> > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>>> > > >> >
>>> > > >> > Sincerely,
>>> > > >> > Dmitriy Pavlov
>>> > > >> >
>>> > > >> > вт, 27 февр. 2018 г. в 16:14, Maxim Muzafarov <
>>> maxmu...@gmail.com>:
>>> > > >> >
>>> > > >> > > Hi all,
>>> > > >> > >
>>> > > >> > > I think, I've done with this issue, what should we do next?
>>> > > >> > >
>>> > > >> > > PR: https://github.com/apache/ignite/pull/3542

Re: Stop nodes after test by default - IGNITE-6842

2018-03-19 Thread Maxim Muzafarov

Dmitry and other igniters,

Will you have time to review this issue?
I've preperated PR and TC for this, also I've fixed all comments made by
Andrey Kuznetsov and Vyacheslav Daradur.

I think this is important issue and will make test framework more stable
and clear.

TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151
JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
PR: https://github.com/apache/ignite/pull/3542

чт, 15 мар. 2018 г. в 13:31, Maxim Muzafarov :

> Dmtry,
>
> Can we proceed with this change?
> I've done with fixing review comments and tests that you mentioned before.
>
> TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151
> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
> PR: https://github.com/apache/ignite/pull/3542
>
>
>
> вт, 6 мар. 2018 г. в 20:42, Dmitry Pavlov :
>
>> Ok, thank you.
>>
>> Please let me know when we can proceed with review
>> https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>>
>>
>> вт, 6 мар. 2018 г. в 20:17, Maxim Muzafarov :
>>
>> > Hello Dmitry,
>> >
>> > Yes, I've updated test classes as you metioned before.
>> > Now i'm fixing review comments. Within next few days I'll prepare final
>> > version of this PR.
>> >
>> > вт, 6 мар. 2018 г. в 20:12, Dmitry Pavlov :
>> >
>> > > Hi Maxim,
>> > >
>> > > are there any news on these test fails?
>> > >
>> > > Is issue ready for review?
>> > >
>> > > Sincerely,
>> > > Dmitiry Pavlov
>> > >
>> > > вт, 27 февр. 2018 г. в 17:12, Dmitry Pavlov :
>> > >
>> > > > Hi, thank you!
>> > > >
>> > > > I've found several suspicious fails: such test fails have rate less
>> > than
>> > > > 1%, it is probably new failures.
>> > > >
>> > > > It would be great if we can fix it before merge. Could you address
>> this
>> > > > fails?
>> > > >
>> > > > Sincerely,
>> > > > Dmitriy Pavlov
>> > > >
>> > > > IgniteCacheTestSuite5: IgniteCacheStoreCollectionTest.testStoreMap
>> > (fail
>> > > > rate 0,0%)
>> > > > IgniteCacheTestSuite5:
>> > > > CacheLateAffinityAssignmentTest.testDelayAssignmentClientJoin (fail
>> > rate
>> > > > 0,0%)
>> > > > IgniteCacheWithIndexingTestSuite:
>> > > > CacheRandomOperationsMultithreadedTest.testAtomicOffheapEviction
>> (fail
>> > > rate
>> > > > 0,0%)
>> > > > IgniteCacheWithIndexingTestSuite:
>> > > >
>> > CacheRandomOperationsMultithreadedTest.testAtomicOffheapEvictionIndexing
>> > > > (fail rate 0,0%)
>> > > > IgniteCacheWithIndexingTestSuite:
>> > > > CacheRandomOperationsMultithreadedTest.testTxOffheapEviction (fail
>> rate
>> > > > 0,0%)
>> > > > IgniteCacheWithIndexingTestSuite:
>> > > > CacheRandomOperationsMultithreadedTest.testTxOffheapEvictionIndexing
>> > > (fail
>> > > > rate 0,0%)
>> > > >
>> > > > IgniteBinarySimpleNameMapperCacheFullApiTestSuite:
>> > > >
>> > >
>> >
>> GridCachePartitionedNearDisabledMultiNodeWithGroupFullApiSelfTest.testWithSkipStoreTx
>> > > > (fail rate 0,0%)
>> > > >
>> > > > вт, 27 февр. 2018 г. в 17:04, Maxim Muzafarov :
>> > > >
>> > > >> Yep, link may not be correct.
>> > > >>
>> > > >> Here is correct version:
>> > > >> TC: *
>> > > >>
>> > >
>> >
>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
>> > > >> <
>> > > >>
>> > >
>> >
>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
>> > > >> >*
>> > > >>
>> > > >>
>> > > >> вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov > >:
>> > > >>
>> > > >> > Hi Maxim,
>> > > >> >
>> > > >> > could you please provide link to TC run on your PR? It seems link
>> > > >> provided
>> > > >> > points to run of master. In changes field you may select
>> > > pull/3542/head
>> > > >> > before starting RunAll.
>> > > >> >
>> > > >> > Igniters,
>> > > >> >
>> > > >> > this change is related to our test framework, so change may
>> affect
>> > > your
>> > > >> > tests. Please join to review
>> > > >> > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>> > > >> >
>> > > >> > Sincerely,
>> > > >> > Dmitriy Pavlov
>> > > >> >
>> > > >> > вт, 27 февр. 2018 г. в 16:14, Maxim Muzafarov <
>> maxmu...@gmail.com>:
>> > > >> >
>> > > >> > > Hi all,
>> > > >> > >
>> > > >> > > I think, I've done with this issue, what should we do next?
>> > > >> > >
>> > > >> > > PR: https://github.com/apache/ignite/pull/3542
>> > > >> > > Upsource:
>> > > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>> > > >> > > TC:
>> > > >> > >
>> > > >> > >
>> > > >> >
>> > > >>
>> > >
>> >
>> https://ci.ignite.apache.org/viewModification.html?modId=723895=false==vcsModificationTests
>> > > >> > > JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
>> > > >> > >
>> > > >> > >
>> > > >> > > чт, 22 февр. 2018

Re: Stop nodes after test by default - IGNITE-6842

2018-03-15 Thread Maxim Muzafarov

Dmtry,

Can we proceed with this change?
I've done with fixing review comments and tests that you mentioned before.

TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151
JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
PR: https://github.com/apache/ignite/pull/3542



вт, 6 мар. 2018 г. в 20:42, Dmitry Pavlov :

> Ok, thank you.
>
> Please let me know when we can proceed with review
> https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>
>
> вт, 6 мар. 2018 г. в 20:17, Maxim Muzafarov :
>
> > Hello Dmitry,
> >
> > Yes, I've updated test classes as you metioned before.
> > Now i'm fixing review comments. Within next few days I'll prepare final
> > version of this PR.
> >
> > вт, 6 мар. 2018 г. в 20:12, Dmitry Pavlov :
> >
> > > Hi Maxim,
> > >
> > > are there any news on these test fails?
> > >
> > > Is issue ready for review?
> > >
> > > Sincerely,
> > > Dmitiry Pavlov
> > >
> > > вт, 27 февр. 2018 г. в 17:12, Dmitry Pavlov :
> > >
> > > > Hi, thank you!
> > > >
> > > > I've found several suspicious fails: such test fails have rate less
> > than
> > > > 1%, it is probably new failures.
> > > >
> > > > It would be great if we can fix it before merge. Could you address
> this
> > > > fails?
> > > >
> > > > Sincerely,
> > > > Dmitriy Pavlov
> > > >
> > > > IgniteCacheTestSuite5: IgniteCacheStoreCollectionTest.testStoreMap
> > (fail
> > > > rate 0,0%)
> > > > IgniteCacheTestSuite5:
> > > > CacheLateAffinityAssignmentTest.testDelayAssignmentClientJoin (fail
> > rate
> > > > 0,0%)
> > > > IgniteCacheWithIndexingTestSuite:
> > > > CacheRandomOperationsMultithreadedTest.testAtomicOffheapEviction
> (fail
> > > rate
> > > > 0,0%)
> > > > IgniteCacheWithIndexingTestSuite:
> > > >
> > CacheRandomOperationsMultithreadedTest.testAtomicOffheapEvictionIndexing
> > > > (fail rate 0,0%)
> > > > IgniteCacheWithIndexingTestSuite:
> > > > CacheRandomOperationsMultithreadedTest.testTxOffheapEviction (fail
> rate
> > > > 0,0%)
> > > > IgniteCacheWithIndexingTestSuite:
> > > > CacheRandomOperationsMultithreadedTest.testTxOffheapEvictionIndexing
> > > (fail
> > > > rate 0,0%)
> > > >
> > > > IgniteBinarySimpleNameMapperCacheFullApiTestSuite:
> > > >
> > >
> >
> GridCachePartitionedNearDisabledMultiNodeWithGroupFullApiSelfTest.testWithSkipStoreTx
> > > > (fail rate 0,0%)
> > > >
> > > > вт, 27 февр. 2018 г. в 17:04, Maxim Muzafarov :
> > > >
> > > >> Yep, link may not be correct.
> > > >>
> > > >> Here is correct version:
> > > >> TC: *
> > > >>
> > >
> >
> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
> > > >> <
> > > >>
> > >
> >
> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
> > > >> >*
> > > >>
> > > >>
> > > >> вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov  >:
> > > >>
> > > >> > Hi Maxim,
> > > >> >
> > > >> > could you please provide link to TC run on your PR? It seems link
> > > >> provided
> > > >> > points to run of master. In changes field you may select
> > > pull/3542/head
> > > >> > before starting RunAll.
> > > >> >
> > > >> > Igniters,
> > > >> >
> > > >> > this change is related to our test framework, so change may affect
> > > your
> > > >> > tests. Please join to review
> > > >> > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
> > > >> >
> > > >> > Sincerely,
> > > >> > Dmitriy Pavlov
> > > >> >
> > > >> > вт, 27 февр. 2018 г. в 16:14, Maxim Muzafarov  >:
> > > >> >
> > > >> > > Hi all,
> > > >> > >
> > > >> > > I think, I've done with this issue, what should we do next?
> > > >> > >
> > > >> > > PR: https://github.com/apache/ignite/pull/3542
> > > >> > > Upsource:
> > > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
> > > >> > > TC:
> > > >> > >
> > > >> > >
> > > >> >
> > > >>
> > >
> >
> https://ci.ignite.apache.org/viewModification.html?modId=723895=false==vcsModificationTests
> > > >> > > JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
> > > >> > >
> > > >> > >
> > > >> > > чт, 22 февр. 2018 г. в 14:12, Dmitry Pavlov <
> > dpavlov@gmail.com
> > > >:
> > > >> > >
> > > >> > > > Hi Maxim,
> > > >> > > >
> > > >> > > > Thank you.
> > > >> > > >
> > > >> > > > To my mind stopAllGrids call should be kept in
> afterTestsStop().
> > > If
> > > >> > > > developer forgot to call super(), he will see exception from
> > > >> > > > beforeTestsStart()
> > > >> > > > added by you.
> > > >> > > >
> > > >> > > > If you think patch is ready to be reviewed, please change JIRA
> > > >> status
> > > >> > to
> > > >> > > > "Patch Available".
> > > >> > > >
> > > >> > > > Optionally you can create Upsource review. It would be easier
> > for
> > > >> > > multiple
> > > >> > > > reviewers to handle this patch. This test

Re: Stop nodes after test by default - IGNITE-6842

2018-03-06 Thread Dmitry Pavlov

Ok, thank you.

Please let me know when we can proceed with review
https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502


вт, 6 мар. 2018 г. в 20:17, Maxim Muzafarov :

> Hello Dmitry,
>
> Yes, I've updated test classes as you metioned before.
> Now i'm fixing review comments. Within next few days I'll prepare final
> version of this PR.
>
> вт, 6 мар. 2018 г. в 20:12, Dmitry Pavlov :
>
> > Hi Maxim,
> >
> > are there any news on these test fails?
> >
> > Is issue ready for review?
> >
> > Sincerely,
> > Dmitiry Pavlov
> >
> > вт, 27 февр. 2018 г. в 17:12, Dmitry Pavlov :
> >
> > > Hi, thank you!
> > >
> > > I've found several suspicious fails: such test fails have rate less
> than
> > > 1%, it is probably new failures.
> > >
> > > It would be great if we can fix it before merge. Could you address this
> > > fails?
> > >
> > > Sincerely,
> > > Dmitriy Pavlov
> > >
> > > IgniteCacheTestSuite5: IgniteCacheStoreCollectionTest.testStoreMap
> (fail
> > > rate 0,0%)
> > > IgniteCacheTestSuite5:
> > > CacheLateAffinityAssignmentTest.testDelayAssignmentClientJoin (fail
> rate
> > > 0,0%)
> > > IgniteCacheWithIndexingTestSuite:
> > > CacheRandomOperationsMultithreadedTest.testAtomicOffheapEviction (fail
> > rate
> > > 0,0%)
> > > IgniteCacheWithIndexingTestSuite:
> > >
> CacheRandomOperationsMultithreadedTest.testAtomicOffheapEvictionIndexing
> > > (fail rate 0,0%)
> > > IgniteCacheWithIndexingTestSuite:
> > > CacheRandomOperationsMultithreadedTest.testTxOffheapEviction (fail rate
> > > 0,0%)
> > > IgniteCacheWithIndexingTestSuite:
> > > CacheRandomOperationsMultithreadedTest.testTxOffheapEvictionIndexing
> > (fail
> > > rate 0,0%)
> > >
> > > IgniteBinarySimpleNameMapperCacheFullApiTestSuite:
> > >
> >
> GridCachePartitionedNearDisabledMultiNodeWithGroupFullApiSelfTest.testWithSkipStoreTx
> > > (fail rate 0,0%)
> > >
> > > вт, 27 февр. 2018 г. в 17:04, Maxim Muzafarov :
> > >
> > >> Yep, link may not be correct.
> > >>
> > >> Here is correct version:
> > >> TC: *
> > >>
> >
> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
> > >> <
> > >>
> >
> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
> > >> >*
> > >>
> > >>
> > >> вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov :
> > >>
> > >> > Hi Maxim,
> > >> >
> > >> > could you please provide link to TC run on your PR? It seems link
> > >> provided
> > >> > points to run of master. In changes field you may select
> > pull/3542/head
> > >> > before starting RunAll.
> > >> >
> > >> > Igniters,
> > >> >
> > >> > this change is related to our test framework, so change may affect
> > your
> > >> > tests. Please join to review
> > >> > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
> > >> >
> > >> > Sincerely,
> > >> > Dmitriy Pavlov
> > >> >
> > >> > вт, 27 февр. 2018 г. в 16:14, Maxim Muzafarov :
> > >> >
> > >> > > Hi all,
> > >> > >
> > >> > > I think, I've done with this issue, what should we do next?
> > >> > >
> > >> > > PR: https://github.com/apache/ignite/pull/3542
> > >> > > Upsource:
> > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
> > >> > > TC:
> > >> > >
> > >> > >
> > >> >
> > >>
> >
> https://ci.ignite.apache.org/viewModification.html?modId=723895=false==vcsModificationTests
> > >> > > JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
> > >> > >
> > >> > >
> > >> > > чт, 22 февр. 2018 г. в 14:12, Dmitry Pavlov <
> dpavlov@gmail.com
> > >:
> > >> > >
> > >> > > > Hi Maxim,
> > >> > > >
> > >> > > > Thank you.
> > >> > > >
> > >> > > > To my mind stopAllGrids call should be kept in afterTestsStop().
> > If
> > >> > > > developer forgot to call super(), he will see exception from
> > >> > > > beforeTestsStart()
> > >> > > > added by you.
> > >> > > >
> > >> > > > If you think patch is ready to be reviewed, please change JIRA
> > >> status
> > >> > to
> > >> > > > "Patch Available".
> > >> > > >
> > >> > > > Optionally you can create Upsource review. It would be easier
> for
> > >> > > multiple
> > >> > > > reviewers to handle this patch. This test framework is used by
> all
> > >> > > Igniters
> > >> > > > so Upsource would be good option (
> > >> https://reviews.ignite.apache.org/
> > >> > ).
> > >> > > >
> > >> > > > Sincerely,
> > >> > > > Dmitriy Pavlov
> > >> > > >
> > >> > > > чт, 22 февр. 2018 г. в 13:44, Maxim Muzafarov <
> maxmu...@gmail.com
> > >:
> > >> > > >
> > >> > > > > Hi all,
> > >> > > > >
> > >> > > > > I've made some changes based on our previous discusstions,
> > please
> > >> see
> > >> > > PR
> > >> > > > > [1]:
> > >> > > > > 1) Remove duplicated code for stopGrid (by index and by name);
> > >> > > > > 2) Add new method that thows exception if not all grids
> haven't
> > >> > stopped
> > >> > > > > correctly;
> > >> > > > > 3)  Change tests that have

Re: Stop nodes after test by default - IGNITE-6842

2018-03-06 Thread Maxim Muzafarov

Hello Dmitry,

Yes, I've updated test classes as you metioned before.
Now i'm fixing review comments. Within next few days I'll prepare final
version of this PR.

вт, 6 мар. 2018 г. в 20:12, Dmitry Pavlov :

> Hi Maxim,
>
> are there any news on these test fails?
>
> Is issue ready for review?
>
> Sincerely,
> Dmitiry Pavlov
>
> вт, 27 февр. 2018 г. в 17:12, Dmitry Pavlov :
>
> > Hi, thank you!
> >
> > I've found several suspicious fails: such test fails have rate less than
> > 1%, it is probably new failures.
> >
> > It would be great if we can fix it before merge. Could you address this
> > fails?
> >
> > Sincerely,
> > Dmitriy Pavlov
> >
> > IgniteCacheTestSuite5: IgniteCacheStoreCollectionTest.testStoreMap (fail
> > rate 0,0%)
> > IgniteCacheTestSuite5:
> > CacheLateAffinityAssignmentTest.testDelayAssignmentClientJoin (fail rate
> > 0,0%)
> > IgniteCacheWithIndexingTestSuite:
> > CacheRandomOperationsMultithreadedTest.testAtomicOffheapEviction (fail
> rate
> > 0,0%)
> > IgniteCacheWithIndexingTestSuite:
> > CacheRandomOperationsMultithreadedTest.testAtomicOffheapEvictionIndexing
> > (fail rate 0,0%)
> > IgniteCacheWithIndexingTestSuite:
> > CacheRandomOperationsMultithreadedTest.testTxOffheapEviction (fail rate
> > 0,0%)
> > IgniteCacheWithIndexingTestSuite:
> > CacheRandomOperationsMultithreadedTest.testTxOffheapEvictionIndexing
> (fail
> > rate 0,0%)
> >
> > IgniteBinarySimpleNameMapperCacheFullApiTestSuite:
> >
> GridCachePartitionedNearDisabledMultiNodeWithGroupFullApiSelfTest.testWithSkipStoreTx
> > (fail rate 0,0%)
> >
> > вт, 27 февр. 2018 г. в 17:04, Maxim Muzafarov :
> >
> >> Yep, link may not be correct.
> >>
> >> Here is correct version:
> >> TC: *
> >>
> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
> >> <
> >>
> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
> >> >*
> >>
> >>
> >> вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov :
> >>
> >> > Hi Maxim,
> >> >
> >> > could you please provide link to TC run on your PR? It seems link
> >> provided
> >> > points to run of master. In changes field you may select
> pull/3542/head
> >> > before starting RunAll.
> >> >
> >> > Igniters,
> >> >
> >> > this change is related to our test framework, so change may affect
> your
> >> > tests. Please join to review
> >> > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
> >> >
> >> > Sincerely,
> >> > Dmitriy Pavlov
> >> >
> >> > вт, 27 февр. 2018 г. в 16:14, Maxim Muzafarov :
> >> >
> >> > > Hi all,
> >> > >
> >> > > I think, I've done with this issue, what should we do next?
> >> > >
> >> > > PR: https://github.com/apache/ignite/pull/3542
> >> > > Upsource:
> https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
> >> > > TC:
> >> > >
> >> > >
> >> >
> >>
> https://ci.ignite.apache.org/viewModification.html?modId=723895=false==vcsModificationTests
> >> > > JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
> >> > >
> >> > >
> >> > > чт, 22 февр. 2018 г. в 14:12, Dmitry Pavlov  >:
> >> > >
> >> > > > Hi Maxim,
> >> > > >
> >> > > > Thank you.
> >> > > >
> >> > > > To my mind stopAllGrids call should be kept in afterTestsStop().
> If
> >> > > > developer forgot to call super(), he will see exception from
> >> > > > beforeTestsStart()
> >> > > > added by you.
> >> > > >
> >> > > > If you think patch is ready to be reviewed, please change JIRA
> >> status
> >> > to
> >> > > > "Patch Available".
> >> > > >
> >> > > > Optionally you can create Upsource review. It would be easier for
> >> > > multiple
> >> > > > reviewers to handle this patch. This test framework is used by all
> >> > > Igniters
> >> > > > so Upsource would be good option (
> >> https://reviews.ignite.apache.org/
> >> > ).
> >> > > >
> >> > > > Sincerely,
> >> > > > Dmitriy Pavlov
> >> > > >
> >> > > > чт, 22 февр. 2018 г. в 13:44, Maxim Muzafarov  >:
> >> > > >
> >> > > > > Hi all,
> >> > > > >
> >> > > > > I've made some changes based on our previous discusstions,
> please
> >> see
> >> > > PR
> >> > > > > [1]:
> >> > > > > 1) Remove duplicated code for stopGrid (by index and by name);
> >> > > > > 2) Add new method that thows exception if not all grids haven't
> >> > stopped
> >> > > > > correctly;
> >> > > > > 3)  Change tests that have been affected by this changes;
> >> > > > >
> >> > > > > Also, I have some thoughts for clarification:
> >> > > > > 1) beforeTestsStart() - I expect here in subtests that grids are
> >> not
> >> > > > > started yet. Am I right?
> >> > > > > 2) I think we should call stopAllGrids in tearDown method. So,
> if
> >> in
> >> > > some
> >> > > > > cases we'll override afterTestsStop in subclasses and forgot to
> >> call
> >> > > > > super() it won't lead us to exception.
> >> > > > >
> >> > > > > [1]

Re: Stop nodes after test by default - IGNITE-6842

2018-03-06 Thread Dmitry Pavlov

Hi Maxim,

are there any news on these test fails?

Is issue ready for review?

Sincerely,
Dmitiry Pavlov

вт, 27 февр. 2018 г. в 17:12, Dmitry Pavlov :

> Hi, thank you!
>
> I've found several suspicious fails: such test fails have rate less than
> 1%, it is probably new failures.
>
> It would be great if we can fix it before merge. Could you address this
> fails?
>
> Sincerely,
> Dmitriy Pavlov
>
> IgniteCacheTestSuite5: IgniteCacheStoreCollectionTest.testStoreMap (fail
> rate 0,0%)
> IgniteCacheTestSuite5:
> CacheLateAffinityAssignmentTest.testDelayAssignmentClientJoin (fail rate
> 0,0%)
> IgniteCacheWithIndexingTestSuite:
> CacheRandomOperationsMultithreadedTest.testAtomicOffheapEviction (fail rate
> 0,0%)
> IgniteCacheWithIndexingTestSuite:
> CacheRandomOperationsMultithreadedTest.testAtomicOffheapEvictionIndexing
> (fail rate 0,0%)
> IgniteCacheWithIndexingTestSuite:
> CacheRandomOperationsMultithreadedTest.testTxOffheapEviction (fail rate
> 0,0%)
> IgniteCacheWithIndexingTestSuite:
> CacheRandomOperationsMultithreadedTest.testTxOffheapEvictionIndexing (fail
> rate 0,0%)
>
> IgniteBinarySimpleNameMapperCacheFullApiTestSuite:
> GridCachePartitionedNearDisabledMultiNodeWithGroupFullApiSelfTest.testWithSkipStoreTx
> (fail rate 0,0%)
>
> вт, 27 февр. 2018 г. в 17:04, Maxim Muzafarov :
>
>> Yep, link may not be correct.
>>
>> Here is correct version:
>> TC: *
>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
>> <
>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
>> >*
>>
>>
>> вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov :
>>
>> > Hi Maxim,
>> >
>> > could you please provide link to TC run on your PR? It seems link
>> provided
>> > points to run of master. In changes field you may select pull/3542/head
>> > before starting RunAll.
>> >
>> > Igniters,
>> >
>> > this change is related to our test framework, so change may affect your
>> > tests. Please join to review
>> > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>> >
>> > Sincerely,
>> > Dmitriy Pavlov
>> >
>> > вт, 27 февр. 2018 г. в 16:14, Maxim Muzafarov :
>> >
>> > > Hi all,
>> > >
>> > > I think, I've done with this issue, what should we do next?
>> > >
>> > > PR: https://github.com/apache/ignite/pull/3542
>> > > Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>> > > TC:
>> > >
>> > >
>> >
>> https://ci.ignite.apache.org/viewModification.html?modId=723895=false==vcsModificationTests
>> > > JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
>> > >
>> > >
>> > > чт, 22 февр. 2018 г. в 14:12, Dmitry Pavlov :
>> > >
>> > > > Hi Maxim,
>> > > >
>> > > > Thank you.
>> > > >
>> > > > To my mind stopAllGrids call should be kept in afterTestsStop(). If
>> > > > developer forgot to call super(), he will see exception from
>> > > > beforeTestsStart()
>> > > > added by you.
>> > > >
>> > > > If you think patch is ready to be reviewed, please change JIRA
>> status
>> > to
>> > > > "Patch Available".
>> > > >
>> > > > Optionally you can create Upsource review. It would be easier for
>> > > multiple
>> > > > reviewers to handle this patch. This test framework is used by all
>> > > Igniters
>> > > > so Upsource would be good option (
>> https://reviews.ignite.apache.org/
>> > ).
>> > > >
>> > > > Sincerely,
>> > > > Dmitriy Pavlov
>> > > >
>> > > > чт, 22 февр. 2018 г. в 13:44, Maxim Muzafarov :
>> > > >
>> > > > > Hi all,
>> > > > >
>> > > > > I've made some changes based on our previous discusstions, please
>> see
>> > > PR
>> > > > > [1]:
>> > > > > 1) Remove duplicated code for stopGrid (by index and by name);
>> > > > > 2) Add new method that thows exception if not all grids haven't
>> > stopped
>> > > > > correctly;
>> > > > > 3)  Change tests that have been affected by this changes;
>> > > > >
>> > > > > Also, I have some thoughts for clarification:
>> > > > > 1) beforeTestsStart() - I expect here in subtests that grids are
>> not
>> > > > > started yet. Am I right?
>> > > > > 2) I think we should call stopAllGrids in tearDown method. So, if
>> in
>> > > some
>> > > > > cases we'll override afterTestsStop in subclasses and forgot to
>> call
>> > > > > super() it won't lead us to exception.
>> > > > >
>> > > > > [1] https://github.com/apache/ignite/pull/3542
>> > > > > [2]
>> https://ci.ignite.apache.org/viewModification.html?modId=717275
>> > > > > [3] https://issues.apache.org/jira/browse/IGNITE-6842
>> > > > >
>> > > > >
>> > > > > ср, 7 февр. 2018 г. в 18:28, Maxim Muzafarov > >:
>> > > > >
>> > > > > > Thank you all,
>> > > > > >
>> > > > > > I'll add this comment's for JIRA ticket, if you don't mind.
>> > > > > >
>> > > > > > ср, 7 февр. 2018 г. в 15:16, Dmitry Pavlov <
>> dpavlov@gmail.com
>> > >:
>> > > > > >
>> > > > > >> Yes, this

Re: Stop nodes after test by default - IGNITE-6842

2018-02-27 Thread Maxim Muzafarov

Yep, link may not be correct.

Here is correct version:
TC: 
*https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F3542%2Fhead
*


вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov :

> Hi Maxim,
>
> could you please provide link to TC run on your PR? It seems link provided
> points to run of master. In changes field you may select pull/3542/head
> before starting RunAll.
>
> Igniters,
>
> this change is related to our test framework, so change may affect your
> tests. Please join to review
> https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
>
> Sincerely,
> Dmitriy Pavlov
>
> вт, 27 февр. 2018 г. в 16:14, Maxim Muzafarov :
>
> > Hi all,
> >
> > I think, I've done with this issue, what should we do next?
> >
> > PR: https://github.com/apache/ignite/pull/3542
> > Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
> > TC:
> >
> >
> https://ci.ignite.apache.org/viewModification.html?modId=723895=false==vcsModificationTests
> > JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
> >
> >
> > чт, 22 февр. 2018 г. в 14:12, Dmitry Pavlov :
> >
> > > Hi Maxim,
> > >
> > > Thank you.
> > >
> > > To my mind stopAllGrids call should be kept in afterTestsStop(). If
> > > developer forgot to call super(), he will see exception from
> > > beforeTestsStart()
> > > added by you.
> > >
> > > If you think patch is ready to be reviewed, please change JIRA status
> to
> > > "Patch Available".
> > >
> > > Optionally you can create Upsource review. It would be easier for
> > multiple
> > > reviewers to handle this patch. This test framework is used by all
> > Igniters
> > > so Upsource would be good option (https://reviews.ignite.apache.org/
> ).
> > >
> > > Sincerely,
> > > Dmitriy Pavlov
> > >
> > > чт, 22 февр. 2018 г. в 13:44, Maxim Muzafarov :
> > >
> > > > Hi all,
> > > >
> > > > I've made some changes based on our previous discusstions, please see
> > PR
> > > > [1]:
> > > > 1) Remove duplicated code for stopGrid (by index and by name);
> > > > 2) Add new method that thows exception if not all grids haven't
> stopped
> > > > correctly;
> > > > 3)  Change tests that have been affected by this changes;
> > > >
> > > > Also, I have some thoughts for clarification:
> > > > 1) beforeTestsStart() - I expect here in subtests that grids are not
> > > > started yet. Am I right?
> > > > 2) I think we should call stopAllGrids in tearDown method. So, if in
> > some
> > > > cases we'll override afterTestsStop in subclasses and forgot to call
> > > > super() it won't lead us to exception.
> > > >
> > > > [1] https://github.com/apache/ignite/pull/3542
> > > > [2] https://ci.ignite.apache.org/viewModification.html?modId=717275
> > > > [3] https://issues.apache.org/jira/browse/IGNITE-6842
> > > >
> > > >
> > > > ср, 7 февр. 2018 г. в 18:28, Maxim Muzafarov :
> > > >
> > > > > Thank you all,
> > > > >
> > > > > I'll add this comment's for JIRA ticket, if you don't mind.
> > > > >
> > > > > ср, 7 февр. 2018 г. в 15:16, Dmitry Pavlov  >:
> > > > >
> > > > >> Yes, this solution allows to cover both cases:
> > > > >> a) not stopped node from previous test and
> > > > >> b) allows to remove useless code that stops Ignite nodes from each
> > > test.
> > > > >>
> > > > >> ср, 7 февр. 2018 г. в 15:13, Anton Vinogradov <
> > > avinogra...@gridgain.com
> > > > >:
> > > > >>
> > > > >> > Maxim,
> > > > >> >
> > > > >> > We discussed with Dima privately, and decided
> > > > >> >
> > > > >> > 1) We have to assert that there is no alive nodes at
> > > > GridAbstractTest's
> > > > >> > beforeTestsStarted
> > > > >> > 2) We have to kill all alive nodes (without force) at
> > > > GridAbstractTest's
> > > > >> > afterTestsStopped
> > > > >> > 3) In case of any exceptions at #2 we have to see test error
> > > > >> > 4) We can get rid of all useless stopAllGrids at
> > GridAbstractTest's
> > > > >> > subclasses.
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > On Wed, Feb 7, 2018 at 2:32 PM, Dmitry Pavlov <
> > > dpavlov@gmail.com>
> > > > >> > wrote:
> > > > >> >
> > > > >> > > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> > > > >> > > afterTestsStopped
> > > > >> > > method body.
> > > > >> > >
> > > > >> > > Can't agree with it becase implicit silent shutdown of nodes
> > from
> > > > test
> > > > >> > > framework may cause a lot of bugs introduced to Ignite.
> > > > >> > >
> > > > >> > > I think stop+test fail should occur.
> > > > >> > > In that case author of incorrect test or not consistent Ignite
> > > > >> shutdown
> > > > >> > > will see problem.
> > > > >> > >
> > > > >> > > 'Fail fast' if always better than hidding real problem under
> > > > automatic
> > > > >> > > framework feature.
> > > > >> > >
> > > > >> > > ср,

Re: Stop nodes after test by default - IGNITE-6842

2018-02-27 Thread Dmitry Pavlov

Hi Maxim,

could you please provide link to TC run on your PR? It seems link provided
points to run of master. In changes field you may select pull/3542/head
before starting RunAll.

Igniters,

this change is related to our test framework, so change may affect your
tests. Please join to review
https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502

Sincerely,
Dmitriy Pavlov

вт, 27 февр. 2018 г. в 16:14, Maxim Muzafarov :

> Hi all,
>
> I think, I've done with this issue, what should we do next?
>
> PR: https://github.com/apache/ignite/pull/3542
> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
> TC:
>
> https://ci.ignite.apache.org/viewModification.html?modId=723895=false==vcsModificationTests
> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842
>
>
> чт, 22 февр. 2018 г. в 14:12, Dmitry Pavlov :
>
> > Hi Maxim,
> >
> > Thank you.
> >
> > To my mind stopAllGrids call should be kept in afterTestsStop(). If
> > developer forgot to call super(), he will see exception from
> > beforeTestsStart()
> > added by you.
> >
> > If you think patch is ready to be reviewed, please change JIRA status to
> > "Patch Available".
> >
> > Optionally you can create Upsource review. It would be easier for
> multiple
> > reviewers to handle this patch. This test framework is used by all
> Igniters
> > so Upsource would be good option (https://reviews.ignite.apache.org/ ).
> >
> > Sincerely,
> > Dmitriy Pavlov
> >
> > чт, 22 февр. 2018 г. в 13:44, Maxim Muzafarov :
> >
> > > Hi all,
> > >
> > > I've made some changes based on our previous discusstions, please see
> PR
> > > [1]:
> > > 1) Remove duplicated code for stopGrid (by index and by name);
> > > 2) Add new method that thows exception if not all grids haven't stopped
> > > correctly;
> > > 3)  Change tests that have been affected by this changes;
> > >
> > > Also, I have some thoughts for clarification:
> > > 1) beforeTestsStart() - I expect here in subtests that grids are not
> > > started yet. Am I right?
> > > 2) I think we should call stopAllGrids in tearDown method. So, if in
> some
> > > cases we'll override afterTestsStop in subclasses and forgot to call
> > > super() it won't lead us to exception.
> > >
> > > [1] https://github.com/apache/ignite/pull/3542
> > > [2] https://ci.ignite.apache.org/viewModification.html?modId=717275
> > > [3] https://issues.apache.org/jira/browse/IGNITE-6842
> > >
> > >
> > > ср, 7 февр. 2018 г. в 18:28, Maxim Muzafarov :
> > >
> > > > Thank you all,
> > > >
> > > > I'll add this comment's for JIRA ticket, if you don't mind.
> > > >
> > > > ср, 7 февр. 2018 г. в 15:16, Dmitry Pavlov :
> > > >
> > > >> Yes, this solution allows to cover both cases:
> > > >> a) not stopped node from previous test and
> > > >> b) allows to remove useless code that stops Ignite nodes from each
> > test.
> > > >>
> > > >> ср, 7 февр. 2018 г. в 15:13, Anton Vinogradov <
> > avinogra...@gridgain.com
> > > >:
> > > >>
> > > >> > Maxim,
> > > >> >
> > > >> > We discussed with Dima privately, and decided
> > > >> >
> > > >> > 1) We have to assert that there is no alive nodes at
> > > GridAbstractTest's
> > > >> > beforeTestsStarted
> > > >> > 2) We have to kill all alive nodes (without force) at
> > > GridAbstractTest's
> > > >> > afterTestsStopped
> > > >> > 3) In case of any exceptions at #2 we have to see test error
> > > >> > 4) We can get rid of all useless stopAllGrids at
> GridAbstractTest's
> > > >> > subclasses.
> > > >> >
> > > >> >
> > > >> >
> > > >> > On Wed, Feb 7, 2018 at 2:32 PM, Dmitry Pavlov <
> > dpavlov@gmail.com>
> > > >> > wrote:
> > > >> >
> > > >> > > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> > > >> > > afterTestsStopped
> > > >> > > method body.
> > > >> > >
> > > >> > > Can't agree with it becase implicit silent shutdown of nodes
> from
> > > test
> > > >> > > framework may cause a lot of bugs introduced to Ignite.
> > > >> > >
> > > >> > > I think stop+test fail should occur.
> > > >> > > In that case author of incorrect test or not consistent Ignite
> > > >> shutdown
> > > >> > > will see problem.
> > > >> > >
> > > >> > > 'Fail fast' if always better than hidding real problem under
> > > automatic
> > > >> > > framework feature.
> > > >> > >
> > > >> > > ср, 7 февр. 2018 г. в 14:05, Anton Vinogradov <
> > > >> avinogra...@gridgain.com
> > > >> > >:
> > > >> > >
> > > >> > > > Hi all,
> > > >> > > >
> > > >> > > > > I've made some research about this problem and i think that
> in
> > > >> > general
> > > >> > > we
> > > >> > > > > should move stopAllGrids method in GridAbstractTest class to
> > > >> > > > > afterTestsStopped method with some changes. Am I right?
> > > >> > > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> > > >> > > > afterTestsStopped method
> > > >> > > > body.
> > > >> > > >
> > > >> > > > > I have a question about stopAllGrids(boolean

Re: Stop nodes after test by default - IGNITE-6842

2018-02-27 Thread Maxim Muzafarov

Hi all,

I think, I've done with this issue, what should we do next?

PR: https://github.com/apache/ignite/pull/3542
Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502
TC:
https://ci.ignite.apache.org/viewModification.html?modId=723895=false==vcsModificationTests
JIRA: https://issues.apache.org/jira/browse/IGNITE-6842


чт, 22 февр. 2018 г. в 14:12, Dmitry Pavlov :

> Hi Maxim,
>
> Thank you.
>
> To my mind stopAllGrids call should be kept in afterTestsStop(). If
> developer forgot to call super(), he will see exception from
> beforeTestsStart()
> added by you.
>
> If you think patch is ready to be reviewed, please change JIRA status to
> "Patch Available".
>
> Optionally you can create Upsource review. It would be easier for multiple
> reviewers to handle this patch. This test framework is used by all Igniters
> so Upsource would be good option (https://reviews.ignite.apache.org/ ).
>
> Sincerely,
> Dmitriy Pavlov
>
> чт, 22 февр. 2018 г. в 13:44, Maxim Muzafarov :
>
> > Hi all,
> >
> > I've made some changes based on our previous discusstions, please see PR
> > [1]:
> > 1) Remove duplicated code for stopGrid (by index and by name);
> > 2) Add new method that thows exception if not all grids haven't stopped
> > correctly;
> > 3)  Change tests that have been affected by this changes;
> >
> > Also, I have some thoughts for clarification:
> > 1) beforeTestsStart() - I expect here in subtests that grids are not
> > started yet. Am I right?
> > 2) I think we should call stopAllGrids in tearDown method. So, if in some
> > cases we'll override afterTestsStop in subclasses and forgot to call
> > super() it won't lead us to exception.
> >
> > [1] https://github.com/apache/ignite/pull/3542
> > [2] https://ci.ignite.apache.org/viewModification.html?modId=717275
> > [3] https://issues.apache.org/jira/browse/IGNITE-6842
> >
> >
> > ср, 7 февр. 2018 г. в 18:28, Maxim Muzafarov :
> >
> > > Thank you all,
> > >
> > > I'll add this comment's for JIRA ticket, if you don't mind.
> > >
> > > ср, 7 февр. 2018 г. в 15:16, Dmitry Pavlov :
> > >
> > >> Yes, this solution allows to cover both cases:
> > >> a) not stopped node from previous test and
> > >> b) allows to remove useless code that stops Ignite nodes from each
> test.
> > >>
> > >> ср, 7 февр. 2018 г. в 15:13, Anton Vinogradov <
> avinogra...@gridgain.com
> > >:
> > >>
> > >> > Maxim,
> > >> >
> > >> > We discussed with Dima privately, and decided
> > >> >
> > >> > 1) We have to assert that there is no alive nodes at
> > GridAbstractTest's
> > >> > beforeTestsStarted
> > >> > 2) We have to kill all alive nodes (without force) at
> > GridAbstractTest's
> > >> > afterTestsStopped
> > >> > 3) In case of any exceptions at #2 we have to see test error
> > >> > 4) We can get rid of all useless stopAllGrids at GridAbstractTest's
> > >> > subclasses.
> > >> >
> > >> >
> > >> >
> > >> > On Wed, Feb 7, 2018 at 2:32 PM, Dmitry Pavlov <
> dpavlov@gmail.com>
> > >> > wrote:
> > >> >
> > >> > > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> > >> > > afterTestsStopped
> > >> > > method body.
> > >> > >
> > >> > > Can't agree with it becase implicit silent shutdown of nodes from
> > test
> > >> > > framework may cause a lot of bugs introduced to Ignite.
> > >> > >
> > >> > > I think stop+test fail should occur.
> > >> > > In that case author of incorrect test or not consistent Ignite
> > >> shutdown
> > >> > > will see problem.
> > >> > >
> > >> > > 'Fail fast' if always better than hidding real problem under
> > automatic
> > >> > > framework feature.
> > >> > >
> > >> > > ср, 7 февр. 2018 г. в 14:05, Anton Vinogradov <
> > >> avinogra...@gridgain.com
> > >> > >:
> > >> > >
> > >> > > > Hi all,
> > >> > > >
> > >> > > > > I've made some research about this problem and i think that in
> > >> > general
> > >> > > we
> > >> > > > > should move stopAllGrids method in GridAbstractTest class to
> > >> > > > > afterTestsStopped method with some changes. Am I right?
> > >> > > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> > >> > > > afterTestsStopped method
> > >> > > > body.
> > >> > > >
> > >> > > > > I have a question about stopAllGrids(boolean cancel) this
> > "cancel"
> > >> > > > That's just a flag means "do not wait for any operations finish"
> > >> > > > See no reason to set it true in that case.
> > >> > > >
> > >> > > > > If you delegate closing to afterTestsStopped this will affect
> > only
> > >> > > > > last test (method).
> > >> > > > The idea is to stop all nodes at last test's method finish.
> > >> > > >
> > >> > > > >  Nodes that survive between tests can affect successive
> > >> > > > tests.
> > >> > > > Thats ok. we have a lot tests where nodes reusable between
> test's
> > >> > > methods.
> > >> > > >
> > >> > > >
> > >> > > > On Wed, Feb 7, 2018 at 1:20 PM, Dmitry Pavlov <
> > >> dpavlov@gmail.com>
> > >> > > > wrote:
> > >> > >

Re: Stop nodes after test by default - IGNITE-6842

2018-02-22 Thread Dmitry Pavlov

Hi Maxim,

Thank you.

To my mind stopAllGrids call should be kept in afterTestsStop(). If
developer forgot to call super(), he will see exception from beforeTestsStart()
added by you.

If you think patch is ready to be reviewed, please change JIRA status to
"Patch Available".

Optionally you can create Upsource review. It would be easier for multiple
reviewers to handle this patch. This test framework is used by all Igniters
so Upsource would be good option (https://reviews.ignite.apache.org/ ).

Sincerely,
Dmitriy Pavlov

чт, 22 февр. 2018 г. в 13:44, Maxim Muzafarov :

> Hi all,
>
> I've made some changes based on our previous discusstions, please see PR
> [1]:
> 1) Remove duplicated code for stopGrid (by index and by name);
> 2) Add new method that thows exception if not all grids haven't stopped
> correctly;
> 3)  Change tests that have been affected by this changes;
>
> Also, I have some thoughts for clarification:
> 1) beforeTestsStart() - I expect here in subtests that grids are not
> started yet. Am I right?
> 2) I think we should call stopAllGrids in tearDown method. So, if in some
> cases we'll override afterTestsStop in subclasses and forgot to call
> super() it won't lead us to exception.
>
> [1] https://github.com/apache/ignite/pull/3542
> [2] https://ci.ignite.apache.org/viewModification.html?modId=717275
> [3] https://issues.apache.org/jira/browse/IGNITE-6842
>
>
> ср, 7 февр. 2018 г. в 18:28, Maxim Muzafarov :
>
> > Thank you all,
> >
> > I'll add this comment's for JIRA ticket, if you don't mind.
> >
> > ср, 7 февр. 2018 г. в 15:16, Dmitry Pavlov :
> >
> >> Yes, this solution allows to cover both cases:
> >> a) not stopped node from previous test and
> >> b) allows to remove useless code that stops Ignite nodes from each test.
> >>
> >> ср, 7 февр. 2018 г. в 15:13, Anton Vinogradov  >:
> >>
> >> > Maxim,
> >> >
> >> > We discussed with Dima privately, and decided
> >> >
> >> > 1) We have to assert that there is no alive nodes at
> GridAbstractTest's
> >> > beforeTestsStarted
> >> > 2) We have to kill all alive nodes (without force) at
> GridAbstractTest's
> >> > afterTestsStopped
> >> > 3) In case of any exceptions at #2 we have to see test error
> >> > 4) We can get rid of all useless stopAllGrids at GridAbstractTest's
> >> > subclasses.
> >> >
> >> >
> >> >
> >> > On Wed, Feb 7, 2018 at 2:32 PM, Dmitry Pavlov 
> >> > wrote:
> >> >
> >> > > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> >> > > afterTestsStopped
> >> > > method body.
> >> > >
> >> > > Can't agree with it becase implicit silent shutdown of nodes from
> test
> >> > > framework may cause a lot of bugs introduced to Ignite.
> >> > >
> >> > > I think stop+test fail should occur.
> >> > > In that case author of incorrect test or not consistent Ignite
> >> shutdown
> >> > > will see problem.
> >> > >
> >> > > 'Fail fast' if always better than hidding real problem under
> automatic
> >> > > framework feature.
> >> > >
> >> > > ср, 7 февр. 2018 г. в 14:05, Anton Vinogradov <
> >> avinogra...@gridgain.com
> >> > >:
> >> > >
> >> > > > Hi all,
> >> > > >
> >> > > > > I've made some research about this problem and i think that in
> >> > general
> >> > > we
> >> > > > > should move stopAllGrids method in GridAbstractTest class to
> >> > > > > afterTestsStopped method with some changes. Am I right?
> >> > > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> >> > > > afterTestsStopped method
> >> > > > body.
> >> > > >
> >> > > > > I have a question about stopAllGrids(boolean cancel) this
> "cancel"
> >> > > > That's just a flag means "do not wait for any operations finish"
> >> > > > See no reason to set it true in that case.
> >> > > >
> >> > > > > If you delegate closing to afterTestsStopped this will affect
> only
> >> > > > > last test (method).
> >> > > > The idea is to stop all nodes at last test's method finish.
> >> > > >
> >> > > > >  Nodes that survive between tests can affect successive
> >> > > > tests.
> >> > > > Thats ok. we have a lot tests where nodes reusable between test's
> >> > > methods.
> >> > > >
> >> > > >
> >> > > > On Wed, Feb 7, 2018 at 1:20 PM, Dmitry Pavlov <
> >> dpavlov@gmail.com>
> >> > > > wrote:
> >> > > >
> >> > > > > Hi Igniters,
> >> > > > >
> >> > > > > IMO nodes that survive between tests is not general practice and
> >> I'm
> >> > > not
> >> > > > > sure is a best practice. I suggest to mark such tests with some
> >> > method
> >> > > > > overriden from AbstractTest.
> >> > > > >
> >> > > > > About cancel flag, please note stopAllGrids(boolean cancel)
> >> > > cancel=false
> >> > > > > allows to wait of checkpoint ends in case persistence enabled.
> >> > > > >
> >> > > > > I still suggest to avoid stopping any nodes by test, but
> validate
> >> not
> >> > > > > stopped node exist and fail test instead of siltent implicit
> >> actions.
> >> > > > >
> >> >

Re: Stop nodes after test by default - IGNITE-6842

2018-02-22 Thread Maxim Muzafarov

Hi all,

I've made some changes based on our previous discusstions, please see PR
[1]:
1) Remove duplicated code for stopGrid (by index and by name);
2) Add new method that thows exception if not all grids haven't stopped
correctly;
3)  Change tests that have been affected by this changes;

Also, I have some thoughts for clarification:
1) beforeTestsStart() - I expect here in subtests that grids are not
started yet. Am I right?
2) I think we should call stopAllGrids in tearDown method. So, if in some
cases we'll override afterTestsStop in subclasses and forgot to call
super() it won't lead us to exception.

[1] https://github.com/apache/ignite/pull/3542
[2] https://ci.ignite.apache.org/viewModification.html?modId=717275
[3] https://issues.apache.org/jira/browse/IGNITE-6842


ср, 7 февр. 2018 г. в 18:28, Maxim Muzafarov :

> Thank you all,
>
> I'll add this comment's for JIRA ticket, if you don't mind.
>
> ср, 7 февр. 2018 г. в 15:16, Dmitry Pavlov :
>
>> Yes, this solution allows to cover both cases:
>> a) not stopped node from previous test and
>> b) allows to remove useless code that stops Ignite nodes from each test.
>>
>> ср, 7 февр. 2018 г. в 15:13, Anton Vinogradov :
>>
>> > Maxim,
>> >
>> > We discussed with Dima privately, and decided
>> >
>> > 1) We have to assert that there is no alive nodes at GridAbstractTest's
>> > beforeTestsStarted
>> > 2) We have to kill all alive nodes (without force) at GridAbstractTest's
>> > afterTestsStopped
>> > 3) In case of any exceptions at #2 we have to see test error
>> > 4) We can get rid of all useless stopAllGrids at GridAbstractTest's
>> > subclasses.
>> >
>> >
>> >
>> > On Wed, Feb 7, 2018 at 2:32 PM, Dmitry Pavlov 
>> > wrote:
>> >
>> > > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
>> > > afterTestsStopped
>> > > method body.
>> > >
>> > > Can't agree with it becase implicit silent shutdown of nodes from test
>> > > framework may cause a lot of bugs introduced to Ignite.
>> > >
>> > > I think stop+test fail should occur.
>> > > In that case author of incorrect test or not consistent Ignite
>> shutdown
>> > > will see problem.
>> > >
>> > > 'Fail fast' if always better than hidding real problem under automatic
>> > > framework feature.
>> > >
>> > > ср, 7 февр. 2018 г. в 14:05, Anton Vinogradov <
>> avinogra...@gridgain.com
>> > >:
>> > >
>> > > > Hi all,
>> > > >
>> > > > > I've made some research about this problem and i think that in
>> > general
>> > > we
>> > > > > should move stopAllGrids method in GridAbstractTest class to
>> > > > > afterTestsStopped method with some changes. Am I right?
>> > > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
>> > > > afterTestsStopped method
>> > > > body.
>> > > >
>> > > > > I have a question about stopAllGrids(boolean cancel) this "cancel"
>> > > > That's just a flag means "do not wait for any operations finish"
>> > > > See no reason to set it true in that case.
>> > > >
>> > > > > If you delegate closing to afterTestsStopped this will affect only
>> > > > > last test (method).
>> > > > The idea is to stop all nodes at last test's method finish.
>> > > >
>> > > > >  Nodes that survive between tests can affect successive
>> > > > tests.
>> > > > Thats ok. we have a lot tests where nodes reusable between test's
>> > > methods.
>> > > >
>> > > >
>> > > > On Wed, Feb 7, 2018 at 1:20 PM, Dmitry Pavlov <
>> dpavlov@gmail.com>
>> > > > wrote:
>> > > >
>> > > > > Hi Igniters,
>> > > > >
>> > > > > IMO nodes that survive between tests is not general practice and
>> I'm
>> > > not
>> > > > > sure is a best practice. I suggest to mark such tests with some
>> > method
>> > > > > overriden from AbstractTest.
>> > > > >
>> > > > > About cancel flag, please note stopAllGrids(boolean cancel)
>> > > cancel=false
>> > > > > allows to wait of checkpoint ends in case persistence enabled.
>> > > > >
>> > > > > I still suggest to avoid stopping any nodes by test, but validate
>> not
>> > > > > stopped node exist and fail test instead of siltent implicit
>> actions.
>> > > > >
>> > > > > Sincerely,
>> > > > > Dmitriy Pavlov
>> > > > >
>> > > > > ср, 7 февр. 2018 г. в 13:04, Andrey Kuznetsov > >:
>> > > > >
>> > > > > > Hi Maxim,
>> > > > > >
>> > > > > > Regarding your first question, the use of afterTestsStopped is
>> not
>> > > > enough
>> > > > > > to stop all nodes, since each individual test (method) can start
>> > > custom
>> > > > > set
>> > > > > > of notes during its operation, and this very test should stop
>> all
>> > > those
>> > > > > > nodes. If you delegate closing to afterTestsStopped this will
>> > affect
>> > > > only
>> > > > > > last test (method). Nodes that survive between tests can affect
>> > > > > successive
>> > > > > > tests.
>> > > > > >
>> > > > > > 2018-02-07 1:10 GMT+03:00 Maxim Muzafarov :
>> > > > > >
>> > > > > > > Hello,
>> > > > > > >
>> >

Re: Stop nodes after test by default - IGNITE-6842

2018-02-07 Thread Maxim Muzafarov

Thank you all,

I'll add this comment's for JIRA ticket, if you don't mind.

ср, 7 февр. 2018 г. в 15:16, Dmitry Pavlov :

> Yes, this solution allows to cover both cases:
> a) not stopped node from previous test and
> b) allows to remove useless code that stops Ignite nodes from each test.
>
> ср, 7 февр. 2018 г. в 15:13, Anton Vinogradov :
>
> > Maxim,
> >
> > We discussed with Dima privately, and decided
> >
> > 1) We have to assert that there is no alive nodes at GridAbstractTest's
> > beforeTestsStarted
> > 2) We have to kill all alive nodes (without force) at GridAbstractTest's
> > afterTestsStopped
> > 3) In case of any exceptions at #2 we have to see test error
> > 4) We can get rid of all useless stopAllGrids at GridAbstractTest's
> > subclasses.
> >
> >
> >
> > On Wed, Feb 7, 2018 at 2:32 PM, Dmitry Pavlov 
> > wrote:
> >
> > > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> > > afterTestsStopped
> > > method body.
> > >
> > > Can't agree with it becase implicit silent shutdown of nodes from test
> > > framework may cause a lot of bugs introduced to Ignite.
> > >
> > > I think stop+test fail should occur.
> > > In that case author of incorrect test or not consistent Ignite
> shutdown
> > > will see problem.
> > >
> > > 'Fail fast' if always better than hidding real problem under automatic
> > > framework feature.
> > >
> > > ср, 7 февр. 2018 г. в 14:05, Anton Vinogradov <
> avinogra...@gridgain.com
> > >:
> > >
> > > > Hi all,
> > > >
> > > > > I've made some research about this problem and i think that in
> > general
> > > we
> > > > > should move stopAllGrids method in GridAbstractTest class to
> > > > > afterTestsStopped method with some changes. Am I right?
> > > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> > > > afterTestsStopped method
> > > > body.
> > > >
> > > > > I have a question about stopAllGrids(boolean cancel) this "cancel"
> > > > That's just a flag means "do not wait for any operations finish"
> > > > See no reason to set it true in that case.
> > > >
> > > > > If you delegate closing to afterTestsStopped this will affect only
> > > > > last test (method).
> > > > The idea is to stop all nodes at last test's method finish.
> > > >
> > > > >  Nodes that survive between tests can affect successive
> > > > tests.
> > > > Thats ok. we have a lot tests where nodes reusable between test's
> > > methods.
> > > >
> > > >
> > > > On Wed, Feb 7, 2018 at 1:20 PM, Dmitry Pavlov  >
> > > > wrote:
> > > >
> > > > > Hi Igniters,
> > > > >
> > > > > IMO nodes that survive between tests is not general practice and
> I'm
> > > not
> > > > > sure is a best practice. I suggest to mark such tests with some
> > method
> > > > > overriden from AbstractTest.
> > > > >
> > > > > About cancel flag, please note stopAllGrids(boolean cancel)
> > > cancel=false
> > > > > allows to wait of checkpoint ends in case persistence enabled.
> > > > >
> > > > > I still suggest to avoid stopping any nodes by test, but validate
> not
> > > > > stopped node exist and fail test instead of siltent implicit
> actions.
> > > > >
> > > > > Sincerely,
> > > > > Dmitriy Pavlov
> > > > >
> > > > > ср, 7 февр. 2018 г. в 13:04, Andrey Kuznetsov :
> > > > >
> > > > > > Hi Maxim,
> > > > > >
> > > > > > Regarding your first question, the use of afterTestsStopped is
> not
> > > > enough
> > > > > > to stop all nodes, since each individual test (method) can start
> > > custom
> > > > > set
> > > > > > of notes during its operation, and this very test should stop all
> > > those
> > > > > > nodes. If you delegate closing to afterTestsStopped this will
> > affect
> > > > only
> > > > > > last test (method). Nodes that survive between tests can affect
> > > > > successive
> > > > > > tests.
> > > > > >
> > > > > > 2018-02-07 1:10 GMT+03:00 Maxim Muzafarov :
> > > > > >
> > > > > > > Hello,
> > > > > > >
> > > > > > > I've made some research about this problem and i think that in
> > > > general
> > > > > we
> > > > > > > should move stopAllGrids method in GridAbstractTest class to
> > > > > > > afterTestsStopped method with some changes. Am I right?
> > > > > > >
> > > > > > > Also, I have a question about stopAllGrids(boolean cancel) this
> > > > > "cancel"
> > > > > > > argument. Why in some cases we should interrupt ComputeJob and
> in
> > > > some
> > > > > > > cases shouldn't? For example here:
> > > > > > > IgniteBaselineAffinityTopologyActivationTest#afterTest
> > > > > > > we call method stopAllGrids(false) this way. Why not "true"
> > > argument
> > > > > > > instead?
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > Best regards,
> > > > > >   Andrey Kuznetsov.
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Stop nodes after test by default - IGNITE-6842

2018-02-07 Thread Dmitry Pavlov

Yes, this solution allows to cover both cases:
a) not stopped node from previous test and
b) allows to remove useless code that stops Ignite nodes from each test.

ср, 7 февр. 2018 г. в 15:13, Anton Vinogradov :

> Maxim,
>
> We discussed with Dima privately, and decided
>
> 1) We have to assert that there is no alive nodes at GridAbstractTest's
> beforeTestsStarted
> 2) We have to kill all alive nodes (without force) at GridAbstractTest's
> afterTestsStopped
> 3) In case of any exceptions at #2 we have to see test error
> 4) We can get rid of all useless stopAllGrids at GridAbstractTest's
> subclasses.
>
>
>
> On Wed, Feb 7, 2018 at 2:32 PM, Dmitry Pavlov 
> wrote:
>
> > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> > afterTestsStopped
> > method body.
> >
> > Can't agree with it becase implicit silent shutdown of nodes from test
> > framework may cause a lot of bugs introduced to Ignite.
> >
> > I think stop+test fail should occur.
> > In that case author of incorrect test or not consistent Ignite  shutdown
> > will see problem.
> >
> > 'Fail fast' if always better than hidding real problem under automatic
> > framework feature.
> >
> > ср, 7 февр. 2018 г. в 14:05, Anton Vinogradov  >:
> >
> > > Hi all,
> > >
> > > > I've made some research about this problem and i think that in
> general
> > we
> > > > should move stopAllGrids method in GridAbstractTest class to
> > > > afterTestsStopped method with some changes. Am I right?
> > > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> > > afterTestsStopped method
> > > body.
> > >
> > > > I have a question about stopAllGrids(boolean cancel) this "cancel"
> > > That's just a flag means "do not wait for any operations finish"
> > > See no reason to set it true in that case.
> > >
> > > > If you delegate closing to afterTestsStopped this will affect only
> > > > last test (method).
> > > The idea is to stop all nodes at last test's method finish.
> > >
> > > >  Nodes that survive between tests can affect successive
> > > tests.
> > > Thats ok. we have a lot tests where nodes reusable between test's
> > methods.
> > >
> > >
> > > On Wed, Feb 7, 2018 at 1:20 PM, Dmitry Pavlov 
> > > wrote:
> > >
> > > > Hi Igniters,
> > > >
> > > > IMO nodes that survive between tests is not general practice and I'm
> > not
> > > > sure is a best practice. I suggest to mark such tests with some
> method
> > > > overriden from AbstractTest.
> > > >
> > > > About cancel flag, please note stopAllGrids(boolean cancel)
> > cancel=false
> > > > allows to wait of checkpoint ends in case persistence enabled.
> > > >
> > > > I still suggest to avoid stopping any nodes by test, but validate not
> > > > stopped node exist and fail test instead of siltent implicit actions.
> > > >
> > > > Sincerely,
> > > > Dmitriy Pavlov
> > > >
> > > > ср, 7 февр. 2018 г. в 13:04, Andrey Kuznetsov :
> > > >
> > > > > Hi Maxim,
> > > > >
> > > > > Regarding your first question, the use of afterTestsStopped is not
> > > enough
> > > > > to stop all nodes, since each individual test (method) can start
> > custom
> > > > set
> > > > > of notes during its operation, and this very test should stop all
> > those
> > > > > nodes. If you delegate closing to afterTestsStopped this will
> affect
> > > only
> > > > > last test (method). Nodes that survive between tests can affect
> > > > successive
> > > > > tests.
> > > > >
> > > > > 2018-02-07 1:10 GMT+03:00 Maxim Muzafarov :
> > > > >
> > > > > > Hello,
> > > > > >
> > > > > > I've made some research about this problem and i think that in
> > > general
> > > > we
> > > > > > should move stopAllGrids method in GridAbstractTest class to
> > > > > > afterTestsStopped method with some changes. Am I right?
> > > > > >
> > > > > > Also, I have a question about stopAllGrids(boolean cancel) this
> > > > "cancel"
> > > > > > argument. Why in some cases we should interrupt ComputeJob and in
> > > some
> > > > > > cases shouldn't? For example here:
> > > > > > IgniteBaselineAffinityTopologyActivationTest#afterTest
> > > > > > we call method stopAllGrids(false) this way. Why not "true"
> > argument
> > > > > > instead?
> > > > > >
> > > > > >
> > > > > > --
> > > > > Best regards,
> > > > >   Andrey Kuznetsov.
> > > > >
> > > >
> > >
> >
>

Re: Stop nodes after test by default - IGNITE-6842

2018-02-07 Thread Anton Vinogradov

Maxim,

We discussed with Dima privately, and decided

1) We have to assert that there is no alive nodes at GridAbstractTest's
beforeTestsStarted
2) We have to kill all alive nodes (without force) at GridAbstractTest's
afterTestsStopped
3) In case of any exceptions at #2 we have to see test error
4) We can get rid of all useless stopAllGrids at GridAbstractTest's
subclasses.



On Wed, Feb 7, 2018 at 2:32 PM, Dmitry Pavlov  wrote:

> > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> afterTestsStopped
> method body.
>
> Can't agree with it becase implicit silent shutdown of nodes from test
> framework may cause a lot of bugs introduced to Ignite.
>
> I think stop+test fail should occur.
> In that case author of incorrect test or not consistent Ignite  shutdown
> will see problem.
>
> 'Fail fast' if always better than hidding real problem under automatic
> framework feature.
>
> ср, 7 февр. 2018 г. в 14:05, Anton Vinogradov :
>
> > Hi all,
> >
> > > I've made some research about this problem and i think that in general
> we
> > > should move stopAllGrids method in GridAbstractTest class to
> > > afterTestsStopped method with some changes. Am I right?
> > Let's just add stopAllGrids(flase) to GridAbstractTest 's
> > afterTestsStopped method
> > body.
> >
> > > I have a question about stopAllGrids(boolean cancel) this "cancel"
> > That's just a flag means "do not wait for any operations finish"
> > See no reason to set it true in that case.
> >
> > > If you delegate closing to afterTestsStopped this will affect only
> > > last test (method).
> > The idea is to stop all nodes at last test's method finish.
> >
> > >  Nodes that survive between tests can affect successive
> > tests.
> > Thats ok. we have a lot tests where nodes reusable between test's
> methods.
> >
> >
> > On Wed, Feb 7, 2018 at 1:20 PM, Dmitry Pavlov 
> > wrote:
> >
> > > Hi Igniters,
> > >
> > > IMO nodes that survive between tests is not general practice and I'm
> not
> > > sure is a best practice. I suggest to mark such tests with some method
> > > overriden from AbstractTest.
> > >
> > > About cancel flag, please note stopAllGrids(boolean cancel)
> cancel=false
> > > allows to wait of checkpoint ends in case persistence enabled.
> > >
> > > I still suggest to avoid stopping any nodes by test, but validate not
> > > stopped node exist and fail test instead of siltent implicit actions.
> > >
> > > Sincerely,
> > > Dmitriy Pavlov
> > >
> > > ср, 7 февр. 2018 г. в 13:04, Andrey Kuznetsov :
> > >
> > > > Hi Maxim,
> > > >
> > > > Regarding your first question, the use of afterTestsStopped is not
> > enough
> > > > to stop all nodes, since each individual test (method) can start
> custom
> > > set
> > > > of notes during its operation, and this very test should stop all
> those
> > > > nodes. If you delegate closing to afterTestsStopped this will affect
> > only
> > > > last test (method). Nodes that survive between tests can affect
> > > successive
> > > > tests.
> > > >
> > > > 2018-02-07 1:10 GMT+03:00 Maxim Muzafarov :
> > > >
> > > > > Hello,
> > > > >
> > > > > I've made some research about this problem and i think that in
> > general
> > > we
> > > > > should move stopAllGrids method in GridAbstractTest class to
> > > > > afterTestsStopped method with some changes. Am I right?
> > > > >
> > > > > Also, I have a question about stopAllGrids(boolean cancel) this
> > > "cancel"
> > > > > argument. Why in some cases we should interrupt ComputeJob and in
> > some
> > > > > cases shouldn't? For example here:
> > > > > IgniteBaselineAffinityTopologyActivationTest#afterTest
> > > > > we call method stopAllGrids(false) this way. Why not "true"
> argument
> > > > > instead?
> > > > >
> > > > >
> > > > > --
> > > > Best regards,
> > > >   Andrey Kuznetsov.
> > > >
> > >
> >
>

Re: Stop nodes after test by default - IGNITE-6842

2018-02-07 Thread Dmitry Pavlov

> Let's just add stopAllGrids(flase) to GridAbstractTest 's afterTestsStopped
method body.

Can't agree with it becase implicit silent shutdown of nodes from test
framework may cause a lot of bugs introduced to Ignite.

I think stop+test fail should occur.
In that case author of incorrect test or not consistent Ignite  shutdown
will see problem.

'Fail fast' if always better than hidding real problem under automatic
framework feature.

ср, 7 февр. 2018 г. в 14:05, Anton Vinogradov :

> Hi all,
>
> > I've made some research about this problem and i think that in general we
> > should move stopAllGrids method in GridAbstractTest class to
> > afterTestsStopped method with some changes. Am I right?
> Let's just add stopAllGrids(flase) to GridAbstractTest 's
> afterTestsStopped method
> body.
>
> > I have a question about stopAllGrids(boolean cancel) this "cancel"
> That's just a flag means "do not wait for any operations finish"
> See no reason to set it true in that case.
>
> > If you delegate closing to afterTestsStopped this will affect only
> > last test (method).
> The idea is to stop all nodes at last test's method finish.
>
> >  Nodes that survive between tests can affect successive
> tests.
> Thats ok. we have a lot tests where nodes reusable between test's methods.
>
>
> On Wed, Feb 7, 2018 at 1:20 PM, Dmitry Pavlov 
> wrote:
>
> > Hi Igniters,
> >
> > IMO nodes that survive between tests is not general practice and I'm not
> > sure is a best practice. I suggest to mark such tests with some method
> > overriden from AbstractTest.
> >
> > About cancel flag, please note stopAllGrids(boolean cancel)  cancel=false
> > allows to wait of checkpoint ends in case persistence enabled.
> >
> > I still suggest to avoid stopping any nodes by test, but validate not
> > stopped node exist and fail test instead of siltent implicit actions.
> >
> > Sincerely,
> > Dmitriy Pavlov
> >
> > ср, 7 февр. 2018 г. в 13:04, Andrey Kuznetsov :
> >
> > > Hi Maxim,
> > >
> > > Regarding your first question, the use of afterTestsStopped is not
> enough
> > > to stop all nodes, since each individual test (method) can start custom
> > set
> > > of notes during its operation, and this very test should stop all those
> > > nodes. If you delegate closing to afterTestsStopped this will affect
> only
> > > last test (method). Nodes that survive between tests can affect
> > successive
> > > tests.
> > >
> > > 2018-02-07 1:10 GMT+03:00 Maxim Muzafarov :
> > >
> > > > Hello,
> > > >
> > > > I've made some research about this problem and i think that in
> general
> > we
> > > > should move stopAllGrids method in GridAbstractTest class to
> > > > afterTestsStopped method with some changes. Am I right?
> > > >
> > > > Also, I have a question about stopAllGrids(boolean cancel) this
> > "cancel"
> > > > argument. Why in some cases we should interrupt ComputeJob and in
> some
> > > > cases shouldn't? For example here:
> > > > IgniteBaselineAffinityTopologyActivationTest#afterTest
> > > > we call method stopAllGrids(false) this way. Why not "true" argument
> > > > instead?
> > > >
> > > >
> > > > --
> > > Best regards,
> > >   Andrey Kuznetsov.
> > >
> >
>

Re: Stop nodes after test by default - IGNITE-6842

2018-02-07 Thread Anton Vinogradov

Hi all,

> I've made some research about this problem and i think that in general we
> should move stopAllGrids method in GridAbstractTest class to
> afterTestsStopped method with some changes. Am I right?
Let's just add stopAllGrids(flase) to GridAbstractTest 's
afterTestsStopped method
body.

> I have a question about stopAllGrids(boolean cancel) this "cancel"
That's just a flag means "do not wait for any operations finish"
See no reason to set it true in that case.

> If you delegate closing to afterTestsStopped this will affect only
> last test (method).
The idea is to stop all nodes at last test's method finish.

>  Nodes that survive between tests can affect successive
tests.
Thats ok. we have a lot tests where nodes reusable between test's methods.


On Wed, Feb 7, 2018 at 1:20 PM, Dmitry Pavlov  wrote:

> Hi Igniters,
>
> IMO nodes that survive between tests is not general practice and I'm not
> sure is a best practice. I suggest to mark such tests with some method
> overriden from AbstractTest.
>
> About cancel flag, please note stopAllGrids(boolean cancel)  cancel=false
> allows to wait of checkpoint ends in case persistence enabled.
>
> I still suggest to avoid stopping any nodes by test, but validate not
> stopped node exist and fail test instead of siltent implicit actions.
>
> Sincerely,
> Dmitriy Pavlov
>
> ср, 7 февр. 2018 г. в 13:04, Andrey Kuznetsov :
>
> > Hi Maxim,
> >
> > Regarding your first question, the use of afterTestsStopped is not enough
> > to stop all nodes, since each individual test (method) can start custom
> set
> > of notes during its operation, and this very test should stop all those
> > nodes. If you delegate closing to afterTestsStopped this will affect only
> > last test (method). Nodes that survive between tests can affect
> successive
> > tests.
> >
> > 2018-02-07 1:10 GMT+03:00 Maxim Muzafarov :
> >
> > > Hello,
> > >
> > > I've made some research about this problem and i think that in general
> we
> > > should move stopAllGrids method in GridAbstractTest class to
> > > afterTestsStopped method with some changes. Am I right?
> > >
> > > Also, I have a question about stopAllGrids(boolean cancel) this
> "cancel"
> > > argument. Why in some cases we should interrupt ComputeJob and in some
> > > cases shouldn't? For example here:
> > > IgniteBaselineAffinityTopologyActivationTest#afterTest
> > > we call method stopAllGrids(false) this way. Why not "true" argument
> > > instead?
> > >
> > >
> > > --
> > Best regards,
> >   Andrey Kuznetsov.
> >
>

Re: Stop nodes after test by default - IGNITE-6842

2018-02-07 Thread Dmitry Pavlov

Hi Igniters,

IMO nodes that survive between tests is not general practice and I'm not
sure is a best practice. I suggest to mark such tests with some method
overriden from AbstractTest.

About cancel flag, please note stopAllGrids(boolean cancel)  cancel=false
allows to wait of checkpoint ends in case persistence enabled.

I still suggest to avoid stopping any nodes by test, but validate not
stopped node exist and fail test instead of siltent implicit actions.

Sincerely,
Dmitriy Pavlov

ср, 7 февр. 2018 г. в 13:04, Andrey Kuznetsov :

> Hi Maxim,
>
> Regarding your first question, the use of afterTestsStopped is not enough
> to stop all nodes, since each individual test (method) can start custom set
> of notes during its operation, and this very test should stop all those
> nodes. If you delegate closing to afterTestsStopped this will affect only
> last test (method). Nodes that survive between tests can affect successive
> tests.
>
> 2018-02-07 1:10 GMT+03:00 Maxim Muzafarov :
>
> > Hello,
> >
> > I've made some research about this problem and i think that in general we
> > should move stopAllGrids method in GridAbstractTest class to
> > afterTestsStopped method with some changes. Am I right?
> >
> > Also, I have a question about stopAllGrids(boolean cancel) this "cancel"
> > argument. Why in some cases we should interrupt ComputeJob and in some
> > cases shouldn't? For example here:
> > IgniteBaselineAffinityTopologyActivationTest#afterTest
> > we call method stopAllGrids(false) this way. Why not "true" argument
> > instead?
> >
> >
> > --
> Best regards,
>   Andrey Kuznetsov.
>

Re: Stop nodes after test by default - IGNITE-6842

2018-02-07 Thread Andrey Kuznetsov

Hi Maxim,

Regarding your first question, the use of afterTestsStopped is not enough
to stop all nodes, since each individual test (method) can start custom set
of notes during its operation, and this very test should stop all those
nodes. If you delegate closing to afterTestsStopped this will affect only
last test (method). Nodes that survive between tests can affect successive
tests.

2018-02-07 1:10 GMT+03:00 Maxim Muzafarov :

> Hello,
>
> I've made some research about this problem and i think that in general we
> should move stopAllGrids method in GridAbstractTest class to
> afterTestsStopped method with some changes. Am I right?
>
> Also, I have a question about stopAllGrids(boolean cancel) this "cancel"
> argument. Why in some cases we should interrupt ComputeJob and in some
> cases shouldn't? For example here:
> IgniteBaselineAffinityTopologyActivationTest#afterTest
> we call method stopAllGrids(false) this way. Why not "true" argument
> instead?
>
>
> --
Best regards,
  Andrey Kuznetsov.

Re: Stop nodes after test by default - IGNITE-6842

2018-02-06 Thread Maxim Muzafarov

Hello,

I've made some research about this problem and i think that in general we
should move stopAllGrids method in GridAbstractTest class to
afterTestsStopped method with some changes. Am I right?

Also, I have a question about stopAllGrids(boolean cancel) this "cancel"
argument. Why in some cases we should interrupt ComputeJob and in some
cases shouldn't? For example here:
IgniteBaselineAffinityTopologyActivationTest#afterTest
we call method stopAllGrids(false) this way. Why not "true" argument
instead?



чт, 1 февр. 2018 г. в 16:12, Dmitry Pavlov :

> Hi Maxim,
>
> I would be happy if this issue is completed.
>
> I think best solultion is
> 1) to validate remained up and running nodes
> 2) and fail test if not-stopped nodes remained
>
> instead of silently stop node. If nodes will be stopped silently, probable
> errors may be missed both in test and in Ignite code.
>
> Sincerely,
> Dmitriy Pavlov
>
> чт, 1 февр. 2018 г. в 3:54, Denis Magda :
>
> > Hello Maxim,
> >
> > Granted you all the required permissions in JIRA. Feel free to assign the
> > ticket to your account.
> >
> > In general, review how the testing framework works in Ignite and you’ll
> > get why we stop all the nodes manually. It will help to get answers on
> most
> > of the questions. Please propose your solution or ask for advice here
> after
> > that.
> >
> > —
> > Denis
> >
> > > On Jan 31, 2018, at 8:01 AM, Maxim Muzafarov 
> wrote:
> > >
> > > Hello everyone!
> > >
> > > I would like to take this one issue for implicating myself into Ignite
> > > community.
> > >
> > > https://issues.apache.org/jira/browse/IGNITE-6842
> > >
> > > Can you help me with granting contributor permissons?
> > > My JIRA ID: mmuzaf
> > > e-mail: maxmu...@gmail.com
> > > So I can assign this ticket for myself, If you don't mind of course.
> > >
> > > Also, it will be great to have something like general roadmap for
> fixing
> > > this issue. What should I look at on my first step?
> >
> >
>

Re: Stop nodes after test by default - IGNITE-6842

2018-02-01 Thread Dmitry Pavlov

Hi Maxim,

I would be happy if this issue is completed.

I think best solultion is
1) to validate remained up and running nodes
2) and fail test if not-stopped nodes remained

instead of silently stop node. If nodes will be stopped silently, probable
errors may be missed both in test and in Ignite code.

Sincerely,
Dmitriy Pavlov

чт, 1 февр. 2018 г. в 3:54, Denis Magda :

> Hello Maxim,
>
> Granted you all the required permissions in JIRA. Feel free to assign the
> ticket to your account.
>
> In general, review how the testing framework works in Ignite and you’ll
> get why we stop all the nodes manually. It will help to get answers on most
> of the questions. Please propose your solution or ask for advice here after
> that.
>
> —
> Denis
>
> > On Jan 31, 2018, at 8:01 AM, Maxim Muzafarov  wrote:
> >
> > Hello everyone!
> >
> > I would like to take this one issue for implicating myself into Ignite
> > community.
> >
> > https://issues.apache.org/jira/browse/IGNITE-6842
> >
> > Can you help me with granting contributor permissons?
> > My JIRA ID: mmuzaf
> > e-mail: maxmu...@gmail.com
> > So I can assign this ticket for myself, If you don't mind of course.
> >
> > Also, it will be great to have something like general roadmap for fixing
> > this issue. What should I look at on my first step?
>
>

Re: Stop nodes after test by default - IGNITE-6842

2018-01-31 Thread Denis Magda

Hello Maxim, 

Granted you all the required permissions in JIRA. Feel free to assign the 
ticket to your account.

In general, review how the testing framework works in Ignite and you’ll get why 
we stop all the nodes manually. It will help to get answers on most of the 
questions. Please propose your solution or ask for advice here after that.

—
Denis

> On Jan 31, 2018, at 8:01 AM, Maxim Muzafarov  wrote:
> 
> Hello everyone!
> 
> I would like to take this one issue for implicating myself into Ignite
> community.
> 
> https://issues.apache.org/jira/browse/IGNITE-6842
> 
> Can you help me with granting contributor permissons?
> My JIRA ID: mmuzaf
> e-mail: maxmu...@gmail.com
> So I can assign this ticket for myself, If you don't mind of course.
> 
> Also, it will be great to have something like general roadmap for fixing
> this issue. What should I look at on my first step?

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

Re: Stop nodes after test by default - IGNITE-6842

24 matches

Site Navigation

Mail list logo

Footer information