Fix go e2e tests

- refactor go e2e test TestTaskRunPipelineRunCancel to reduce flakiness.
- refactor yaml e2e test bash script. yaml script checks for both
taskruns, pipelineruns to be in unknown status and then check results.
- taskrun `UpdateStatus` updates subresource status and cancel status
sets the status to empty object sometimes.

Fixes tektoncd#511

pkg/reconciler/v1alpha1/pipelinerun/cancel.go

@@ -47,7 +47,7 @@ func cancelPipelineRun(pr *v1alpha1.PipelineRun, pipelineState []*resources.Reso
 			continue
 		}
 		rprt.TaskRun.Spec.Status = v1alpha1.TaskRunSpecStatusCancelled
-		if _, err := clientSet.PipelineV1alpha1().TaskRuns(pr.Namespace).UpdateStatus(rprt.TaskRun); err != nil {
+		if _, err := clientSet.PipelineV1alpha1().TaskRuns(pr.Namespace).Update(rprt.TaskRun); err != nil {
 			errs = append(errs, err.Error())
 			continue
 		}

test/cancel_test.go

@@ -52,12 +52,13 @@ func TestTaskRunPipelineRunCancel(t *testing.T) {
 	pipeline := tb.Pipeline("tomatoes", namespace,
 		tb.PipelineSpec(tb.PipelineTask("foo", "banana")),
 	)
-
-	logger.Infof("Creating PipelineRun in namespace %s", namespace)
-	pipelineRun := tb.PipelineRun("pear", namespace, tb.PipelineRunSpec(pipeline.Name))
 	if _, err := c.PipelineClient.Create(pipeline); err != nil {
 		t.Fatalf("Failed to create Pipeline `%s`: %s", "tomatoes", err)
 	}
+
+	pipelineRun := tb.PipelineRun("pear", namespace, tb.PipelineRunSpec(pipeline.Name))
+
+	logger.Infof("Creating PipelineRun in namespace %s", namespace)
 	if _, err := c.PipelineRunClient.Create(pipelineRun); err != nil {
 		t.Fatalf("Failed to create PipelineRun `%s`: %s", "pear", err)
 	}
@@ -87,25 +88,24 @@ func TestTaskRunPipelineRunCancel(t *testing.T) {
 	defer close(errChan)
 
 	for _, taskrunItem := range taskrunList.Items {
-		go func() {
+		go func(chan error) {
 			err := WaitForTaskRunState(c, taskrunItem.Name, func(tr *v1alpha1.TaskRun) (bool, error) {
-				c := tr.Status.GetCondition(duckv1alpha1.ConditionSucceeded)
-				if c != nil {
-					if c.Status == corev1.ConditionTrue || c.Status == corev1.ConditionFalse {
+				if c := tr.Status.GetCondition(duckv1alpha1.ConditionSucceeded); c != nil {
+					if c.IsTrue() || c.IsFalse() {
 						return true, fmt.Errorf("taskRun %s already finished!", taskrunItem.Name)
-					} else if c.Status == corev1.ConditionUnknown && (c.Reason == "Running" || c.Reason == "Pending") {
+					} else if c.IsUnknown() && (c.Reason == "Running" || c.Reason == "Pending") {
 						return true, nil
 					}
 				}
 				return false, nil
 			}, "TaskRunRunning")
 			errChan <- err
-		}()
-
+		}(errChan)
 	}
+
 	for i := 1; i <= len(taskrunList.Items); i++ {
 		if <-errChan != nil {
-			t.Errorf("Error waiting for TaskRun %s to be running: %s", taskrunList.Items[i-1].Name, err)
+			t.Errorf("Error waiting for TaskRun %s to be running: %v", taskrunList.Items[i-1].Name, err)
 		}
 	}
 
@@ -121,49 +121,46 @@ func TestTaskRunPipelineRunCancel(t *testing.T) {
 
 	logger.Infof("Waiting for PipelineRun %s in namespace %s to be cancelled", "pear", namespace)
 	if err := WaitForPipelineRunState(c, "pear", pipelineRunTimeout, func(pr *v1alpha1.PipelineRun) (bool, error) {
-		c := pr.Status.GetCondition(duckv1alpha1.ConditionSucceeded)
-		if c != nil {
-			if c.Status == corev1.ConditionFalse {
+		if c := pr.Status.GetCondition(duckv1alpha1.ConditionSucceeded); c != nil {
+			if c.IsFalse() {
 				if c.Reason == "PipelineRunCancelled" {
 					return true, nil
 				}
 				return true, fmt.Errorf("pipelineRun %s completed with the wrong reason: %s", "pear", c.Reason)
-			} else if c.Status == corev1.ConditionTrue {
+			} else if c.IsTrue() {
 				return true, fmt.Errorf("pipelineRun %s completed successfully, should have been cancelled", "pear")
 			}
 		}
 		return false, nil
 	}, "PipelineRunCancelled"); err != nil {
-		t.Errorf("Error waiting for PipelineRun `pear` to finish: %s", err)
+		t.Errorf("Error waiting for PipelineRun `pear` to finished: %s", err)
 	}
 
 	logger.Infof("Waiting for TaskRuns in PipelineRun %s in namespace %s to be cancelled", "pear", namespace)
 	errChan2 := make(chan error, len(taskrunList.Items))
 	defer close(errChan2)
 	for _, taskrunItem := range taskrunList.Items {
-		go func() {
+		go func(errChan2 chan error) {
 			err := WaitForTaskRunState(c, taskrunItem.Name, func(tr *v1alpha1.TaskRun) (bool, error) {
-				c := tr.Status.GetCondition(duckv1alpha1.ConditionSucceeded)
-				if c != nil {
-					if c.Status == corev1.ConditionFalse {
+				if c := tr.Status.GetCondition(duckv1alpha1.ConditionSucceeded); c != nil {
+					if c.IsFalse() {
 						if c.Reason == "TaskRunCancelled" {
 							return true, nil
 						}
 						return true, fmt.Errorf("taskRun %s completed with the wrong reason: %s", taskrunItem.Name, c.Reason)
-					} else if c.Status == corev1.ConditionTrue {
+					} else if c.IsTrue() {
 						return true, fmt.Errorf("taskRun %s completed successfully, should have been cancelled", taskrunItem.Name)
 					}
 				}
 				return false, nil
 			}, "TaskRunCancelled")
 			errChan2 <- err
-		}()
+		}(errChan2)
 
 	}
 	for i := 1; i <= len(taskrunList.Items); i++ {
 		if <-errChan2 != nil {
-			t.Errorf("Error waiting for TaskRun %s to be finish: %s", taskrunList.Items[i-1].Name, err)
+			t.Errorf("Error waiting for TaskRun %s to be finished: %v", taskrunList.Items[i-1].Name, err)
 		}
 	}
-
 }

test/e2e-common.sh

@@ -63,8 +63,6 @@ function dump_extra_cluster_state() {
 }
 
 function validate_run() {
-  # Wait for tests to finish.
-  echo ">> Waiting for tests to finish"
   local tests_finished=0
   for i in {1..60}; do
     local finished="$(kubectl get $1.pipeline.knative.dev --output=jsonpath='{.items[*].status.conditions[*].status}')"
@@ -74,14 +72,12 @@ function validate_run() {
     fi
     sleep 10
   done
-  if (( ! tests_finished )); then
-    echo "ERROR: tests timed out"
-    return 1
-  fi
 
-  # Check that tests passed.
+  return ${tests_finished}
+}
+
+function check_results() {
   local failed=0
-  echo ">> Checking test results"
   results="$(kubectl get $1.pipeline.knative.dev --output=jsonpath='{range .items[*]}{.metadata.name}={.status.conditions[*].type}{.status.conditions[*].status}{" "}{end}')"
   for result in ${results}; do
     if [[ ! "${result,,}" == *"=succeededtrue" ]]; then
@@ -106,10 +102,22 @@ function run_yaml_tests() {
     perl -p -e 's/gcr.io\/christiewilson-catfactory/$ENV{KO_DOCKER_REPO}/g' ${file} | ko apply -f - || return 1
   done
 
-  if validate_run $1; then
-    echo ">> All YAML tests passed"
-    return 0
-  fi
+  # Wait for tests to finish.
+  echo ">> Waiting for tests to finish"
+  for test in taskrun pipelinerun; do
+     if validate_run ${test}; then
+      echo "ERROR: tests timed out"
+     fi
+  done
+
+  # Check that tests passed.
+  echo ">> Checking test results"
+  for test in taskrun pipelinerun; do
+    if check_results ${test}; then
+      echo ">> All YAML tests passed"
+      return 0
+    fi
+  done
 
   return 1
 }

test/wait.go

@@ -58,7 +58,7 @@ import (
 
 const (
 	interval = 1 * time.Second
-	timeout  = 5 * time.Minute
+	timeout  = 10 * time.Minute
 )
 
 // TaskRunStateFn is a condition function on TaskRun used polling functions