Unreliable checks in tests for string presence in GridStringLogger contents

[Andrey Kuznetsov]

Folks,

While testing my PR on TeamCity I found a floating test failure. Sometimes
we use {{GridStringLogger}} test facility to capture Ignite node logs and
then check whether some marker strings exist in log output. Since logger's
buffer is limited (32K chars by default) unrelated changes that add more
logging can damage a test.

I propose to add some stuff to {{GridStringLogger}} in order to catch
preset marker strings during logging, then we will not depend on buffer
size.

Thoughts?

--
Best regards,
  Andrey Kuznetsov.

[daradurvs]

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

On Wed, May 23, 2018 at 11:46 AM, Andrey Kuznetsov <[hidden email]> wrote:

> Folks,
>
> While testing my PR on TeamCity I found a floating test failure. Sometimes
> we use {{GridStringLogger}} test facility to capture Ignite node logs and
> then check whether some marker strings exist in log output. Since logger's
> buffer is limited (32K chars by default) unrelated changes that add more
> logging can damage a test.
>
> I propose to add some stuff to {{GridStringLogger}} in order to catch
> preset marker strings during logging, then we will not depend on buffer
> size.
>
> Thoughts?
>
> --
> Best regards,
>   Andrey Kuznetsov.

--
Best Regards, Vyacheslav D.

[Andrey Kuznetsov]

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 <[hidden email]>:

> 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.

[Nikita Amelchev]

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 <[hidden email]>:

>
> 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 <[hidden email]>:
>
> > 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

[Andrey Kuznetsov]

Nikita,

I like your suggestion. It looks more expressive for me than existing
throwing version.

чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <[hidden email]>:

> 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 <[hidden email]>:
> >
> > 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 <[hidden email]>:
> >
> > > 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.

[Pavel Pereslegin]

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 <[hidden email]>:

> Nikita,
>
> I like your suggestion. It looks more expressive for me than existing
> throwing version.
>
> чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <[hidden email]>:
>
> > 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 <[hidden email]>:
> > >
> > > 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 <[hidden email]>:
> > >
> > > > 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.
>

[Dmitrii Ryabov]

> waitForCondition(lsnr::check, timeout);
Agree, it is more convenient to use.
пт, 26 окт. 2018 г. в 13:01, Pavel Pereslegin <[hidden email]>:

>
> 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 <[hidden email]>:
>
> > Nikita,
> >
> > I like your suggestion. It looks more expressive for me than existing
> > throwing version.
> >
> > чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <[hidden email]>:
> >
> > > 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 <[hidden email]>:
> > > >
> > > > 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 <[hidden email]>:
> > > >
> > > > > 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.
> >

[Nikita Amelchev]

Thanks for comments,

I have filed a ticket [1] and will implement it if you don't mind.

1. https://issues.apache.org/jira/browse/IGNITE-10023
пт, 26 окт. 2018 г. в 15:56, Dmitrii Ryabov <[hidden email]>:

>
> > waitForCondition(lsnr::check, timeout);
> Agree, it is more convenient to use.
> пт, 26 окт. 2018 г. в 13:01, Pavel Pereslegin <[hidden email]>:
> >
> > 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 <[hidden email]>:
> >
> > > Nikita,
> > >
> > > I like your suggestion. It looks more expressive for me than existing
> > > throwing version.
> > >
> > > чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <[hidden email]>:
> > >
> > > > 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 <[hidden email]>:
> > > > >
> > > > > 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 <[hidden email]>:
> > > > >
> > > > > > 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.
> > >

--
Best wishes,
Amelchev Nikita

[Sergey Kosarev]

Hi!
I appreciate your efforts in replcacing GridStringLogger, just a remark.
I think it was a mistake to change check() to boolean.
I supppose method should have not be changed, but added as both methods are
useful.

Now we've lost error description messages existed in previous
implementation.

I mean if we previously matched specific counts, the following error
message returned:

String err =  errMsg != null ? errMsg :
    "\"" + subj + "\" matches " + matchesCnt + " times, expected: " +

        (exp.getMaximum() == exp.getMinimum() ? exp.getMinimum() : exp) + ".";


But now in case of error we have less information.

Hence, I suppose we should add  new method ( name can be assert() ) with
old implementation returning AssertionError.

What do you think?

Best regards,
Sergey Kosarev.


пт, 26 окт. 2018 г. в 16:53, Nikita Amelchev <[hidden email]>:

> Thanks for comments,
>
> I have filed a ticket [1] and will implement it if you don't mind.
>
> 1. https://issues.apache.org/jira/browse/IGNITE-10023
> пт, 26 окт. 2018 г. в 15:56, Dmitrii Ryabov <[hidden email]>:
> >
> > > waitForCondition(lsnr::check, timeout);
> > Agree, it is more convenient to use.
> > пт, 26 окт. 2018 г. в 13:01, Pavel Pereslegin <[hidden email]>:
> > >
> > > 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 <[hidden email]>:
> > >
> > > > Nikita,
> > > >
> > > > I like your suggestion. It looks more expressive for me than existing
> > > > throwing version.
> > > >
> > > > чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <[hidden email]>:
> > > >
> > > > > 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 <[hidden email]>:
> > > > > >
> > > > > > 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 <
> [hidden email]>:
> > > > > >
> > > > > > > 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.
> > > >
>
>
>
> --
> Best wishes,
> Amelchev Nikita
>

[Andrey Kuznetsov]

Hi, Sergey!

Your note sounds convincing. +1 for adding throwing version of {{check}}.

Best regards,
Andrey Kuznetsov.

вт, 29 янв. 2019, 19:14 Sergey [hidden email]:

> Hi!
> I appreciate your efforts in replcacing GridStringLogger, just a remark.
> I think it was a mistake to change check() to boolean.
> I supppose method should have not be changed, but added as both methods are
> useful.
>
> Now we've lost error description messages existed in previous
> implementation.
>
> I mean if we previously matched specific counts, the following error
> message returned:
>
> String err =  errMsg != null ? errMsg :
>     "\"" + subj + "\" matches " + matchesCnt + " times, expected: " +
>
>         (exp.getMaximum() == exp.getMinimum() ? exp.getMinimum() : exp) +
> ".";
>
>
> But now in case of error we have less information.
>
> Hence, I suppose we should add  new method ( name can be assert() ) with
> old implementation returning AssertionError.
>
> What do you think?
>
> Best regards,
> Sergey Kosarev.
>
>
> пт, 26 окт. 2018 г. в 16:53, Nikita Amelchev <[hidden email]>:
>
> > Thanks for comments,
> >
> > I have filed a ticket [1] and will implement it if you don't mind.
> >
> > 1. https://issues.apache.org/jira/browse/IGNITE-10023
> > пт, 26 окт. 2018 г. в 15:56, Dmitrii Ryabov <[hidden email]>:
> > >
> > > > waitForCondition(lsnr::check, timeout);
> > > Agree, it is more convenient to use.
> > > пт, 26 окт. 2018 г. в 13:01, Pavel Pereslegin <[hidden email]>:
> > > >
> > > > 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 <[hidden email]>:
> > > >
> > > > > Nikita,
> > > > >
> > > > > I like your suggestion. It looks more expressive for me than
> existing
> > > > > throwing version.
> > > > >
> > > > > чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <[hidden email]
> >:
> > > > >
> > > > > > 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 <[hidden email]
> >:
> > > > > > >
> > > > > > > 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 <
> > [hidden email]>:
> > > > > > >
> > > > > > > > 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.
> > > > >
> >
> >
> >
> > --
> > Best wishes,
> > Amelchev Nikita
> >
>