ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pavel Pereslegin <xxt...@gmail.com>
Subject Re: Unreliable checks in tests for string presence in GridStringLogger contents
Date Fri, 26 Oct 2018 10:01:04 GMT
Nikita,
personally, I don’t like that "check()" throws an AssertionError, but in
the case of a composite listener, it will indicate which of the conditions
did not work.
Btw, your case can be solved with custom listener, but I think it's good
improvement, let's do it.

чт, 25 окт. 2018 г. в 21:31, Andrey Kuznetsov <stkuzma@gmail.com>:

> Nikita,
>
> I like your suggestion. It looks more expressive for me than existing
> throwing version.
>
> чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <nsamelchev@gmail.com>:
>
> > Hi, Igniters.
> >
> > I suggest improving new listening test logger.
> >
> > I found usage case when needs wait for conditions for test duration
> > optimization.
> > For example, that messages A and B will be logged.
> >
> > For now, LogListener.check() doesn't return checking result as boolean.
> > It throws the exception if conditions fail. Code for this case:
> >
> > waitForCondition(() -> {
> >  try {
> >    lsnr.check();
> >
> >    return true;
> >  }
> >  catch (AssertionError ignored) {
> >    return false;
> >  }
> > }, timeout);
> >
> > For code readability, I suggest make LogListener.check() with boolean
> type:
> >
> > waitForCondition(lsnr::check, timeout);
> >
> > Also, it's more understandable when we write explicit assert in tests:
> > assertTrue("Fail reason.", lsnr.check());
> > ср, 23 мая 2018 г. в 14:36, Andrey Kuznetsov <stkuzma@gmail.com>:
> > >
> > > Thanks, Vyacheslav.
> > >
> > > Created the issue [1] based on your idea.
> > >
> > > [1]  https://issues.apache.org/jira/browse/IGNITE-8570
> > >
> > >
> > > 2018-05-23 12:41 GMT+03:00 Vyacheslav Daradur <daradurvs@gmail.com>:
> > >
> > > > Hi, Andrey, I have faced this problem too.
> > > >
> > > > I'd suggest introducing new logger for tests instead of extending API
> > > > of *GridStringLogger*.
> > > >
> > > > The new logger should be some kind of *listened*, for example with
> the
> > > > folowing API:
> > > >
> > > > void addListener(String pattern, CountDownLatch latch);
> > > > void addListener(IgniteInClosure<String> lsnr);
> > > >
> > > > This approach reduces memory load in comparison with
> > *GridStringLogger*.
> > > >
> > > > Just for example these should demonstrate my idea, *listened logger*
> -
> > > > [1], *listener* - [2]:
> > > >
> > > > [1] https://github.com/apache/ignite/blob/master/modules/
> > > >
> >
> compatibility/src/test/java/org/apache/ignite/compatibility/testframework/
> > > > junits/logger/ListenedGridTestLog4jLogger.java
> > > > [2] https://github.com/apache/ignite/blob/master/modules/
> > > >
> >
> compatibility/src/test/java/org/apache/ignite/compatibility/testframework/
> > > > junits/IgniteCompatibilityAbstractTest.java#L304
> > > >
> > > >
> > > >
> > > --
> > > Best regards,
> > >   Andrey Kuznetsov.
> >
> >
> >
> > --
> > Best wishes,
> > Amelchev Nikita
> >
>
>
> --
> Best regards,
>   Andrey Kuznetsov.
>

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