Refactor updateTaskRunsStatus to getTaskRunsStatus

Instead of modifying the passed in pipelinerun's status, return the status to be updated. Also, updates the multiple status tests with conditionChecks to a single one with multiple inputs
tektoncd · Jul 29, 2019 · c8174e7 · c8174e7
1 parent f737fec
commit c8174e7
Show file tree

Hide file tree

Showing 8 changed files with 132 additions and 230 deletions.
diff --git a/pkg/apis/pipeline/v1alpha1/condition_types_test.go b/pkg/apis/pipeline/v1alpha1/condition_types_test.go
@@ -27,7 +27,7 @@ import (
 )
 
 func TestConditionCheck_IsDone(t *testing.T) {
-	tr := tb.TaskRun("", "", tb.TaskRunStatus(tb.Condition(
+	tr := tb.TaskRun("", "", tb.TaskRunStatus(tb.StatusCondition(
 		apis.Condition{
 			Type:   apis.ConditionSucceeded,
 			Status: corev1.ConditionFalse,
@@ -41,15 +41,15 @@ func TestConditionCheck_IsDone(t *testing.T) {
 }
 
 func TestConditionCheck_IsSuccessful(t *testing.T) {
-	tr := tb.TaskRun("", "", tb.TaskRunStatus(tb.Condition(
+	tr := tb.TaskRun("", "", tb.TaskRunStatus(tb.StatusCondition(
 		apis.Condition{
 			Type:   apis.ConditionSucceeded,
 			Status: corev1.ConditionTrue,
 		},
 	)))
 
 	cc := v1alpha1.ConditionCheck(*tr)
-	if !cc.IsDone() {
+	if !cc.IsSuccessful() {
 		t.Fatal("Expected conditionCheck status to be done")
 	}
 }
diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
@@ -369,7 +369,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er
 				return xerrors.Errorf("error creating TaskRun called %s for PipelineTask %s from PipelineRun %s: %w", rprt.TaskRunName, rprt.PipelineTask.Name, pr.Name, err)
 			}
 		} else if !rprt.ResolvedConditionChecks.HasStarted() {
-			// FIXME: Move this on to its own function
 			for _, rcc := range rprt.ResolvedConditionChecks {
 				rcc.ConditionCheck, err = c.makeConditionCheckContainer(rprt, rcc, pr)
 				if err != nil {
@@ -384,17 +383,19 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er
 	pr.Status.SetCondition(after)
 	reconciler.EmitEvent(c.Recorder, before, after, pr)
 
-	updateTaskRunsStatus(pr, pipelineState)
+	pr.Status.TaskRuns = getTaskRunsStatus(pr, pipelineState)
 
 	c.Logger.Infof("PipelineRun %s status is being set to %s", pr.Name, pr.Status.GetCondition(apis.ConditionSucceeded))
 	return nil
 }
 
-func updateTaskRunsStatus(pr *v1alpha1.PipelineRun, pipelineState []*resources.ResolvedPipelineRunTask) {
-	for _, rprt := range pipelineState {
+func getTaskRunsStatus(pr *v1alpha1.PipelineRun, state []*resources.ResolvedPipelineRunTask) map[string]*v1alpha1.PipelineRunTaskRunStatus {
+	status := make(map[string]*v1alpha1.PipelineRunTaskRunStatus)
+	for _, rprt := range state {
 		if rprt.TaskRun == nil && rprt.ResolvedConditionChecks == nil {
 			continue
 		}
+
 		var prtrs *v1alpha1.PipelineRunTaskRunStatus
 		if rprt.TaskRun != nil {
 			prtrs = pr.Status.TaskRuns[rprt.TaskRun.Name]
@@ -416,8 +417,7 @@ func updateTaskRunsStatus(pr *v1alpha1.PipelineRun, pipelineState []*resources.R
 					ConditionName: c.Condition.Name,
 				}
 				if c.ConditionCheck != nil {
-					ccStatus := c.NewConditionCheckStatus()
-					cStatus[c.ConditionCheckName].Status = &ccStatus
+					cStatus[c.ConditionCheckName].Status = c.NewConditionCheckStatus()
 				}
 			}
 			prtrs.ConditionChecks = cStatus
@@ -433,8 +433,9 @@ func updateTaskRunsStatus(pr *v1alpha1.PipelineRun, pipelineState []*resources.R
 				})
 			}
 		}
-		pr.Status.TaskRuns[rprt.TaskRunName] = prtrs
+		status[rprt.TaskRunName] = prtrs
 	}
+	return status
 }
 
 func (c *Reconciler) updateTaskRunsStatusDirectly(pr *v1alpha1.PipelineRun) error {