GC use of deprecated junit assertThat and unify

4 files changed, 54 insertions, 74 deletions

diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
index 3c5af5d1447..eba8d3d4d66 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
@@ -51,12 +51,8 @@ import java.util.Set;
 
 import static com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN;
 import static com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus.NO_REMARKS;
-import static org.hamcrest.collection.IsEmptyCollection.empty;
-import static org.hamcrest.core.Is.is;
-import static org.hamcrest.core.IsCollectionContaining.hasItem;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
@@ -123,44 +119,44 @@ public class OrchestratorImplTest {
 
     @Test
     public void application_has_initially_no_remarks() throws Exception {
-        assertThat(orchestrator.getApplicationInstanceStatus(app1), is(NO_REMARKS));
+        assertEquals(NO_REMARKS, orchestrator.getApplicationInstanceStatus(app1));
     }
 
     @Test
     public void application_can_be_set_in_suspend() throws Exception {
         orchestrator.suspend(app1);
-        assertThat(orchestrator.getApplicationInstanceStatus(app1), is(ALLOWED_TO_BE_DOWN));
+        assertEquals(ALLOWED_TO_BE_DOWN, orchestrator.getApplicationInstanceStatus(app1));
     }
 
     @Test
     public void application_can_be_removed_from_suspend() throws Exception {
         orchestrator.suspend(app1);
         orchestrator.resume(app1);
-        assertThat(orchestrator.getApplicationInstanceStatus(app1), is(NO_REMARKS));
+        assertEquals(NO_REMARKS, orchestrator.getApplicationInstanceStatus(app1));
     }
 
     @Test
     public void appliations_list_returns_empty_initially() {
-        assertThat(orchestrator.getAllSuspendedApplications(), is(empty()));
+        assertTrue(orchestrator.getAllSuspendedApplications().isEmpty());
     }
 
     @Test
     public void appliations_list_returns_suspended_apps() throws Exception {
         // One suspended app
         orchestrator.suspend(app1);
-        assertThat(orchestrator.getAllSuspendedApplications().size(), is(1));
-        assertThat(orchestrator.getAllSuspendedApplications(), hasItem(app1));
+        assertEquals(1, orchestrator.getAllSuspendedApplications().size());
+        assertTrue(orchestrator.getAllSuspendedApplications().contains(app1));
 
         // Two suspended apps
         orchestrator.suspend(app2);
-        assertThat(orchestrator.getAllSuspendedApplications().size(), is(2));
-        assertThat(orchestrator.getAllSuspendedApplications(), hasItem(app1));
-        assertThat(orchestrator.getAllSuspendedApplications(), hasItem(app2));
+        assertEquals(2, orchestrator.getAllSuspendedApplications().size());
+        assertTrue(orchestrator.getAllSuspendedApplications().contains(app1));
+        assertTrue(orchestrator.getAllSuspendedApplications().contains(app2));
 
         // Back to one when resetting one app to no_remarks
         orchestrator.resume(app1);
-        assertThat(orchestrator.getAllSuspendedApplications().size(), is(1));
-        assertThat(orchestrator.getAllSuspendedApplications(), hasItem(app2));
+        assertEquals(1, orchestrator.getAllSuspendedApplications().size());
+        assertTrue(orchestrator.getAllSuspendedApplications().contains(app2));
     }
 
 
@@ -169,23 +165,23 @@ public class OrchestratorImplTest {
         // Two suspends
         orchestrator.suspend(app1);
         orchestrator.suspend(app1);
-        assertThat(orchestrator.getApplicationInstanceStatus(app1), is(ALLOWED_TO_BE_DOWN));
-        assertThat(orchestrator.getApplicationInstanceStatus(app2), is(NO_REMARKS));
+        assertEquals(ALLOWED_TO_BE_DOWN, orchestrator.getApplicationInstanceStatus(app1));
+        assertEquals(NO_REMARKS, orchestrator.getApplicationInstanceStatus(app2));
 
         // Three no_remarks
         orchestrator.resume(app1);
         orchestrator.resume(app1);
         orchestrator.resume(app1);
-        assertThat(orchestrator.getApplicationInstanceStatus(app1), is(NO_REMARKS));
-        assertThat(orchestrator.getApplicationInstanceStatus(app2), is(NO_REMARKS));
+        assertEquals(NO_REMARKS, orchestrator.getApplicationInstanceStatus(app1));
+        assertEquals(NO_REMARKS, orchestrator.getApplicationInstanceStatus(app2));
 
         // Two suspends and two on two applications interleaved
         orchestrator.suspend(app2);
         orchestrator.resume(app1);
         orchestrator.suspend(app2);
         orchestrator.resume(app1);
-        assertThat(orchestrator.getApplicationInstanceStatus(app1), is(NO_REMARKS));
-        assertThat(orchestrator.getApplicationInstanceStatus(app2), is(ALLOWED_TO_BE_DOWN));
+        assertEquals(NO_REMARKS, orchestrator.getApplicationInstanceStatus(app1));
+        assertEquals(ALLOWED_TO_BE_DOWN, orchestrator.getApplicationInstanceStatus(app2));
     }
 
     @Test
@@ -274,7 +270,7 @@ public class OrchestratorImplTest {
     }
 
     @Test
-    public void suspendAllWorks() throws Exception {
+    public void suspendAllWorks() {
         // A spy is preferential because suspendAll() relies on delegating the hard work to suspend() and resume().
         OrchestratorImpl orchestrator = spy(this.orchestrator);
 
@@ -307,7 +303,7 @@ public class OrchestratorImplTest {
     }
 
     @Test
-    public void whenSuspendAllFails() throws Exception {
+    public void whenSuspendAllFails() {
         // A spy is preferential because suspendAll() relies on delegating the hard work to suspend() and resume().
         OrchestratorImpl orchestrator = spy(this.orchestrator);
 
@@ -340,7 +336,7 @@ public class OrchestratorImplTest {
     }
 
     @Test
-    public void testLargeLocks() throws Exception {
+    public void testLargeLocks() {
         var tenantId = new TenantId("tenant");
         var applicationInstanceId = new ApplicationInstanceId("app:dev:us-east-1:default");
         var applicationInstanceReference = new ApplicationInstanceReference(tenantId, applicationInstanceId);
@@ -480,7 +476,7 @@ public class OrchestratorImplTest {
     }
 
     @Test
-    public void testGetHost() throws Exception {
+    public void testGetHost() {
         ClusterControllerClientFactory clusterControllerClientFactory = new ClusterControllerClientFactoryMock();
         StatusService statusService = new ZkStatusService(
                 new MockCurator(),
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorTest.java
index 5951a3b0e21..698c7b544b3 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorTest.java
@@ -32,8 +32,7 @@ import org.junit.Test;
 import java.util.List;
 import java.util.Map;
 
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
@@ -101,8 +100,8 @@ public class OrchestratorTest {
             orchestrator.acquirePermissionToRemove(toApplicationModelHostName(cfg2));
             fail();
         } catch (HostStateChangeDeniedException e) {
-            assertThat(e.getMessage(), containsString("Changing the state of cfg2 would violate enough-services-up"));
-            assertThat(e.getMessage(), containsString("[cfg1] are suspended."));
+            assertTrue(e.getMessage().contains("Changing the state of cfg2 would violate enough-services-up"));
+            assertTrue(e.getMessage().contains("[cfg1] are suspended."));
         }
 
         // cfg1 is removed from the application
@@ -113,8 +112,8 @@ public class OrchestratorTest {
             orchestrator.acquirePermissionToRemove(toApplicationModelHostName(cfg2));
             fail();
         } catch (HostStateChangeDeniedException e) {
-            assertThat(e.getMessage(), containsString("Changing the state of cfg2 would violate enough-services-up"));
-            assertThat(e.getMessage(), containsString("[1 missing config server] are down."));
+            assertTrue(e.getMessage().contains("Changing the state of cfg2 would violate enough-services-up"));
+            assertTrue(e.getMessage().contains("[1 missing config server] are down."));
         }
 
         // cfg1 is reprovisioned, added to the node repo, and activated
@@ -128,8 +127,8 @@ public class OrchestratorTest {
             orchestrator.acquirePermissionToRemove(toApplicationModelHostName(cfg1));
             fail();
         } catch (HostStateChangeDeniedException e) {
-            assertThat(e.getMessage(), containsString("Changing the state of cfg1 would violate enough-services-up"));
-            assertThat(e.getMessage(), containsString("[cfg2] are suspended"));
+            assertTrue(e.getMessage().contains("Changing the state of cfg1 would violate enough-services-up"));
+            assertTrue(e.getMessage().contains("[cfg2] are suspended"));
         }
 
         // etc (should be the same as for cfg1)
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java
index babfa666577..1563a54a029 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java
@@ -36,10 +36,8 @@ import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
-import static org.hamcrest.core.StringContains.containsString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
@@ -180,9 +178,8 @@ public class ClusterApiImplTest {
             policy.verifyGroupGoingDownIsFine(clusterApi);
             fail();
         } catch (HostStateChangeDeniedException e) {
-            assertThat(e.getMessage(),
-                    containsString("Changing the state of cfg1 would violate enough-services-up: 33% of the config " +
-                                   "servers are down or suspended already: [1 missing config server] are down."));
+            assertTrue(e.getMessage().contains("Changing the state of cfg1 would violate enough-services-up: 33% of the config " +
+                    "servers are down or suspended already: [1 missing config server] are down."));
         }
     }
 
@@ -200,9 +197,8 @@ public class ClusterApiImplTest {
             policy.verifyGroupGoingDownIsFine(clusterApi);
             fail();
         } catch (HostStateChangeDeniedException e) {
-            assertThat(e.getMessage(),
-                    containsString("Changing the state of cfg1 would violate enough-services-up: 33% of the config " +
-                                   "server hosts are down or suspended already: [1 missing config server host] are down."));
+            assertTrue(e.getMessage().contains("Changing the state of cfg1 would violate enough-services-up: 33% of the config " +
+                    "server hosts are down or suspended already: [1 missing config server host] are down."));
         }
     }
 
@@ -216,9 +212,8 @@ public class ClusterApiImplTest {
             policy.verifyGroupGoingDownIsFine(clusterApi);
             fail();
         } catch (HostStateChangeDeniedException e) {
-            assertThat(e.getMessage(),
-                       containsString("Changing the state of cfg1 would violate enough-services-up: 33% of the config " +
-                                      "servers are down or suspended already: [1 missing config server] are down."));
+            assertTrue(e.getMessage().contains("Changing the state of cfg1 would violate enough-services-up: 33% of the config " +
+                    "servers are down or suspended already: [1 missing config server] are down."));
         }
     }
 
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusServiceTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusServiceTest.java
index 874b3d32325..c5ce69bb648 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusServiceTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusServiceTest.java
@@ -10,7 +10,6 @@ import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
 import com.yahoo.vespa.applicationmodel.HostName;
 import com.yahoo.vespa.curator.Curator;
-import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.orchestrator.OrchestratorContext;
 import com.yahoo.vespa.orchestrator.OrchestratorUtil;
 import com.yahoo.vespa.orchestrator.TestIds;
@@ -51,11 +50,10 @@ import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import static org.hamcrest.core.Is.is;
-import static org.hamcrest.core.IsCollectionContaining.hasItem;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -108,14 +106,13 @@ public class ZkStatusServiceTest {
     }
 
     @Test
-    public void host_state_for_unknown_hosts_is_no_remarks() throws Exception {
-        assertThat(
-                statusService.getHostInfo(TestIds.APPLICATION_INSTANCE_REFERENCE, TestIds.HOST_NAME1).status(),
-                is(HostStatus.NO_REMARKS));
+    public void host_state_for_unknown_hosts_is_no_remarks() {
+        assertEquals(HostStatus.NO_REMARKS,
+                statusService.getHostInfo(TestIds.APPLICATION_INSTANCE_REFERENCE, TestIds.HOST_NAME1).status());
     }
 
     @Test
-    public void setting_host_state_is_idempotent() throws Exception {
+    public void setting_host_state_is_idempotent() {
         when(timer.currentTime()).thenReturn(
                 Instant.ofEpochMilli((1)),
                 Instant.ofEpochMilli((3)),
@@ -128,8 +125,7 @@ public class ZkStatusServiceTest {
                 for (int i = 0; i < 2; i++) {
                     lock.setHostState(TestIds.HOST_NAME1, hostStatus);
 
-                    assertThat(lock.getHostInfos().getOrNoRemarks(TestIds.HOST_NAME1).status(),
-                               is(hostStatus));
+                    assertEquals(hostStatus, lock.getHostInfos().getOrNoRemarks(TestIds.HOST_NAME1).status());
                 }
             }
         }
@@ -238,13 +234,11 @@ public class ZkStatusServiceTest {
     }
 
     @Test
-    public void suspend_and_resume_application_works_and_is_symmetric() throws Exception {
+    public void suspend_and_resume_application_works_and_is_symmetric() {
 
         // Initial state is NO_REMARK
-        assertThat(
-                statusService
-                        .getApplicationInstanceStatus(TestIds.APPLICATION_INSTANCE_REFERENCE),
-                is(ApplicationInstanceStatus.NO_REMARKS));
+        assertEquals(ApplicationInstanceStatus.NO_REMARKS,
+                statusService.getApplicationInstanceStatus(TestIds.APPLICATION_INSTANCE_REFERENCE));
 
         // Suspend
         try (ApplicationLock lock = statusService
@@ -252,10 +246,8 @@ public class ZkStatusServiceTest {
             lock.setApplicationInstanceStatus(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN);
         }
 
-        assertThat(
-                statusService
-                        .getApplicationInstanceStatus(TestIds.APPLICATION_INSTANCE_REFERENCE),
-                is(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN));
+        assertEquals(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN,
+                statusService.getApplicationInstanceStatus(TestIds.APPLICATION_INSTANCE_REFERENCE));
 
         // Resume
         try (ApplicationLock lock = statusService
@@ -263,16 +255,14 @@ public class ZkStatusServiceTest {
             lock.setApplicationInstanceStatus(ApplicationInstanceStatus.NO_REMARKS);
         }
 
-        assertThat(
-                statusService
-                        .getApplicationInstanceStatus(TestIds.APPLICATION_INSTANCE_REFERENCE),
-                is(ApplicationInstanceStatus.NO_REMARKS));
+        assertEquals(ApplicationInstanceStatus.NO_REMARKS,
+                statusService.getApplicationInstanceStatus(TestIds.APPLICATION_INSTANCE_REFERENCE));
     }
 
     @Test
-    public void suspending_two_applications_returns_two_applications() throws Exception {
+    public void suspending_two_applications_returns_two_applications() {
         Set<ApplicationInstanceReference> suspendedApps = statusService.getAllSuspendedApplications();
-        assertThat(suspendedApps.size(), is(0));
+        assertEquals(0, suspendedApps.size());
 
         try (ApplicationLock statusRegistry = statusService
                 .lockApplication(context, TestIds.APPLICATION_INSTANCE_REFERENCE)) {
@@ -285,13 +275,13 @@ public class ZkStatusServiceTest {
         }
 
         suspendedApps = statusService.getAllSuspendedApplications();
-        assertThat(suspendedApps.size(), is(2));
-        assertThat(suspendedApps, hasItem(TestIds.APPLICATION_INSTANCE_REFERENCE));
-        assertThat(suspendedApps, hasItem(TestIds.APPLICATION_INSTANCE_REFERENCE2));
+        assertEquals(2, suspendedApps.size());
+        assertTrue(suspendedApps.contains(TestIds.APPLICATION_INSTANCE_REFERENCE));
+        assertTrue(suspendedApps.contains(TestIds.APPLICATION_INSTANCE_REFERENCE2));
     }
 
     @Test
-    public void zookeeper_cleanup() throws Exception {
+    public void zookeeper_cleanup() {
         HostName strayHostname = new HostName("stray1.com");
 
         verify(antiServiceMonitor, times(0)).disallowDuperModelLockAcquisition(any());