ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitriy Pavlov <dpav...@apache.org>
Subject Re: Default failure handler was changed for tests
Date Wed, 05 Dec 2018 15:28:51 GMT
Folks, let me explain one thing which is not related much to fix itself,
but it is more about how we interact. If someone will just come to the list
and say it is not good commit, it is a silly solution and say to others to
rework these patches - it is a road to nowhere.

If someone sees the potential to make things better he or she suggest help
(or commits patch). This is named do-ocracy, those who do can make a
decision.



And this topic it is a perfect example of how do-ocracy should (and should
not) work. We have a potentially hidden problem (we had it before Dmitriy
R. commit), I believe 3 or 7 tests may be found after re-checks of tests.
Eventually, these tests will get their stop-node handler after revisiting
no-op test list.



We have ~100 tests and several people who care. Anton, Andrew,  Dmitrii &
Dmitriy, Nikolay, probably Ed, and we have 100/6 = 18 tests to double check
for each contributor. We can make things better if we go together. And this
is how a community works.



If someone just come to list to criticize and enforces someone else to do
all things, he or she probably don't want to improve project code but has
other goals.

ср, 5 дек. 2018 г. в 18:08, Andrey Kuznetsov <stkuzma@gmail.com>:

> As I can see from the above discussion,
>
> >  Tests in these classes check fail cases when we expect critical failure
> like node stop or exception thrown
>
> So, this copy-n-paste-style change is caused by the imperfect logic of
> existing tests, that should be reworked in more robust way, e.g. using
> custom failure handlers. Dmitrii just revealed the existing flaws, IMO.
>
> ср, 5 дек. 2018 г. в 17:54, Nikolay Izhikov <nizhikov@apache.org>:
>
> > Hello, Igniters.
> >
> > I'm agree with Anton Vinogradov.
> >
> > I think we should avoid commits like [1]
> > Copy paste coding style is well known anti pattern.
> >
> > Don't we have another option to do same fix with better styling?
> >
> > Accepting such patches leads to the further tickets to cleanup mess that
> > patches brings to the code base.
> > Example of cleanup [2]
> >
> > It's take a significant amount of my and Maxim time to made and review
> this
> > cleanup patch.
> >
> > We shouldn't accept patch with copy paste "improvements".
> >
> > > I really like your perfectionism
> >
> > It's not about perfectionism it's about keeping code base clean.
> >
> > > And I'm going to rollback changes in case arguments will not be
> provided.
> >
> > +1 to rollback and rework this commit.
> > At least, we should reduce copy paste code.
> >
> > [1]
> >
> >
> https://github.com/apache/ignite/commit/b94a3c2fe3a272a31fad62b80505d16f87eab2dd
> > [2]
> >
> >
> https://github.com/apache/ignite/commit/eb8038f65285559c5424eba2882b0de0583ea7af
> >
> > ср, 5 дек. 2018 г. в 17:28, Anton Vinogradov <av@apache.org>:
> >
> > > Andrey,
> > >
> > > >> But why should we make all things perfect
> > > >> in a single fix?
> > > As I said, I'm ok in case someone ready to continue :)
> > > But, we should avoid such over-copy-pasted commits in the future.
> > >
> > > On Wed, Dec 5, 2018 at 5:13 PM Andrey Mashenkov <
> > > andrey.mashenkov@gmail.com>
> > > wrote:
> > >
> > > > Dmitry,
> > > >
> > > > Do we have TC run results for the PR before massive failure handler
> > > > fallbacks were added?
> > > > Let's create a ticket to investigate possibility of using any
> > meaningful
> > > > failure handler for such tests with TC report attached.
> > > >
> > > > On Wed, Dec 5, 2018 at 4:41 PM Anton Vinogradov <av@apache.org>
> wrote:
> > > >
> > > > > Dmitriy,
> > > > >
> > > > > It's ok in case someone ready to do this (get rid of all no-op or
> > > explain
> > > > > why it's a better choice).
> > > > > Explicit confirmation required.
> > > > >
> > > > > Otherwise, only rollback is an option.
> > > > >
> > > > > On Wed, Dec 5, 2018 at 4:29 PM Dmitriy Pavlov <dpavlov@apache.org>
> > > > wrote:
> > > > >
> > > > > > Anton, if you care enough here will you try to research a couple
> of
> > > > these
> > > > > > tests? Or you are asking others to do things for you, aren't you?
> > > > > >
> > > > > > I like idea from Andrew to create ticket and check these test to
> > keep
> > > > > > moving towards 0....10 tests with noop. It is easy to locate
> these
> > > > > > overridden method now.
> > > > > >
> > > > > > So threat this change as contributed mechanism for failing tests.
> > Is
> > > it
> > > > > Ok
> > > > > > for you?
> > > > > >
> > > > > > ср, 5 дек. 2018 г., 15:59 Anton Vinogradov <av@apache.org>:
> > > > > >
> > > > > > > >> I didn't get. What is the problem in saving No-Op for
> several
> > > > tests?
> > > > > > Why
> > > > > > > >> should we keep No-Op for all?
> > > > > > > Several (less than 10) is ok to me with the proper explanation
> > why
> > > > > tests
> > > > > > > fail and why no-op is a better choice.
> > > > > > >
> > > > > > > 100+++ copy-pasted no-op handlers are not ok!
> > > > > > >
> > > > > > > >> I don't ask you to re-do this change, I ask to demonstrate
> any
> > > > > better
> > > > > > > >> approach for tests which intentionally activate failure
> > handler.
> > > > > > > You asking me to provide approach without explanation why tests
> > > fail
> > > > > > > without no-op handler?
> > > > > > > My approach is to rollback this fix, reopen the issue and make
> > > > > everything
> > > > > > > properly.
> > > > > > > Make a proper investigation first.
> > > > > > >
> > > > > > >
> > > > > > > Finally, let's stop this game.
> > > > > > > We have to discuss the reasons why tests fail.
> > > > > > > In case no-one checked "why" before the fix was merged we will
> be
> > > > able
> > > > > to
> > > > > > > start doing this after rollback.
> > > > > > >
> > > > > > > On Wed, Dec 5, 2018 at 3:49 PM Eduard Shangareev <
> > > > > > > eduard.shangareev@gmail.com> wrote:
> > > > > > >
> > > > > > > > Guys,
> > > > > > > >
> > > > > > > > I didn't get. What is the problem in saving No-Op for several
> > > > tests?
> > > > > > Why
> > > > > > > > should we keep No-Op for all?
> > > > > > > >
> > > > > > > > On Wed, Dec 5, 2018 at 3:20 PM Павлухин Иван <
> > > vololo100@gmail.com>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Anton,
> > > > > > > > >
> > > > > > > > > Yes I meant that patch. And I would like to respell a name
> > > > "massive
> > > > > > > > > no-op handler restore" to "use no-op failure handler only
> > where
> > > > it
> > > > > is
> > > > > > > > > assumed".
> > > > > > > > > ср, 5 дек. 2018 г. в 15:09, Dmitriy Pavlov <
> > dpavlov@apache.org
> > > >:
> > > > > > > > > >
> > > > > > > > > > Dmitrii Ryabov explained these tests are perfectly ok to
> > have
> > > > > > > failures
> > > > > > > > as
> > > > > > > > > > these tests do test failures.
> > > > > > > > > >
> > > > > > > > > > Anton, there is no reason to revert other's contributions
> > > > because
> > > > > > you
> > > > > > > > > know
> > > > > > > > > > how to do things better. A lot of people can do things
> > better
> > > > > than
> > > > > > > me.
> > > > > > > > > > Should we revert everything I've contributed? I hope -
> no.
> > > > > > > > > >
> > > > > > > > > > If you can do things better, just commit further
> > > improvements.
> > > > > And
> > > > > > I
> > > > > > > > will
> > > > > > > > > > be happy if you contribute some improvements later.
> > > > > > > > > >
> > > > > > > > > > If you would like to revert by veto, please justify your
> > > > intent.
> > > > > If
> > > > > > > you
> > > > > > > > > > would discuss it with all community, please feel free to
> > > > convince
> > > > > > me
> > > > > > > > and
> > > > > > > > > > others.
> > > > > > > > > >
> > > > > > > > > > ср, 5 дек. 2018 г. в 14:53, Павлухин Иван <
> > > vololo100@gmail.com
> > > > >:
> > > > > > > > > >
> > > > > > > > > > > Hi Anton,
> > > > > > > > > > >
> > > > > > > > > > > Could you please summarize what does aforementioned
> patch
> > > > made
> > > > > > > really
> > > > > > > > > > > worse?
> > > > > > > > > > >
> > > > > > > > > > > As I see, the patch added a very good thing --
> meaningful
> > > > > failure
> > > > > > > > > > > handler in tests. And I think it is really important.
> But
> > > was
> > > > > is
> > > > > > > the
> > > > > > > > > > > harm and does it overweight positive result? And why?
> > > > > > > > > > > ср, 5 дек. 2018 г. в 14:03, Anton Vinogradov <
> > > av@apache.org
> > > > >:
> > > > > > > > > > > >
> > > > > > > > > > > > Dmitriy,
> > > > > > > > > > > >
> > > > > > > > > > > > That's an incorrect idea to ask me to provide PR or
> to
> > > fix
> > > > > > these
> > > > > > > > test
> > > > > > > > > > > > properly since I'm not an author or reviewer.
> > > > > > > > > > > >
> > > > > > > > > > > > But, I, as a community member, ask you to explain
> what
> > > > > problems
> > > > > > > the
> > > > > > > > > fix
> > > > > > > > > > > > fixes.
> > > > > > > > > > > > In case you're not able to provide the explanation I
> > will
> > > > > > > rollback
> > > > > > > > > the
> > > > > > > > > > > > changes.
> > > > > > > > > > > >
> > > > > > > > > > > > That's not acceptable to merge fix of unknown
> problems.
> > > At
> > > > > > least,
> > > > > > > > > such
> > > > > > > > > > > "100
> > > > > > > > > > > > times copy-paste fix".
> > > > > > > > > > > > Please provide the explanation of the problem we're
> > > fixing
> > > > > for
> > > > > > > each
> > > > > > > > > test
> > > > > > > > > > > > group.
> > > > > > > > > > > >
> > > > > > > > > > > > P.s. My goal is not to rollback something, but to
> > prevent
> > > > > merge
> > > > > > > > > without
> > > > > > > > > > > > understanding what it fixes.
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Dec 5, 2018 at 1:40 PM Dmitriy Pavlov <
> > > > > > > dpavlov@apache.org>
> > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Anton, please provide PR to demo your idea. Code
> > speaks
> > > > > > louder
> > > > > > > > than
> > > > > > > > > > > words
> > > > > > > > > > > > > sometimes.
> > > > > > > > > > > > >
> > > > > > > > > > > > > No reason to revert a contribution if someone has
> an
> > > > idea,
> > > > > > > which
> > > > > > > > > is not
> > > > > > > > > > > > > clear for others.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Again, we should discuss not Dmitrii contribution,
> > but
> > > > the
> > > > > > > > initial
> > > > > > > > > > > > > selection of no-op.
> > > > > > > > > > > > >
> > > > > > > > > > > > > If you will do a test failure fixes later and you
> > will
> > > > set
> > > > > > new
> > > > > > > > > handler
> > > > > > > > > > > > > StopNode+FailTest as the only option - ok for me.
> > > > > > > > > > > > >
> > > > > > > > > > > > > ср, 5 дек. 2018 г. в 13:35, Anton Vinogradov <
> > > > > av@apache.org
> > > > > > >:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Dmitriy,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > As I said before, these changes allow tests to be
> > > > > > successful
> > > > > > > in
> > > > > > > > > case
> > > > > > > > > > > of
> > > > > > > > > > > > > > unexpected failures.
> > > > > > > > > > > > > > That's not acceptable.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > As a reviewer, you have to be ready to provide
> > > > arguments
> > > > > > why
> > > > > > > > > these
> > > > > > > > > > > tests
> > > > > > > > > > > > > > have to be fixed this way and what was the
> problem,
> > > in
> > > > > case
> > > > > > > you
> > > > > > > > > > > merged
> > > > > > > > > > > > > such
> > > > > > > > > > > > > > changes.
> > > > > > > > > > > > > > That's unacceptable to hide issues instead of
> fix.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Now, I ask you, as a reviewer, to provide the
> > > > > explanation.
> > > > > > > > > > > > > > What problem and at what test we solved by no-op
> > > > handler.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > And I'm going to rollback changes in case
> arguments
> > > > will
> > > > > > not
> > > > > > > be
> > > > > > > > > > > provided.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Wed, Dec 5, 2018 at 1:10 PM Dmitriy Pavlov <
> > > > > > > > > dpavlov@apache.org>
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > I will not do any rollback because changes make
> > > tests
> > > > > > > better.
> > > > > > > > > > > Please
> > > > > > > > > > > > > pay
> > > > > > > > > > > > > > > attention that no-op became default long time
> > ago.
> > > > > Please
> > > > > > > > > discuss
> > > > > > > > > > > this
> > > > > > > > > > > > > > > selection with authors of the previous commit.
> > New
> > > > > commit
> > > > > > > > > changes
> > > > > > > > > > > > > > > NoOp->FailTest+stopNode.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Please provide a PR to demonstrate your idea
> how
> > to
> > > > > > > transfer
> > > > > > > > > and
> > > > > > > > > > > handle
> > > > > > > > > > > > > > > exceptions. I believe it will not work because
> > the
> > > > fail
> > > > > > > > > handler is
> > > > > > > > > > > > > > > activated from any pool inside a node.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > ср, 5 дек. 2018 г. в 13:05, Anton Vinogradov <
> > > > > > > av@apache.org
> > > > > > > > >:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Dmitriy,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >> Which code block will do a throw?
> > > > > > > > > > > > > > > > Depends on the test.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Looks like we make the *bad *test even
> *worse*.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > That's not a correct fix.
> > > > > > > > > > > > > > > > In case you expect failure you have to check
> > this
> > > > > > > > expectation
> > > > > > > > > > > inside
> > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > special handler.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > I'd like to ask you to rollback these changes
> > and
> > > > > > replace
> > > > > > > > > them
> > > > > > > > > > > with
> > > > > > > > > > > > > > > correct
> > > > > > > > > > > > > > > > fixes.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Wed, Dec 5, 2018 at 12:39 PM Andrey
> > Mashenkov
> > > <
> > > > > > > > > > > > > > > > andrey.mashenkov@gmail.com>
> > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Hi,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Dmitri,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > The meaningful failure handler as a default
> > one
> > > > > looks
> > > > > > > > > > > reasonable.
> > > > > > > > > > > > > > > > > Thanks a lot.
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > But what is the reason to fallback to noop
> > for
> > > > 100+
> > > > > > > test?
> > > > > > > > > > > > > > > > > Does it means these test become failed
> after
> > > > > changing
> > > > > > > > > default
> > > > > > > > > > > > > failure
> > > > > > > > > > > > > > > > > handler?
> > > > > > > > > > > > > > > > > If so, let's create a ticket (may be
> > umbrella)
> > > to
> > > > > > > > > investigate
> > > > > > > > > > > and
> > > > > > > > > > > > > fix
> > > > > > > > > > > > > > > > this.
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > I see 100+ touched files in PR and some of
> > them
> > > > are
> > > > > > > > > abstract
> > > > > > > > > > > > > classes,
> > > > > > > > > > > > > > > so,
> > > > > > > > > > > > > > > > > we have much more affected tests.
> > > > > > > > > > > > > > > > > Seems, most of failover test doesn't
> expects
> > if
> > > > any
> > > > > > > > > critical
> > > > > > > > > > > > > internal
> > > > > > > > > > > > > > > > issue
> > > > > > > > > > > > > > > > > occur and there is no need to fallback to
> > noop.
> > > > > > > > > > > > > > > > > Other test should set custom failure
> handler
> > to
> > > > > > detect
> > > > > > > > > expected
> > > > > > > > > > > > > > > failures
> > > > > > > > > > > > > > > > or
> > > > > > > > > > > > > > > > > if grid hanging simulation is needed (to
> keep
> > > > > hanged
> > > > > > > grid
> > > > > > > > > under
> > > > > > > > > > > > > > > control).
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > On Wed, Dec 5, 2018 at 12:16 PM Anton
> > > Vinogradov
> > > > <
> > > > > > > > > > > av@apache.org>
> > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > Dmitrii,
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > No-op means "hide any problem", so, we
> lose
> > > the
> > > > > > > > > guarantees.
> > > > > > > > > > > > > > > > > > Could you please share some examples
> where
> > > > > "no-op"
> > > > > > > > better
> > > > > > > > > > > than
> > > > > > > > > > > > > > > "strict
> > > > > > > > > > > > > > > > > > try-catch with a check"?
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > On Wed, Dec 5, 2018 at 11:37 AM Dmitrii
> > > Ryabov
> > > > <
> > > > > > > > > > > > > > > somefireone@gmail.com>
> > > > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > Anton, I think wrapping every
> > disconnecting
> > > > > node
> > > > > > > with
> > > > > > > > > > > try-catch
> > > > > > > > > > > > > > > will
> > > > > > > > > > > > > > > > be
> > > > > > > > > > > > > > > > > > > less readable than no-op handler.
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > ср, 5 дек. 2018 г., 9:26 Dmitriy Pavlov
> > > > > > > > > dpavlov@apache.org
> > > > > > > > > > > :
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > Folks let me remind you that Dmitry
> > > changed
> > > > > > > default
> > > > > > > > > of
> > > > > > > > > > > ALL
> > > > > > > > > > > > > > tests
> > > > > > > > > > > > > > > > from
> > > > > > > > > > > > > > > > > > > noop
> > > > > > > > > > > > > > > > > > > > to a meaningful handler. So we should
> > > start
> > > > > > every
> > > > > > > > > message
> > > > > > > > > > > > > here
> > > > > > > > > > > > > > > from
> > > > > > > > > > > > > > > > > > > saying
> > > > > > > > > > > > > > > > > > > > thank you to Dmitry.
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > Please review remaining tests and
> > remove
> > > > noop
> > > > > > > where
> > > > > > > > > > > possible.
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > вт, 4 дек. 2018 г., 23:48 Andrey
> > > Mashenkov
> > > > <
> > > > > > > > > > > > > > > > > andrey.mashenkov@gmail.com
> > > > > > > > > > > > > > > > > > >:
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > Hi all,
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > Really, why noop?
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > If you expect failure handler
> should
> > be
> > > > > > > > triggered,
> > > > > > > > > you
> > > > > > > > > > > can
> > > > > > > > > > > > > > > > override
> > > > > > > > > > > > > > > > > > > > default
> > > > > > > > > > > > > > > > > > > > > one and rise some flag, which can
> be
> > > > > checked
> > > > > > in
> > > > > > > > > test.
> > > > > > > > > > > > > > > > > > > > > This will make test clearer.
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > With noop, you'll get previous
> > unwanted
> > > > > > > > behavior,
> > > > > > > > > > > that you
> > > > > > > > > > > > > > are
> > > > > > > > > > > > > > > > > > trying
> > > > > > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > > > > > improve, isnt'it?
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > 4 дек. 2018 г. 23:25 пользователь
> > > "Anton
> > > > > > > > > Vinogradov" <
> > > > > > > > > > > > > > > > > av@apache.org>
> > > > > > > > > > > > > > > > > > > > > написал:
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > And you have to check the reason of
> > > > failure
> > > > > > > > inside
> > > > > > > > > the
> > > > > > > > > > > > > > > try-catch
> > > > > > > > > > > > > > > > > > block,
> > > > > > > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > > > > > course.
> > > > > > > > > > > > > > > > > > > > > In case found not equals to
> expected
> > > then
> > > > > > test
> > > > > > > > > should
> > > > > > > > > > > > > rethrow
> > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > exception.
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > вт, 4 дек. 2018 г. в 23:21, Anton
> > > > > Vinogradov
> > > > > > <
> > > > > > > > > > > > > av@apache.org
> > > > > > > > > > > > > > >:
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > Dmitrii,
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > The solution is not clear to me.
> > > > > > > > > > > > > > > > > > > > > > In case you expect the failure
> > then a
> > > > > > correct
> > > > > > > > > case
> > > > > > > > > > > is to
> > > > > > > > > > > > > > wrap
> > > > > > > > > > > > > > > > it
> > > > > > > > > > > > > > > > > > with
> > > > > > > > > > > > > > > > > > > > > > try-catch block instead of no-op
> > > > failure
> > > > > > > > handler
> > > > > > > > > > > usage.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > вт, 4 дек. 2018 г. в 21:41,
> Dmitrii
> > > > > Ryabov
> > > > > > <
> > > > > > > > > > > > > > > > > somefireone@gmail.com
> > > > > > > > > > > > > > > > > > >:
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >> Anton,
> > > > > > > > > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > > > > > > > > >> Tests in these classes check
> fail
> > > > cases
> > > > > > when
> > > > > > > > we
> > > > > > > > > > > expect
> > > > > > > > > > > > > > > > critical
> > > > > > > > > > > > > > > > > > > > > >> failure like node stop or
> > exception
> > > > > > thrown.
> > > > > > > > Such
> > > > > > > > > > > tests
> > > > > > > > > > > > > > > trigger
> > > > > > > > > > > > > > > > > > > failure
> > > > > > > > > > > > > > > > > > > > > >> handler and it fails test when
> > > > > everything
> > > > > > > goes
> > > > > > > > > as it
> > > > > > > > > > > > > > should
> > > > > > > > > > > > > > > > go.
> > > > > > > > > > > > > > > > > > > That's
> > > > > > > > > > > > > > > > > > > > > >> why we need no-op handler here.
> > > > > > > > > > > > > > > > > > > > > >> вт, 4 дек. 2018 г. в 20:06,
> > Dmitriy
> > > > > > Pavlov <
> > > > > > > > > > > > > > > > dpavlov@apache.org
> > > > > > > > > > > > > > > > > >:
> > > > > > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > > > > > >> > Hi Igniters,
> > > > > > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > > > > > >> > BTW, if you find in any of
> your
> > > > tests
> > > > > it
> > > > > > > > > does't
> > > > > > > > > > > need
> > > > > > > > > > > > > an
> > > > > > > > > > > > > > > old
> > > > > > > > > > > > > > > > > > value
> > > > > > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > > > > > >> > handler (=NoOp), feel free to
> > > remove
> > > > > it.
> > > > > > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > > > > > >> > Sincerely,
> > > > > > > > > > > > > > > > > > > > > >> > Dmitriy Pavlov
> > > > > > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > > > > > >> > вт, 4 дек. 2018 г. в 20:02,
> > Anton
> > > > > > > > Vinogradov <
> > > > > > > > > > > > > > > av@apache.org
> > > > > > > > > > > > > > > > >:
> > > > > > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > > > > > >> > > Dmitrii,
> > > > > > > > > > > > > > > > > > > > > >> > >
> > > > > > > > > > > > > > > > > > > > > >> > > Could you please explain the
> > > > reason
> > > > > of
> > > > > > > > > explicit
> > > > > > > > > > > set
> > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > 100+
> > > > > > > > > > > > > > > > > > > > > >> > > NoOpFailureHandlers?
> > > > > > > > > > > > > > > > > > > > > >> > >
> > > > > > > > > > > > > > > > > > > > > >> > >
> > > > > > > > > > > > > > > > > > > > > >> > > вт, 4 дек. 2018 г. в 19:12,
> > > > Dmitrii
> > > > > > > > Ryabov <
> > > > > > > > > > > > > > > > > > > somefireone@gmail.com
> > > > > > > > > > > > > > > > > > > > >:
> > > > > > > > > > > > > > > > > > > > > >> > >
> > > > > > > > > > > > > > > > > > > > > >> > > > Hello, Igniters!
> > > > > > > > > > > > > > > > > > > > > >> > > >
> > > > > > > > > > > > > > > > > > > > > >> > > > Today the test framework's
> > > > default
> > > > > > > no-op
> > > > > > > > > > > failure
> > > > > > > > > > > > > > > handler
> > > > > > > > > > > > > > > > > was
> > > > > > > > > > > > > > > > > > > > > >> changed to
> > > > > > > > > > > > > > > > > > > > > >> > > the
> > > > > > > > > > > > > > > > > > > > > >> > > > handler, which stops the
> > node
> > > > and
> > > > > > > fails
> > > > > > > > > the
> > > > > > > > > > > test.
> > > > > > > > > > > > > > > > > > > > > >> > > >
> > > > > > > > > > > > > > > > > > > > > >> > > > Over 100 tests kept no-op
> > > > failure
> > > > > > > > handler
> > > > > > > > > by
> > > > > > > > > > > > > > overrided
> > > > > > > > > > > > > > > > > > > > > >> > > > `getFailureHandler()`
> > method.
> > > > > > > > > > > > > > > > > > > > > >> > > >
> > > > > > > > > > > > > > > > > > > > > >> > > > If you'll found a problem
> or
> > > > > > something
> > > > > > > > > > > unexpected
> > > > > > > > > > > > > -
> > > > > > > > > > > > > > > > write
> > > > > > > > > > > > > > > > > > here
> > > > > > > > > > > > > > > > > > > > or
> > > > > > > > > > > > > > > > > > > > > >> in the
> > > > > > > > > > > > > > > > > > > > > >> > > > ticket [1].
> > > > > > > > > > > > > > > > > > > > > >> > > >
> > > > > > > > > > > > > > > > > > > > > >> > > > [1]
> > > > > > > > > > > > > >
> https://issues.apache.org/jira/browse/IGNITE-8227
> > > > > > > > > > > > > > > > > > > > > >> > > >
> > > > > > > > > > > > > > > > > > > > > >> > >
> > > > > > > > > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > --
> > > > > > > > > > > > > > > > > Best regards,
> > > > > > > > > > > > > > > > > Andrey V. Mashenkov
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > --
> > > > > > > > > > > Best regards,
> > > > > > > > > > > Ivan Pavlukhin
> > > > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Best regards,
> > > > > > > > > Ivan Pavlukhin
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > Best regards,
> > > > Andrey V. Mashenkov
> > > >
> > >
> >
>
>
> --
> Best regards,
>   Andrey Kuznetsov.
>

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