Mailing-List: contact commits-help@zookeeper.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@zookeeper.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: hanm@apache.org
To: commits@zookeeper.apache.org
Message-Id: <3d68cd9e962b49aa8a466be1f2142814@git.apache.org>
Subject: zookeeper git commit: ZOOKEEPER-2786: Flaky test:
 org.apache.zookeeper.test.ClientTest.testNonExistingOpCode
Date: Fri, 30 Jun 2017 20:43:48 +0000 (UTC)
archived-at: Fri, 30 Jun 2017 20:43:50 -0000

Repository: zookeeper
Updated Branches:
  refs/heads/branch-3.5 749b6024b -> 74e45a0d6


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

As hanm noticed and mentioned on the JIRA this flaky test can still fail. The reason is that we can reconnect before calling `watcher.waitForDisconnected(5000);` We need to make sure that we only process a single event from the watcher. Using a `synchronized (watcher)` provides that guarantee here.

This issue can be reproduced by dropping a Thread.sleep before `watcher.waitForDisconnected(5000);`.

Author: Abraham Fine <afine@apache.org>

Reviewers: Michael Han <hanm@apache.org>

Closes #286 from afine/ZOOKEEPER-2786_3.5_fix


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

Branch: refs/heads/branch-3.5
Commit: 74e45a0d635702f1a1f57adffdaba9acdc9efee2
Parents: 749b602
Author: Abraham Fine <afine@apache.org>
Authored: Fri Jun 30 13:43:45 2017 -0700
Committer: Michael Han <hanm@apache.org>
Committed: Fri Jun 30 13:43:45 2017 -0700

----------------------------------------------------------------------
 .../org/apache/zookeeper/test/ClientTest.java   | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/74e45a0d/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 ffc81c1..6373bb3 100644
--- a/src/java/test/org/apache/zookeeper/test/ClientTest.java
+++ b/src/java/test/org/apache/zookeeper/test/ClientTest.java
@@ -18,12 +18,11 @@
 
 package org.apache.zookeeper.test;
 
-import static org.junit.Assert.fail;
-
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
 
@@ -33,6 +32,7 @@ import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.KeeperException.InvalidACLException;
 import org.apache.zookeeper.TestableZooKeeper;
 import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.Watcher.Event.EventType;
 import org.apache.zookeeper.Watcher.Event.KeeperState;
 import org.apache.zookeeper.ZooDefs.Ids;
@@ -828,8 +828,16 @@ public class ClientTest extends ClientBase {
      */
     @Test
     public void testNonExistingOpCode() throws Exception  {
-        CountdownWatcher watcher = new CountdownWatcher();
-        TestableZooKeeper zk = createClient(watcher);
+        final CountDownLatch clientDisconnected = new CountDownLatch(1);
+        Watcher watcher = new Watcher() {
+            @Override
+            public synchronized void process(WatchedEvent event) {
+                if (event.getState() == KeeperState.Disconnected) {
+                    clientDisconnected.countDown();
+                }
+            }
+        };
+        TestableZooKeeper zk = new TestableZooKeeper(hostPort, CONNECTION_TIMEOUT, watcher);
 
         final String path = "/m1";
 
@@ -839,12 +847,14 @@ public class ClientTest extends ClientBase {
         request.setPath(path);
         request.setWatch(false);
         ExistsResponse response = new ExistsResponse();
+
         ReplyHeader r = zk.submitRequest(h, request, response, null);
 
         Assert.assertEquals(r.getErr(), Code.UNIMPLEMENTED.intValue());
 
         // Sending a nonexisting opcode should cause the server to disconnect
-        watcher.waitForDisconnected(5000);
+        Assert.assertTrue("failed to disconnect",
+                clientDisconnected.await(5000, TimeUnit.MILLISECONDS));
     }
 
     @Test