Merge pull request #1795 from metamx/announcerTestTransactions

Try and make AnnouncerTest a bit more predictable
apache · Oct 2, 2015 · 166c4fc · 166c4fc
2 parents 42e971d + 7d635a2
commit 166c4fc
Showing 1 changed file with 60 additions and 39 deletions.
diff --git a/server/src/test/java/io/druid/curator/announcement/AnnouncerTest.java b/server/src/test/java/io/druid/curator/announcement/AnnouncerTest.java
@@ -56,10 +56,11 @@ public void tearDown()
     tearDownServerAndCurator();
   }
 
-  @Test
+  @Test(timeout = 60_000L)
   public void testSanity() throws Exception
   {
     curator.start();
+    curator.blockUntilConnected();
     Announcer announcer = new Announcer(curator, exec);
 
     final byte[] billy = "billy".getBytes();
@@ -72,50 +73,66 @@ public void testSanity() throws Exception
 
     announcer.start();
 
-    Assert.assertArrayEquals("/test1 has data", billy, curator.getData().decompressed().forPath(testPath1));
-    Assert.assertNull("/somewhere/test2 still does not exist", curator.checkExists().forPath(testPath2));
+    try {
+      Assert.assertArrayEquals("/test1 has data", billy, curator.getData().decompressed().forPath(testPath1));
+      Assert.assertNull("/somewhere/test2 still does not exist", curator.checkExists().forPath(testPath2));
 
-    announcer.announce(testPath2, billy);
+      announcer.announce(testPath2, billy);
 
-    Assert.assertArrayEquals("/test1 still has data", billy, curator.getData().decompressed().forPath(testPath1));
-    Assert.assertArrayEquals("/somewhere/test2 has data", billy, curator.getData().decompressed().forPath(testPath2));
+      Assert.assertArrayEquals("/test1 still has data", billy, curator.getData().decompressed().forPath(testPath1));
+      Assert.assertArrayEquals("/somewhere/test2 has data", billy, curator.getData().decompressed().forPath(testPath2));
 
-    final CountDownLatch latch = new CountDownLatch(1);
-    curator.getCuratorListenable().addListener(
-        new CuratorListener()
-        {
-          @Override
-          public void eventReceived(CuratorFramework client, CuratorEvent event) throws Exception
+      final CountDownLatch latch = new CountDownLatch(1);
+      curator.getCuratorListenable().addListener(
+          new CuratorListener()
           {
-            if (event.getType() == CuratorEventType.CREATE && event.getPath().equals(testPath1)) {
-              latch.countDown();
+            @Override
+            public void eventReceived(CuratorFramework client, CuratorEvent event) throws Exception
+            {
+              if (event.getType() == CuratorEventType.CREATE && event.getPath().equals(testPath1)) {
+                latch.countDown();
+              }
             }
           }
-        }
-    );
-    curator.delete().forPath(testPath1);
-    Assert.assertTrue("Wait for /test1 to be created", timing.forWaiting().awaitLatch(latch));
-
-    Assert.assertArrayEquals("expect /test1 data is restored", billy, curator.getData().decompressed().forPath(testPath1));
-    Assert.assertArrayEquals("expect /somewhere/test2 is still there", billy, curator.getData().decompressed().forPath(testPath2));
+      );
+      curator.inTransaction().delete().forPath(testPath1).and().commit();
+      Assert.assertTrue("Wait for /test1 to be created", timing.forWaiting().awaitLatch(latch));
 
-    announcer.unannounce(testPath1);
-    Assert.assertNull("expect /test1 unannounced", curator.checkExists().forPath(testPath1));
-    Assert.assertArrayEquals("expect /somewhere/test2 is still still there", billy, curator.getData().decompressed().forPath(testPath2));
+      Assert.assertArrayEquals(
+          "expect /test1 data is restored",
+          billy,
+          curator.getData().decompressed().forPath(testPath1)
+      );
+      Assert.assertArrayEquals(
+          "expect /somewhere/test2 is still there",
+          billy,
+          curator.getData().decompressed().forPath(testPath2)
+      );
 
-    announcer.stop();
+      announcer.unannounce(testPath1);
+      Assert.assertNull("expect /test1 unannounced", curator.checkExists().forPath(testPath1));
+      Assert.assertArrayEquals(
+          "expect /somewhere/test2 is still still there",
+          billy,
+          curator.getData().decompressed().forPath(testPath2)
+      );
+    }
+    finally {
+      announcer.stop();
+    }
 
     Assert.assertNull("expect /test1 remains unannounced", curator.checkExists().forPath(testPath1));
     Assert.assertNull("expect /somewhere/test2 unannounced", curator.checkExists().forPath(testPath2));
   }
 
-  @Test
+  @Test(timeout = 60_000L)
   public void testSessionKilled() throws Exception
   {
     curator.start();
+    curator.blockUntilConnected();
     Announcer announcer = new Announcer(curator, exec);
     try {
-      curator.create().forPath("/somewhere");
+      curator.inTransaction().create().forPath("/somewhere").and().commit();
       announcer.start();
 
       final byte[] billy = "billy".getBytes();
@@ -172,14 +189,16 @@ public void testCleansUpItsLittleTurdlings() throws Exception
     final String parent = ZKPaths.getPathAndNode(testPath).getPath();
 
     announcer.start();
+    try {
+      Assert.assertNull(curator.checkExists().forPath(parent));
 
-    Assert.assertNull(curator.checkExists().forPath(parent));
-
-    announcer.announce(testPath, billy);
-
-    Assert.assertNotNull(curator.checkExists().forPath(parent));
+      announcer.announce(testPath, billy);
 
-    announcer.stop();
+      Assert.assertNotNull(curator.checkExists().forPath(parent));
+    }
+    finally {
+      announcer.stop();
+    }
 
     Assert.assertNull(curator.checkExists().forPath(parent));
   }
@@ -198,14 +217,16 @@ public void testLeavesBehindTurdlingsThatAlreadyExisted() throws Exception
     final Stat initialStat = curator.checkExists().forPath(parent);
 
     announcer.start();
+    try {
+      Assert.assertEquals(initialStat.getMzxid(), curator.checkExists().forPath(parent).getMzxid());
 
-    Assert.assertEquals(initialStat.getMzxid(), curator.checkExists().forPath(parent).getMzxid());
-
-    announcer.announce(testPath, billy);
-
-    Assert.assertEquals(initialStat.getMzxid(), curator.checkExists().forPath(parent).getMzxid());
+      announcer.announce(testPath, billy);
 
-    announcer.stop();
+      Assert.assertEquals(initialStat.getMzxid(), curator.checkExists().forPath(parent).getMzxid());
+    }
+    finally {
+      announcer.stop();
+    }
 
     Assert.assertEquals(initialStat.getMzxid(), curator.checkExists().forPath(parent).getMzxid());
   }