chore: remove job status: finished (#844)

* chore: remove job status: finished

* chore: fix status ring test

Author: yashmehrotra

job/job_test.go

@@ -22,7 +22,8 @@ func TestStatusRing(t *testing.T) {
 		{Success: 3, Failed: 3},
 	}
 	var total int
-	var expected = 2000 - (5 * 6 * 2)
+	loops := 100
+	var expected = (len(cases) * loops * 3) - (3 * 3 * len(cases))
 
 	eg, _ := errgroup.WithContext(context.TODO())
 	eg.Go(func() error {
@@ -40,10 +41,8 @@ func TestStatusRing(t *testing.T) {
 		td := cases[i]
 		eg.Go(func() error {
 			sr := newStatusRing(td, false, ch)
-			for i := 0; i < 100; i++ {
+			for i := 0; i < loops; i++ {
 				sr.Add(&models.JobHistory{ID: uuid.New(), Status: string(models.StatusSuccess)})
-				sr.Add(&models.JobHistory{ID: uuid.New(), Status: string(models.StatusFinished)})
-
 				sr.Add(&models.JobHistory{ID: uuid.New(), Status: string(models.StatusFailed)})
 				sr.Add(&models.JobHistory{ID: uuid.New(), Status: string(models.StatusWarning)})
 			}
@@ -54,8 +53,8 @@ func TestStatusRing(t *testing.T) {
 	_ = eg.Wait()
 	total += len(ch)
 
-	// we have added 2000 job  history to the status rings
-	// based on retention, 5*6*2 jobs remain in the status rings
+	// we have added 1500 job  history to the status rings
+	// based on retention, 5*3*3 (cases * uniq status * retention for uniq status) jobs remain in the status rings
 	// while the rest of them should have been moved to the evicted channel
 	if total != expected {
 		t.Fatalf("Expected %d job ids in the channel. Got %d", expected, total)

models/job_history.go

@@ -141,17 +141,12 @@ func (h *JobHistory) evaluateStatus() {
 	if h.Status != StatusRunning {
 		return
 	}
-	if h.SuccessCount == 0 {
-		if h.ErrorCount > 0 {
-			h.Status = StatusFailed
-		} else {
-			h.Status = StatusFinished
-		}
+
+	if h.ErrorCount > 0 && h.SuccessCount > 0 {
+		h.Status = StatusWarning
+	} else if h.ErrorCount > 0 && h.SuccessCount == 0 {
+		h.Status = StatusFailed
 	} else {
-		if h.ErrorCount == 0 {
-			h.Status = StatusSuccess
-		} else {
-			h.Status = StatusWarning
-		}
+		h.Status = StatusSuccess
 	}
 }

tests/job_test.go

@@ -53,9 +53,9 @@ var _ = Describe("Job", Ordered, func() {
 		counts := lo.CountValuesBy(items, func(j models.JobHistory) string { return j.Status })
 
 		Expect(len(items)).To(BeNumerically("==", 4))
-		Expect(counts[models.StatusFinished]).To(Equal(2))
+		Expect(counts[models.StatusSuccess]).To(Equal(2))
 		Expect(counts[models.StatusSkipped]).To(Equal(2))
-		for _, item := range groups[models.StatusFinished] {
+		for _, item := range groups[models.StatusSuccess] {
 			Expect(item.TimeEnd).ToNot(BeNil())
 			Expect(item.TimeEnd.Sub(item.TimeStart).Milliseconds()).To(BeNumerically("~", 50, 10))
 		}

tests/setup/common.go

@@ -226,7 +226,7 @@ func ExpectJobToPass(j *job.Job) {
 	history, err := j.FindHistory()
 	Expect(err).To(BeNil())
 	Expect(len(history)).To(BeNumerically(">=", 1))
-	Expect(history[0].Status).To(BeElementOf(models.StatusFinished, models.StatusSuccess))
+	Expect(history[0].Status).To(BeElementOf(models.StatusSuccess))
 }
 
 func DumpEventQueue(ctx context.Context) {