Nikolay,

Answering your questions a couple of emails ago.

Only one valid reason to avoid NoOp it the risk
- we don't correctly understand test meaning by class name, we don't catch
it's expected flow and
- there is some test which uses NoOp now, but should not.

Any failure in such test included to set of NoOp handlers test can be
hidden and the test will pass for whatever reason, e.g. poor validation of
expected behavior. So suggested analog to this is muted failures, which
runs on TC but don't signal us that something is wrong.

Sincerely,
Dmitriy Pavlov

чт, 6 дек. 2018 г. в 13:53, Dmitriy Pavlov <dpav...@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 <a...@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 Павлухин Иван <vololo...@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 <somefire...@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 a...@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 Павлухин Иван <vololo...@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 <nizhi...@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 dpav...@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 <
>> nizhi...@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 <
>> > nizhi...@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 <
>> > > > nizhi...@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 <
>> > > > dpav...@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 <
>> > > > > nizhi...@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 <
>> > > > > dpav...@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 <
>> > > > > > > > stku...@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 <
>> > > > > > > > nizhi...@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 <
>> > > > > > > > a...@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.mashen...@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 <
>> > > > > > > > > > > >
>> > > > > > > > > > > > a...@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 <
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > dpav...@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
>> > > > <
>> > > > > > > > a...@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.shangar...@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
>> > Павлухин
>> > > > > Иван
>> > > > > > > <
>> > > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > > vololo...@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
>> > > > > > > <
>> > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > dpav...@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,
>> > > > Павлухин
>> > > > > > > Иван <
>> > > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > > vololo...@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 <
>> > > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > > a...@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
>> > > > > > > > > > > >
>> > > > > > > > > > > > <
>> > > > > > > > > > > > > > > > > > > > > dpav...@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 <
>> > > > > > > > > > > > > > > > > > > a...@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 <
>> > > > > > > > > > > > > > > > > > > > > > > dpav...@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
>> > > > > > > > > > > > > > <
>> > > > > > > > > > > > > > > > > > > > > a...@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.mashen...@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?
>> > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
>
>

Reply via email to