Cleanup tests a bit

3 files changed, 49 insertions, 43 deletions

diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateViewTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateViewTest.java
index b7f4b0c7ddb..7427c882ae8 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateViewTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateViewTest.java
@@ -1,4 +1,4 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.clustercontroller.core;
 
 import com.yahoo.vdslib.state.*;
@@ -33,8 +33,6 @@ public class ClusterStateViewTest {
         verify(statsAggregator, never()).updateForDistributor(anyInt(), any());
     }
 
-
-
     @Test
     public void testStateVersionMismatch() {
         when(nodeInfo.isDistributor()).thenReturn(true);
@@ -100,4 +98,5 @@ public class ClusterStateViewTest {
         assert(indices.contains(2));
         assert(indices.contains(6));
     }
+
 }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseTest.java
index 850484188a2..b8c0d2e0c54 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseTest.java
@@ -1,4 +1,4 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.clustercontroller.core;
 
 import com.yahoo.jrt.ErrorCode;
@@ -26,24 +26,6 @@ public class DatabaseTest extends FleetControllerTest {
 
     private static final Logger log = Logger.getLogger(DatabaseTest.class.getName());
 
-    // Note: different semantics than FleetControllerTest.setWantedState
-    private void setWantedState(Node n, NodeState ns, Map<Node, NodeState> wantedStates) {
-        int rpcPort = fleetController.getRpcPort();
-        if (supervisor == null) {
-            supervisor = new Supervisor(new Transport());
-        }
-        Target connection = supervisor.connect(new Spec("localhost", rpcPort));
-        assertTrue(connection.isValid());
-
-        Request req = new Request("setNodeState");
-        req.parameters().add(new StringValue("storage/cluster.mycluster/" + n.getType().toString() + "/" + n.getIndex()));
-        req.parameters().add(new StringValue(ns.serialize(true)));
-        connection.invokeSync(req, timeoutS);
-        assertEquals(req.toString(), ErrorCode.NONE, req.errorCode());
-        assertTrue(req.toString(), req.checkReturnTypes("s"));
-        wantedStates.put(n, ns);
-    }
-
     // These tests work in isolation but causes other tests to hang
     @Ignore
     @Test
@@ -62,42 +44,42 @@ public class DatabaseTest extends FleetControllerTest {
         for (DummyVdsNode node : nodes) {
             wantedStates.put(node.getNode(), new NodeState(node.getType(), State.UP));
         }
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
         log.info("SET A WANTED STATE AND SEE THAT IT GETS PROPAGATED");
         setWantedState(new Node(NodeType.STORAGE, 3), new NodeState(NodeType.STORAGE, State.MAINTENANCE).setDescription("Yoo"), wantedStates);
         waitForState("version:\\d+ distributor:10 storage:10 .3.s:m");
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
         log.info("SET ANOTHER WANTED STATE AND SEE THAT IT GETS PROPAGATED");
         setWantedState(new Node(NodeType.DISTRIBUTOR, 2), new NodeState(NodeType.DISTRIBUTOR, State.DOWN), wantedStates);
         waitForState("version:\\d+ distributor:10 .2.s:d storage:10 .3.s:m");
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
         log.info("SET YET ANOTHER WANTED STATE AND SEE THAT IT GETS PROPAGATED");
         setWantedState(new Node(NodeType.STORAGE, 7), new NodeState(NodeType.STORAGE, State.RETIRED).setDescription("We wanna replace this node"), wantedStates);
         waitForState("version:\\d+ distributor:10 .2.s:d storage:10 .3.s:m .7.s:r");
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
         log.info("CHECK THAT WANTED STATES PERSIST FLEETCONTROLLER RESTART");
         stopFleetController();
         startFleetController();
 
         waitForState("version:\\d+ distributor:10 .2.s:d storage:10 .3.s:m .7.s:r");
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
         log.info("CLEAR WANTED STATE");
         setWantedState(new Node(NodeType.STORAGE, 7), new NodeState(NodeType.STORAGE, State.UP), wantedStates);
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
         setWantedState(new Node(NodeType.DISTRIBUTOR, 5), new NodeState(NodeType.DISTRIBUTOR, State.DOWN), wantedStates);
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
         setWantedState(new Node(NodeType.DISTRIBUTOR, 2), new NodeState(NodeType.DISTRIBUTOR, State.UP), wantedStates);
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
         setWantedState(new Node(NodeType.STORAGE, 9), new NodeState(NodeType.STORAGE, State.DOWN), wantedStates);
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
     }
 
     // These tests work in isolation but causes other tests to hang
@@ -119,31 +101,31 @@ public class DatabaseTest extends FleetControllerTest {
             wantedStates.put(node.getNode(), new NodeState(node.getType(), State.UP));
         }
 
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
         setWantedState(new Node(NodeType.STORAGE, 1), new NodeState(NodeType.STORAGE, State.MAINTENANCE).setDescription("Yoo"), wantedStates);
         waitForState("version:\\d+ distributor:10 storage:10 .1.s:m");
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
-            // This should not show up, as it is down
+        // This should not show up, as it is down
         setWantedState(new Node(NodeType.DISTRIBUTOR, 8), new NodeState(NodeType.DISTRIBUTOR, State.DOWN), wantedStates);
         waitForState("version:\\d+ distributor:10 .8.s:d storage:10 .1.s:m");
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
-            // This should show up, as down nodes can be turned to maintenance
+        // This should show up, as down nodes can be turned to maintenance
         setWantedState(new Node(NodeType.STORAGE, 6), new NodeState(NodeType.STORAGE, State.MAINTENANCE).setDescription("foobar"), wantedStates);
         waitForState("version:\\d+ distributor:10 .8.s:d storage:10 .1.s:m .6.s:m");
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
-            // This should not show up, as we cannot turn a down node retired
+        // This should not show up, as we cannot turn a down node retired
         setWantedState(new Node(NodeType.STORAGE, 7), new NodeState(NodeType.STORAGE, State.RETIRED).setDescription("foobar"), wantedStates);
         waitForState("version:\\d+ distributor:10 .8.s:d storage:10 .1.s:m .6.s:m .7.s:r");
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
-            // This should not show up, as it is down
+        // This should not show up, as it is down
         setWantedState(new Node(NodeType.STORAGE, 8), new NodeState(NodeType.STORAGE, State.DOWN).setDescription("foobar"), wantedStates);
         waitForState("version:\\d+ distributor:10 .8.s:d storage:10 .1.s:m .6.s:m .7.s:r .8.s:d");
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
 
         stopFleetController();
         for (int i=6; i<nodes.size(); ++i) nodes.get(i).disconnect();
@@ -156,7 +138,32 @@ public class DatabaseTest extends FleetControllerTest {
 
         for (int i=6; i<nodes.size(); ++i) nodes.get(i).connect();
         waitForState("version:\\d+ distributor:10 .8.s:d storage:10 .1.s:m .7.s:r .8.s:d");
-        for (DummyVdsNode node : nodes) { assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode())); }
+        assertWantedStates(wantedStates);
     }
 
