From beeeb56f687714dbe84d55e4afc8e8f4ccc5a0de Mon Sep 17 00:00:00 2001
From: dthomson25 <dthomson25@gmail.com>
Date: Thu, 6 Feb 2020 14:27:53 -0800
Subject: [PATCH] Experiments passed duration succeed with running analysis
 (#392)

---
 experiments/analysisrun_test.go     | 48 ++++++++++++++++++++++++++---
 experiments/experiment.go           |  7 +++--
 utils/experiment/experiment.go      |  9 ++++++
 utils/experiment/experiment_test.go | 15 +++++++++
 4 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/experiments/analysisrun_test.go b/experiments/analysisrun_test.go
index 86e5ecb5c0..4cb5603811 100644
--- a/experiments/analysisrun_test.go
+++ b/experiments/analysisrun_test.go
@@ -306,12 +306,14 @@ func TestAssessAnalysisRunStatusesAfterTemplateSuccess(t *testing.T) {
 	e := newExperiment("foo", templates, "")
 	e.Spec.Analyses = []v1alpha1.ExperimentAnalysisTemplateRef{
 		{
-			Name:         "success-rate",
-			TemplateName: "success-rate",
+			Name:                  "success-rate",
+			TemplateName:          "success-rate",
+			RequiredForCompletion: true,
 		},
 		{
-			Name:         "latency",
-			TemplateName: "latency",
+			Name:                  "latency",
+			TemplateName:          "latency",
+			RequiredForCompletion: true,
 		},
 	}
 	e.Status.Phase = v1alpha1.AnalysisPhaseRunning
@@ -569,6 +571,44 @@ func TestDoNotCompleteExperimentWithRemainingRequiredAnalysisRun(t *testing.T) {
 	assert.NotEqual(t, patchedEx.Status.Phase, v1alpha1.AnalysisPhaseSuccessful)
 }
 
+func TestCompleteExperimentWithNoRequiredAnalysis(t *testing.T) {
+	templates := generateTemplates("bar")
+	e := newExperiment("foo", templates, "1m")
+	e.Spec.Analyses = []v1alpha1.ExperimentAnalysisTemplateRef{
+		{
+			Name:         "success-rate",
+			TemplateName: "success-rate",
+		},
+	}
+	e.Status.Phase = v1alpha1.AnalysisPhaseRunning
+	e.Status.AvailableAt = secondsAgo(61)
+	rs := templateToRS(e, templates[0], 0)
+	rs.Spec.Replicas = new(int32)
+	ar := analysisTemplateToRun("success-rate", e, &v1alpha1.AnalysisTemplateSpec{})
+	ar.Status = v1alpha1.AnalysisRunStatus{
+		Phase: v1alpha1.AnalysisPhaseRunning,
+	}
+	e.Status.TemplateStatuses = []v1alpha1.TemplateStatus{{
+		Name:   "bar",
+		Status: v1alpha1.TemplateStatusSuccessful,
+	}}
+	e.Status.AnalysisRuns = []v1alpha1.ExperimentAnalysisRunStatus{
+		{
+			Name:        e.Spec.Analyses[0].Name,
+			Phase:       v1alpha1.AnalysisPhaseRunning,
+			AnalysisRun: ar.Name,
+		},
+	}
+
+	f := newFixture(t, e, rs, ar)
+	defer f.Close()
+	patchIndex := f.expectPatchExperimentAction(e)
+	f.run(getKey(e, t))
+	patchedEx := f.getPatchedExperimentAsObj(patchIndex)
+	//assert.True(t, patchedEx.Spec.Terminate)
+	assert.Equal(t, patchedEx.Status.Phase, v1alpha1.AnalysisPhaseSuccessful)
+}
+
 // TestTerminateAnalysisRuns verifies we terminate analysis runs when experiment is terminating
 func TestTerminateAnalysisRuns(t *testing.T) {
 	templates := generateTemplates("bar")
diff --git a/experiments/experiment.go b/experiments/experiment.go
index 1889b1cd35..fd4990d670 100644
--- a/experiments/experiment.go
+++ b/experiments/experiment.go
@@ -388,10 +388,11 @@ func (ec *experimentContext) calculateStatus() *v1alpha1.ExperimentStatus {
 			ec.log.Infof("Marked AvailableAt: %v", now)
 		}
 		if templateStatus.Completed() {
-			if templateStatus == v1alpha1.AnalysisPhaseSuccessful {
+			analysisRun := analysesStatus.Completed() || experimentutil.HasRequiredAnalysisRuns(ec.ex)
+			if templateStatus == v1alpha1.AnalysisPhaseSuccessful && analysisRun {
 				// If the templates have completed successfully (e.g. it ran without degrading for
-				// the entire duration), then the status of the Experiment is deferred to the status
-				// the analyses results.
+				// the entire duration), and there are required analysis runs or they have completed, then
+				// the status of the Experiment is deferred to the status of the analyses results.
 				ec.newStatus.Phase = analysesStatus
 				ec.newStatus.Message = analysesMessage
 			} else {
diff --git a/utils/experiment/experiment.go b/utils/experiment/experiment.go
index 5e26accd1b..57f4337efc 100644
--- a/utils/experiment/experiment.go
+++ b/utils/experiment/experiment.go
@@ -49,6 +49,15 @@ func IsTerminating(experiment *v1alpha1.Experiment) bool {
 	return RequiredAnalysisRunsSuccessful(experiment, &experiment.Status)
 }
 
+func HasRequiredAnalysisRuns(ex *v1alpha1.Experiment) bool {
+	for _, analysis := range ex.Spec.Analyses {
+		if analysis.RequiredForCompletion {
+			return true
+		}
+	}
+	return false
+}
+
 // RequiredAnalysisRunsSuccessful has at least one required for completition analysis run
 // and it completed successfully
 func RequiredAnalysisRunsSuccessful(ex *v1alpha1.Experiment, exStatus *v1alpha1.ExperimentStatus) bool {
diff --git a/utils/experiment/experiment_test.go b/utils/experiment/experiment_test.go
index 4df7ada659..691c250755 100644
--- a/utils/experiment/experiment_test.go
+++ b/utils/experiment/experiment_test.go
@@ -389,3 +389,18 @@ func TestRequiredAnalysisRunsSuccessful(t *testing.T) {
 	e.Status.AnalysisRuns[0].Phase = v1alpha1.AnalysisPhaseSuccessful
 	assert.True(t, RequiredAnalysisRunsSuccessful(e, &e.Status))
 }
+
+func TestHasRequiredAnalysisRuns(t *testing.T) {
+	e := &v1alpha1.Experiment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "foo",
+		},
+	}
+	assert.False(t, HasRequiredAnalysisRuns(e))
+	e.Spec.Analyses = []v1alpha1.ExperimentAnalysisTemplateRef{{
+		Name: "foo",
+	}}
+	assert.False(t, HasRequiredAnalysisRuns(e))
+	e.Spec.Analyses[0].RequiredForCompletion = true
+	assert.True(t, HasRequiredAnalysisRuns(e))
+}