Split ResolveResultRefs in two

ResolveResultRefs is always invoked with either the 2nd or the 3rd
parameter set to nil, so it's actually two different things into
one function.
The second part of the function had not tests, so split it into a
separate function and add tests for it.

Author: afrittoli

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -318,7 +318,7 @@ func (c *Reconciler) updatePipelineResults(ctx context.Context, pr *v1alpha1.Pip
 				})
 			}
 		}
-		resolvedResultRefs, _ := resources.ResolveResultRefs(pipelineState, nil, pipelineSpec.Results)
+		resolvedResultRefs := resources.ResolvePipelineResultRefs(pipelineState, pipelineSpec.Results)
 		pr.Status.PipelineResults = getPipelineRunResults(pipelineSpec, resolvedResultRefs)
 	}
 }
@@ -557,7 +557,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er
 	}
 
 	nextRprts := pipelineState.GetNextTasks(candidateTasks)
-	resolvedResultRefs, err := resources.ResolveResultRefs(pipelineState, nextRprts, nil)
+	resolvedResultRefs, err := resources.ResolveResultRefs(pipelineState, nextRprts)
 	if err != nil {
 		c.Logger.Infof("Failed to resolve all task params for %q with error %v", pr.Name, err)
 		pr.Status.SetCondition(&apis.Condition{

pkg/reconciler/pipelinerun/resources/resultrefresolution.go

@@ -37,7 +37,7 @@ type ResolvedResultRef struct {
 }
 
 // ResolveResultRefs resolves any ResultReference that are found in the target ResolvedPipelineRunTask
-func ResolveResultRefs(pipelineRunState PipelineRunState, targets PipelineRunState, pipelineResults []v1beta1.PipelineResult) (ResolvedResultRefs, error) {
+func ResolveResultRefs(pipelineRunState PipelineRunState, targets PipelineRunState) (ResolvedResultRefs, error) {
 	var allResolvedResultRefs ResolvedResultRefs
 	for _, target := range targets {
 		resolvedResultRefs, err := convertParamsToResultRefs(pipelineRunState, target)
@@ -46,13 +46,19 @@ func ResolveResultRefs(pipelineRunState PipelineRunState, targets PipelineRunSta
 		}
 		allResolvedResultRefs = append(allResolvedResultRefs, resolvedResultRefs...)
 	}
+	return removeDup(allResolvedResultRefs), nil
+}
+
+// ResolvePipelineResultRefs resolves any ResultReference that are found in the target ResolvedPipelineRunTask
+func ResolvePipelineResultRefs(pipelineRunState PipelineRunState, pipelineResults []v1beta1.PipelineResult) ResolvedResultRefs {
+	var allResolvedResultRefs ResolvedResultRefs
 	for _, result := range pipelineResults {
 		resolvedResultRefs := convertPipelineResultToResultRefs(pipelineRunState, result)
 		if resolvedResultRefs != nil {
 			allResolvedResultRefs = append(allResolvedResultRefs, resolvedResultRefs...)
 		}
 	}
-	return removeDup(allResolvedResultRefs), nil
+	return removeDup(allResolvedResultRefs)
 }
 
 // extractResultRefs resolves any ResultReference that are found in param or pipeline result

pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go

@@ -10,6 +10,8 @@ import (
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	tb "github.com/tektoncd/pipeline/test/builder"
+	corev1 "k8s.io/api/core/v1"
+	"knative.dev/pkg/apis"
 )
 
 func TestTaskParamResolver_ResolveResultRefs(t *testing.T) {
@@ -366,7 +368,7 @@ func TestResolveResultRefs(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := ResolveResultRefs(tt.args.pipelineRunState, tt.args.targets, nil)
+			got, err := ResolveResultRefs(tt.args.pipelineRunState, tt.args.targets)
 			sort.SliceStable(got, func(i, j int) bool {
 				return strings.Compare(got[i].FromTaskRun, got[j].FromTaskRun) < 0
 			})
@@ -380,3 +382,118 @@ func TestResolveResultRefs(t *testing.T) {
 		})
 	}
 }
+
+func TestResolvePipelineResultRefs(t *testing.T) {
+	type args struct {
+		pipelineRunState PipelineRunState
+		pipelineResults  []v1beta1.PipelineResult
+	}
+	pipelineRunState := PipelineRunState{
+		{
+			TaskRunName: "aTaskRun",
+			TaskRun: tb.TaskRun("aTaskRun", tb.TaskRunStatus(
+				tb.TaskRunResult("aResult", "aResultValue"),
+			)),
+			PipelineTask: &v1alpha1.PipelineTask{
+				Name:    "aTask",
+				TaskRef: &v1alpha1.TaskRef{Name: "aTask"},
+			},
+		}, {
+			TaskRunName: "bTaskRun",
+			TaskRun: tb.TaskRun("bTaskRun", tb.TaskRunStatus(
+				tb.StatusCondition(apis.Condition{
+					Type:   apis.ConditionSucceeded,
+					Status: corev1.ConditionFalse})),
+			),
+			PipelineTask: &v1alpha1.PipelineTask{
+				Name:    "bTask",
+				TaskRef: &v1alpha1.TaskRef{Name: "bTask"},
+			},
+		}, {
+			PipelineTask: &v1alpha1.PipelineTask{
+				Name:    "cTask",
+				TaskRef: &v1alpha1.TaskRef{Name: "cTask"},
+			},
+		},
+	}
+
+	tests := []struct {
+		name string
+		args args
+		want ResolvedResultRefs
+	}{
+		{
+			name: "Test pipeline result from a successful task",
+			args: args{
+				pipelineRunState: pipelineRunState,
+				pipelineResults: []v1beta1.PipelineResult{
+					v1beta1.PipelineResult{
+						Name:        "from-a",
+						Value:       "$(tasks.aTask.results.aResult)",
+						Description: "a result from a",
+					},
+				},
+			},
+			want: ResolvedResultRefs{
+				{
+					Value: v1beta1.ArrayOrString{
+						Type:      v1beta1.ParamTypeString,
+						StringVal: "aResultValue",
+					},
+					ResultReference: v1beta1.ResultRef{
+						PipelineTask: "aTask",
+						Result:       "aResult",
+					},
+					FromTaskRun: "aTaskRun",
+				},
+			},
+		},
+		{
+			name: "Test results from a task that did not run and one and failed",
+			args: args{
+				pipelineRunState: pipelineRunState,
+				pipelineResults: []v1beta1.PipelineResult{
+					v1beta1.PipelineResult{
+						Name:        "from-a",
+						Value:       "$(tasks.aTask.results.aResult)",
+						Description: "a result from a",
+					},
+					v1beta1.PipelineResult{
+						Name:        "from-b",
+						Value:       "$(tasks.bTask.results.bResult)",
+						Description: "a result from b",
+					},
+					v1beta1.PipelineResult{
+						Name:        "from-c",
+						Value:       "$(tasks.cTask.results.cResult)",
+						Description: "a result from c",
+					},
+				},
+			},
+			want: ResolvedResultRefs{
+				{
+					Value: v1beta1.ArrayOrString{
+						Type:      v1beta1.ParamTypeString,
+						StringVal: "aResultValue",
+					},
+					ResultReference: v1beta1.ResultRef{
+						PipelineTask: "aTask",
+						Result:       "aResult",
+					},
+					FromTaskRun: "aTaskRun",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ResolvePipelineResultRefs(tt.args.pipelineRunState, tt.args.pipelineResults)
+			sort.SliceStable(got, func(i, j int) bool {
+				return strings.Compare(got[i].FromTaskRun, got[j].FromTaskRun) < 0
+			})
+			if d := cmp.Diff(tt.want, got); d != "" {
+				t.Fatalf("ResolveResultRef  -want, +got: %v", d)
+			}
+		})
+	}
+}