This is an automated email from the ASF dual-hosted git repository.

eolivelli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/curator.git


The following commit(s) were added to refs/heads/master by this push:
     new 9aafdec  CURATOR-610: Refactor CountCuratorWatcher in 
TestWatcherIdentity.java …
9aafdec is described below

commit 9aafdec9f2607cc6b652371a7687ed38889ddd2b
Author: wx930910 <wx19930...@gmail.com>
AuthorDate: Mon Aug 9 14:07:51 2021 +0200

    CURATOR-610: Refactor CountCuratorWatcher in TestWatcherIdentity.java …
    
    Fixes [CURATOR-610](https://issues.apache.org/jira/browse/CURATOR-610)
    
    ### Description
    Refactor test class 
[CountCuratorWatcher](https://github.com/apache/curator/blob/4a11aaef8b190dc220d35b7a91df294bfa06250e/curator-framework/src/test/java/org/apache/curator/framework/imps/TestWatcherIdentity.java#L42)
 by using mocking object created by Mockito.
    
    <hr>
    
    ##### Key changed/added classes in this PR
    - Create mocking object to replace test subclass `CountCuratorWatcher`, 
decouple test from production code.
    - Make test logic more clear by using method stub instead of method 
overriding.
    - Extract AtomicInteger variable out of the test subclass. Use the 
extracted variable in assertation statement to check `process(WatchedEvent)` 
method invocation status.
    
    <hr>
    
    Author: wx930910 <wx19930...@gmail.com>
    
    Reviewers: Enrico Olivelli <eolive...@apache.org>
    
    Closes #397 from wx930910/CURATOR-610
---
 curator-framework/pom.xml                          |   7 +
 .../framework/imps/TestWatcherIdentity.java        | 286 ++++++++++-----------
 2 files changed, 143 insertions(+), 150 deletions(-)

diff --git a/curator-framework/pom.xml b/curator-framework/pom.xml
index 867b14c..f883546 100644
--- a/curator-framework/pom.xml
+++ b/curator-framework/pom.xml
@@ -86,6 +86,13 @@
             <artifactId>slf4j-log4j12</artifactId>
             <scope>test</scope>
         </dependency>
+        
+        <dependency>
+            <groupId>org.mockito</groupId>
+             <artifactId>mockito-core</artifactId>
+             <scope>test</scope>
+        </dependency>
+        
         <dependency>
             <groupId>org.awaitility</groupId>
             <artifactId>awaitility</artifactId>
diff --git 
a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestWatcherIdentity.java
 
b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestWatcherIdentity.java
index 1d16450..70e99ca 100644
--- 
a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestWatcherIdentity.java
+++ 
b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestWatcherIdentity.java
@@ -18,8 +18,6 @@
  */
 package org.apache.curator.framework.imps;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import com.google.common.collect.Sets;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.CuratorFrameworkFactory;
@@ -30,169 +28,157 @@ import org.apache.curator.test.Timing;
 import org.apache.curator.utils.CloseableUtils;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
+import org.awaitility.Awaitility;
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+
 public class TestWatcherIdentity extends BaseClassForTests
 {
-    private static final String PATH = "/foo";
+  private static final String PATH = "/foo";
+  private static final int TIMEOUT_MS = 100000;
 
-    private static class CountCuratorWatcher implements CuratorWatcher
-    {
-        private final AtomicInteger count = new AtomicInteger(0);
-
-        @Override
-        public void process(WatchedEvent event) throws Exception
-        {
-            count.incrementAndGet();
-        }
-    }
+  private static class CountZKWatcher implements Watcher
+  {
+    private final AtomicInteger count = new AtomicInteger(0);
 
-    private static class CountZKWatcher implements Watcher
+    @Override
+    public void process(WatchedEvent event)
     {
-        private final AtomicInteger count = new AtomicInteger(0);
-
-        @Override
-        public void process(WatchedEvent event)
-        {
-            count.incrementAndGet();
-        }
+      count.incrementAndGet();
     }
-
-    @Test
-    public void testSameWatcherPerZKDocs() throws Exception
-    {
-        CountZKWatcher actualWatcher = new CountZKWatcher();
-        Timing timing = new Timing();
-        CuratorFramework client = 
CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), 
timing.connection(), new RetryOneTime(1));
-        try
-        {
-            client.start();
-            client.create().forPath("/test");
-
-            // per ZK docs, this watcher should only trigger once
-            client.checkExists().usingWatcher(actualWatcher).forPath("/test");
-            client.getData().usingWatcher(actualWatcher).forPath("/test");
-
-            client.setData().forPath("/test", "foo".getBytes());
-            client.delete().forPath("/test");
-            timing.sleepABit();
-            assertEquals(actualWatcher.count.getAndSet(0), 1);
-
-            client.create().forPath("/test");
-            client.checkExists().usingWatcher(actualWatcher).forPath("/test");
-            client.delete().forPath("/test");
-            timing.sleepABit();
-            assertEquals(actualWatcher.count.get(), 1);
-        }
-        finally
-        {
-            CloseableUtils.closeQuietly(client);
-        }
+  }
+
+  @Test
+  public void testSameWatcherPerZKDocs() throws Exception
+  {
+    CountZKWatcher actualWatcher = new CountZKWatcher();
+    Timing timing = new Timing();
+    CuratorFramework client = 
CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(),
+        timing.connection(), new RetryOneTime(1));
+    try {
+      client.start();
+      client.create().forPath("/test");
+
+      // per ZK docs, this watcher should only trigger once
+      client.checkExists().usingWatcher(actualWatcher).forPath("/test");
+      client.getData().usingWatcher(actualWatcher).forPath("/test");
+
+      client.setData().forPath("/test", "foo".getBytes());
+      client.delete().forPath("/test");
+      Awaitility.await()
+          .untilAsserted(() -> assertEquals(1, 
actualWatcher.count.getAndSet(0)));
+
+      client.create().forPath("/test");
+      client.checkExists().usingWatcher(actualWatcher).forPath("/test");
+      client.delete().forPath("/test");
+      Awaitility.await()
+          .untilAsserted(() -> assertEquals(1, actualWatcher.count.get()));
+    } finally {
+      CloseableUtils.closeQuietly(client);
     }
-
-    @Test
-    public void testSameCuratorWatcherPerZKDocs() throws Exception
-    {
-        CountCuratorWatcher actualWatcher = new CountCuratorWatcher();
-        Timing timing = new Timing();
-        CuratorFramework client = 
CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), 
timing.connection(), new RetryOneTime(1));
-        try
-        {
-            client.start();
-            client.create().forPath("/test");
-
-            // per ZK docs, this watcher should only trigger once
-            client.checkExists().usingWatcher(actualWatcher).forPath("/test");
-            client.getData().usingWatcher(actualWatcher).forPath("/test");
-
-            client.setData().forPath("/test", "foo".getBytes());
-            client.delete().forPath("/test");
-            timing.sleepABit();
-            assertEquals(actualWatcher.count.getAndSet(0), 1);
-
-            client.create().forPath("/test");
-            client.checkExists().usingWatcher(actualWatcher).forPath("/test");
-            client.delete().forPath("/test");
-            timing.sleepABit();
-            assertEquals(actualWatcher.count.get(), 1);
-        }
-        finally
-        {
-            CloseableUtils.closeQuietly(client);
-        }
+  }
+
+  @Test
+  public void testSameCuratorWatcherPerZKDocs() throws Exception
+  {
+    // Construct mock object
+    CuratorWatcher actualWatcher = mock(CuratorWatcher.class);
+
+    Timing timing = new Timing();
+    CuratorFramework client = 
CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(),
+        timing.connection(), new RetryOneTime(1));
+    try {
+      client.start();
+      client.create().forPath("/test");
+
+      // per ZK docs, this watcher should only trigger once
+      client.checkExists().usingWatcher(actualWatcher).forPath("/test");
+      client.getData().usingWatcher(actualWatcher).forPath("/test");
+
+      client.setData().forPath("/test", "foo".getBytes());
+      client.delete().forPath("/test");
+      Mockito.verify(actualWatcher, 
Mockito.timeout(TIMEOUT_MS).times(1)).process(any(WatchedEvent.class));
+
+      client.create().forPath("/test");
+      client.checkExists().usingWatcher(actualWatcher).forPath("/test");
+      client.delete().forPath("/test");
+      Mockito.verify(actualWatcher, 
Mockito.timeout(TIMEOUT_MS).times(2)).process(any(WatchedEvent.class));
+    } finally {
+      CloseableUtils.closeQuietly(client);
     }
