Optimize NodeMetricsClientTest

1 files changed, 21 insertions, 30 deletions

diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/NodeMetricsClientTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/NodeMetricsClientTest.java
index b0852776a89..93212889b3c 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/NodeMetricsClientTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/NodeMetricsClientTest.java
@@ -6,50 +6,59 @@ package ai.vespa.metricsproxy.http.application;
 
 import ai.vespa.metricsproxy.http.application.NodeMetricsClient.Node;
 import ai.vespa.metricsproxy.metric.model.MetricsPacket;
-import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import com.github.tomakehurst.wiremock.junit.WireMockClassRule;
 import com.yahoo.test.ManualClock;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClients;
 import org.junit.Before;
-import org.junit.Rule;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
 import org.junit.Test;
 
-import java.io.IOException;
-import java.net.ServerSocket;
 import java.util.List;
 
 import static ai.vespa.metricsproxy.TestUtil.getFileContents;
 import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
 import static com.github.tomakehurst.wiremock.client.WireMock.get;
-import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
 import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
 import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
 import static org.junit.Assert.assertEquals;
 
 /**
+ * Two optimizations worth noting:
+ *
+ * 1. Using a ClassRule for the wire mocking means it is reused between test methods.
+ * 2. Configuring stubs on the rule is faster than using the static WireMock.stubFor method.
+ *
  * @author gjoranv
  */
 public class NodeMetricsClientTest {
 
     private static final String TEST_FILE = "generic-sample.json";
     private static final String RESPONSE = getFileContents(TEST_FILE);
-    private static final int PORT = getAvailablePort();
-
     private static final CloseableHttpClient httpClient = HttpClients.createDefault();
 
-    private final Node node = new Node("id", "localhost", PORT);
+    private static Node node;
+
     private ManualClock clock;
     private NodeMetricsClient nodeMetricsClient;
 
+    @ClassRule
+    public static WireMockClassRule wireMockRule = new WireMockClassRule(options().dynamicPort());
+
+    @BeforeClass
+    public static void setupWireMock() {
+        node = new Node("id", "localhost", wireMockRule.port());
+        wireMockRule.stubFor(get(urlEqualTo(node.metricsUri.getPath()))
+                                     .willReturn(aResponse().withBody(RESPONSE)));
+    }
+
     @Before
-    public void setup() {
+    public void setupClient() {
         clock = new ManualClock();
         nodeMetricsClient = new NodeMetricsClient(httpClient, node, clock);
     }
 
-    @Rule
-    public WireMockRule wireMockRule = new WireMockRule(options().port(PORT));
-
     @Test
     public void metrics_are_not_retrieved_until_first_request() {
         assertEquals(0, nodeMetricsClient.snapshotsRetrieved());
@@ -57,9 +66,6 @@ public class NodeMetricsClientTest {
 
     @Test
     public void metrics_are_retrieved_upon_first_request() {
-        stubFor(get(urlEqualTo(node.metricsUri.getPath()))
-                        .willReturn(aResponse().withBody(RESPONSE)));
-
         List<MetricsPacket.Builder> metrics = nodeMetricsClient.getMetrics();
         assertEquals(1, nodeMetricsClient.snapshotsRetrieved());
         assertEquals(4, metrics.size());
@@ -67,9 +73,6 @@ public class NodeMetricsClientTest {
 
     @Test
     public void cached_metrics_are_used_when_ttl_has_not_expired() {
-        stubFor(get(urlEqualTo(node.metricsUri.getPath()))
-                        .willReturn(aResponse().withBody(RESPONSE)));
-
         nodeMetricsClient.getMetrics();
         assertEquals(1, nodeMetricsClient.snapshotsRetrieved());
 
@@ -80,9 +83,6 @@ public class NodeMetricsClientTest {
 
     @Test
     public void metrics_are_refreshed_when_ttl_has_expired() {
-        stubFor(get(urlEqualTo(node.metricsUri.getPath()))
-                        .willReturn(aResponse().withBody(RESPONSE)));
-
         nodeMetricsClient.getMetrics();
         assertEquals(1, nodeMetricsClient.snapshotsRetrieved());
 
@@ -91,13 +91,4 @@ public class NodeMetricsClientTest {
         assertEquals(2, nodeMetricsClient.snapshotsRetrieved());
     }
 
-    private static int getAvailablePort() {
-        try (ServerSocket socket = new ServerSocket(0)) {
-            socket.setReuseAddress(true);
-            return socket.getLocalPort();
-        } catch (IOException e) {
-            throw new RuntimeException("Could not find available port: ", e);
-        }
-    }
-
 }