ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitrii Ryabov <somefire...@gmail.com>
Subject Re: Default failure handler was changed for tests
Date Thu, 06 Dec 2018 11:05:10 GMT
Anton, I mean `copy-paste reduce` ticket. I'll try to describe reasons for
no-op in tests. Then, we can create tickets to fix this cases if needed.

чт, 6 дек. 2018 г., 13:53 Dmitriy Pavlov dpavlov@apache.org:

> BTW, No-Op or StopNode-FailTest in case of a deep investigation will always
> require to understand what test does and what it tests.
>
> So we can get a positive outcome from this research if we agree to add
> - a small description to each test about the reason for existing of this
> test,
> - what is the expected behavior of the product in the test, and how it is
> checked?
> - failure handler influence, etc.
>
> I still hope Anton will do the first bunch of tests research to demonstrate
> the idea.
>
> чт, 6 дек. 2018 г. в 13:39, Anton Vinogradov <av@apache.org>:
>
> > Dmitrii,
> >
> > >> I agree with Nikolay's solution. If no one minds, I'll create ticket
> for
> > >> appropriate changes and recheck issues.
> > Do you mean 'copy-paste reduce' ticket or check/fix of all tests with
> no-op
> > to have a proper handler?
> >
> > Just want to make sure that copy-paste minimization is not the final
> step.
> >
> > On Thu, Dec 6, 2018 at 1:24 PM Павлухин Иван <vololo100@gmail.com>
> wrote:
> >
> > > Dmitrii Ryabov,
> > >
> > > Your comments sounds reasonable to me. Marker base class approach
> > > looks good to me so far.
> > >
> > > P.S. I had even worse name in mind 'StopGaps' =)
> > > чт, 6 дек. 2018 г. в 13:08, Dmitrii Ryabov <somefireone@gmail.com>:
> > > >
> > > > Ivan, I think `Workarounds` class isn't good idea, because it looks
> > like
> > > we
> > > > create stable workarounds, which will never be fixed.
> > > >
> > > > I agree with Nikolay's solution. If no one minds, I'll create ticket
> > for
> > > > appropriate changes and recheck issues.
> > > >
> > > > чт, 6 дек. 2018 г., 12:17 Anton Vinogradov av@apache.org:
> > > >
> > > > > Folks, thank's everyone for solution research.
> > > > > I'm ok with Nikolay approach in case that's not a final step.
> > > > >
> > > > > On Thu, Dec 6, 2018 at 12:11 PM Павлухин Иван <vololo100@gmail.com
> >
> > > wrote:
> > > > >
> > > > > > Nikolay,
> > > > > >
> > > > > > I meant "not expensive" by "cheap". And I meant that it is good
> > that
> > > > > > it cheap =). And I said it to contrast with "expensive" ~100
> tests
> > > > > > investigation. And if we agree (mostly I would like an opinion
> from
> > > > > > Dmitriy Ryabov as an original author) on a way how to improve the
> > > > > > patch then let's do it.
> > > > > > чт, 6 дек. 2018 г. в 10:41, Nikolay Izhikov <nizhikov@apache.org
> >:
> > > > > > >
> > > > > > > Dmitriy Ryabov, Dmitriy Pavlov, sorry.
> > > > > > >
> > > > > > > Of course it should be "NOT to blame author".
> > > > > > >
> > > > > > > Sorry, one more time.
> > > > > > >
> > > > > > > чт, 6 дек. 2018 г., 10:40 Dmitriy Pavlov dpavlov@apache.org:
> > > > > > >
> > > > > > > > I hope you've misprinted here
> > > > > > > > > I'm here to blame the author.
> > > > > > > >
> > > > > > > > We can blame code but never coders.
> > > > > > > >
> > > > > > > > Please see https://discourse.pi-hole.net/faq - has
> absolutely
> > > > > nothing
> > > > > > in
> > > > > > > > common with Apache Guides, but says the same things. It is a
> > > > > practical
> > > > > > > > necessity to maintain a friendly atmosphere.
> > > > > > > >
> > > > > > > > чт, 6 дек. 2018 г. в 10:31, Nikolay Izhikov <
> > nizhikov@apache.org
> > > >:
> > > > > > > >
> > > > > > > > > Ivan.
> > > > > > > > >
> > > > > > > > > > 1. Accept the patch and bring an improvement to Ignite
> (and
> > > > > create
> > > > > > a>
> > > > > > > > > ticket for further investigation).
> > > > > > > > >
> > > > > > > > > I support this idea.
> > > > > > > > > Do we create the tickets already?
> > > > > > > > >
> > > > > > > > > > Nikolay's patch [1] suggests a slightly different
> approach
> > > how to
> > > > > > the
> > > > > > > > > > same thing. And implementing that idea looks like a cheap
> > > > > > refactoring.
> > > > > > > > >
> > > > > > > > > I don't agree with your term "cheap".
> > > > > > > > > Do you think reducing copy paste code not worth it?
> > > > > > > > >
> > > > > > > > > I see a hundreds issues that bring copypasted code in the
> > > > > > product(Ignite
> > > > > > > > > and others).
> > > > > > > > > I insist, that we shouldn't accept patches with it.
> > > > > > > > >
> > > > > > > > > I'm here to blame the author.
> > > > > > > > > I want to improve this patch and make it easier to find all
> > > places
> > > > > > with
> > > > > > > > > NoOp handler to do the further investigation.
> > > > > > > > >
> > > > > > > > > В Чт, 06/12/2018 в 10:19 +0300, Павлухин Иван пишет:
> > > > > > > > > > Guys,
> > > > > > > > > >
> > > > > > > > > > I asked what harm will applying the patch bring I have
> not
> > > got a
> > > > > > > > > > direct answer. But I think I got some pain points:
> > > > > > > > > > 1. Anton does not like that reasons why ~100 tests
> require
> > > noop
> > > > > > > > > > handler are not clear. And might be several problems are
> > > covered
> > > > > > > > > > there.
> > > > > > > > > > 2. Nikolay suggests some code improvements.
> > > > > > > > > >
> > > > > > > > > > Nikolay's patch [1] suggests a slightly different
> approach
> > > how to
> > > > > > the
> > > > > > > > > > same thing. And implementing that idea looks like a cheap
> > > > > > refactoring.
> > > > > > > > > > But the idea of course could be discussed. Straight away
> I
> > > can
> > > > > > suggest
> > > > > > > > > > another slightly different trick [2].
> > > > > > > > > >
> > > > > > > > > > Investigating why ~100 tests require noop handler could
> be
> > > > > costly.
> > > > > > So,
> > > > > > > > > > in that direction I see following options which can
> happen
> > > for
> > > > > > sure:
> > > > > > > > > > 1. Accept the patch and bring an improvement to Ignite
> (and
> > > > > create
> > > > > > a
> > > > > > > > > > ticket for further investigation).
> > > > > > > > > > 2. Revert the patch and loose an improvement.
> > > > > > > > > >
> > > > > > > > > > One might say that there is an option "Revert the patch
> and
> > > then
> > > > > > do it
> > > > > > > > > > better" but I does not see anything (anyone) what can
> > > guarantee
> > > > > it.
> > > > > > > > > > So, I personally prefer an option 1 against 2 because I
> > > believe
> > > > > > that
> > > > > > > > > > it is good if the system "can make a progress".
> > > > > > > > > >
> > > > > > > > > > [1] https://github.com/apache/ignite/pull/5584/files
> > > > > > > > > > [2] https://github.com/apache/ignite/pull/5586/files
> > > > > > > > > > ср, 5 дек. 2018 г. в 21:22, Nikolay Izhikov <
> > > nizhikov@apache.org
> > > > > >:
> > > > > > > > > > >
> > > > > > > > > > > Dmitriy.
> > > > > > > > > > >
> > > > > > > > > > > > The closest analog to Noop handler is mute of test
> > > failure.
> > > > > > > > > > > > By this commit, we had unmuted (possible) failures in
> > > > > > > > > ~50000-~100=~49900
> > > > > > > > > > >
> > > > > > > > > > > tests, and we’re still concerned about style or minor
> > > details
> > > > > if
> > > > > > > > no-op
> > > > > > > > > was
> > > > > > > > > > > copy-pasted, aren’t we?
> > > > > > > > > > >
> > > > > > > > > > > Can you explain this idea a bit more?
> > > > > > > > > > > I don't understand what is unmuted by discussed commit.
> > > > > > > > > > >
> > > > > > > > > > > ср, 5 дек. 2018 г. в 20:40, Nikolay Izhikov <
> > > > > nizhikov@apache.org
> > > > > > >:
> > > > > > > > > > >
> > > > > > > > > > > > > Thanks, as an improvement to the code, this may be
> > > better.
> > > > > > > > > > > >
> > > > > > > > > > > > I can prepare a full patch for NoOp handler.
> > > > > > > > > > > > What do you think?
> > > > > > > > > > > >
> > > > > > > > > > > > Anton Vinogradov, do you agree with this approach?
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > ср, 5 дек. 2018 г. в 20:33, Dmitriy Pavlov <
> > > > > dpavlov@apache.org
> > > > > > >:
> > > > > > > > > > > >
> > > > > > > > > > > > > Thanks, as an improvement to the code, this may be
> > > better.
> > > > > > But
> > > > > > > > > still, it
> > > > > > > > > > > > > is
> > > > > > > > > > > > > not a reason to revert. And Anton mentioned
> something
> > > with
> > > > > > better
> > > > > > > > > > > > > exception
> > > > > > > > > > > > > handling/logging. Probably we will see an
> > > implementation as
> > > > > > well.
> > > > > > > > > > > > >
> > > > > > > > > > > > > This case here is a big thing related to The Apache
> > > Way, -
> > > > > > and
> > > > > > > > I'll
> > > > > > > > > > > > > explain
> > > > > > > > > > > > > why it makes me switched into fight-mode - until we
> > > stop
> > > > > this
> > > > > > > > > nonsense. If
> > > > > > > > > > > > > PMCs (at least) are aware of patterns and
> > > anti-patterns in
> > > > > > the
> > > > > > > > > community,
> > > > > > > > > > > > > we will succeed as a project much more as with
> (only)
> > > > > perfect
> > > > > > > > code.
> > > > > > > > > > > > >
> > > > > > > > > > > > > The closest analog to Noop handler is mute of test
> > > failure.
> > > > > > By
> > > > > > > > this
> > > > > > > > > > > > > commit,
> > > > > > > > > > > > > we had unmuted (possible) failures in
> > > ~50000-~100=~49900
> > > > > > tests,
> > > > > > > > > and we’re
> > > > > > > > > > > > > still concerned about style or minor details if
> no-op
> > > was
> > > > > > > > > copy-pasted,
> > > > > > > > > > > > > aren’t we?
> > > > > > > > > > > > >
> > > > > > > > > > > > > To everyone arguing about the number of tests we
> are
> > > > > allowed
> > > > > > to
> > > > > > > > > have with
> > > > > > > > > > > > > no-op: please visit this page
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > > >
> > >
> >
> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&tab=mutedProblems&branch_IgniteTests24Java8=__all_branches__
> > > > > > > > > > > > >
> > > > > > > > > > > > > It says: Muted tests: 3154. Are there any
> > disagreements
> > > > > > here? Why
> > > > > > > > > there
> > > > > > > > > > > > > are
> > > > > > > > > > > > > no insistent disagreement/not happy PMCs with
> > > absolutely
> > > > > > > > > unconditionally
> > > > > > > > > > > > > muted failures?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Any reason now to continue the discussion about
> > > reverting
> > > > > > > > > absolutely
> > > > > > > > > > > > > positive contribution into product stability from
> > > Dmitrii
> > > > > R.?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Moreover, Dmitrii Ryabov is trying to solve odd
> mutes
> > > > > > problem, as
> > > > > > > > > well, to
> > > > > > > > > > > > > locate mutes with links resolved issues in the TC
> > Bot.
> > > Is
> > > > > he
> > > > > > > > > deserved to
> > > > > > > > > > > > > read denouncing comments about the contribution? I
> > > guess,
> > > > > no,
> > > > > > > > > especially
> > > > > > > > > > > > > if
> > > > > > > > > > > > > the commenter is not going to help/contribute a
> > better
> > > fix.
> > > > > > > > > > > > >
> > > > > > > > > > > > > This is now a paramount thing for me if people in
> > this
> > > > > thread
> > > > > > > > will
> > > > > > > > > join
> > > > > > > > > > > > > the
> > > > > > > > > > > > > process or not. People may be not happy with some
> > > > > > > > > decisions/code/style,
> > > > > > > > > > > > > and
> > > > > > > > > > > > > some people are more often unhappy than others.
> More
> > > you
> > > > > > > > > contribute,- more
> > > > > > > > > > > > > you can decide. If you don't contribute at all - I
> > > don't
> > > > > > care too
> > > > > > > > > much
> > > > > > > > > > > > > about just opinions, I can accept facts. To provide
> > > facts
> > > > > we
> > > > > > need
> > > > > > > > > to do
> > > > > > > > > > > > > deep research, how can someone know if the test
> > should
> > > be
> > > > > > no-op
> > > > > > > > or
> > > > > > > > > not
> > > > > > > > > > > > > without deep analysis?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Again, if someone comes to list and provide just
> > > negative
> > > > > > > > > feedback, people
> > > > > > > > > > > > > will stop writing here. Probably no-op was enabled
> > > without
> > > > > > proper
> > > > > > > > > > > > > discussion because of this, someone may be afraid
> of
> > > > > sharing
> > > > > > > > this.
> > > > > > > > > Result:
> > > > > > > > > > > > > some of us knew it only now.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Do you need to make Ignite quite toxic place to
> have
> > an
> > > > > > > > absolutely
> > > > > > > > > perfect
> > > > > > > > > > > > > code with just a few of arguing-resistant
> > > contributors? I
> > > > > > believe
> > > > > > > > > not, and
> > > > > > > > > > > > > you don't need to be reminded 'community first
> > > principle'.
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > ср, 5 дек. 2018 г. в 19:43, Nikolay Izhikov <
> > > > > > nizhikov@apache.org
> > > > > > > > >:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > 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
> > > > > > > > > > > > > > > >

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