+  }
 
-    @Test
-    public void testSetAddition()
+  @Test
+  public void testSetAddition()
+  {
+    Watcher watcher = new Watcher()
     {
-        Watcher watcher = new Watcher()
-        {
-            @Override
-            public void process(WatchedEvent event)
-            {
-
-            }
-        };
-        NamespaceWatcher namespaceWatcher1 = new NamespaceWatcher(null, 
watcher, "/foo");
-        NamespaceWatcher namespaceWatcher2 = new NamespaceWatcher(null, 
watcher, "/foo");
-        assertEquals(namespaceWatcher1, namespaceWatcher2);
-        assertFalse(namespaceWatcher1.equals(watcher));
-        assertFalse(watcher.equals(namespaceWatcher1));
-        Set<Watcher> set = Sets.newHashSet();
-        set.add(namespaceWatcher1);
-        set.add(namespaceWatcher2);
-        assertEquals(set.size(), 1);
+      @Override
+      public void process(WatchedEvent event)
+      {
+
+      }
+    };
+    NamespaceWatcher namespaceWatcher1 = new NamespaceWatcher(null, watcher, 
"/foo");
+    NamespaceWatcher namespaceWatcher2 = new NamespaceWatcher(null, watcher, 
"/foo");
+    assertEquals(namespaceWatcher1, namespaceWatcher2);
+    assertFalse(namespaceWatcher1.equals(watcher));
+    assertFalse(watcher.equals(namespaceWatcher1));
+    Set<Watcher> set = Sets.newHashSet();
+    set.add(namespaceWatcher1);
+    set.add(namespaceWatcher2);
+    assertEquals(set.size(), 1);
+  }
+
+  @Test
+  public void testCuratorWatcher() throws Exception
+  {
+    Timing timing = new Timing();
+    // Construct mock object
+    CuratorWatcher watcher = mock(CuratorWatcher.class);
+    CuratorFramework client = 
CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(),
+        timing.connection(), new RetryOneTime(1));
+    try {
+      client.start();
+      client.create().forPath(PATH);
+      // Add twice the same watcher on the same path
+      client.getData().usingWatcher(watcher).forPath(PATH);
+      client.getData().usingWatcher(watcher).forPath(PATH);
+      // Ok, let's test it
+      client.setData().forPath(PATH, new byte[] {});
+      Mockito.verify(watcher, 
Mockito.timeout(TIMEOUT_MS).times(1)).process(any(WatchedEvent.class));
+    } finally {
+      CloseableUtils.closeQuietly(client);
     }
