Repository: zookeeper
Updated Branches:
  refs/heads/branch-3.5 20dafe7be -> c89784cc5


ZOOKEEPER-2786: Flaky test: 
org.apache.zookeeper.test.ClientTest.testNonExistingOpCode

On branch 3.5, testNonExistingOpCode appears to always take 30 seconds to 
execute (far too long): 
https://builds.apache.org/job/ZooKeeper_branch35_jdk7/967/testReport/org.apache.zookeeper.test/ClientTest/testNonExistingOpCode/history/

The reason for this is a call to 
`zk.testableWaitForShutdown(CONNECTION_TIMEOUT)` while waiting for the client 
to disconnect after it sent a request with a bad opcode to the server. The call 
to `testableWaitForShutdown` never actually asserts anything and effectively 
just hangs for CONNECTION_TIMEOUT (30 seconds) and returns because the client 
reconnects to the server.

This patch replaces `zk.testableWaitForShutdown(CONNECTION_TIMEOUT)` with 
`watcher.waitForDisconnected(5000)` which is a better way to detect if we have 
been disconnected.

Author: Abraham Fine <af...@apache.org>

Reviewers: Michael Han <h...@apache.org>

Closes #259 from afine/ZOOKEEPER-2786_br3.5


Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/c89784cc
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/c89784cc
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/c89784cc

Branch: refs/heads/branch-3.5
Commit: c89784cc57bdcc88de89da6993d758225585d33e
Parents: 20dafe7
Author: Abraham Fine <af...@apache.org>
Authored: Thu Jun 15 15:34:59 2017 -0700
Committer: Michael Han <h...@apache.org>
Committed: Thu Jun 15 15:34:59 2017 -0700

----------------------------------------------------------------------
 src/java/test/org/apache/zookeeper/TestableZooKeeper.java | 6 ------
 src/java/test/org/apache/zookeeper/test/ClientTest.java   | 7 +++++--
 2 files changed, 5 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/c89784cc/src/java/test/org/apache/zookeeper/TestableZooKeeper.java
----------------------------------------------------------------------
diff --git a/src/java/test/org/apache/zookeeper/TestableZooKeeper.java 
b/src/java/test/org/apache/zookeeper/TestableZooKeeper.java
index c69033c..cd4c53c 100644
--- a/src/java/test/org/apache/zookeeper/TestableZooKeeper.java
+++ b/src/java/test/org/apache/zookeeper/TestableZooKeeper.java
@@ -96,12 +96,6 @@ public class TestableZooKeeper extends ZooKeeperAdmin {
             return false;
         }
     }
-    
-    public boolean testableWaitForShutdown(int wait)
-        throws InterruptedException
-    {
-        return super.testableWaitForShutdown(wait);
-    }
 
     public SocketAddress testableLocalSocketAddress() {
         return super.testableLocalSocketAddress();

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/c89784cc/src/java/test/org/apache/zookeeper/test/ClientTest.java
----------------------------------------------------------------------
diff --git a/src/java/test/org/apache/zookeeper/test/ClientTest.java 
b/src/java/test/org/apache/zookeeper/test/ClientTest.java
index 1aaef75..ffc81c1 100644
--- a/src/java/test/org/apache/zookeeper/test/ClientTest.java
+++ b/src/java/test/org/apache/zookeeper/test/ClientTest.java
@@ -828,7 +828,8 @@ public class ClientTest extends ClientBase {
      */
     @Test
     public void testNonExistingOpCode() throws Exception  {
-        TestableZooKeeper zk = createClient();
+        CountdownWatcher watcher = new CountdownWatcher();
+        TestableZooKeeper zk = createClient(watcher);
 
         final String path = "/m1";
 
@@ -841,7 +842,9 @@ public class ClientTest extends ClientBase {
         ReplyHeader r = zk.submitRequest(h, request, response, null);
 
         Assert.assertEquals(r.getErr(), Code.UNIMPLEMENTED.intValue());
-        zk.testableWaitForShutdown(CONNECTION_TIMEOUT);
+
+        // Sending a nonexisting opcode should cause the server to disconnect
+        watcher.waitForDisconnected(5000);
     }
 
     @Test

Reply via email to