You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Andrey Kuznetsov <st...@gmail.com> on 2018/05/23 08:46:56 UTC
Unreliable checks in tests for string presence in GridStringLogger contents
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.
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.
Re: Unreliable checks in tests for string presence in
GridStringLogger contents
Posted by 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
Re: Unreliable checks in tests for string presence in
GridStringLogger contents
Posted by 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.
Re: Unreliable checks in tests for string presence in
GridStringLogger contents
Posted by 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
On Wed, May 23, 2018 at 11:46 AM, Andrey Kuznetsov <st...@gmail.com> 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.