1 files changed, 61 insertions, 10 deletions
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
index 0f6aec804e2..f7d451ab931 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
@@ -189,7 +190,7 @@ public class JobRunnerTest {
 
         // Start a third run, then unregister and wait for data to be deleted.
         jobs.start(id, systemTest, versions);
-        jobs.unregister(appId);
+        tester.applications().deleteInstance(id);
         runner.maintain();
         assertFalse(jobs.last(id, systemTest).isPresent());
         assertTrue(jobs.runs(id, systemTest).isEmpty());
@@ -242,24 +243,74 @@ public class JobRunnerTest {
                                          inThreadExecutor(), (id, step) -> Optional.of(running));
 
         TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id();
-        ApplicationId id = appId.defaultInstance();
+        ApplicationId instanceId = appId.defaultInstance();
+        JobId jobId = new JobId(instanceId, systemTest);
         jobs.submit(appId, versions.targetApplication().source().get(), "a@b", 2, applicationPackage, new byte[0]);
+        assertFalse(jobs.lastSuccess(jobId).isPresent());
 
         for (int i = 0; i < jobs.historyLength(); i++) {
-            jobs.start(id, systemTest, versions);
+            jobs.start(instanceId, systemTest, versions);
             runner.run();
         }
 
-        assertEquals(256, jobs.runs(id, systemTest).size());
-        assertTrue(jobs.details(new RunId(id, systemTest, 1)).isPresent());
+        assertEquals(256, jobs.runs(jobId).size());
+        assertTrue(jobs.details(new RunId(instanceId, systemTest, 1)).isPresent());
 
-        jobs.start(id, systemTest, versions);
+        jobs.start(instanceId, systemTest, versions);
         runner.run();
 
-        assertEquals(256, jobs.runs(id, systemTest).size());
-        assertEquals(2, jobs.runs(id, systemTest).keySet().iterator().next().number());
-        assertFalse(jobs.details(new RunId(id, systemTest, 1)).isPresent());
-        assertTrue(jobs.details(new RunId(id, systemTest, 257)).isPresent());
+        assertEquals(256, jobs.runs(jobId).size());
+        assertEquals(2, jobs.runs(jobId).keySet().iterator().next().number());
+        assertFalse(jobs.details(new RunId(instanceId, systemTest, 1)).isPresent());
+        assertTrue(jobs.details(new RunId(instanceId, systemTest, 257)).isPresent());
+
+        JobRunner failureRunner = new JobRunner(tester.controller(), Duration.ofDays(1), new JobControl(tester.controller().curator()),
+                                                inThreadExecutor(), (id, step) -> Optional.of(error));
+
+        // Make all but the oldest of the 256 jobs a failure.
+        for (int i = 0; i < jobs.historyLength() - 1; i++) {
+            jobs.start(instanceId, systemTest, versions);
+            failureRunner.run();
+        }
+        assertEquals(256, jobs.runs(jobId).size());
+        assertEquals(257, jobs.runs(jobId).keySet().iterator().next().number());
+        assertEquals(257, jobs.lastSuccess(jobId).get().id().number());
+        assertEquals(258, jobs.firstFailing(jobId).get().id().number());
+
+        // Oldest success is kept even though it would normally overflow.
+        jobs.start(instanceId, systemTest, versions);
+        failureRunner.run();
+        assertEquals(257, jobs.runs(jobId).size());
+        assertEquals(257, jobs.runs(jobId).keySet().iterator().next().number());
+        assertEquals(257, jobs.lastSuccess(jobId).get().id().number());
+        assertEquals(258, jobs.firstFailing(jobId).get().id().number());
+
+        // First failure after the last success is also kept.
+        jobs.start(instanceId, systemTest, versions);
+        failureRunner.run();
+        assertEquals(258, jobs.runs(jobId).size());
+        assertEquals(257, jobs.runs(jobId).keySet().iterator().next().number());
+        assertEquals(258, jobs.runs(jobId).keySet().stream().skip(1).iterator().next().number());
+        assertEquals(257, jobs.lastSuccess(jobId).get().id().number());
+        assertEquals(258, jobs.firstFailing(jobId).get().id().number());
+
+        // No other jobs are kept with repeated failures.
+        jobs.start(instanceId, systemTest, versions);
+        failureRunner.run();
+        assertEquals(258, jobs.runs(jobId).size());
+        assertEquals(257, jobs.runs(jobId).keySet().iterator().next().number());
+        assertEquals(258, jobs.runs(jobId).keySet().stream().skip(1).iterator().next().number());
+        assertEquals(260, jobs.runs(jobId).keySet().stream().skip(2).iterator().next().number());
+        assertEquals(257, jobs.lastSuccess(jobId).get().id().number());
+        assertEquals(258, jobs.firstFailing(jobId).get().id().number());
+
+        // history length returns to 256 when a new success is recorded.
+        jobs.start(instanceId, systemTest, versions);
+        runner.run();
+        assertEquals(256, jobs.runs(jobId).size());
+        assertEquals(261, jobs.runs(jobId).keySet().iterator().next().number());
+        assertEquals(516, jobs.lastSuccess(jobId).get().id().number());
+        assertFalse(jobs.firstFailing(jobId).isPresent());
     }
 
     @Test