Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Unit Tests for TestMissingResultWhenStepErrorIsIgnored and Update e2e test: TestFailingStepOnContinue #6771

Merged
merged 1 commit into from
Jun 13, 2023
Merged

Add Unit Tests for TestMissingResultWhenStepErrorIsIgnored and Update e2e test: TestFailingStepOnContinue #6771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 173 additions & 0 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,179 @@ spec:
}
}

func TestMissingResultWhenStepErrorIsIgnored(t *testing.T) {
prs := []*v1beta1.PipelineRun{parse.MustParseV1beta1PipelineRun(t, `
metadata:
name: test-pipeline-missing-results
namespace: foo
spec:
serviceAccountName: test-sa-0
pipelineSpec:
tasks:
- name: task1
taskSpec:
results:
- name: result1
type: string
- name: result2
type: string
steps:
- name: failing-step
onError: continue
image: busybox
script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)'
- name: task2
runAfter: [ task1 ]
params:
- name: param1
value: $(tasks.task1.results.result1)
- name: param2
value: $(tasks.task1.results.result2)
taskSpec:
params:
- name: param1
type: string
- name: param2
type: string
steps:
- name: foo
image: busybox
script: 'exit 0'
`)}

trs := []*v1beta1.TaskRun{mustParseTaskRunWithObjectMeta(t,
taskRunObjectMeta("test-pipeline-missing-results-task1", "foo",
"test-pipeline-missing-results", "test-pipeline", "task1", true),
`
spec:
serviceAccountName: test-sa
timeout: 1h0m0s
status:
conditions:
- status: "True"
type: Succeeded
taskResults:
- name: result1
value: 123
`)}

expectedPipelineRun :=
parse.MustParseV1beta1PipelineRun(t, `
metadata:
name: test-pipeline-missing-results
namespace: foo
annotations: {}
labels:
tekton.dev/pipeline: test-pipeline-missing-results
spec:
serviceAccountName: test-sa-0
pipelineSpec:
tasks:
- name: task1
taskSpec:
results:
- name: result1
type: string
- name: result2
type: string
steps:
- name: failing-step
onError: continue
image: busybox
script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)'
- name: task2
runAfter: [ task1 ]
params:
- name: param1
value: $(tasks.task1.results.result1)
- name: param2
value: $(tasks.task1.results.result2)
taskSpec:
params:
- name: param1
type: string
- name: param2
type: string
steps:
- name: foo
image: busybox
script: 'exit 0'
status:
pipelineSpec:
tasks:
- name: task1
taskSpec:
results:
- name: result1
type: string
- name: result2
type: string
steps:
- name: failing-step
onError: continue
image: busybox
script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)'
- name: task2
runAfter: [ task1 ]
params:
- name: param1
value: $(tasks.task1.results.result1)
- name: param2
value: $(tasks.task1.results.result2)
taskSpec:
params:
- name: param1
type: string
- name: param2
type: string
steps:
- name: foo
image: busybox
script: 'exit 0'
conditions:
- message: "Could not find result with name result2 for task task1"
reason: InvalidTaskResultReference
status: "False"
type: Succeeded
childReferences:
- apiVersion: tekton.dev/v1beta1
kind: TaskRun
name: test-pipeline-missing-results-task1
pipelineTaskName: task1
provenance:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provenance field looks like irrelevant of this test and maybe we could consider ignoring the comparison for it. But it is totally optional 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QuanZhang-William I'm not sure if we have an ignoreProvenance field. It looks like the provenance field was introduced in this PR: #6495 and added to all of the expectedPR statuses. Maybe for consistency we leave as is and then can scope to a separate PR if we want to add an ignoreProvenance field?

featureFlags:
RunningInEnvWithInjectedSidecars: true
EnableAPIFields: "beta"
AwaitSidecarReadiness: true
VerificationNoMatchPolicy: "ignore"
EnableProvenanceInStatus: true
ResultExtractionMethod: "termination-message"
MaxResultSize: 4096
`)
d := test.Data{
PipelineRuns: prs,
TaskRuns: trs,
}
prt := newPipelineRunTest(t, d)
defer prt.Cancel()

reconciledRun, clients := prt.reconcileRun("foo", "test-pipeline-missing-results", []string{}, true)
if reconciledRun.Status.CompletionTime == nil {
t.Errorf("Expected a CompletionTime on invalid PipelineRun but was nil")
}

// The PipelineRun should be marked as failed due to InvalidTaskResultReference.
if d := cmp.Diff(reconciledRun, expectedPipelineRun, ignoreResourceVersion, ignoreLastTransitionTime, ignoreTypeMeta, ignoreStartTime, ignoreCompletionTime); d != "" {
t.Errorf("Expected to see PipelineRun run marked as failed with the reason: InvalidTaskResultReference. Diff %s", diff.PrintWantGot(d))
}

// Check that the expected TaskRun was created
taskRuns := getTaskRunsForPipelineRun(prt.TestAssets.Ctx, t, clients, "foo", "test-pipeline-missing-results")

// We expect only 1 TaskRun to be created, since the PipelineRun should fail before creating the 2nd TaskRun due to the InvalidTaskResultReference
validateTaskRunsCount(t, taskRuns, 1)
}

