From c1a191c547c0fb8e0bce4f17751e356b213f1ddb Mon Sep 17 00:00:00 2001 From: Jiri Danek Date: Tue, 7 Aug 2018 18:22:57 +0200 Subject: [PATCH] ARTEMIS-2016 fix flaky test QueueControlTest#testRemoveAllWithPagingMode Parameters going into Wait.waitFor were originally wrong, because `durationMillis: 3, sleepMillis: 100` means you would test the condition only once. This commit is changing the durationMillis from 3ms to 3s, swapping the two numbers (duration 100ms, sleep 3ms) would also be reasonable, I think. Next, Wait.assertEquals is here being used, instead of Assert.assertTrue. I saw the test fail only once, and never was able to reproduce it again, but I think this commit does improve the test and so it is worthwhile. java.lang.AssertionError at org.apache.activemq.artemis.tests.integration.management.QueueControlTest.assertMetrics(QueueControlTest.java:2651) at org.apache.activemq.artemis.tests.integration.management.QueueControlTest.assertMessageMetrics(QueueControlTest.java:2615) at org.apache.activemq.artemis.tests.integration.management.QueueControlTest.testRemoveAllWithPagingMode(QueueControlTest.java:1554) --- .../apache/activemq/artemis/junit/Wait.java | 6 +++++- .../management/QueueControlTest.java | 20 +++++++++---------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/artemis-junit/src/main/java/org/apache/activemq/artemis/junit/Wait.java b/artemis-junit/src/main/java/org/apache/activemq/artemis/junit/Wait.java index e37539a3a6..c0aa55d40c 100644 --- a/artemis-junit/src/main/java/org/apache/activemq/artemis/junit/Wait.java +++ b/artemis-junit/src/main/java/org/apache/activemq/artemis/junit/Wait.java @@ -52,7 +52,11 @@ public class Wait { } public static void assertEquals(long size, LongCondition condition, long timeout) throws Exception { - boolean result = waitFor(() -> condition.getCount() == size, timeout); + assertEquals(size, condition, timeout, SLEEP_MILLIS); + } + + public static void assertEquals(Long size, LongCondition condition, long timeout, long sleepMillis) throws Exception { + boolean result = waitFor(() -> condition.getCount() == size, timeout, sleepMillis); if (!result) { Assert.fail(size + " != " + condition.getCount()); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/QueueControlTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/QueueControlTest.java index 0faa055f53..123f581248 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/QueueControlTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/QueueControlTest.java @@ -2632,25 +2632,25 @@ public class QueueControlTest extends ManagementTestBase { Supplier durableCount, Supplier durableSize) throws Exception { //make sure count stat equals message count - Assert.assertTrue(Wait.waitFor(() -> count.get().longValue() == messageCount, 3, 100)); + Assert.assertTrue(Wait.waitFor(() -> count.get().longValue() == messageCount, 3 * 1000, 100)); if (messageCount > 0) { //verify size stat greater than 0 - Assert.assertTrue(Wait.waitFor(() -> size.get().longValue() > 0, 3, 100)); + Assert.assertTrue(Wait.waitFor(() -> size.get().longValue() > 0, 3 * 1000, 100)); //If durable then make sure durable count and size are correct if (durable) { - Assert.assertTrue(Wait.waitFor(() -> durableCount.get().longValue() == messageCount, 3, 100)); - Assert.assertTrue(Wait.waitFor(() -> durableSize.get().longValue() > 0, 3, 100)); + Wait.assertEquals(messageCount, () -> durableCount.get().longValue(), 3 * 1000, 100); + Assert.assertTrue(Wait.waitFor(() -> durableSize.get().longValue() > 0, 3 * 1000, 100)); } else { - Assert.assertTrue(Wait.waitFor(() -> durableCount.get().longValue() == 0, 3, 100)); - Assert.assertTrue(Wait.waitFor(() -> durableSize.get().longValue() == 0, 3, 100)); + Wait.assertEquals(0L, () -> durableCount.get().longValue(), 3 * 1000, 100); + Wait.assertEquals(0L, () -> durableSize.get().longValue(), 3 * 1000, 100); } } else { - Assert.assertTrue(Wait.waitFor(() -> count.get().longValue() == 0, 3, 100)); - Assert.assertTrue(Wait.waitFor(() -> durableCount.get().longValue() == 0, 3, 100)); - Assert.assertTrue(Wait.waitFor(() -> size.get().longValue() == 0, 3, 100)); - Assert.assertTrue(Wait.waitFor(() -> durableSize.get().longValue() == 0, 3, 100)); + Wait.assertEquals(0L, () -> count.get().longValue(), 3 * 1000, 100); + Wait.assertEquals(0L, () -> durableCount.get().longValue(), 3 * 1000, 100); + Wait.assertEquals(0L, () -> size.get().longValue(), 3 * 1000, 100); + Wait.assertEquals(0L, () -> durableSize.get().longValue(), 3 * 1000, 100); } } }