Skip to content

Commit

Permalink
divide ApplyTaskResultsToPipelineResults tests into success and error
Browse files Browse the repository at this point in the history 
This commit divides ApplyTaskResultsToPipelineResults unit tests into
success and error, two separate tests to improve the readibility and
easier to find bugs. Before this commit, the unit test doesn't check the
case when the return error is not nil but expect nil, and some errors
are not checked.

Signed-off-by: Yongxuan Zhang [email protected]
  • Loading branch information
@Yongxuanzhang
Yongxuanzhang committed Mar 17, 2023
1 parent 03512d8 commit f0e0dda
Showing 1 changed file with 170 additions and 142 deletions.
312 changes: 170 additions & 142 deletions pkg/reconciler/pipelinerun/resources/apply_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3505,7 +3505,7 @@ func TestApplyFinallyResultsToPipelineResults(t *testing.T) {
}
}

func TestApplyTaskResultsToPipelineResults(t *testing.T) {
func TestApplyTaskResultsToPipelineResults_Success(t *testing.T) {
for _, tc := range []struct {
description string
results []v1beta1.PipelineResult
Expand All @@ -3529,60 +3529,6 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) {
},
},
expectedResults: nil,
}, {
description: "array-index-out-of-bound",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.pt1.results.foo[4])"),
}},
taskResults: map[string][]v1beta1.TaskRunResult{
"pt1": {
{
Name: "foo",
Value: *v1beta1.NewStructuredValues("do", "rae", "mi"),
},
},
},
expectedResults: nil,
expectedError: fmt.Errorf("invalid pipelineresults [pipeline-result-1], the referred results don't exist"),
}, {
description: "object-reference-key-not-exist",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.pt1.results.foo.key3)"),
}},
taskResults: map[string][]v1beta1.TaskRunResult{
"pt1": {
{
Name: "foo",
Value: *v1beta1.NewObject(map[string]string{
"key1": "val1",
"key2": "val2",
}),
},
},
},
expectedResults: nil,
expectedError: fmt.Errorf("invalid pipelineresults [pipeline-result-1], the referred results don't exist"),
}, {
description: "object-results-resultname-not-exist",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.pt1.results.bar.key1)"),
}},
taskResults: map[string][]v1beta1.TaskRunResult{
"pt1": {
{
Name: "foo",
Value: *v1beta1.NewObject(map[string]string{
"key1": "val1",
"key2": "val2",
}),
},
},
},
expectedResults: nil,
expectedError: fmt.Errorf("invalid pipelineresults [pipeline-result-1], the referred results don't exist"),
}, {
description: "apply-array-results",
results: []v1beta1.PipelineResult{{
Expand Down Expand Up @@ -3764,6 +3710,165 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) {
}},
},
expectedResults: nil,
}, {
description: "multiple-results-multiple-successful-tasks ",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.pt1.results.foo)"),
}, {
Name: "pipeline-result-2",
Value: *v1beta1.NewStructuredValues("$(tasks.pt1.results.foo), $(tasks.pt2.results.baz), $(tasks.pt1.results.bar), $(tasks.pt2.results.baz), $(tasks.pt1.results.foo)"),
}},
taskResults: map[string][]v1beta1.TaskRunResult{
"pt1": {
{
Name: "foo",
Value: *v1beta1.NewStructuredValues("do"),
}, {
Name: "bar",
Value: *v1beta1.NewStructuredValues("mi"),
},
},
"pt2": {{
Name: "baz",
Value: *v1beta1.NewStructuredValues("rae"),
}},
},
expectedResults: []v1beta1.PipelineRunResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("do"),
}, {
Name: "pipeline-result-2",
Value: *v1beta1.NewStructuredValues("do, rae, mi, rae, do"),
}},
}, {
description: "multiple-results-custom-and-normal-tasks",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.customtask.results.foo)"),
}, {
Name: "pipeline-result-2",
Value: *v1beta1.NewStructuredValues("$(tasks.customtask.results.foo), $(tasks.normaltask.results.baz), $(tasks.customtask.results.bar), $(tasks.normaltask.results.baz), $(tasks.customtask.results.foo)"),
}},
runResults: map[string][]v1beta1.CustomRunResult{
"customtask": {
{
Name: "foo",
Value: "do",
}, {
Name: "bar",
Value: "mi",
},
},
},
taskResults: map[string][]v1beta1.TaskRunResult{
"normaltask": {{
Name: "baz",
Value: *v1beta1.NewStructuredValues("rae"),
}},
},
expectedResults: []v1beta1.PipelineRunResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("do"),
}, {
Name: "pipeline-result-2",
Value: *v1beta1.NewStructuredValues("do, rae, mi, rae, do"),
}},
}, {
description: "multiple-results-skipped-and-normal-tasks",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.skippedTask.results.foo)"),
}, {
Name: "pipeline-result-2",
Value: *v1beta1.NewStructuredValues("$(tasks.skippedTask.results.foo), $(tasks.normaltask.results.baz)"),
}},
taskResults: map[string][]v1beta1.TaskRunResult{
"normaltask": {{
Name: "baz",
Value: *v1beta1.NewStructuredValues("rae"),
}},
},
skippedTasks: []v1beta1.SkippedTask{{
Name: "skippedTask",
}},
expectedResults: nil,
}} {
t.Run(tc.description, func(t *testing.T) {
received, err := ApplyTaskResultsToPipelineResults(context.Background(), tc.results, tc.taskResults, tc.runResults, tc.skippedTasks)
if err != nil {
t.Errorf("Got unecpected error:%v", err)
}
if d := cmp.Diff(tc.expectedResults, received); d != "" {
t.Errorf(diff.PrintWantGot(d))
}
})
}
}