func TestReconcile_InvalidPipelineRunNames(t *testing.T) {
// TestReconcile_InvalidPipelineRunNames runs "Reconcile" on several PipelineRuns that have invalid names.
// It verifies that reconcile fails, how it fails and which events are triggered.
Expand Down
89 changes: 35 additions & 54 deletions test/ignore_step_error_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,83 +24,64 @@ import (
"fmt"
"testing"

"github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun"
"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
"github.com/tektoncd/pipeline/test/diff"
"github.com/tektoncd/pipeline/test/parse"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/pkg/apis"
knativetest "knative.dev/pkg/test"
"knative.dev/pkg/test/helpers"
)

func TestMissingResultWhenStepErrorIsIgnored(t *testing.T) {
func TestFailingStepOnContinue(t *testing.T) {
ctx := context.Background()
ctx, cancel := context.WithCancel(ctx)
defer cancel()
c, namespace := setup(ctx, t)
knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf)
defer tearDown(ctx, t, c, namespace)

pipelineRun := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(`
taskRunName := "mytaskrun"
tr := parse.MustParseV1beta1TaskRun(t, fmt.Sprintf(`
metadata:
name: %s
namespace: %s
spec:
pipelineSpec:
tasks:
- name: task1
taskSpec:
results:
- name: result1
- name: result2
steps:
- name: failing-step
onError: continue
image: busybox
script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)'
- name: task2
runAfter: [ task1 ]
params:
- name: param1
value: $(tasks.task1.results.result1)
- name: param2
value: $(tasks.task1.results.result2)
taskSpec:
params:
- name: param1
- name: param2
steps:
- name: foo
image: busybox
script: 'exit 0'`, helpers.ObjectNameForTest(t)))

if _, err := c.V1beta1PipelineRunClient.Create(ctx, pipelineRun, metav1.CreateOptions{}); err != nil {
t.Fatalf("Failed to create PipelineRun `%s`: %s", pipelineRun.Name, err)
taskSpec:
results:
- name: result1
- name: result2
steps:
- name: failing-step
onError: continue
image: busybox
script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)'
`, taskRunName, namespace))

if _, err := c.V1beta1TaskRunClient.Create(ctx, tr, metav1.CreateOptions{}); err != nil {
t.Fatalf("Failed to create TaskRun: %s", err)
}

t.Logf("Waiting for PipelineRun in namespace %s to fail", namespace)
if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, FailedWithReason(pipelinerun.ReasonInvalidTaskResultReference, pipelineRun.Name), "InvalidTaskResultReference", v1beta1Version); err != nil {
t.Errorf("Error waiting for PipelineRun to fail: %s", err)
t.Logf("Waiting for TaskRun in namespace %s to finish", namespace)
if err := WaitForTaskRunState(ctx, c, taskRunName, TaskRunSucceed(taskRunName), "TaskRunSucceeded", v1beta1Version); err != nil {
t.Errorf("Error waiting for TaskRun to finish: %s", err)
}

taskrunList, err := c.V1beta1TaskRunClient.List(ctx, metav1.ListOptions{LabelSelector: "tekton.dev/pipelineRun=" + pipelineRun.Name})
taskRun, err := c.V1beta1TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{})
if err != nil {
t.Fatalf("Error listing TaskRuns for PipelineRun %s: %s", pipelineRun.Name, err)
t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err)
}

if len(taskrunList.Items) != 1 {
t.Fatalf("The pipelineRun should have exactly 1 taskRun for the first task \"task1\"")
if !taskRun.Status.GetCondition(apis.ConditionSucceeded).IsTrue() {
t.Fatalf("Expected TaskRun %s to have succeeded but Status is %v", taskRunName, taskRun.Status)
}

taskrunItem := taskrunList.Items[0]
if taskrunItem.Labels["tekton.dev/pipelineTask"] != "task1" {
t.Fatalf("TaskRun was not found for the task \"task1\"")
}

if len(taskrunItem.Status.TaskRunResults) != 1 {
t.Fatalf("task1 should have produced a result before failing the step")
}
expectedResults := []v1beta1.TaskRunResult{{
Name: "result1",
Type: "string",
Value: *v1beta1.NewArrayOrString("123"),
}}

for _, r := range taskrunItem.Status.TaskRunResults {
if r.Name == "result1" && r.Value.StringVal != "123" {
t.Fatalf("task1 should have initialized a result \"result1\" to \"123\"")
}
if d := cmp.Diff(expectedResults, taskRun.Status.TaskRunResults); d != "" {
t.Errorf("Got unexpected results %s", diff.PrintWantGot(d))
EmmaMunley marked this conversation as resolved.
Show resolved Hide resolved
}
}