1 files changed, 67 insertions, 76 deletions
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java
index 685ad3f0b43..1b52194b270 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java
@@ -18,9 +18,7 @@ import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStat
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.SetResponse;
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitResponse;
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitState;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
+import org.junit.jupiter.api.Test;
 
 import java.time.Clock;
 import java.time.Duration;
@@ -32,11 +30,7 @@ import java.util.regex.Pattern;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.Is.is;
 import static org.hamcrest.core.StringContains.containsString;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.Mockito.mock;
@@ -44,10 +38,6 @@ import static org.mockito.Mockito.when;
 
 public class SetNodeStateTest extends StateRestApiTest {
 
-    @SuppressWarnings("deprecation")
-    @Rule
-    public ExpectedException expectedException = ExpectedException.none();
-
     public static class SetUnitStateRequestImpl extends StateRequest implements SetUnitStateRequest {
         private final Map<String, UnitState> newStates = new LinkedHashMap<>();
         private Condition condition = Condition.FORCE;
@@ -176,7 +166,7 @@ public class SetNodeStateTest extends StateRestApiTest {
     }
 
     @Test
-    public void testSimple() throws Exception {
+    void testSimple() throws Exception {
         setUp(true);
         verifyStateSet("down", "testing");
         verifyStateSet("up", "foo");
@@ -186,14 +176,14 @@ public class SetNodeStateTest extends StateRestApiTest {
     }
 
     @Test
-    public void testSetNodesForCluster() throws Exception {
+    void testSetNodesForCluster() throws Exception {
         setUp(true);
         verifyClusterSet("maintenance", "prepare for maintenance");
         verifyClusterSet("up", "and we're back online");
     }
 
     @Test
-    public void testShouldNotModifyDistributorSafe() throws Exception {
+    void testShouldNotModifyDistributorSafe() throws Exception {
         setUp(false);
         SetResponse setResponse = restAPI.setUnitState(new SetUnitStateRequestImpl("music/distributor/1")
                 .setNewState("user", "up", "whatever reason.")
@@ -204,7 +194,7 @@ public class SetNodeStateTest extends StateRestApiTest {
     }
 
     @Test
-    public void testShouldModifyStorageSafeOk() throws Exception {
+    void testShouldModifyStorageSafeOk() throws Exception {
         setUp(false);
         SetResponse setResponse = restAPI.setUnitState(new SetUnitStateRequestImpl("music/storage/2")
                 .setNewState("user", "maintenance", "whatever reason.")
@@ -214,7 +204,7 @@ public class SetNodeStateTest extends StateRestApiTest {
     }
 
     @Test
-    public void testShouldModifyStorageSafeBlocked() throws Exception {
+    void testShouldModifyStorageSafeBlocked() throws Exception {
         // Sets up 2 groups: [0, 2, 4] and [1, 3, 5]
         setUpMusicGroup(6, "");
 
@@ -254,7 +244,7 @@ public class SetNodeStateTest extends StateRestApiTest {
     }
 
     @Test
-    public void settingSafeMaintenanceWhenNodeDown() throws Exception {
+    void settingSafeMaintenanceWhenNodeDown() throws Exception {
         // Sets up 2 groups: [0, 2, 4] and [1, 3, 5], with 1 being down
         setUpMusicGroup(6, " .1.s:d");
         assertUnitState(1, "generated", State.DOWN, "");
@@ -289,7 +279,7 @@ public class SetNodeStateTest extends StateRestApiTest {
         UnitResponse response = restAPI.getState(new StateRequest(path, 0));
         Response.NodeResponse nodeResponse = (Response.NodeResponse) response;
         UnitState unitState = nodeResponse.getStatePerType().get(type);
-        assertNotNull("No such type " + type + " at path " + path, unitState);
+        assertNotNull(unitState, "No such type " + type + " at path " + path);
         assertEquals(state.toString().toLowerCase(), unitState.getId());
         assertEquals(reason, unitState.getReason());
     }
@@ -326,19 +316,18 @@ public class SetNodeStateTest extends StateRestApiTest {
         Matcher matcher = Pattern.compile(reasonRegex).matcher(setResponse.getReason());
 
         String errorMessage = "Expected reason to match '" + reasonRegex + "', but got: " + setResponse.getReason() + "'";
-        assertTrue(errorMessage, matcher.find());
+        assertTrue(matcher.find(), errorMessage);
 
         int alreadyMaintainedIndex = Integer.parseInt(matcher.group(1));
         // Example: Say index 1 is in maintenance, and we try to set 2 in maintenance. This should
         // NOT be allowed, since 2 is in a different group than 1.
-        assertEquals("Tried to set " + index + " in maintenance, but got: " + setResponse.getReason(),
-                index % 2, (alreadyMaintainedIndex + 1) % 2);
+        assertEquals(index % 2, (alreadyMaintainedIndex + 1) % 2, "Tried to set " + index + " in maintenance, but got: " + setResponse.getReason());
 
         assertThat(setResponse.getWasModified(), is(false));
     }
 
     @Test
-    public void testSetWantedStateOnNodeNotInSlobrok() throws Exception {
+    void testSetWantedStateOnNodeNotInSlobrok() throws Exception {
         // Node 2 in cluster music does not have a valid NodeInfo due to passing true to setUp
         setUp(true);
         restAPI.setUnitState(new SetUnitStateRequestImpl("music/distributor/2").setNewState("user", "down", "borked node"));
@@ -348,73 +337,73 @@ public class SetNodeStateTest extends StateRestApiTest {
     }
 
     @Test
-    public void testWrongUnit() throws Exception {
+    void testWrongUnit() throws Exception {
         setUp(true);
 
         String wrongUnitMessage = "State can only be set at cluster or node level";
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "").setNewState("user", "down", "testing"));
             fail();
         } catch (OperationNotSupportedForUnitException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains(wrongUnitMessage));
+            assertTrue(e.getMessage().contains(wrongUnitMessage), e.getMessage());
         }
 
         // ... setting at cluster-level is allowed
 
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/distributor").setNewState("user", "down", "testing"));
             fail();
         } catch (OperationNotSupportedForUnitException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains(wrongUnitMessage));
+            assertTrue(e.getMessage().contains(wrongUnitMessage), e.getMessage());
         }
 
         // ... setting at node-level is allowed
 
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/storage/1/0").setNewState("user", "down", "testing"));
             fail();
         } catch (OperationNotSupportedForUnitException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains(wrongUnitMessage));
+            assertTrue(e.getMessage().contains(wrongUnitMessage), e.getMessage());
         }
     }
 
     @Test
-    public void testInvalidUnit() throws Exception {
+    void testInvalidUnit() throws Exception {
         setUp(true);
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "foo").setNewState("user", "down", "testing"));
             fail();
         } catch (MissingUnitException e) {
         }
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/content").setNewState("user", "down", "testing"));
             fail();
         } catch (MissingUnitException e) {
         }
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/storage/bah").setNewState("user", "down", "testing"));
             fail();
         } catch (MissingUnitException e) {
         }
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/storage/10").setNewState("user", "down", "testing"));
             fail();
         } catch (MissingUnitException e) {
         }
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/storage/1/0/1").setNewState("user", "down", "testing"));
             fail();
         } catch (MissingUnitException e) {
         }
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/storage/1/bar").setNewState("user", "down", "testing"));
             fail();
@@ -423,71 +412,71 @@ public class SetNodeStateTest extends StateRestApiTest {
     }
 
     @Test
-    public void testSettingInvalidStateType() throws Exception {
+    void testSettingInvalidStateType() throws Exception {
         setUp(true);
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/distributor/1").setNewState("foo", "down", "testing"));
         } catch (InvalidContentException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains("No new user state given"));
+            assertTrue(e.getMessage().contains("No new user state given"), e.getMessage());
         }
     }
 
     @Test
-    public void testSafeIsInvalidForSetNodesStatesForCluster() throws Exception {
+    void testSafeIsInvalidForSetNodesStatesForCluster() throws Exception {
         setUp(true);
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl("music")
                     .setNewState("user", "maintenance", "example reason")
                     .setCondition(SetUnitStateRequest.Condition.SAFE));
         } catch (InvalidContentException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains(
-                    "Setting all nodes in a cluster to a state is only supported with FORCE"));
+            assertTrue(e.getMessage().contains(
+                    "Setting all nodes in a cluster to a state is only supported with FORCE"), e.getMessage());
         }
     }
 
     @Test
-    public void testSettingWrongStateType() throws Exception {
+    void testSettingWrongStateType() throws Exception {
         setUp(true);
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/distributor/1").setNewState("generated", "down", "testing"));
         } catch (InvalidContentException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains("No new user state given"));
+            assertTrue(e.getMessage().contains("No new user state given"), e.getMessage());
         }
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/distributor/1").setNewState("unit", "down", "testing"));
         } catch (InvalidContentException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains("No new user state given"));
+            assertTrue(e.getMessage().contains("No new user state given"), e.getMessage());
         }
     }
 
     @Test
