ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Vinogradov <avinogra...@gridgain.com>
Subject Re: Stop nodes after test by default - IGNITE-6842
Date Wed, 07 Feb 2018 12:13:33 GMT
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.spb@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 <avinogradov@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.spb@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 <stkuzma@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 <maxmuzaf@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.
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message