You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Ivan Zlenko <iz...@gridgain.com> on 2025/10/29 12:58:39 UTC
Deprecation of waitForCondition in tests
Hi Ignite community!
I have a small proposition in regards to our testing.
We have an utility method called waitForCondition which we are using, as it
implies from the name of the method, to wait until some condition will be
met in the test. As soon as the condition is met this method returns true,
or false if the specified condition is not met for specified timeout.
After that our common pattern is to use assertTrue to verify whether
waitForCondition is completed successfully.
However we have a library awaitility in our test classpath in many modules
which are made for similar purposes. The main difference here is that this
library provides a clear message on what condition failed to meet in what
time frame.
Let's take a look at following example:
assertTrue(waitForCondition(() -> 60 == 61, 5_000));
In that case result of the assertion will be the following:
org.opentest4j.AssertionFailedError:
Expected :true
Actual :false
As we can see it is hard to understand what went wrong.
The same scenario using awaitility:
await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
Will result in the following error:
org.awaitility.core.ConditionTimeoutException: Lambda expression in
org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
expected <61> but was <60> within 5 seconds.
Not only is it easier to understand what we want to achieve in the
testing scenario, but the error message is much clearer compared to
waitForCondition.
Also, we have a very huge bug in waitForCondition where it will not
work at all if the supplier is frozen or executing more than specified
timeout. For example:
assertTrue(waitForCondition(() -> {
try {
Thread.sleep(10_000);
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
return true;
}, 5_000));
I would've expected that this code snippet would fail after 5 seconds,
but it completes successfully after 10 seconds.
So my proposition to the community is to deprecate waitForCondition
completely and eventually switch to awaitility through the whole
Ignite code base.
Sincerely yours,
Ivan Zlenko.
Re: Deprecation of waitForCondition in tests
Posted by Ivan Zlenko <iz...@gridgain.com>.
I'm talking about Ignite 3 only. Though I do not think it will be hard to
add awaitility in Ignite 2 as well.
On Wed, Oct 29, 2025 at 5:12 PM Aleksandr Polovtsev <al...@gmail.com>
wrote:
> It's also worth noting that "waitForCondition" requires "assertTrue", which
> is easy to forget and we had a lot of bugs because of that.
>
> Also, are you talking about both Ignite 2 and Ignite 3? Does Ignite 2 use
> Awaitility?
>
> On Wed, Oct 29, 2025 at 3:07 PM Pavel Tupitsyn <pt...@apache.org>
> wrote:
>
> > +1, sounds great. Did not know about that library.
> >
> > On Wed, Oct 29, 2025 at 2:58 PM Ivan Zlenko <iz...@gridgain.com>
> wrote:
> > >
> > > Hi Ignite community!
> > > I have a small proposition in regards to our testing.
> > > We have an utility method called waitForCondition which we are using,
> as
> > it
> > > implies from the name of the method, to wait until some condition will
> be
> > > met in the test. As soon as the condition is met this method returns
> > true,
> > > or false if the specified condition is not met for specified timeout.
> > > After that our common pattern is to use assertTrue to verify whether
> > > waitForCondition is completed successfully.
> > > However we have a library awaitility in our test classpath in many
> > modules
> > > which are made for similar purposes. The main difference here is that
> > this
> > > library provides a clear message on what condition failed to meet in
> what
> > > time frame.
> > > Let's take a look at following example:
> > >
> > > assertTrue(waitForCondition(() -> 60 == 61, 5_000));
> > >
> > > In that case result of the assertion will be the following:
> > >
> > > org.opentest4j.AssertionFailedError:
> > > Expected :true
> > > Actual :false
> > >
> > > As we can see it is hard to understand what went wrong.
> > >
> > > The same scenario using awaitility:
> > >
> > > await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
> > >
> > > Will result in the following error:
> > >
> > > org.awaitility.core.ConditionTimeoutException: Lambda expression in
> > >
> >
> org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
> > > expected <61> but was <60> within 5 seconds.
> > >
> > > Not only is it easier to understand what we want to achieve in the
> > > testing scenario, but the error message is much clearer compared to
> > > waitForCondition.
> > >
> > > Also, we have a very huge bug in waitForCondition where it will not
> > > work at all if the supplier is frozen or executing more than specified
> > > timeout. For example:
> > >
> > > assertTrue(waitForCondition(() -> {
> > > try {
> > > Thread.sleep(10_000);
> > > } catch (InterruptedException e) {
> > > throw new RuntimeException(e);
> > > }
> > >
> > > return true;
> > > }, 5_000));
> > >
> > > I would've expected that this code snippet would fail after 5 seconds,
> > > but it completes successfully after 10 seconds.
> > >
> > > So my proposition to the community is to deprecate waitForCondition
> > > completely and eventually switch to awaitility through the whole
> > > Ignite code base.
> > >
> > > Sincerely yours,
> > >
> > > Ivan Zlenko.
> >
>
>
> --
> With regards,
> Aleksandr Polovtsev
>
Re: Deprecation of waitForCondition in tests
Posted by Aleksandr Polovtsev <al...@gmail.com>.
It's also worth noting that "waitForCondition" requires "assertTrue", which
is easy to forget and we had a lot of bugs because of that.
Also, are you talking about both Ignite 2 and Ignite 3? Does Ignite 2 use
Awaitility?
On Wed, Oct 29, 2025 at 3:07 PM Pavel Tupitsyn <pt...@apache.org> wrote:
> +1, sounds great. Did not know about that library.
>
> On Wed, Oct 29, 2025 at 2:58 PM Ivan Zlenko <iz...@gridgain.com> wrote:
> >
> > Hi Ignite community!
> > I have a small proposition in regards to our testing.
> > We have an utility method called waitForCondition which we are using, as
> it
> > implies from the name of the method, to wait until some condition will be
> > met in the test. As soon as the condition is met this method returns
> true,
> > or false if the specified condition is not met for specified timeout.
> > After that our common pattern is to use assertTrue to verify whether
> > waitForCondition is completed successfully.
> > However we have a library awaitility in our test classpath in many
> modules
> > which are made for similar purposes. The main difference here is that
> this
> > library provides a clear message on what condition failed to meet in what
> > time frame.
> > Let's take a look at following example:
> >
> > assertTrue(waitForCondition(() -> 60 == 61, 5_000));
> >
> > In that case result of the assertion will be the following:
> >
> > org.opentest4j.AssertionFailedError:
> > Expected :true
> > Actual :false
> >
> > As we can see it is hard to understand what went wrong.
> >
> > The same scenario using awaitility:
> >
> > await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
> >
> > Will result in the following error:
> >
> > org.awaitility.core.ConditionTimeoutException: Lambda expression in
> >
> org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
> > expected <61> but was <60> within 5 seconds.
> >
> > Not only is it easier to understand what we want to achieve in the
> > testing scenario, but the error message is much clearer compared to
> > waitForCondition.
> >
> > Also, we have a very huge bug in waitForCondition where it will not
> > work at all if the supplier is frozen or executing more than specified
> > timeout. For example:
> >
> > assertTrue(waitForCondition(() -> {
> > try {
> > Thread.sleep(10_000);
> > } catch (InterruptedException e) {
> > throw new RuntimeException(e);
> > }
> >
> > return true;
> > }, 5_000));
> >
> > I would've expected that this code snippet would fail after 5 seconds,
> > but it completes successfully after 10 seconds.
> >
> > So my proposition to the community is to deprecate waitForCondition
> > completely and eventually switch to awaitility through the whole
> > Ignite code base.
> >
> > Sincerely yours,
> >
> > Ivan Zlenko.
>
--
With regards,
Aleksandr Polovtsev
Re: Deprecation of waitForCondition in tests
Posted by Pavel Tupitsyn <pt...@apache.org>.
+1, sounds great. Did not know about that library.
On Wed, Oct 29, 2025 at 2:58 PM Ivan Zlenko <iz...@gridgain.com> wrote:
>
> Hi Ignite community!
> I have a small proposition in regards to our testing.
> We have an utility method called waitForCondition which we are using, as it
> implies from the name of the method, to wait until some condition will be
> met in the test. As soon as the condition is met this method returns true,
> or false if the specified condition is not met for specified timeout.
> After that our common pattern is to use assertTrue to verify whether
> waitForCondition is completed successfully.
> However we have a library awaitility in our test classpath in many modules
> which are made for similar purposes. The main difference here is that this
> library provides a clear message on what condition failed to meet in what
> time frame.
> Let's take a look at following example:
>
> assertTrue(waitForCondition(() -> 60 == 61, 5_000));
>
> In that case result of the assertion will be the following:
>
> org.opentest4j.AssertionFailedError:
> Expected :true
> Actual :false
>
> As we can see it is hard to understand what went wrong.
>
> The same scenario using awaitility:
>
> await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
>
> Will result in the following error:
>
> org.awaitility.core.ConditionTimeoutException: Lambda expression in
> org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
> expected <61> but was <60> within 5 seconds.
>
> Not only is it easier to understand what we want to achieve in the
> testing scenario, but the error message is much clearer compared to
> waitForCondition.
>
> Also, we have a very huge bug in waitForCondition where it will not
> work at all if the supplier is frozen or executing more than specified
> timeout. For example:
>
> assertTrue(waitForCondition(() -> {
> try {
> Thread.sleep(10_000);
> } catch (InterruptedException e) {
> throw new RuntimeException(e);
> }
>
> return true;
> }, 5_000));
>
> I would've expected that this code snippet would fail after 5 seconds,
> but it completes successfully after 10 seconds.
>
> So my proposition to the community is to deprecate waitForCondition
> completely and eventually switch to awaitility through the whole
> Ignite code base.
>
> Sincerely yours,
>
> Ivan Zlenko.
Re: Deprecation of waitForCondition in tests
Posted by Mikhail Pochatkin <m....@gmail.com>.
+1
ср, 29 окт. 2025 г. в 16:23, Vadim Pakhnushev <vp...@unison.team>:
> +1
>
> Also sometimes we use awaitility in the following manner:
> await().untilAsserted(() -> assertThat(foo(), is(bar)));
> This is equivalent to the
> await().until(() -> foo(), is(bar));
> So keep that in mind when writing new assertions.
>
> On Wed, Oct 29, 2025 at 4:19 PM Aleksandr Pakhomov <ap...@gmail.com>
> wrote:
>
> > +1, Good idea!
> >
> > > On 29 Oct 2025, at 15:58, Ivan Zlenko <iz...@gridgain.com> wrote:
> > >
> > > Hi Ignite community!
> > > I have a small proposition in regards to our testing.
> > > We have an utility method called waitForCondition which we are using,
> as
> > it
> > > implies from the name of the method, to wait until some condition will
> be
> > > met in the test. As soon as the condition is met this method returns
> > true,
> > > or false if the specified condition is not met for specified timeout.
> > > After that our common pattern is to use assertTrue to verify whether
> > > waitForCondition is completed successfully.
> > > However we have a library awaitility in our test classpath in many
> > modules
> > > which are made for similar purposes. The main difference here is that
> > this
> > > library provides a clear message on what condition failed to meet in
> what
> > > time frame.
> > > Let's take a look at following example:
> > >
> > > assertTrue(waitForCondition(() -> 60 == 61, 5_000));
> > >
> > > In that case result of the assertion will be the following:
> > >
> > > org.opentest4j.AssertionFailedError:
> > > Expected :true
> > > Actual :false
> > >
> > > As we can see it is hard to understand what went wrong.
> > >
> > > The same scenario using awaitility:
> > >
> > > await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
> > >
> > > Will result in the following error:
> > >
> > > org.awaitility.core.ConditionTimeoutException: Lambda expression in
> > >
> >
> org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
> > > expected <61> but was <60> within 5 seconds.
> > >
> > > Not only is it easier to understand what we want to achieve in the
> > > testing scenario, but the error message is much clearer compared to
> > > waitForCondition.
> > >
> > > Also, we have a very huge bug in waitForCondition where it will not
> > > work at all if the supplier is frozen or executing more than specified
> > > timeout. For example:
> > >
> > > assertTrue(waitForCondition(() -> {
> > > try {
> > > Thread.sleep(10_000);
> > > } catch (InterruptedException e) {
> > > throw new RuntimeException(e);
> > > }
> > >
> > > return true;
> > > }, 5_000));
> > >
> > > I would've expected that this code snippet would fail after 5 seconds,
> > > but it completes successfully after 10 seconds.
> > >
> > > So my proposition to the community is to deprecate waitForCondition
> > > completely and eventually switch to awaitility through the whole
> > > Ignite code base.
> > >
> > > Sincerely yours,
> > >
> > > Ivan Zlenko.
> >
> >
>
--
best regards,
Pochatkin Mikhail.
Re: Deprecation of waitForCondition in tests
Posted by Vadim Pakhnushev <vp...@unison.team>.
+1
Also sometimes we use awaitility in the following manner:
await().untilAsserted(() -> assertThat(foo(), is(bar)));
This is equivalent to the
await().until(() -> foo(), is(bar));
So keep that in mind when writing new assertions.
On Wed, Oct 29, 2025 at 4:19 PM Aleksandr Pakhomov <ap...@gmail.com> wrote:
> +1, Good idea!
>
> > On 29 Oct 2025, at 15:58, Ivan Zlenko <iz...@gridgain.com> wrote:
> >
> > Hi Ignite community!
> > I have a small proposition in regards to our testing.
> > We have an utility method called waitForCondition which we are using, as
> it
> > implies from the name of the method, to wait until some condition will be
> > met in the test. As soon as the condition is met this method returns
> true,
> > or false if the specified condition is not met for specified timeout.
> > After that our common pattern is to use assertTrue to verify whether
> > waitForCondition is completed successfully.
> > However we have a library awaitility in our test classpath in many
> modules
> > which are made for similar purposes. The main difference here is that
> this
> > library provides a clear message on what condition failed to meet in what
> > time frame.
> > Let's take a look at following example:
> >
> > assertTrue(waitForCondition(() -> 60 == 61, 5_000));
> >
> > In that case result of the assertion will be the following:
> >
> > org.opentest4j.AssertionFailedError:
> > Expected :true
> > Actual :false
> >
> > As we can see it is hard to understand what went wrong.
> >
> > The same scenario using awaitility:
> >
> > await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
> >
> > Will result in the following error:
> >
> > org.awaitility.core.ConditionTimeoutException: Lambda expression in
> >
> org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
> > expected <61> but was <60> within 5 seconds.
> >
> > Not only is it easier to understand what we want to achieve in the
> > testing scenario, but the error message is much clearer compared to
> > waitForCondition.
> >
> > Also, we have a very huge bug in waitForCondition where it will not
> > work at all if the supplier is frozen or executing more than specified
> > timeout. For example:
> >
> > assertTrue(waitForCondition(() -> {
> > try {
> > Thread.sleep(10_000);
> > } catch (InterruptedException e) {
> > throw new RuntimeException(e);
> > }
> >
> > return true;
> > }, 5_000));
> >
> > I would've expected that this code snippet would fail after 5 seconds,
> > but it completes successfully after 10 seconds.
> >
> > So my proposition to the community is to deprecate waitForCondition
> > completely and eventually switch to awaitility through the whole
> > Ignite code base.
> >
> > Sincerely yours,
> >
> > Ivan Zlenko.
>
>
Re: Deprecation of waitForCondition in tests
Posted by Aleksandr Pakhomov <ap...@gmail.com>.
+1, Good idea!
> On 29 Oct 2025, at 15:58, Ivan Zlenko <iz...@gridgain.com> wrote:
>
> Hi Ignite community!
> I have a small proposition in regards to our testing.
> We have an utility method called waitForCondition which we are using, as it
> implies from the name of the method, to wait until some condition will be
> met in the test. As soon as the condition is met this method returns true,
> or false if the specified condition is not met for specified timeout.
> After that our common pattern is to use assertTrue to verify whether
> waitForCondition is completed successfully.
> However we have a library awaitility in our test classpath in many modules
> which are made for similar purposes. The main difference here is that this
> library provides a clear message on what condition failed to meet in what
> time frame.
> Let's take a look at following example:
>
> assertTrue(waitForCondition(() -> 60 == 61, 5_000));
>
> In that case result of the assertion will be the following:
>
> org.opentest4j.AssertionFailedError:
> Expected :true
> Actual :false
>
> As we can see it is hard to understand what went wrong.
>
> The same scenario using awaitility:
>
> await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
>
> Will result in the following error:
>
> org.awaitility.core.ConditionTimeoutException: Lambda expression in
> org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
> expected <61> but was <60> within 5 seconds.
>
> Not only is it easier to understand what we want to achieve in the
> testing scenario, but the error message is much clearer compared to
> waitForCondition.
>
> Also, we have a very huge bug in waitForCondition where it will not
> work at all if the supplier is frozen or executing more than specified
> timeout. For example:
>
> assertTrue(waitForCondition(() -> {
> try {
> Thread.sleep(10_000);
> } catch (InterruptedException e) {
> throw new RuntimeException(e);
> }
>
> return true;
> }, 5_000));
>
> I would've expected that this code snippet would fail after 5 seconds,
> but it completes successfully after 10 seconds.
>
> So my proposition to the community is to deprecate waitForCondition
> completely and eventually switch to awaitility through the whole
> Ignite code base.
>
> Sincerely yours,
>
> Ivan Zlenko.
Re: Deprecation of waitForCondition in tests
Posted by Mirza Aliev <al...@gmail.com>.
+1
ср, 29 окт. 2025 г. в 17:30, Tiago Godinho <me...@gmail.com>:
> +1
>
> On Wed, 29 Oct 2025, 12:58 Ivan Zlenko, <iz...@gridgain.com> wrote:
>
> > Hi Ignite community!
> > I have a small proposition in regards to our testing.
> > We have an utility method called waitForCondition which we are using, as
> it
> > implies from the name of the method, to wait until some condition will be
> > met in the test. As soon as the condition is met this method returns
> true,
> > or false if the specified condition is not met for specified timeout.
> > After that our common pattern is to use assertTrue to verify whether
> > waitForCondition is completed successfully.
> > However we have a library awaitility in our test classpath in many
> modules
> > which are made for similar purposes. The main difference here is that
> this
> > library provides a clear message on what condition failed to meet in what
> > time frame.
> > Let's take a look at following example:
> >
> > assertTrue(waitForCondition(() -> 60 == 61, 5_000));
> >
> > In that case result of the assertion will be the following:
> >
> > org.opentest4j.AssertionFailedError:
> > Expected :true
> > Actual :false
> >
> > As we can see it is hard to understand what went wrong.
> >
> > The same scenario using awaitility:
> >
> > await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
> >
> > Will result in the following error:
> >
> > org.awaitility.core.ConditionTimeoutException: Lambda expression in
> >
> >
> org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
> > expected <61> but was <60> within 5 seconds.
> >
> > Not only is it easier to understand what we want to achieve in the
> > testing scenario, but the error message is much clearer compared to
> > waitForCondition.
> >
> > Also, we have a very huge bug in waitForCondition where it will not
> > work at all if the supplier is frozen or executing more than specified
> > timeout. For example:
> >
> > assertTrue(waitForCondition(() -> {
> > try {
> > Thread.sleep(10_000);
> > } catch (InterruptedException e) {
> > throw new RuntimeException(e);
> > }
> >
> > return true;
> > }, 5_000));
> >
> > I would've expected that this code snippet would fail after 5 seconds,
> > but it completes successfully after 10 seconds.
> >
> > So my proposition to the community is to deprecate waitForCondition
> > completely and eventually switch to awaitility through the whole
> > Ignite code base.
> >
> > Sincerely yours,
> >
> > Ivan Zlenko.
> >
>
Re: Deprecation of waitForCondition in tests
Posted by Ivan Zlenko <iz...@gridgain.com>.
Since in the last week there is no voice against it, I’ve created a ticket
to make this change: https://issues.apache.org/jira/browse/IGNITE-26922
Sincerely yours,
Ivan Zlenko.
On Mon, Nov 3, 2025 at 2:42 PM Roman Puchkovskiy <
roman.puchkovskiy@gmail.com> wrote:
> +1
>
> ср, 29 окт. 2025 г. в 17:33, Denis C <mo...@gmail.com>:
> >
> > +1
> >
> > ср, 29 окт. 2025 г., 22:30 Tiago Godinho <me...@gmail.com>:
> >
> > > +1
> > >
> > > On Wed, 29 Oct 2025, 12:58 Ivan Zlenko, <iz...@gridgain.com> wrote:
> > >
> > > > Hi Ignite community!
> > > > I have a small proposition in regards to our testing.
> > > > We have an utility method called waitForCondition which we are
> using, as
> > > it
> > > > implies from the name of the method, to wait until some condition
> will be
> > > > met in the test. As soon as the condition is met this method returns
> > > true,
> > > > or false if the specified condition is not met for specified timeout.
> > > > After that our common pattern is to use assertTrue to verify whether
> > > > waitForCondition is completed successfully.
> > > > However we have a library awaitility in our test classpath in many
> > > modules
> > > > which are made for similar purposes. The main difference here is that
> > > this
> > > > library provides a clear message on what condition failed to meet in
> what
> > > > time frame.
> > > > Let's take a look at following example:
> > > >
> > > > assertTrue(waitForCondition(() -> 60 == 61, 5_000));
> > > >
> > > > In that case result of the assertion will be the following:
> > > >
> > > > org.opentest4j.AssertionFailedError:
> > > > Expected :true
> > > > Actual :false
> > > >
> > > > As we can see it is hard to understand what went wrong.
> > > >
> > > > The same scenario using awaitility:
> > > >
> > > > await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
> > > >
> > > > Will result in the following error:
> > > >
> > > > org.awaitility.core.ConditionTimeoutException: Lambda expression in
> > > >
> > > >
> > >
> org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
> > > > expected <61> but was <60> within 5 seconds.
> > > >
> > > > Not only is it easier to understand what we want to achieve in the
> > > > testing scenario, but the error message is much clearer compared to
> > > > waitForCondition.
> > > >
> > > > Also, we have a very huge bug in waitForCondition where it will not
> > > > work at all if the supplier is frozen or executing more than
> specified
> > > > timeout. For example:
> > > >
> > > > assertTrue(waitForCondition(() -> {
> > > > try {
> > > > Thread.sleep(10_000);
> > > > } catch (InterruptedException e) {
> > > > throw new RuntimeException(e);
> > > > }
> > > >
> > > > return true;
> > > > }, 5_000));
> > > >
> > > > I would've expected that this code snippet would fail after 5
> seconds,
> > > > but it completes successfully after 10 seconds.
> > > >
> > > > So my proposition to the community is to deprecate waitForCondition
> > > > completely and eventually switch to awaitility through the whole
> > > > Ignite code base.
> > > >
> > > > Sincerely yours,
> > > >
> > > > Ivan Zlenko.
> > > >
> > >
>
Re: Deprecation of waitForCondition in tests
Posted by Roman Puchkovskiy <ro...@gmail.com>.
+1
ср, 29 окт. 2025 г. в 17:33, Denis C <mo...@gmail.com>:
>
> +1
>
> ср, 29 окт. 2025 г., 22:30 Tiago Godinho <me...@gmail.com>:
>
> > +1
> >
> > On Wed, 29 Oct 2025, 12:58 Ivan Zlenko, <iz...@gridgain.com> wrote:
> >
> > > Hi Ignite community!
> > > I have a small proposition in regards to our testing.
> > > We have an utility method called waitForCondition which we are using, as
> > it
> > > implies from the name of the method, to wait until some condition will be
> > > met in the test. As soon as the condition is met this method returns
> > true,
> > > or false if the specified condition is not met for specified timeout.
> > > After that our common pattern is to use assertTrue to verify whether
> > > waitForCondition is completed successfully.
> > > However we have a library awaitility in our test classpath in many
> > modules
> > > which are made for similar purposes. The main difference here is that
> > this
> > > library provides a clear message on what condition failed to meet in what
> > > time frame.
> > > Let's take a look at following example:
> > >
> > > assertTrue(waitForCondition(() -> 60 == 61, 5_000));
> > >
> > > In that case result of the assertion will be the following:
> > >
> > > org.opentest4j.AssertionFailedError:
> > > Expected :true
> > > Actual :false
> > >
> > > As we can see it is hard to understand what went wrong.
> > >
> > > The same scenario using awaitility:
> > >
> > > await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
> > >
> > > Will result in the following error:
> > >
> > > org.awaitility.core.ConditionTimeoutException: Lambda expression in
> > >
> > >
> > org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
> > > expected <61> but was <60> within 5 seconds.
> > >
> > > Not only is it easier to understand what we want to achieve in the
> > > testing scenario, but the error message is much clearer compared to
> > > waitForCondition.
> > >
> > > Also, we have a very huge bug in waitForCondition where it will not
> > > work at all if the supplier is frozen or executing more than specified
> > > timeout. For example:
> > >
> > > assertTrue(waitForCondition(() -> {
> > > try {
> > > Thread.sleep(10_000);
> > > } catch (InterruptedException e) {
> > > throw new RuntimeException(e);
> > > }
> > >
> > > return true;
> > > }, 5_000));
> > >
> > > I would've expected that this code snippet would fail after 5 seconds,
> > > but it completes successfully after 10 seconds.
> > >
> > > So my proposition to the community is to deprecate waitForCondition
> > > completely and eventually switch to awaitility through the whole
> > > Ignite code base.
> > >
> > > Sincerely yours,
> > >
> > > Ivan Zlenko.
> > >
> >
Re: Deprecation of waitForCondition in tests
Posted by Denis C <mo...@gmail.com>.
+1
ср, 29 окт. 2025 г., 22:30 Tiago Godinho <me...@gmail.com>:
> +1
>
> On Wed, 29 Oct 2025, 12:58 Ivan Zlenko, <iz...@gridgain.com> wrote:
>
> > Hi Ignite community!
> > I have a small proposition in regards to our testing.
> > We have an utility method called waitForCondition which we are using, as
> it
> > implies from the name of the method, to wait until some condition will be
> > met in the test. As soon as the condition is met this method returns
> true,
> > or false if the specified condition is not met for specified timeout.
> > After that our common pattern is to use assertTrue to verify whether
> > waitForCondition is completed successfully.
> > However we have a library awaitility in our test classpath in many
> modules
> > which are made for similar purposes. The main difference here is that
> this
> > library provides a clear message on what condition failed to meet in what
> > time frame.
> > Let's take a look at following example:
> >
> > assertTrue(waitForCondition(() -> 60 == 61, 5_000));
> >
> > In that case result of the assertion will be the following:
> >
> > org.opentest4j.AssertionFailedError:
> > Expected :true
> > Actual :false
> >
> > As we can see it is hard to understand what went wrong.
> >
> > The same scenario using awaitility:
> >
> > await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
> >
> > Will result in the following error:
> >
> > org.awaitility.core.ConditionTimeoutException: Lambda expression in
> >
> >
> org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
> > expected <61> but was <60> within 5 seconds.
> >
> > Not only is it easier to understand what we want to achieve in the
> > testing scenario, but the error message is much clearer compared to
> > waitForCondition.
> >
> > Also, we have a very huge bug in waitForCondition where it will not
> > work at all if the supplier is frozen or executing more than specified
> > timeout. For example:
> >
> > assertTrue(waitForCondition(() -> {
> > try {
> > Thread.sleep(10_000);
> > } catch (InterruptedException e) {
> > throw new RuntimeException(e);
> > }
> >
> > return true;
> > }, 5_000));
> >
> > I would've expected that this code snippet would fail after 5 seconds,
> > but it completes successfully after 10 seconds.
> >
> > So my proposition to the community is to deprecate waitForCondition
> > completely and eventually switch to awaitility through the whole
> > Ignite code base.
> >
> > Sincerely yours,
> >
> > Ivan Zlenko.
> >
>
Re: Deprecation of waitForCondition in tests
Posted by Tiago Godinho <me...@gmail.com>.
+1
On Wed, 29 Oct 2025, 12:58 Ivan Zlenko, <iz...@gridgain.com> wrote:
> Hi Ignite community!
> I have a small proposition in regards to our testing.
> We have an utility method called waitForCondition which we are using, as it
> implies from the name of the method, to wait until some condition will be
> met in the test. As soon as the condition is met this method returns true,
> or false if the specified condition is not met for specified timeout.
> After that our common pattern is to use assertTrue to verify whether
> waitForCondition is completed successfully.
> However we have a library awaitility in our test classpath in many modules
> which are made for similar purposes. The main difference here is that this
> library provides a clear message on what condition failed to meet in what
> time frame.
> Let's take a look at following example:
>
> assertTrue(waitForCondition(() -> 60 == 61, 5_000));
>
> In that case result of the assertion will be the following:
>
> org.opentest4j.AssertionFailedError:
> Expected :true
> Actual :false
>
> As we can see it is hard to understand what went wrong.
>
> The same scenario using awaitility:
>
> await().timeout(Duration.ofSeconds(5)).until(() -> 60, equalTo(61));
>
> Will result in the following error:
>
> org.awaitility.core.ConditionTimeoutException: Lambda expression in
>
> org.apache.ignite.internal.ItApplyPartitionRaftLogOnAnotherNodesCompatibilityTest
> expected <61> but was <60> within 5 seconds.
>
> Not only is it easier to understand what we want to achieve in the
> testing scenario, but the error message is much clearer compared to
> waitForCondition.
>
> Also, we have a very huge bug in waitForCondition where it will not
> work at all if the supplier is frozen or executing more than specified
> timeout. For example:
>
> assertTrue(waitForCondition(() -> {
> try {
> Thread.sleep(10_000);
> } catch (InterruptedException e) {
> throw new RuntimeException(e);
> }
>
> return true;
> }, 5_000));
>
> I would've expected that this code snippet would fail after 5 seconds,
> but it completes successfully after 10 seconds.
>
> So my proposition to the community is to deprecate waitForCondition
> completely and eventually switch to awaitility through the whole
> Ignite code base.
>
> Sincerely yours,
>
> Ivan Zlenko.
>