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

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>:

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

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@g

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

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 Muzafa

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

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 fo

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

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.o

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

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-C

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

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 fe

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

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

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

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

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

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 (f

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

Yep, link may not be correct. Here is correct version: TC: *https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&branch_IgniteTests24Java8=pull%2F3542%2Fhead *

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

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 http

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

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&personal=false&buildTypeId=&tab=vcsModificat

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

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 c

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

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

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

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. > > ср,

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

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 th

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

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

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

> 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 n

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

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

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

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

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

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 affe

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

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 case

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

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

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

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 y

Stop nodes after test by default - IGNITE-6842

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 d