-
-    @Test
-    public void testCuratorWatcher() throws Exception
-    {
-        Timing timing = new Timing();
-        CountCuratorWatcher watcher = new CountCuratorWatcher();
-        CuratorFramework client = 
CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), 
timing.connection(), new RetryOneTime(1));
-        try
-        {
-            client.start();
-            client.create().forPath(PATH);
-            // Add twice the same watcher on the same path
-            client.getData().usingWatcher(watcher).forPath(PATH);
-            client.getData().usingWatcher(watcher).forPath(PATH);
-            // Ok, let's test it
-            client.setData().forPath(PATH, new byte[]{});
-            timing.sleepABit();
-            assertEquals(1, watcher.count.get());
-        }
-        finally
-        {
-            CloseableUtils.closeQuietly(client);
-        }
-    }
-
-
-    @Test
-    public void testZKWatcher() throws Exception
-    {
-        Timing timing = new Timing();
-        CountZKWatcher watcher = new CountZKWatcher();
-        CuratorFramework client = 
CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), 
timing.connection(), new RetryOneTime(1));
-        try
-        {
-            client.start();
-            client.create().forPath(PATH);
-            // Add twice the same watcher on the same path
-            client.getData().usingWatcher(watcher).forPath(PATH);
-            client.getData().usingWatcher(watcher).forPath(PATH);
-            // Ok, let's test it
-            client.setData().forPath(PATH, new byte[]{});
-            timing.sleepABit();
-            assertEquals(1, watcher.count.get());
-        }
-        finally
-        {
-            CloseableUtils.closeQuietly(client);
-        }
+  }
+
+  @Test
+  public void testZKWatcher() throws Exception
+  {
+    Timing timing = new Timing();
+    CountZKWatcher watcher = new CountZKWatcher();
+    CuratorFramework client = 
CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(),
+        timing.connection(), new RetryOneTime(1));
+    try {
+      client.start();
+      client.create().forPath(PATH);
+      // Add twice the same watcher on the same path
+      client.getData().usingWatcher(watcher).forPath(PATH);
+      client.getData().usingWatcher(watcher).forPath(PATH);
+      // Ok, let's test it
+      client.setData().forPath(PATH, new byte[] {});
+      Awaitility.await()
+          .untilAsserted(() -> assertEquals(1, watcher.count.get()));
+    } finally {
+      CloseableUtils.closeQuietly(client);
     }
+  }
 }

Reply via email to