1 files changed, 24 insertions, 26 deletions

diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
index 519c457e73b..4f97c078c9b 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.versions;
 
+import com.google.common.collect.ImmutableSet;
 import com.yahoo.component.Version;
 import com.yahoo.component.Vtag;
 import com.yahoo.config.provision.Environment;
@@ -19,6 +20,8 @@ import org.junit.Test;
 
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.time.Duration;
+import java.util.Collections;
 import java.util.List;
 
 import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.component;
@@ -28,6 +31,7 @@ import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobTy
 import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.systemTest;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -95,23 +99,20 @@ public class VersionStatusTest {
         List<VespaVersion> versions = tester.controller().versionStatus().versions();
         assertEquals("The two versions above exist", 2, versions.size());
 
+        System.err.println(tester.controller().applications().deploymentTrigger().jobTimeoutLimit());
+
         VespaVersion v1 = versions.get(0);
         assertEquals(version1, v1.versionNumber());
-        assertEquals(0, v1.statistics().failing().size());
-        // All applications are on v1 in at least one zone
-        assertEquals(3, v1.statistics().production().size());
-        assertTrue(v1.statistics().production().contains(app2.id()));
-        assertTrue(v1.statistics().production().contains(app1.id()));
+        assertEquals("No applications are failing on version1.", ImmutableSet.of(), v1.statistics().failing());
+        assertEquals("All applications have at least one active production deployment on version 1.", ImmutableSet.of(app1.id(), app2.id(), app3.id()), v1.statistics().production());
+        assertEquals("No applications have active deployment jobs on version1.", ImmutableSet.of(), v1.statistics().deploying());
 
         VespaVersion v2 = versions.get(1);
         assertEquals(version2, v2.versionNumber());
-        // All applications have failed on v2 in at least one zone
-        assertEquals(3, v2.statistics().failing().size());
-        assertTrue(v2.statistics().failing().contains(app1.id()));
-        assertTrue(v2.statistics().failing().contains(app3.id()));
-        // Only one application is on v2 in at least one zone
-        assertEquals(1, v2.statistics().production().size());
-        assertTrue(v2.statistics().production().contains(app2.id()));
+        assertEquals("All applications have failed on version2 in at least one zone.", ImmutableSet.of(app1.id(), app2.id(), app3.id()), v2.statistics().failing());
+        assertEquals("Only app2 has successfully deployed to production on version2.", ImmutableSet.of(app2.id()), v2.statistics().production());
+        // Should test the below, but can't easily be done with current test framework. This test passes in DeploymentApiTest.
+        // assertEquals("All applications are being retried on version2.", ImmutableSet.of(app1.id(), app2.id(), app3.id()), v2.statistics().deploying());
     }
     
     @Test
@@ -161,6 +162,12 @@ public class VersionStatusTest {
         assertEquals("One canary failed: Broken",
                      Confidence.broken, confidence(tester.controller(), version1));
 
+        // Finish running jobs
+        tester.deployAndNotify(canary2, DeploymentTester.applicationPackage("canary"), false, systemTest);
+        tester.clock().advance(Duration.ofHours(1));
+        tester.deployAndNotify(canary1, DeploymentTester.applicationPackage("canary"), false, productionUsWest1);
+        tester.deployAndNotify(canary2, DeploymentTester.applicationPackage("canary"), false, systemTest);
+
         // New version is released
         Version version2 = new Version("5.2");
         tester.upgradeSystem(version2);
@@ -204,6 +211,7 @@ public class VersionStatusTest {
         
         // Another default application upgrades, raising confidence to high
         tester.completeUpgrade(default8, version2, "default");
+        tester.completeUpgrade(default9, version2, "default");
         tester.updateVersionStatus();
 
         assertEquals("Confidence remains unchanged for version0: High",
@@ -241,7 +249,7 @@ public class VersionStatusTest {
     }
 
     @Test
-    public void testIgnoreConfigdeince() {
+    public void testIgnoreConfidence() {
         DeploymentTester tester = new DeploymentTester();
 
         Version version0 = new Version("5.0");
@@ -270,7 +278,6 @@ public class VersionStatusTest {
         tester.completeUpgradeWithError(default3, version1, "default", stagingTest);
         tester.completeUpgradeWithError(default4, version1, "default", stagingTest);
         tester.updateVersionStatus();
-
         assertEquals("Canaries have upgraded, 1 of 4 default apps failing: Broken",
                      Confidence.broken, confidence(tester.controller(), version1));
 
@@ -295,8 +302,9 @@ public class VersionStatusTest {
         Version versionWithUnknownTag = new Version("6.1.2");
 
         Application app = tester.createAndDeploy("tenant1", "domain1","application1", Environment.test, 11);
-        applications.notifyJobCompletion(mockReport(app, component, true));
-        applications.notifyJobCompletion(mockReport(app, systemTest, true));
+        tester.clock().advance(Duration.ofMillis(1));
+        applications.notifyJobCompletion(DeploymentTester.jobReport(app, component, true));
+        applications.notifyJobCompletion(DeploymentTester.jobReport(app, systemTest, true));
 
         List<VespaVersion> vespaVersions = VersionStatus.compute(tester.controller()).versions();
 
@@ -313,14 +321,4 @@ public class VersionStatusTest {
                 .orElseThrow(() -> new IllegalArgumentException("Expected to find version: " + version));
     }
 
-    private DeploymentJobs.JobReport mockReport(Application application, DeploymentJobs.JobType jobType, boolean success) {
-        return new DeploymentJobs.JobReport(
-                application.id(),
-                jobType,
-                application.deploymentJobs().projectId().get(),
-                42,
-                JobError.from(success)
-        );
-    }
-
 }