Updated tests

3 files changed, 22 insertions, 24 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
index 99aecab376c..5697384ebbc 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
@@ -153,21 +153,20 @@ public class VersionStatus {
                     .filter(jobStatus -> jobStatus.jobError().isPresent())
                     .filter(jobStatus -> jobStatus.jobError().get() != DeploymentJobs.JobError.outOfCapacity)
                     .map(jobStatus -> jobStatus.lastCompleted().get().version())
-                    .forEach(version -> versionMap.merge(version, DeploymentStatistics.empty(version), (statistics, __ ) -> statistics.withFailing(application.id())));
+                    .forEach(version -> versionMap.put(version, versionMap.getOrDefault(version, DeploymentStatistics.empty(version)).withFailing(application.id())));
 
             // Succeeding versions
             jobs.jobStatus().values().stream()
                     .filter(jobStatus -> jobStatus.lastSuccess().isPresent())
                     .filter(jobStatus -> jobStatus.type().isProduction())
                     .map(jobStatus -> jobStatus.lastSuccess().get().version())
-                    .forEach(version -> versionMap.merge(version, DeploymentStatistics.empty(version), (statistics, __ ) -> statistics.withProduction(application.id())));
+                    .forEach(version -> versionMap.put(version, versionMap.getOrDefault(version, DeploymentStatistics.empty(version)).withProduction(application.id())));
 
             // Deploying versions
             jobs.jobStatus().values().stream()
-                    .filter(jobStatus -> jobStatus.type() != JobType.component)
                     .filter(jobStatus -> jobStatus.isRunning(jobTimeoutLimit))
                     .map(jobStatus -> jobStatus.lastTriggered().get().version())
-                    .forEach(version -> versionMap.merge(version, DeploymentStatistics.empty(version), (statistics, __ ) -> statistics.withDeploying(application.id())));
+                    .forEach(version -> versionMap.put(version, versionMap.getOrDefault(version, DeploymentStatistics.empty(version)).withDeploying(application.id())));
         }
         return versionMap.values();
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
index 5af5dac714f..299681b65bb 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
@@ -23,6 +23,7 @@ import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
 
 import java.time.Duration;
 import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.Optional;
 import java.util.UUID;
 import java.util.stream.Collectors;
@@ -224,21 +225,20 @@ public class DeploymentTester {
     /** Assert that the sceduled jobs of this application are exactly those given, and take them */
     private void consumeJobs(Application application, boolean expectOnlyTheseJobs, JobType... jobs) {
         for (JobType job : jobs) {
-            Optional<BuildService.BuildJob> buildJob = findJob(application, job);
-            assertTrue(String.format("Job %s is scheduled for %s", job, application), buildJob.isPresent());
-            assertEquals((long) application.deploymentJobs().projectId().get(), buildJob.get().projectId());
-            assertEquals(job.id(), buildJob.get().jobName());
+            BuildService.BuildJob buildJob = findJob(application, job);
+            assertEquals((long) application.deploymentJobs().projectId().get(), buildJob.projectId());
+            assertEquals(job.id(), buildJob.jobName());
         }
         if (expectOnlyTheseJobs)
             assertEquals(jobs.length, countJobsOf(application));
         buildSystem().removeJobs(application.id());
     }
 
-    private Optional<BuildService.BuildJob> findJob(Application application, JobType jobType) {
+    private BuildService.BuildJob findJob(Application application, JobType jobType) {
         for (BuildService.BuildJob job : buildSystem().jobs())
             if (job.projectId() == application.deploymentJobs().projectId().get() && job.jobName().equals(jobType.id()))
-                return Optional.of(job);
-        return Optional.empty();
+                return job;
+        throw new NoSuchElementException(jobType + " is not scheduled for " + application);
     }
 
     private int countJobsOf(Application application) {
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 7d1d507807a..7ed512cc199 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,7 @@ import org.junit.Test;
 
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.Collections;
 import java.util.List;
 
 import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.component;
@@ -28,6 +30,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 +98,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
@@ -264,7 +264,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));