[TEST] Fix ClusterApplierServiceTests.testClusterStateUpdateLogging

This changes the test to not use a `CountDownlatch`, instead adding an assertion for the final logging message and waiting until the `MockAppender` has seen it before proceeding. Resolves #23739
elastic · Nov 15, 2018 · df2c06f · df2c06f
1 parent ee9a271
commit df2c06f
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 15 deletions.
diff --git a/server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java
@@ -128,19 +128,22 @@ public void testClusterStateUpdateLogging() throws Exception {
                         clusterApplierService.getClass().getCanonicalName(),
                         Level.TRACE,
                         "*failed to execute cluster state applier in [2s]*"));
+        mockAppender.addExpectation(
+            new MockLogAppender.SeenEventExpectation(
+                "test3",
+                clusterApplierService.getClass().getCanonicalName(),
+                Level.DEBUG,
+                "*processing [test3]: took [0s] no change in cluster state*"));
 
         Logger clusterLogger = LogManager.getLogger("org.elasticsearch.cluster.service");
         Loggers.addAppender(clusterLogger, mockAppender);
         try {
-            final CountDownLatch latch = new CountDownLatch(3);
             clusterApplierService.currentTimeOverride = System.nanoTime();
             clusterApplierService.runOnApplierThread("test1",
                 currentState -> clusterApplierService.currentTimeOverride += TimeValue.timeValueSeconds(1).nanos(),
                 new ClusterApplyListener() {
                     @Override
-                    public void onSuccess(String source) {
-                        latch.countDown();
-                    }
+                    public void onSuccess(String source) { }
 
                     @Override
                     public void onFailure(String source, Exception e) {
@@ -159,31 +162,25 @@ public void onSuccess(String source) {
                     }
 
                     @Override
-                    public void onFailure(String source, Exception e) {
-                        latch.countDown();
-                    }
+                    public void onFailure(String source, Exception e) { }
                 });
             // Additional update task to make sure all previous logging made it to the loggerName
-            // We don't check logging for this on since there is no guarantee that it will occur before our check
             clusterApplierService.runOnApplierThread("test3",
                 currentState -> {},
                 new ClusterApplyListener() {
                     @Override
-                    public void onSuccess(String source) {
-                        latch.countDown();
-                    }
+                    public void onSuccess(String source) { }
 
                     @Override
                     public void onFailure(String source, Exception e) {
                         fail();
                     }
                 });
-            latch.await();
+            assertBusy(mockAppender::assertAllExpectationsMatched);
         } finally {
             Loggers.removeAppender(clusterLogger, mockAppender);
             mockAppender.stop();
         }
-        mockAppender.assertAllExpectationsMatched();
     }
 
     @TestLogging("org.elasticsearch.cluster.service:WARN") // To ensure that we log cluster state events on WARN level

diff --git a/test/framework/src/main/java/org/elasticsearch/test/MockLogAppender.java b/test/framework/src/main/java/org/elasticsearch/test/MockLogAppender.java
@@ -112,7 +112,7 @@ public UnseenEventExpectation(String name, String logger, Level level, String me
 
         @Override
         public void assertMatched() {
-            assertThat(name, saw, equalTo(false));
+            assertThat("expected to see " + name + " but did not", saw, equalTo(false));
         }
     }
 
@@ -124,7 +124,7 @@ public SeenEventExpectation(String name, String logger, Level level, String mess
 
         @Override
         public void assertMatched() {
-            assertThat(name, saw, equalTo(true));
+            assertThat("expected to see " + name + " but did not", saw, equalTo(true));
         }
     }