ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nikolay Izhikov <nizhi...@apache.org>
Subject Re: Default failure handler was changed for tests
Date Wed, 05 Dec 2018 16:43:33 GMT
Dmitriy.

I think we should avoid copy paste code instead of thinking about Apache
Way all the time :)

Anyway, I propose to return to the code!
I think we should use some kind of marker base class for a cases with
NoOpHandler.
This has several advantages, comparing with current implementation:

1. No copy paste code
2. Reduce changes.
3. All usages of NoOpHandler can be easily found with IDE or grep search.

I've prepared proof of concept pull request to demonstrate my approach [1]
I can go further and prepare full fix.

What do you think?

[1] https://github.com/apache/ignite/pull/5584/files

ср, 5 дек. 2018 г. в 18:29, Dmitriy Pavlov <dpavlov@apache.org>:

> 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