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? >> > > > > > > > > > > > > > > > > > > > If so, let's create a ticket (may be >> > > > > umbrella) >> > > > > > to >> > > > > > > > > > > > investigate >> > > > > > > > > > > > > > and >> > > > > > > > > > > > > > > > fix >> > > > > > > > > > > > > > > > > > > this. >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > I see 100+ touched files in PR and >> some >> > > of >> > > > > them >> > > > > > > are >> > > > > > > > > > > > abstract >> > > > > > > > > > > > > > > > classes, >> > > > > > > > > > > > > > > > > > so, >> > > > > > > > > > > > > > > > > > > > we have much more affected tests. >> > > > > > > > > > > > > > > > > > > > Seems, most of failover test doesn't >> > > > expects >> > > > > if >> > > > > > > any >> > > > > > > > > > > > critical >> > > > > > > > > > > > > > > > internal >> > > > > > > > > > > > > > > > > > > issue >> > > > > > > > > > > > > > > > > > > > occur and there is no need to >> fallback >> > to >> > > > > noop. >> > > > > > > > > > > > > > > > > > > > Other test should set custom failure >> > > > handler >> > > > > to >> > > > > > > > > detect >> > > > > > > > > > > > expected >> > > > > > > > > > > > > > > > > > failures >> > > > > > > > > > > > > > > > > > > or >> > > > > > > > > > > > > > > > > > > > if grid hanging simulation is needed >> > (to >> > > > keep >> > > > > > > > hanged >> > > > > > > > > > grid >> > > > > > > > > > > > under >> > > > > > > > > > > > > > > > > > control). >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > On Wed, Dec 5, 2018 at 12:16 PM >> Anton >> > > > > > Vinogradov >> > > > > > > < >> > > > > > > > > > > > > > a...@apache.org> >> > > > > > > > > > > > > > > > > > wrote: >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > Dmitrii, >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > No-op means "hide any problem", >> so, >> > we >> > > > lose >> > > > > > the >> > > > > > > > > > > > guarantees. >> > > > > > > > > > > > > > > > > > > > > Could you please share some >> examples >> > > > where >> > > > > > > > "no-op" >> > > > > > > > > > > better >> > > > > > > > > > > > > > than >> > > > > > > > > > > > > > > > > > "strict >> > > > > > > > > > > > > > > > > > > > > try-catch with a check"? >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > On Wed, Dec 5, 2018 at 11:37 AM >> > Dmitrii >> > > > > > Ryabov >> > > > > > > < >> > > > > > > > > > > > > > > > > > somefire...@gmail.com> >> > > > > > > > > > > > > > > > > > > > > wrote: >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > Anton, I think wrapping every >> > > > > disconnecting >> > > > > > > > node >> > > > > > > > > > with >> > > > > > > > > > > > > > try-catch >> > > > > > > > > > > > > > > > > > will >> > > > > > > > > > > > > > > > > > > be >> > > > > > > > > > > > > > > > > > > > > > less readable than no-op >> handler. >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > ср, 5 дек. 2018 г., 9:26 Dmitriy >> > > Pavlov >> > > > > > > > > > > > dpav...@apache.org >> > > > > > > > > > > > > > : >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > Folks let me remind you that >> > Dmitry >> > > > > > changed >> > > > > > > > > > default >> > > > > > > > > > > > of >> > > > > > > > > > > > > > ALL >> > > > > > > > > > > > > > > > > tests >> > > > > > > > > > > > > > > > > > > from >> > > > > > > > > > > > > > > > > > > > > > noop >> > > > > > > > > > > > > > > > > > > > > > > to a meaningful handler. So we >> > > should >> > > > > > start >> > > > > > > > > every >> > > > > > > > > > > > message >> > > > > > > > > > > > > > > > here >> > > > > > > > > > > > > > > > > > from >> > > > > > > > > > > > > > > > > > > > > > saying >> > > > > > > > > > > > > > > > > > > > > > > thank you to Dmitry. >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > Please review remaining tests >> and >> > > > > remove >> > > > > > > noop >> > > > > > > > > > where >> > > > > > > > > > > > > > possible. >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > вт, 4 дек. 2018 г., 23:48 >> Andrey >> > > > > > Mashenkov >> > > > > > > < >> > > > > > > > > > > > > > > > > > > > andrey.mashen...@gmail.com >> > > > > > > > > > > > > > > > > > > > > >: >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > Hi all, >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > Really, why noop? >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > If you expect failure >> handler >> > > > should >> > > > > be >> > > > > > > > > > > triggered, >> > > > > > > > > > > > you >> > > > > > > > > > > > > > can >> > > > > > > > > > > > > > > > > > > override >> > > > > > > > > > > > > > > > > > > > > > > default >> > > > > > > > > > > > > > > > > > > > > > > > one and rise some flag, >> which >> > can >> > > > be >> > > > > > > > checked >> > > > > > > > > in >> > > > > > > > > > > > test. >> > > > > > > > > > > > > > > > > > > > > > > > This will make test clearer. >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > With noop, you'll get >> previous >> > > > > unwanted >> > > > > > > > > > > behavior, >> > > > > > > > > > > > > > that you >> > > > > > > > > > > > > > > > > are >> > > > > > > > > > > > > > > > > > > > > trying >> > > > > > > > > > > > > > > > > > > > > > to >> > > > > > > > > > > > > > > > > > > > > > > > improve, isnt'it? >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > 4 дек. 2018 г. 23:25 >> > пользователь >> > > > > > "Anton >> > > > > > > > > > > > Vinogradov" < >> > > > > > > > > > > > > > > > > > > > a...@apache.org> >> > > > > > > > > > > > > > > > > > > > > > > > написал: >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > And you have to check the >> > reason >> > > of >> > > > > > > failure >> > > > > > > > > > > inside >> > > > > > > > > > > > the >> > > > > > > > > > > > > > > > > > try-catch >> > > > > > > > > > > > > > > > > > > > > block, >> > > > > > > > > > > > > > > > > > > > > > > of >> > > > > > > > > > > > > > > > > > > > > > > > course. >> > > > > > > > > > > > > > > > > > > > > > > > In case found not equals to >> > > > expected >> > > > > > then >> > > > > > > > > test >> > > > > > > > > > > > should >> > > > > > > > > > > > > > > > rethrow >> > > > > > > > > > > > > > > > > > the >> > > > > > > > > > > > > > > > > > > > > > > > exception. >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > вт, 4 дек. 2018 г. в 23:21, >> > Anton >> > > > > > > > Vinogradov >> > > > > > > > > < >> > > > > > > > > > > > > > > > a...@apache.org >> > > > > > > > > > > > > > > > > >: >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > Dmitrii, >> > > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > The solution is not clear >> to >> > > me. >> > > > > > > > > > > > > > > > > > > > > > > > > In case you expect the >> > failure >> > > > > then a >> > > > > > > > > correct >> > > > > > > > > > > > case >> > > > > > > > > > > > > > is to >> > > > > > > > > > > > > > > > > wrap >> > > > > > > > > > > > > > > > > > > it >> > > > > > > > > > > > > > > > > > > > > with >> > > > > > > > > > > > > > > > > > > > > > > > > try-catch block instead of >> > > no-op >> > > > > > > failure >> > > > > > > > > > > handler >> > > > > > > > > > > > > > usage. >> > > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > вт, 4 дек. 2018 г. в >> 21:41, >> > > > Dmitrii >> > > > > > > > Ryabov >> > > > > > > > > < >> > > > > > > > > > > > > > > > > > > > somefire...@gmail.com >> > > > > > > > > > > > > > > > > > > > > >: >> > > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> Anton, >> > > > > > > > > > > > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > > > > > > > > > > > >> Tests in these classes >> check >> > > > fail >> > > > > > > cases >> > > > > > > > > when >> > > > > > > > > > > we >> > > > > > > > > > > > > > expect >> > > > > > > > > > > > > > > > > > > critical >> > > > > > > > > > > > > > > > > > > > > > > > >> failure like node stop or >> > > > > exception >> > > > > > > > > thrown. >> > > > > > > > > > > Such >> > > > > > > > > > > > > > tests >> > > > > > > > > > > > > > > > > > trigger >> > > > > > > > > > > > > > > > > > > > > > failure >> > > > > > > > > > > > > > > > > > > > > > > > >> handler and it fails test >> > when >> > > > > > > > everything >> > > > > > > > > > goes >> > > > > > > > > > > > as it >> > > > > > > > > > > > > > > > > should >> > > > > > > > > > > > > > > > > > > go. >> > > > > > > > > > > > > > > > > > > > > > That's >> > > > > > > > > > > > > > > > > > > > > > > > >> why we need no-op handler >> > > here. >> > > > > > > > > > > > > > > > > > > > > > > > >> вт, 4 дек. 2018 г. в >> 20:06, >> > > > > Dmitriy >> > > > > > > > > Pavlov < >> > > > > > > > > > > > > > > > > > > dpav...@apache.org >> > > > > > > > > > > > > > > > > > > > >: >> > > > > > > > > > > > > > > > > > > > > > > > >> > >> > > > > > > > > > > > > > > > > > > > > > > > >> > Hi Igniters, >> > > > > > > > > > > > > > > > > > > > > > > > >> > >> > > > > > > > > > > > > > > > > > > > > > > > >> > BTW, if you find in >> any of >> > > > your >> > > > > > > tests >> > > > > > > > it >> > > > > > > > > > > > does't >> > > > > > > > > > > > > > need >> > > > > > > > > > > > > > > > an >> > > > > > > > > > > > > > > > > > old >> > > > > > > > > > > > > > > > > > > > > value >> > > > > > > > > > > > > > > > > > > > > > of >> > > > > > > > > > > > > > > > > > > > > > > > >> > handler (=NoOp), feel >> free >> > > to >> > > > > > remove >> > > > > > > > it. >> > > > > > > > > > > > > > > > > > > > > > > > >> > >> > > > > > > > > > > > > > > > > > > > > > > > >> > Sincerely, >> > > > > > > > > > > > > > > > > > > > > > > > >> > Dmitriy Pavlov >> > > > > > > > > > > > > > > > > > > > > > > > >> > >> > > > > > > > > > > > > > > > > > > > > > > > >> > вт, 4 дек. 2018 г. в >> > 20:02, >> > > > > Anton >> > > > > > > > > > > Vinogradov < >> > > > > > > > > > > > > > > > > > a...@apache.org >> > > > > > > > > > > > > > > > > > > >: >> > > > > > > > > > > > > > > > > > > > > > > > >> > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > Dmitrii, >> > > > > > > > > > > > > > > > > > > > > > > > >> > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > Could you please >> explain >> > > the >> > > > > > > reason >> > > > > > > > of >> > > > > > > > > > > > explicit >> > > > > > > > > > > > > > set >> > > > > > > > > > > > > > > > of >> > > > > > > > > > > > > > > > > > > 100+ >> > > > > > > > > > > > > > > > > > > > > > > > >> > > NoOpFailureHandlers? >> > > > > > > > > > > > > > > > > > > > > > > > >> > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > вт, 4 дек. 2018 г. в >> > > 19:12, >> > > > > > > Dmitrii >> > > > > > > > > > > Ryabov < >> > > > > > > > > > > > > > > > > > > > > > somefire...@gmail.com >> > > > > > > > > > > > > > > > > > > > > > > >: >> > > > > > > > > > > > > > > > > > > > > > > > >> > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > Hello, Igniters! >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > Today the test >> > > framework's >> > > > > > > default >> > > > > > > > > > no-op >> > > > > > > > > > > > > > failure >> > > > > > > > > > > > > > > > > > handler >> > > > > > > > > > > > > > > > > > > > was >> > > > > > > > > > > > > > > > > > > > > > > > >> changed to >> > > > > > > > > > > > > > > > > > > > > > > > >> > > the >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > handler, which >> stops >> > the >> > > > > node >> > > > > > > and >> > > > > > > > > > fails >> > > > > > > > > > > > the >> > > > > > > > > > > > > > test. >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > Over 100 tests kept >> > > no-op >> > > > > > > failure >> > > > > > > > > > > handler >> > > > > > > > > > > > by >> > > > > > > > > > > > > > > > > overrided >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > >> `getFailureHandler()` >> > > > > method. >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > If you'll found a >> > > problem >> > > > or >> > > > > > > > > something >> > > > > > > > > > > > > > unexpected >> > > > > > > > > > > > > > > > - >> > > > > > > > > > > > > > > > > > > write >> > > > > > > > > > > > > > > > > > > > > here >> > > > > > > > > > > > > > > > > > > > > > > or >> > > > > > > > > > > > > > > > > > > > > > > > >> in the >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > ticket [1]. >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > [1] >> > > > > > > > > > > > > > > > > >> > > > https://issues.apache.org/jira/browse/IGNITE-8227 >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > >> > > > > > > > > > > > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > -- >> > > > > > > > > > > > > > > > > > > > Best regards, >> > > > > > > > > > > > > > > > > > > > Andrey V. Mashenkov >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > > -- >> > > > > > > > > > > > > > Best regards, >> > > > > > > > > > > > > > Ivan Pavlukhin >> > > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > -- >> > > > > > > > > > > > Best regards, >> > > > > > > > > > > > Ivan Pavlukhin >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > -- >> > > > > > > Best regards, >> > > > > > > Andrey V. Mashenkov >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > >> > > > -- >> > > > Best regards, >> > > > Andrey Kuznetsov. >> > > > >> > > >> > >> >