+    private void assertWantedStates(Map<Node, NodeState> wantedStates) {
+        for (DummyVdsNode node : nodes) {
+            assertEquals(node.getNode().toString(), wantedStates.get(node.getNode()), fleetController.getWantedNodeState(node.getNode()));
+        }
+    }
+
+    // Note: different semantics than FleetControllerTest.setWantedState
+    private void setWantedState(Node n, NodeState ns, Map<Node, NodeState> wantedStates) {
+        int rpcPort = fleetController.getRpcPort();
+        if (supervisor == null) {
+            supervisor = new Supervisor(new Transport());
+        }
+        Target connection = supervisor.connect(new Spec("localhost", rpcPort));
+        assertTrue(connection.isValid());
+
+        Request req = new Request("setNodeState");
+        req.parameters().add(new StringValue("storage/cluster.mycluster/" + n.getType().toString() + "/" + n.getIndex()));
+        req.parameters().add(new StringValue(ns.serialize(true)));
+        connection.invokeSync(req, timeoutS);
+        assertEquals(req.toString(), ErrorCode.NONE, req.errorCode());
+        assertTrue(req.toString(), req.checkReturnTypes("s"));
+        wantedStates.put(n, ns);
+    }
+
+
 }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java
index 608d7d6aee3..9f5e672886e 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java
@@ -42,7 +42,7 @@ public class MasterElectionTest extends FleetControllerTest {
     public TestRule cleanupZookeeperLogsOnSuccess = new CleanupZookeeperLogsOnSuccess();
 
     @Rule
-    public Timeout globalTimeout= Timeout.seconds(120);
+    public Timeout globalTimeout = Timeout.seconds(120);
 
     private static int defaultZkSessionTimeoutInMillis() { return 30_000; }