From add630c957cbd37ead0f041c923a54c0043cb0c6 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Mon, 1 Jun 2015 14:44:49 +0900 Subject: [PATCH] Fix sporadic assertion failure in SingleThreadEventLoopTest Motivation: SingleThreadEventLoopTest.testScheduleTaskAtFixedRate() fails often due to: - too little tolerance - incorrect assertion (it compares only with the previous timestamp) Modifications: - Increase the timestamp difference tolerance from 10ms to 20ms - Improve the timestamp assertion so that the comparison is performed against the first recorded timestamp - Misc: Fix broken Javadoc tag Result: More build stability --- .../netty/channel/SingleThreadEventLoop.java | 2 +- .../channel/SingleThreadEventLoopTest.java | 29 ++++++++++++------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/transport/src/main/java/io/netty/channel/SingleThreadEventLoop.java b/transport/src/main/java/io/netty/channel/SingleThreadEventLoop.java index d334fbfaf5..a68e52b61e 100644 --- a/transport/src/main/java/io/netty/channel/SingleThreadEventLoop.java +++ b/transport/src/main/java/io/netty/channel/SingleThreadEventLoop.java @@ -67,7 +67,7 @@ public abstract class SingleThreadEventLoop extends SingleThreadEventExecutor im } /** - * Marker interface for {@linkRunnable} that will not trigger an {@link #wakeup(boolean)} in all cases. + * Marker interface for {@link Runnable} that will not trigger an {@link #wakeup(boolean)} in all cases. */ interface NonWakeupRunnable extends Runnable { } } diff --git a/transport/src/test/java/io/netty/channel/SingleThreadEventLoopTest.java b/transport/src/test/java/io/netty/channel/SingleThreadEventLoopTest.java index 281cedc154..fd862cd567 100644 --- a/transport/src/test/java/io/netty/channel/SingleThreadEventLoopTest.java +++ b/transport/src/test/java/io/netty/channel/SingleThreadEventLoopTest.java @@ -39,7 +39,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; public class SingleThreadEventLoopTest { @@ -146,7 +146,8 @@ public class SingleThreadEventLoopTest { endTime.set(System.nanoTime()); } }, 500, TimeUnit.MILLISECONDS).get(); - assertTrue(endTime.get() - startTime >= TimeUnit.MILLISECONDS.toNanos(500)); + assertThat(endTime.get() - startTime, + is(greaterThanOrEqualTo(TimeUnit.MILLISECONDS.toNanos(500)))); } @Test @@ -177,15 +178,19 @@ public class SingleThreadEventLoopTest { assertEquals(5, timestamps.size()); // Check if the task was run without a lag. - Long previousTimestamp = null; + Long firstTimestamp = null; + int cnt = 0; for (Long t: timestamps) { - if (previousTimestamp == null) { - previousTimestamp = t; + if (firstTimestamp == null) { + firstTimestamp = t; continue; } - assertTrue(t.longValue() - previousTimestamp.longValue() >= TimeUnit.MILLISECONDS.toNanos(90)); - previousTimestamp = t; + long timepoint = t - firstTimestamp; + assertThat(timepoint, is(greaterThanOrEqualTo(TimeUnit.MILLISECONDS.toNanos(100 * cnt + 80)))); + assertThat(timepoint, is(lessThan(TimeUnit.MILLISECONDS.toNanos(100 * (cnt + 1) + 20)))); + + cnt ++; } } @@ -230,9 +235,9 @@ public class SingleThreadEventLoopTest { long diff = t.longValue() - previousTimestamp.longValue(); if (i == 0) { - assertTrue(diff >= TimeUnit.MILLISECONDS.toNanos(400)); + assertThat(diff, is(greaterThanOrEqualTo(TimeUnit.MILLISECONDS.toNanos(400)))); } else { - assertTrue(diff <= TimeUnit.MILLISECONDS.toNanos(10)); + assertThat(diff, is(lessThanOrEqualTo(TimeUnit.MILLISECONDS.toNanos(10)))); } previousTimestamp = t; i ++; @@ -274,7 +279,8 @@ public class SingleThreadEventLoopTest { continue; } - assertTrue(t.longValue() - previousTimestamp.longValue() >= TimeUnit.MILLISECONDS.toNanos(150)); + assertThat(t.longValue() - previousTimestamp.longValue(), + is(greaterThanOrEqualTo(TimeUnit.MILLISECONDS.toNanos(150)))); previousTimestamp = t; } } @@ -408,7 +414,8 @@ public class SingleThreadEventLoopTest { loopA.awaitTermination(Integer.MAX_VALUE, TimeUnit.SECONDS); } - assertTrue(System.nanoTime() - startTime >= TimeUnit.SECONDS.toNanos(1)); + assertThat(System.nanoTime() - startTime, + is(greaterThanOrEqualTo(TimeUnit.SECONDS.toNanos(1)))); } @Test(timeout = 5000)