ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrey Mashenkov <andrey.mashen...@gmail.com>
Subject Re: Default failure handler was changed for tests
Date Wed, 05 Dec 2018 09:38:43 GMT
Hi,

Dmitri,

The meaningful failure handler as a default one looks reasonable.
Thanks a lot.

But what is the reason to fallback to noop for 100+ test?
Does it means these test become failed after changing default failure
handler?
If so, let's create a ticket (may be umbrella) to investigate and fix this.

I see 100+ touched files in PR and some of them are abstract classes, so,
we have much more affected tests.
Seems, most of failover test doesn't expects if any critical internal issue
occur and there is no need to fallback to noop.
Other test should set custom failure handler to detect expected failures or
if grid hanging simulation is needed (to keep hanged grid under control).

On Wed, Dec 5, 2018 at 12:16 PM Anton Vinogradov <av@apache.org> wrote:

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


-- 
Best regards,
Andrey V. Mashenkov

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