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 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: > > > > > > > > > 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 <stku...@gmail.com>: > > > > > > > > > > > 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 <maxmu...@gmail.com>: > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > >
