diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-12-21 15:18:14 +0100 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2021-12-21 15:43:26 +0100 |
commit | e0fb5474ac171436e263a4950d72a2a405d379a2 (patch) | |
tree | 0b754bd09a06ef419a56f9df586446b386106669 /orchestrator/src/test | |
parent | 28ae61202ad963955cf92719bab9b9d97181d5dd (diff) |
GC use of deprecated junit assertThat and unify
Diffstat (limited to 'orchestrator/src/test')
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()); |