Reorganize helper methods

1 files changed, 72 insertions, 77 deletions

diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index 4bb27c72fc4..f5bbec5d832 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -71,11 +71,6 @@ import static org.junit.Assert.fail;
  */
 public class ControllerTest {
 
-    private static final ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
-            .environment(Environment.prod)
-            .region("corp-us-east-1")
-            .build();
-
     @Test
     public void testDeployment() {
         // Setup system
@@ -289,33 +284,6 @@ public class ControllerTest {
         assertEquals(newSystemVersion, tester.configServer().lastPrepareVersion().get());
     }
 
-    /** Adds a new version, higher than the current system version, makes it the system version and returns it */
-    private Version incrementSystemVersion(Controller controller) {
-        Version systemVersion = controller.versionStatus().systemVersion().get().versionNumber();
-        Version newSystemVersion = new Version(systemVersion.getMajor(), systemVersion.getMinor()+1, 0);
-        VespaVersion newSystemVespaVersion = new VespaVersion(DeploymentStatistics.empty(newSystemVersion),
-                                                              "commit1",
-                                                              Instant.now(),
-                                                              true,
-                                                              true,
-                                                              Collections.emptyList(),
-                                                              VespaVersion.Confidence.low
-        );
-        List<VespaVersion> versions = new ArrayList<>(controller.versionStatus().versions());
-        for (int i = 0; i < versions.size(); i++) {
-            VespaVersion c = versions.get(i);
-            if (c.isSystemVersion())
-                versions.set(i, new VespaVersion(c.statistics(), c.releaseCommit(), c.committedAt(),
-                                                 false,
-                                                 false,
-                                                 c.systemApplicationHostnames(),
-                                                 c.confidence()));
-        }
-        versions.add(newSystemVespaVersion);
-        controller.updateVersionStatus(new VersionStatus(versions));
-        return newSystemVersion;
-    }
-
     @Test
     public void testPullRequestDeployment() {
         // Setup system
@@ -362,13 +330,6 @@ public class ControllerTest {
                            .count());
     }
 
-    private void assertStatus(JobStatus expectedStatus, ApplicationId id, Controller controller) {
-        Application app = controller.applications().get(id).get();
-        JobStatus existingStatus = app.deploymentJobs().jobStatus().get(expectedStatus.type());
-        assertNotNull("Status of type " + expectedStatus.type() + " is present", existingStatus);
-        assertEquals(expectedStatus, existingStatus);
-    }
-
     @Test
     public void testGlobalRotations() throws IOException {
         // Setup tester and app def
@@ -652,6 +613,62 @@ public class ControllerTest {
                    tester.applications().require(app.id()).deploymentJobs().jobStatus().isEmpty());
     }
 
+    @Test
+    public void testDeploymentOfNewInstanceWithIllegalApplicationName() {
+        ControllerTester tester = new ControllerTester();
+        String application = "this_application_name_is_far_too_long_and_has_underscores";
+        ZoneId zone = ZoneId.from("test", "us-east-1");
+        DeployOptions options = new DeployOptions(false,
+                                                  Optional.empty(),
+                                                  false,
+                                                  false);
+
+        tester.createTenant("tenant", "domain", null);
+
+        // Deploy an application which doesn't yet exist, and which has an illegal application name.
+        try {
+            tester.controller().applications().deploy(ApplicationId.from("tenant", application, "123"), zone, Optional.empty(), options);
+            fail("Illegal application name should cause validation exception.");
+        }
+        catch (IllegalArgumentException e) {
+            assertTrue(e.getMessage().contains("Invalid id"));
+        }
+
+        // Sneak an illegal application in the back door.
+        tester.createApplication(new ApplicationSerializer().toSlime(new Application(ApplicationId.from("tenant", application, "default"))));
+
+        // Deploy a PR instance for the application, with no NToken.
+        tester.controller().applications().deploy(ApplicationId.from("tenant", application, "456"), zone, Optional.empty(), options);
+        assertTrue(tester.controller().applications().get(ApplicationId.from("tenant", application, "456")).isPresent());
+    }
+
+    /** Adds a new version, higher than the current system version, makes it the system version and returns it */
+    private Version incrementSystemVersion(Controller controller) {
+        Version systemVersion = controller.versionStatus().systemVersion().get().versionNumber();
+        Version newSystemVersion = new Version(systemVersion.getMajor(), systemVersion.getMinor()+1, 0);
+        VespaVersion newSystemVespaVersion = new VespaVersion(DeploymentStatistics.empty(newSystemVersion),
+                                                              "commit1",
+                                                              Instant.now(),
+                                                              true,
+                                                              true,
+                                                              Collections.emptyList(),
+                                                              VespaVersion.Confidence.low
+        );
+        List<VespaVersion> versions = new ArrayList<>(controller.versionStatus().versions());
+        for (int i = 0; i < versions.size(); i++) {
+            VespaVersion c = versions.get(i);
+            if (c.isSystemVersion())
+                versions.set(i, new VespaVersion(c.statistics(), c.releaseCommit(), c.committedAt(),
+                                                 false,
+                                                 false,
+                                                 c.systemApplicationHostnames(),
+                                                 c.confidence()));
+        }
+        versions.add(newSystemVespaVersion);
+        controller.updateVersionStatus(new VersionStatus(versions));
+        return newSystemVersion;
+    }
+
     private void runUpgrade(DeploymentTester tester, ApplicationId application, ApplicationVersion version) {
         Version next = Version.fromString("6.2");
         tester.upgradeSystem(next);
@@ -675,6 +692,13 @@ public class ControllerTest {
         runDeployment(tester, app, version, Optional.empty(), Optional.of(applicationPackage));
     }
 
+    private void assertStatus(JobStatus expectedStatus, ApplicationId id, Controller controller) {
+        Application app = controller.applications().get(id).get();
+        JobStatus existingStatus = app.deploymentJobs().jobStatus().get(expectedStatus.type());
+        assertNotNull("Status of type " + expectedStatus.type() + " is present", existingStatus);
+        assertEquals(expectedStatus, existingStatus);
+    }
+
     private void runDeployment(DeploymentTester tester, Application app, ApplicationVersion version,
                                Optional<Version> upgrade, Optional<ApplicationPackage> applicationPackage) {
         Version vespaVersion = upgrade.orElseGet(tester::defaultPlatformVersion);
@@ -683,23 +707,23 @@ public class ControllerTest {
         tester.deployAndNotify(app, applicationPackage, true, systemTest);
         tester.deployAndNotify(app, applicationPackage, true, stagingTest);
         JobStatus expected = JobStatus.initial(stagingTest)
-                             .withTriggering(vespaVersion, version, productionCorpUsEast1.zone(main).map(tester.application(app.id()).deployments()::get), "",
-                                             tester.clock().instant().truncatedTo(MILLIS))
-                             .withCompletion(42, Optional.empty(), tester.clock().instant().truncatedTo(MILLIS));
+                                      .withTriggering(vespaVersion, version, productionCorpUsEast1.zone(main).map(tester.application(app.id()).deployments()::get), "",
+                                                      tester.clock().instant().truncatedTo(MILLIS))
+                                      .withCompletion(42, Optional.empty(), tester.clock().instant().truncatedTo(MILLIS));
         assertStatus(expected, app.id(), tester.controller());
 
         // Deploy in production
         expected = JobStatus.initial(productionCorpUsEast1)
-                             .withTriggering(vespaVersion, version, productionCorpUsEast1.zone(main).map(tester.application(app.id()).deployments()::get), "",
-                                             tester.clock().instant().truncatedTo(MILLIS))
-                             .withCompletion(42, Optional.empty(), tester.clock().instant().truncatedTo(MILLIS));
+                            .withTriggering(vespaVersion, version, productionCorpUsEast1.zone(main).map(tester.application(app.id()).deployments()::get), "",
+                                            tester.clock().instant().truncatedTo(MILLIS))
+                            .withCompletion(42, Optional.empty(), tester.clock().instant().truncatedTo(MILLIS));
         tester.deployAndNotify(app, applicationPackage, true, productionCorpUsEast1);
         assertStatus(expected, app.id(), tester.controller());
 
         expected = JobStatus.initial(productionUsEast3)
-                             .withTriggering(vespaVersion, version, productionUsEast3.zone(main).map(tester.application(app.id()).deployments()::get), "",
-                                             tester.clock().instant().truncatedTo(MILLIS))
-                             .withCompletion(42, Optional.empty(), tester.clock().instant().truncatedTo(MILLIS));
+                            .withTriggering(vespaVersion, version, productionUsEast3.zone(main).map(tester.application(app.id()).deployments()::get), "",
+                                            tester.clock().instant().truncatedTo(MILLIS))
+                            .withCompletion(42, Optional.empty(), tester.clock().instant().truncatedTo(MILLIS));
         tester.deployAndNotify(app, applicationPackage, true, productionUsEast3);
         assertStatus(expected, app.id(), tester.controller());
 
@@ -711,33 +735,4 @@ public class ControllerTest {
         }
     }
 
-    @Test
-    public void testDeploymentOfNewInstanceWithIllegalApplicationName() {
-        ControllerTester tester = new ControllerTester();
-        String application = "this_application_name_is_far_too_long_and_has_underscores";
-        ZoneId zone = ZoneId.from("test", "us-east-1");
-        DeployOptions options = new DeployOptions(false,
-                                                  Optional.empty(),
-                                                  false,
-                                                  false);
-
-        tester.createTenant("tenant", "domain", null);
-
-        // Deploy an application which doesn't yet exist, and which has an illegal application name.
-        try {
-            tester.controller().applications().deploy(ApplicationId.from("tenant", application, "123"), zone, Optional.empty(), options);
-            fail("Illegal application name should cause validation exception.");
-        }
-        catch (IllegalArgumentException e) {
-            assertTrue(e.getMessage().contains("Invalid id"));
-        }
-
-        // Sneak an illegal application in the back door.
-        tester.createApplication(new ApplicationSerializer().toSlime(new Application(ApplicationId.from("tenant", application, "default"))));
-
-        // Deploy a PR instance for the application, with no NToken.
-        tester.controller().applications().deploy(ApplicationId.from("tenant", application, "456"), zone, Optional.empty(), options);
-        assertTrue(tester.controller().applications().get(ApplicationId.from("tenant", application, "456")).isPresent());
-    }
-
 }