ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitry Pavlov <dpavlov....@gmail.com>
Subject Re: Stop nodes after test by default - IGNITE-6842
Date Wed, 07 Feb 2018 12:16:19 GMT
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 <avinogradov@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.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