-    public void testInvalidState() throws Exception {
+    void testInvalidState() throws Exception {
         setUp(true);
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/distributor/1").setNewState("user", "initializing", "testing"));
         } catch (InvalidContentException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains("Invalid user state"));
+            assertTrue(e.getMessage().contains("Invalid user state"), e.getMessage());
         }
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/distributor/1").setNewState("user", "stopping", "testing"));
         } catch (InvalidContentException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains("Invalid user state"));
+            assertTrue(e.getMessage().contains("Invalid user state"), e.getMessage());
         }
-        try{
+        try {
             restAPI.setUnitState(new SetUnitStateRequestImpl(
                     "music/distributor/1").setNewState("user", "foo", "testing"));
         } catch (InvalidContentException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains("Invalid user state"));
+            assertTrue(e.getMessage().contains("Invalid user state"), e.getMessage());
         }
     }
 
     @Test
-    public void testOverwriteReason() throws Exception {
+    void testOverwriteReason() throws Exception {
         setUp(true);
         restAPI.setUnitState(new SetUnitStateRequestImpl(
                 "music/distributor/1").setNewState("user", "down", "testing"));
@@ -508,19 +497,19 @@ public class SetNodeStateTest extends StateRestApiTest {
     }
 
     @Test
-    public void set_node_state_requests_are_by_default_tagged_as_having_version_ack_dependency() {
+    void set_node_state_requests_are_by_default_tagged_as_having_version_ack_dependency() {
         SetNodeStateRequest request = createDummySetNodeStateRequest();
         assertTrue(request.hasVersionAckDependency());
     }
 
     @Test
-    public void set_node_state_requests_not_initially_marked_as_failed() {
+    void set_node_state_requests_not_initially_marked_as_failed() {
         SetNodeStateRequest request = createDummySetNodeStateRequest();
         assertFalse(request.isFailed());
     }
 
     @Test
-    public void set_node_state_requests_may_override_version_ack_dependency() {
+    void set_node_state_requests_may_override_version_ack_dependency() {
         SetNodeStateRequest request = new SetNodeStateRequest(createDummyId(), new SetUnitStateRequestImpl("music/storage/1")
                 .setNewState("user", "maintenance", "whatever reason.")
                 .setResponseWait(SetUnitStateRequest.ResponseWait.NO_WAIT));
@@ -531,40 +520,42 @@ public class SetNodeStateTest extends StateRestApiTest {
     // important to test (and expected to happen) for requests that have dependencies on cluster
     // state version publishing.
     @Test
-    public void leadership_loss_fails_set_node_state_request() throws Exception {
-        expectedException.expectMessage("Leadership lost before request could complete");
-        expectedException.expect(UnknownMasterException.class);
+    void leadership_loss_fails_set_node_state_request() throws Exception {
+        Throwable exception = assertThrows(UnknownMasterException.class, () -> {
 
-        SetNodeStateRequest request = createDummySetNodeStateRequest();
-        request.handleFailure(RemoteClusterControllerTask.Failure.of(RemoteClusterControllerTask.FailureCondition.LEADERSHIP_LOST));
-        request.getResult();
+            SetNodeStateRequest request = createDummySetNodeStateRequest();
+            request.handleFailure(RemoteClusterControllerTask.Failure.of(RemoteClusterControllerTask.FailureCondition.LEADERSHIP_LOST));
+            request.getResult();
+        });
+        assertTrue(exception.getMessage().contains("Leadership lost before request could complete"));
     }
 
     @Test
-    public void leadership_loss_marks_request_as_failed_for_early_out_response() {
+    void leadership_loss_marks_request_as_failed_for_early_out_response() {
         SetNodeStateRequest request = createDummySetNodeStateRequest();
         request.handleFailure(RemoteClusterControllerTask.Failure.of(RemoteClusterControllerTask.FailureCondition.LEADERSHIP_LOST));
         assertTrue(request.isFailed());
     }
 
     @Test
-    public void deadline_exceeded_fails_set_node_state_request() throws Exception {
-        expectedException.expectMessage("Task exceeded its version wait deadline: gremlins in the computer");
-        expectedException.expect(DeadlineExceededException.class);
+    void deadline_exceeded_fails_set_node_state_request() throws Exception {
+        Throwable exception = assertThrows(DeadlineExceededException.class, () -> {
 
-        SetNodeStateRequest request = createDummySetNodeStateRequest();
-        request.handleFailure(RemoteClusterControllerTask.Failure.of(
-                RemoteClusterControllerTask.FailureCondition.DEADLINE_EXCEEDED, "gremlins in the computer"));
-        request.getResult();
+            SetNodeStateRequest request = createDummySetNodeStateRequest();
+            request.handleFailure(RemoteClusterControllerTask.Failure.of(
+                    RemoteClusterControllerTask.FailureCondition.DEADLINE_EXCEEDED, "gremlins in the computer"));
+            request.getResult();
+        });
+        assertTrue(exception.getMessage().contains("Task exceeded its version wait deadline: gremlins in the computer"));
     }
 
     @Test
-    public void no_fail_if_modified() throws StateRestApiException {
+    void no_fail_if_modified() throws StateRestApiException {
         assertFalse(isFailed(true));
     }
 
     @Test
-    public void fail_if_not_modified() throws StateRestApiException {
+    void fail_if_not_modified() throws StateRestApiException {
         assertTrue(isFailed(false));
     }