From 75324e99181872a55318ee811b0a6fb654a903b6 Mon Sep 17 00:00:00 2001
From: ZHANG Dapeng <zdapeng@google.com>
Date: Fri, 14 Apr 2017 09:42:42 -0700
Subject: [PATCH] netty: fix flaky max connection age tests using sleepAtLeast

resolves #2898
---
 .../io/grpc/netty/NettyServerHandlerTest.java | 16 +++---
 .../main/java/io/grpc/testing/TestUtils.java  | 14 +++++
 .../java/io/grpc/testing/TestUtilsTest.java   | 56 +++++++++++++++++++
 3 files changed, 77 insertions(+), 9 deletions(-)
 create mode 100644 testing/src/test/java/io/grpc/testing/TestUtilsTest.java

diff --git a/netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java b/netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java
index 2db9029f80..11e76a2b0f 100644
--- a/netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java
+++ b/netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java
@@ -72,6 +72,7 @@ import io.grpc.internal.ServerStreamListener;
 import io.grpc.internal.ServerTransportListener;
 import io.grpc.internal.StatsTraceContext;
 import io.grpc.netty.GrpcHttp2HeadersDecoder.GrpcHttp2ServerHeadersDecoder;
+import io.grpc.testing.TestUtils;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufUtil;
 import io.netty.buffer.Unpooled;
@@ -538,7 +539,7 @@ public class NettyServerHandlerTest extends NettyHandlerTestBase<NettyServerHand
     maxConnectionAgeInNanos = TimeUnit.MINUTES.toNanos(30L);
     setUp();
 
-    Thread.sleep(10L);
+    TestUtils.sleepAtLeast(10L);
     channel().runPendingTasks();
 
     // GO_AWAY not sent yet
@@ -549,13 +550,12 @@ public class NettyServerHandlerTest extends NettyHandlerTestBase<NettyServerHand
   }
 
   @Test
-  @Ignore // Flaky. TODO(zdapeng): fix https://github.com/grpc/grpc-java/issues/2898
   public void maxConnectionAge_goAwaySent() throws Exception {
     maxConnectionAgeInNanos = TimeUnit.MILLISECONDS.toNanos(10L);
     setUp();
     assertTrue(channel().isOpen());
 
-    Thread.sleep(10L);
+    TestUtils.sleepAtLeast(10L);
     channel().runPendingTasks();
 
     // GO_AWAY sent
@@ -568,21 +568,20 @@ public class NettyServerHandlerTest extends NettyHandlerTestBase<NettyServerHand
   }
 
   @Test
-  @Ignore // Flaky. TODO(zdapeng): fix https://github.com/grpc/grpc-java/issues/2898
   public void maxConnectionAgeGrace_channelStillOpenDuringGracePeriod() throws Exception {
     maxConnectionAgeInNanos = TimeUnit.MILLISECONDS.toNanos(10L);
     maxConnectionAgeGraceInNanos = TimeUnit.MINUTES.toNanos(30L);
     setUp();
     createStream();
 
-    Thread.sleep(10L);
+    TestUtils.sleepAtLeast(10L);
     channel().runPendingTasks();
 
     verifyWrite().writeGoAway(
         eq(ctx()), eq(Integer.MAX_VALUE), eq(Http2Error.NO_ERROR.code()), any(ByteBuf.class),
         any(ChannelPromise.class));
 
-    Thread.sleep(10L);
+    TestUtils.sleepAtLeast(10L);
     channel().runPendingTasks();
 
     // channel not closed yet
@@ -590,7 +589,6 @@ public class NettyServerHandlerTest extends NettyHandlerTestBase<NettyServerHand
   }
 
   @Test
-  @Ignore // Flaky. TODO(zdapeng): fix https://github.com/grpc/grpc-java/issues/2898
   public void maxConnectionAgeGrace_channelClosedAfterGracePeriod() throws Exception {
     maxConnectionAgeInNanos = TimeUnit.MILLISECONDS.toNanos(10L);
     maxConnectionAgeGraceInNanos = TimeUnit.MILLISECONDS.toNanos(10L);
@@ -598,7 +596,7 @@ public class NettyServerHandlerTest extends NettyHandlerTestBase<NettyServerHand
     createStream();
 
     // runPendingTasks so that GO_AWAY is sent and the forceful shutdown is scheduled
-    Thread.sleep(10L);
+    TestUtils.sleepAtLeast(10L);
     channel().runPendingTasks();
 
     verifyWrite().writeGoAway(
@@ -607,7 +605,7 @@ public class NettyServerHandlerTest extends NettyHandlerTestBase<NettyServerHand
     assertTrue(channel().isOpen());
 
     // need runPendingTasks again so that the forceful shutdown can be executed
-    Thread.sleep(10L);
+    TestUtils.sleepAtLeast(10L);
     channel().runPendingTasks();
 
     // channel closed
diff --git a/testing/src/main/java/io/grpc/testing/TestUtils.java b/testing/src/main/java/io/grpc/testing/TestUtils.java
index 08737442f6..42b135fcd4 100644
--- a/testing/src/main/java/io/grpc/testing/TestUtils.java
+++ b/testing/src/main/java/io/grpc/testing/TestUtils.java
@@ -56,6 +56,7 @@ import java.security.cert.X509Certificate;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSocketFactory;
@@ -219,5 +220,18 @@ public class TestUtils {
     return context.getSocketFactory();
   }
 
+  /**
+   * Sleeps for at least the specified time. When in need of a guaranteed sleep time, use this in
+   * preference to {@code Thread.sleep} which might not sleep for the required time.
+   */
+  public static void sleepAtLeast(long millis) throws InterruptedException {
+    long delay = TimeUnit.MILLISECONDS.toNanos(millis);
+    long end = System.nanoTime() + delay;
+    while (delay > 0) {
+      TimeUnit.NANOSECONDS.sleep(delay);
+      delay = end - System.nanoTime();
+    }
+  }
+
   private TestUtils() {}
 }
diff --git a/testing/src/test/java/io/grpc/testing/TestUtilsTest.java b/testing/src/test/java/io/grpc/testing/TestUtilsTest.java
new file mode 100644
index 0000000000..3265e45114
--- /dev/null
+++ b/testing/src/test/java/io/grpc/testing/TestUtilsTest.java
@@ -0,0 +1,56 @@
+/*
+ * Copyright 2017, Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package io.grpc.testing;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import java.util.concurrent.TimeUnit;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Unit tests for {@link TestUtils}.
+ */
+@RunWith(JUnit4.class)
+public class TestUtilsTest {
+  @Test
+  public void sleepAtLeast() throws Exception {
+    long sleepMilis = 10L;
+
+    long start = System.nanoTime();
+    TestUtils.sleepAtLeast(sleepMilis);
+    long end = System.nanoTime();
+
+    assertThat(end - start).isAtLeast(TimeUnit.MILLISECONDS.toNanos(sleepMilis));
+  }
+}