post-finally-pr.diff

diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
index 575c9fc3..0887a656 100644
--- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
+++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
@@ -141,6 +141,50 @@ func (t ResolvedPipelineRunTask) IsStarted() bool {
 	return true
 }
 
+func inGraph(n string, d *dag.Graph) bool {
+	if _, ok := d.Nodes[n]; ok {
+		return true
+	}
+	return false
+}
+
+func (t ResolvedPipelineRunTask) IsSkipped(root string, state PipelineRunState, d *dag.Graph) bool {
+	// it already has TaskRun associated with it - PipelineTask not skipped
+	if t.IsStarted() {
+		return false
+	}
+
+	// Check if conditionChecks have failed, if so task is skipped
+	if len(t.ResolvedConditionChecks) > 0 {
+		if t.ResolvedConditionChecks.IsDone() && !t.ResolvedConditionChecks.IsSuccess() {
+			return true
+		}
+	}
+
+	// Skip the task if at least one PipelineTask has failed (not same as the one specified)
+	if t.PipelineTask.Name != root && t.IsFailure() {
+		return true
+	}
+
+	// Skip rest of the PipelineTasks if pipeline is stopping
+	if inGraph(t.PipelineTask.Name, d) && state.IsStopping(d) {
+		return true
+	}
+
+	stateMap := state.ToMap()
+	// Recursively look at parent tasks to see if they have been skipped,
+	// if any of the parents have been skipped, skip as well
+	node := d.Nodes[t.PipelineTask.Name]
+	if inGraph(t.PipelineTask.Name, d) {
+		for _, p := range node.Prev {
+			if stateMap[p.Task.HashKey()].IsSkipped(root, state, d) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // ToMap returns a map that maps pipeline task name to the resolved pipeline run task
 func (state PipelineRunState) ToMap() map[string]*ResolvedPipelineRunTask {
 	m := make(map[string]*ResolvedPipelineRunTask)
@@ -219,10 +263,9 @@ func (state PipelineRunState) GetNextTasks(candidateTasks map[string]struct{}) [
 // which have successfully completed or skipped
 func (state PipelineRunState) SuccessfulOrSkippedDAGTasks(d *dag.Graph) []string {
 	tasks := []string{}
-	stateMap := state.ToMap()
 	for _, t := range state {
 		if _, ok := d.Nodes[t.PipelineTask.Name]; ok {
-			if t.IsSuccessful() || isSkipped(t, stateMap, d) {
+			if t.IsSuccessful() || t.IsSkipped(t.PipelineTask.Name, state, d) {
 				tasks = append(tasks, t.PipelineTask.Name)
 			}
 		}
@@ -230,35 +273,17 @@ func (state PipelineRunState) SuccessfulOrSkippedDAGTasks(d *dag.Graph) []string
 	return tasks
 }
 
-// isDAGTasksStopped returns true if any of the pipeline task has failed
-// and none of the pipeline task are still running
-func (state PipelineRunState) isDAGTasksStopped(d *dag.Graph) bool {
-	failed := false
-	for _, t := range state {
-		if t.IsFailure() {
-			failed = true
-			continue
-		}
-		if t.IsStarted() && !t.IsDone() {
-			failed = false
-			break
-		}
-	}
-	return failed
-}
-
 // checkTasksDone returns true if all tasks from the specified graph are finished executing
 // a task is considered done if it has failed/succeeded/skipped
 func (state PipelineRunState) checkTasksDone(d *dag.Graph) (isDone bool) {
 	isDone = true
-	stateMap := state.ToMap()
 	for _, t := range state {
 		if _, ok := d.Nodes[t.PipelineTask.Name]; ok {
 			if t.TaskRun == nil {
 				// this task might have skipped if taskRun is nil
 				// continue and ignore if this task was skipped
 				// skipped task is considered part of done
-				if isSkipped(t, stateMap, d) {
+				if t.IsSkipped(t.PipelineTask.Name, state, d) {
 					continue
 				}
 				return false
@@ -280,7 +305,8 @@ func (state PipelineRunState) GetFinalTasks(d *dag.Graph, dfinally *dag.Graph) [
 	finalCandidates := map[string]struct{}{}
 	// check either pipeline has finished executing all DAG pipelineTasks
 	// or any one of the DAG pipelineTask has failed
-	if state.isDAGTasksStopped(d) || state.checkTasksDone(d) {
+	if state.checkTasksDone(d) {
 		// return list of tasks with all final tasks
 		for _, t := range state {
 			if _, ok := dfinally.Nodes[t.PipelineTask.Name]; ok && !t.IsSuccessful() {
@@ -502,8 +528,6 @@ func GetPipelineConditionStatus(pr *v1beta1.PipelineRun, state PipelineRunState,
 	failedTasks := int(0)
 	cancelledTasks := int(0)
 	reason := v1beta1.PipelineRunReasonSuccessful.String()
-	stateAsMap := state.ToMap()
-	isStopping := state.IsStopping(dag)
 
 	// Check to see if all tasks are success or skipped
 	//
@@ -520,18 +544,9 @@ func GetPipelineConditionStatus(pr *v1beta1.PipelineRun, state PipelineRunState,
 	for _, rprt := range state {
 		allTasks = append(allTasks, rprt.PipelineTask.Name)
 		switch {
-		case !rprt.IsStarted() && isStopping:
-			// If the pipeline is in stopping mode, all tasks that are not running
-			// already will be skipped. Otherwise these tasks end up in the
-			// incomplete count.
-			// this should never be the case for final task
-			if _, ok := dag.Nodes[rprt.PipelineTask.Name]; ok {
-				skipTasks++
-				withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name)
-			}
 		case rprt.IsSuccessful():
 			withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name)
-		case isSkipped(rprt, stateAsMap, dag):
+		case rprt.IsSkipped(rprt.PipelineTask.Name, state, dag):
 			skipTasks++
 			withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name)
 			// At least one is skipped and no failure yet, mark as completed
@@ -584,37 +599,6 @@ func GetPipelineConditionStatus(pr *v1beta1.PipelineRun, state PipelineRunState,
 	}
 }
 
-// isSkipped returns true if a Task in a TaskRun will not be run either because
-// its Condition Checks failed or because one of the parent tasks' conditions failed
-// Note that this means isSkipped returns false if a conditionCheck is in progress
-func isSkipped(rprt *ResolvedPipelineRunTask, stateMap map[string]*ResolvedPipelineRunTask, d *dag.Graph) bool {
-	// Taskrun not skipped if it already exists
-	if rprt.TaskRun != nil {
-		return false
-	}
-
-	// Check if conditionChecks have failed, if so task is skipped
-	if len(rprt.ResolvedConditionChecks) > 0 {
-		// isSkipped is only true iof
-		if rprt.ResolvedConditionChecks.IsDone() && !rprt.ResolvedConditionChecks.IsSuccess() {
-			return true
-		}
-	}
-
-	// Recursively look at parent tasks to see if they have been skipped,
-	// if any of the parents have been skipped, skip as well
-	// continue if the task does not belong to the specified Graph
-	if node, ok := d.Nodes[rprt.PipelineTask.Name]; ok {
-		for _, p := range node.Prev {
-			skip := isSkipped(stateMap[p.Task.HashKey()], stateMap, d)
-			if skip {
-				return true
-			}
-		}
-	}
-	return false
-}
-
 func resolveConditionChecks(pt *v1beta1.PipelineTask, taskRunStatus map[string]*v1beta1.PipelineRunTaskRunStatus, taskRunName string, getTaskRun resources.GetTaskRun, getCondition GetCondition, providedResources map[string]*resourcev1alpha1.PipelineResource) ([]*ResolvedConditionCheck, error) {
 	rccs := []*ResolvedConditionCheck{}
 	for i := range pt.Conditions {
diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go
index 34089f45..1577a0e3 100644
--- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go
+++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go
@@ -1080,7 +1080,7 @@ func TestIsSkipped(t *testing.T) {
 				TaskSpec: &task.Spec,
 			},
 		}},
-		expected: false,
+		expected: true,
 	}, {
 		name:     "tasks-parent-cancelled",
 		taskName: "mytask7",
@@ -1099,7 +1099,7 @@ func TestIsSkipped(t *testing.T) {
 				TaskSpec: &task.Spec,
 			},
 		}},
-		expected: false,
+		expected: true,
 	}, {
 		name:     "tasks-grandparent-failed",
 		taskName: "mytask10",
@@ -1129,7 +1129,7 @@ func TestIsSkipped(t *testing.T) {
 				TaskSpec: &task.Spec,
 			},
 		}},
-		expected: false,
+		expected: true,
 	}, {
 		name:     "tasks-parents-failed-passed",
 		taskName: "mytask8",
@@ -1155,7 +1155,7 @@ func TestIsSkipped(t *testing.T) {
 				TaskSpec: &task.Spec,
 			},
 		}},
-		expected: false,
+		expected: true,
 	}}
 
 	for _, tc := range tcs {
@@ -1169,7 +1169,7 @@ func TestIsSkipped(t *testing.T) {
 			if rprt == nil {
 				t.Fatalf("Could not get task %s from the state: %v", tc.taskName, tc.state)
 			}
-			isSkipped := isSkipped(rprt, stateMap, dag)
+			isSkipped := rprt.IsSkipped(rprt.PipelineTask.Name, tc.state, dag)
 			if d := cmp.Diff(isSkipped, tc.expected); d != "" {
 				t.Errorf("Didn't get expected isSkipped %s", diff.PrintWantGot(d))
 			}
@@ -1197,7 +1197,7 @@ func TestPipelineRunState_SuccessfulOrSkippedDAGTasks(t *testing.T) {
 	}, {
 		name:          "one-task-failed",
 		state:         oneFailedState,
-		expectedNames: []string{},
+		expectedNames: []string{pts[1].Name},
 	}, {
 		name:          "all-finished",
 		state:         allFinishedState,