Add Unit Tests for TestMissingResultWhenStepErrorIsIgnored and update…

… e2e test: TestFailingStepOnContinue This commit adds test coverage for a pipelinerun that has 2 tasks where the first task produces 2 results that are consumed in the 2nd task. However, the first task contains a failing step so only 1 of the 2 results are produced which results in a failing pipeline run. Prior to this commit, this was an E2E integration test, but missing unit test coverage. This increases the test coverage and also modifies the e2e test to only test the behavior of the TaskRun, since the PipelineRun behavior is now being tested in unit tests to reduce redunancies, improve speed, andreduce flakes.
tektoncd · Jun 8, 2023 · 59557e4 · 59557e4
1 parent 87fc1ee
commit 59557e4
Show file tree

Hide file tree

Showing 2 changed files with 210 additions and 50 deletions.
diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go
@@ -1044,6 +1044,188 @@ 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'
+  conditions:
+  - type: Succeeded
+    status: "False"
+    reason: "InvalidTaskResultReference"
+    message: "Could not find result with name result2 for task task1"
+  childReferences:
+  - apiVersion: tekton.dev/v1beta1
+    kind: TaskRun
+    name: pr-test-pipeline-missing-results-0
+    pipelineTaskName: test-pipeline-missing-results-task1`)}
+
+	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:
+    featureFlags:
+      RunningInEnvWithInjectedSidecars: true
+      EnableAPIFields: "stable"
+      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.

diff --git a/test/ignore_step_error_test.go b/test/ignore_step_error_test.go
@@ -24,82 +24,60 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun"
 	"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)
-	}
-
-	if len(taskrunList.Items) != 1 {
-		t.Fatalf("The pipelineRun should have exactly 1 taskRun for the first task \"task1\"")
+		t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err)
 	}
 
-	taskrunItem := taskrunList.Items[0]
-	if taskrunItem.Labels["tekton.dev/pipelineTask"] != "task1" {
-		t.Fatalf("TaskRun was not found for the task \"task1\"")
+	if !taskRun.Status.GetCondition(apis.ConditionSucceeded).IsTrue() {
+		t.Fatalf("Expected TaskRun %s to have succeeded but Status is %v", taskRunName, taskRun.Status)
 	}
 
-	if len(taskrunItem.Status.TaskRunResults) != 1 {
+	if len(taskRun.Status.TaskRunResults) != 1 {
 		t.Fatalf("task1 should have produced a result before failing the step")
 	}
 
-	for _, r := range taskrunItem.Status.TaskRunResults {
-		if r.Name == "result1" && r.Value.StringVal != "123" {
+	for _, r := range taskRun.Status.TaskRunResults {
+		if r.Name == "taskRunesult1" && r.Value.StringVal != "123" {
 			t.Fatalf("task1 should have initialized a result \"result1\" to \"123\"")
 		}
 	}