func TestApplyTaskResultsToPipelineResults_Error(t *testing.T) {
for _, tc := range []struct {
description string
results []v1beta1.PipelineResult
taskResults map[string][]v1beta1.TaskRunResult
runResults map[string][]v1beta1.CustomRunResult
skippedTasks []v1beta1.SkippedTask
expectedResults []v1beta1.PipelineRunResult
expectedError error
}{{
description: "array-index-out-of-bound",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.pt1.results.foo[4])"),
}},
taskResults: map[string][]v1beta1.TaskRunResult{
"pt1": {
{
Name: "foo",
Value: *v1beta1.NewStructuredValues("do", "rae", "mi"),
},
},
},
expectedResults: nil,
expectedError: fmt.Errorf("invalid pipelineresults [pipeline-result-1], the referred results don't exist"),
}, {
description: "object-reference-key-not-exist",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.pt1.results.foo.key3)"),
}},
taskResults: map[string][]v1beta1.TaskRunResult{
"pt1": {
{
Name: "foo",
Value: *v1beta1.NewObject(map[string]string{
"key1": "val1",
"key2": "val2",
}),
},
},
},
expectedResults: nil,
expectedError: fmt.Errorf("invalid pipelineresults [pipeline-result-1], the referred results don't exist"),
}, {
description: "object-results-resultname-not-exist",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.pt1.results.bar.key1)"),
}},
taskResults: map[string][]v1beta1.TaskRunResult{
"pt1": {
{
Name: "foo",
Value: *v1beta1.NewObject(map[string]string{
"key1": "val1",
"key2": "val2",
}),
},
},
},
expectedResults: nil,
expectedError: fmt.Errorf("invalid pipelineresults [pipeline-result-1], the referred results don't exist"),
}, {
description: "invalid-result-variable-no-returned-result",
results: []v1beta1.PipelineResult{{
Expand Down Expand Up @@ -3802,6 +3907,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) {
}},
},
expectedResults: nil,
expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"),
}, {
description: "invalid-result-name-no-returned-result",
results: []v1beta1.PipelineResult{{
Expand All @@ -3824,6 +3930,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) {
}},
taskResults: map[string][]v1beta1.TaskRunResult{},
expectedResults: nil,
expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"),
}, {
description: "mixed-success-tasks-some-returned-results",
results: []v1beta1.PipelineResult{{
Expand All @@ -3844,37 +3951,6 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) {
Value: *v1beta1.NewStructuredValues("rae"),
}},
expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"),
}, {
description: "multiple-results-multiple-successful-tasks ",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.pt1.results.foo)"),
}, {
Name: "pipeline-result-2",
Value: *v1beta1.NewStructuredValues("$(tasks.pt1.results.foo), $(tasks.pt2.results.baz), $(tasks.pt1.results.bar), $(tasks.pt2.results.baz), $(tasks.pt1.results.foo)"),
}},
taskResults: map[string][]v1beta1.TaskRunResult{
"pt1": {
{
Name: "foo",
Value: *v1beta1.NewStructuredValues("do"),
}, {
Name: "bar",
Value: *v1beta1.NewStructuredValues("mi"),
},
},
"pt2": {{
Name: "baz",
Value: *v1beta1.NewStructuredValues("rae"),
}},
},
expectedResults: []v1beta1.PipelineRunResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("do"),
}, {
Name: "pipeline-result-2",
Value: *v1beta1.NewStructuredValues("do, rae, mi, rae, do"),
}},
}, {
description: "no-run-results-no-returned-results",
results: []v1beta1.PipelineResult{{
Expand All @@ -3883,6 +3959,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) {
}},
runResults: map[string][]v1beta1.CustomRunResult{},
expectedResults: nil,
expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"),
}, {
description: "wrong-customtask-name-no-returned-result",
results: []v1beta1.PipelineResult{{
Expand Down Expand Up @@ -3933,66 +4010,17 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) {
},
expectedResults: nil,
expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"),
}, {
description: "multiple-results-custom-and-normal-tasks",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.customtask.results.foo)"),
}, {
Name: "pipeline-result-2",
Value: *v1beta1.NewStructuredValues("$(tasks.customtask.results.foo), $(tasks.normaltask.results.baz), $(tasks.customtask.results.bar), $(tasks.normaltask.results.baz), $(tasks.customtask.results.foo)"),
}},
runResults: map[string][]v1beta1.CustomRunResult{
"customtask": {
{
Name: "foo",
Value: "do",
}, {
Name: "bar",
Value: "mi",
},
},
},
taskResults: map[string][]v1beta1.TaskRunResult{
"normaltask": {{
Name: "baz",
Value: *v1beta1.NewStructuredValues("rae"),
}},
},
expectedResults: []v1beta1.PipelineRunResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("do"),
}, {
Name: "pipeline-result-2",
Value: *v1beta1.NewStructuredValues("do, rae, mi, rae, do"),
}},
}, {
description: "multiple-results-skipped-and-normal-tasks",
results: []v1beta1.PipelineResult{{
Name: "pipeline-result-1",
Value: *v1beta1.NewStructuredValues("$(tasks.skippedTask.results.foo)"),
}, {
Name: "pipeline-result-2",
Value: *v1beta1.NewStructuredValues("$(tasks.skippedTask.results.foo), $(tasks.normaltask.results.baz)"),
}},
taskResults: map[string][]v1beta1.TaskRunResult{
"normaltask": {{
Name: "baz",
Value: *v1beta1.NewStructuredValues("rae"),
}},
},
skippedTasks: []v1beta1.SkippedTask{{
Name: "skippedTask",
}},
expectedResults: nil,
}} {
t.Run(tc.description, func(t *testing.T) {
received, err := ApplyTaskResultsToPipelineResults(context.Background(), tc.results, tc.taskResults, tc.runResults, tc.skippedTasks)
if tc.expectedError != nil {
if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" {
t.Errorf("ApplyTaskResultsToPipelineResults() errors diff %s", diff.PrintWantGot(d))
}
if err == nil {
t.Errorf("Expect error but got nil")
}

if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" {
t.Errorf("ApplyTaskResultsToPipelineResults() errors diff %s", diff.PrintWantGot(d))
}

if d := cmp.Diff(tc.expectedResults, received); d != "" {
t.Errorf(diff.PrintWantGot(d))
}
Expand Down

0 comments on commit f0e0dda

Please sign in to comment.