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); } + } }