You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Nikita Amelchev <ns...@gmail.com> on 2018/10/25 18:06:45 UTC
Re: Unreliable checks in tests for string presence in
GridStringLogger contents
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 <st...@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 <da...@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
Re: Unreliable checks in tests for string presence in
GridStringLogger contents
Posted by Andrey Kuznetsov <st...@gmail.com>.
Hi, Sergey!
Your note sounds convincing. +1 for adding throwing version of {{check}}.
Best regards,
Andrey Kuznetsov.
вт, 29 янв. 2019, 19:14 Sergey macrergate@gmail.com:
> 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 <ns...@gmail.com>:
>
> > 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 <so...@gmail.com>:
> > >
> > > > waitForCondition(lsnr::check, timeout);
> > > Agree, it is more convenient to use.
> > > пт, 26 окт. 2018 г. в 13:01, Pavel Pereslegin <xx...@gmail.com>:
> > > >
> > > > 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 <st...@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.
> > > > >
> >
> >
> >
> > --
> > Best wishes,
> > Amelchev Nikita
> >
>
Re: Unreliable checks in tests for string presence in
GridStringLogger contents
Posted by Sergey <ma...@gmail.com>.
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 <ns...@gmail.com>:
> 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 <so...@gmail.com>:
> >
> > > waitForCondition(lsnr::check, timeout);
> > Agree, it is more convenient to use.
> > пт, 26 окт. 2018 г. в 13:01, Pavel Pereslegin <xx...@gmail.com>:
> > >
> > > 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 <st...@gmail.com>:
> > >
> > > > Nikita,
> > > >
> > > > I like your suggestion. It looks more expressive for me than existing
> > > > throwing version.
> > > >
> > > > чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <ns...@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 <st...@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.
> > > >
>
>
>
> --
> Best wishes,
> Amelchev Nikita
>
Re: Unreliable checks in tests for string presence in
GridStringLogger contents
Posted by Nikita Amelchev <ns...@gmail.com>.
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 <so...@gmail.com>:
>
> > waitForCondition(lsnr::check, timeout);
> Agree, it is more convenient to use.
> пт, 26 окт. 2018 г. в 13:01, Pavel Pereslegin <xx...@gmail.com>:
> >
> > 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 <st...@gmail.com>:
> >
> > > Nikita,
> > >
> > > I like your suggestion. It looks more expressive for me than existing
> > > throwing version.
> > >
> > > чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <ns...@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 <st...@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 <da...@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.
> > >
--
Best wishes,
Amelchev Nikita
Re: Unreliable checks in tests for string presence in
GridStringLogger contents
Posted by Dmitrii Ryabov <so...@gmail.com>.
> waitForCondition(lsnr::check, timeout);
Agree, it is more convenient to use.
пт, 26 окт. 2018 г. в 13:01, Pavel Pereslegin <xx...@gmail.com>:
>
> 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 <st...@gmail.com>:
>
> > Nikita,
> >
> > I like your suggestion. It looks more expressive for me than existing
> > throwing version.
> >
> > чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <ns...@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 <st...@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 <da...@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.
> >
Re: Unreliable checks in tests for string presence in
GridStringLogger contents
Posted by Pavel Pereslegin <xx...@gmail.com>.
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 <st...@gmail.com>:
> Nikita,
>
> I like your suggestion. It looks more expressive for me than existing
> throwing version.
>
> чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <ns...@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 <st...@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 <da...@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.
>
Re: Unreliable checks in tests for string presence in
GridStringLogger contents
Posted by Andrey Kuznetsov <st...@gmail.com>.
Nikita,
I like your suggestion. It looks more expressive for me than existing
throwing version.
чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev <ns...@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 <st...@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 <da...@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.