Adding asserts in unit tests for no leader or failure checks

Signed-off-by: Harsh Garg <[email protected]>
opensearch-project · Apr 9, 2024 · 6166519 · 6166519
1 parent 48e38b0
commit 6166519
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 13 deletions.
diff --git a/server/src/main/java/org/opensearch/cluster/coordination/ClusterManagerMetrics.java b/server/src/main/java/org/opensearch/cluster/coordination/ClusterManagerMetrics.java
@@ -26,12 +26,12 @@ public final class ClusterManagerMetrics {
     public final Counter followerChecksFailureCounter;
 
     public ClusterManagerMetrics(MetricsRegistry metricsRegistry) {
-        this.followerChecksFailureCounter = metricsRegistry.createCounter(
+        followerChecksFailureCounter = metricsRegistry.createCounter(
             "followers.checker.failure.count",
             "Counter for number of failed follower checks",
             COUNTER_METRICS_UNIT
         );
-        this.leaderCheckFailureCounter = metricsRegistry.createCounter(
+        leaderCheckFailureCounter = metricsRegistry.createCounter(
             "leader.checker.failure.count",
             "Counter for number of failed leader checks",
             COUNTER_METRICS_UNIT

diff --git a/server/src/test/java/org/opensearch/cluster/coordination/FollowersCheckerTests.java b/server/src/test/java/org/opensearch/cluster/coordination/FollowersCheckerTests.java
@@ -134,6 +134,8 @@ protected void onSendRequest(long requestId, String action, TransportRequest req
         transportService.start();
         transportService.acceptIncomingRequests();
 
+        TestInMemoryMetricsRegistry metricsRegistry = new TestInMemoryMetricsRegistry();
+
         final FollowersChecker followersChecker = new FollowersChecker(
             settings,
             clusterSettings,
@@ -143,7 +145,7 @@ protected void onSendRequest(long requestId, String action, TransportRequest req
                 assert false : node;
             },
             () -> new StatusInfo(StatusInfo.Status.HEALTHY, "healthy-info"),
-            new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)
+            new ClusterManagerMetrics(metricsRegistry)
         );
 
         followersChecker.setCurrentNodes(discoveryNodesHolder[0]);
@@ -197,6 +199,7 @@ protected void onSendRequest(long requestId, String action, TransportRequest req
         followersChecker.clearCurrentNodes();
         deterministicTaskQueue.runAllTasks();
         assertThat(checkedNodes, empty());
+        assertEquals(Integer.valueOf(0), metricsRegistry.getCounterStore().get("followers.checker.failure.count").getCounterValue());
     }
 
     public void testFailsNodeThatDoesNotRespond() {
@@ -313,6 +316,7 @@ public String toString() {
         transportService.acceptIncomingRequests();
 
         final AtomicBoolean nodeFailed = new AtomicBoolean();
+        TestInMemoryMetricsRegistry metricsRegistry = new TestInMemoryMetricsRegistry();
 
         final FollowersChecker followersChecker = new FollowersChecker(
             settings,
@@ -324,7 +328,7 @@ public String toString() {
                 assertThat(reason, equalTo("disconnected"));
             },
             () -> new StatusInfo(HEALTHY, "healthy-info"),
-            new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)
+            new ClusterManagerMetrics(metricsRegistry)
         );
 
         DiscoveryNodes discoveryNodes = DiscoveryNodes.builder().add(localNode).add(otherNode).localNodeId(localNode.getId()).build();
@@ -335,6 +339,7 @@ public String toString() {
         deterministicTaskQueue.runAllRunnableTasks();
         assertTrue(nodeFailed.get());
         assertThat(followersChecker.getFaultyNodes(), contains(otherNode));
+        assertEquals(Integer.valueOf(1), metricsRegistry.getCounterStore().get("followers.checker.failure.count").getCounterValue());
     }
 
     public void testFailsNodeThatIsUnhealthy() {

diff --git a/server/src/test/java/org/opensearch/cluster/coordination/LeaderCheckerTests.java b/server/src/test/java/org/opensearch/cluster/coordination/LeaderCheckerTests.java
@@ -45,7 +45,6 @@
 import org.opensearch.core.transport.TransportResponse.Empty;
 import org.opensearch.monitor.StatusInfo;
 import org.opensearch.telemetry.TestInMemoryMetricsRegistry;
-import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry;
 import org.opensearch.telemetry.tracing.noop.NoopTracer;
 import org.opensearch.test.EqualsHashCodeTestUtils;
 import org.opensearch.test.EqualsHashCodeTestUtils.CopyFunction;
@@ -466,7 +465,8 @@ public void testLeaderBehaviour() {
         transportService.start();
         transportService.acceptIncomingRequests();
 
-        final ClusterManagerMetrics clusterManagerMetrics = new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE);
+        final TestInMemoryMetricsRegistry metricsRegistry = new TestInMemoryMetricsRegistry();
+        final ClusterManagerMetrics clusterManagerMetrics = new ClusterManagerMetrics(metricsRegistry);
         final LeaderChecker leaderChecker = new LeaderChecker(
             settings,
             clusterSettings,
@@ -538,6 +538,7 @@ public void testLeaderBehaviour() {
                 equalTo("rejecting leader check from [" + otherNode + "] sent to a node that is no longer the cluster-manager")
             );
         }
+        assertEquals(Integer.valueOf(0), metricsRegistry.getCounterStore().get("leader.checker.failure.count").getCounterValue());
     }
 
     private class CapturingTransportResponseHandler implements TransportResponseHandler<Empty> {

diff --git a/server/src/test/java/org/opensearch/telemetry/TestInMemoryMetricsRegistry.java b/server/src/test/java/org/opensearch/telemetry/TestInMemoryMetricsRegistry.java
@@ -25,13 +25,8 @@
  */
 public class TestInMemoryMetricsRegistry implements MetricsRegistry {
 
-    private ConcurrentHashMap<String, TestInMemoryCounter> counterStore;
-    private ConcurrentHashMap<String, TestInMemoryHistogram> histogramStore;
-
-    public TestInMemoryMetricsRegistry() {
-        this.counterStore = new ConcurrentHashMap<>();
-        this.histogramStore = new ConcurrentHashMap<>();
-    }
+    private ConcurrentHashMap<String, TestInMemoryCounter> counterStore = new ConcurrentHashMap<>();
+    private ConcurrentHashMap<String, TestInMemoryHistogram> histogramStore = new ConcurrentHashMap<>();
 
     public ConcurrentHashMap<String, TestInMemoryCounter> getCounterStore() {
         return this.counterStore;