Skip to content

Commit

Permalink
Split ResolveResultRefs in two
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
afrittoli authored and tekton-robot committed May 5, 2020
1 parent 3143aa5 commit d5a91fd
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 6 deletions.
4 changes: 2 additions & 2 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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{
Expand Down
11 changes: 9 additions & 2 deletions pkg/reconciler/pipelinerun/resources/resultrefresolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -46,13 +46,20 @@ func ResolveResultRefs(pipelineRunState PipelineRunState, targets PipelineRunSta
}
allResolvedResultRefs = append(allResolvedResultRefs, resolvedResultRefs...)
}
return removeDup(allResolvedResultRefs), nil
}

// ResolvePipelineResultRefs takes a list of PipelineResults and resolves any references they
// include to Task results in the given PipelineRunState
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
Expand Down
119 changes: 118 additions & 1 deletion pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
})
Expand All @@ -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{
{
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{
{
Name: "from-a",
Value: "$(tasks.aTask.results.aResult)",
Description: "a result from a",
},
{
Name: "from-b",
Value: "$(tasks.bTask.results.bResult)",
Description: "a result from b",
},
{
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)
}
})
}
}
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/resources/validate_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
)

// Validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type.
// ValidateParamTypesMatching validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type.
func ValidateParamTypesMatching(p *v1alpha1.PipelineSpec, pr *v1alpha1.PipelineRun) error {
// Build a map of parameter names/types declared in p.
paramTypes := make(map[string]v1alpha1.ParamType)
Expand Down

0 comments on commit d5a91fd

Please sign in to comment.