From f4e4f9619018289404bd264660f78ec657aa4617 Mon Sep 17 00:00:00 2001
From: Jerop <jerop@google.com>
Date: Thu, 5 Nov 2020 13:21:51 -0500
Subject: [PATCH] Refactor Skip

Previously, the Skip function was big because it had all the logic for
skipping. In this change, we split up the function to have smaller
functions that do only one thing. This will be useful in future work that
need to use these smaller functions.
---
 .../resources/pipelinerunresolution.go        | 27 +++++++++----------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
index 958a162ef59..787a89a1ef8 100644
--- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
+++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
@@ -140,24 +140,27 @@ func (t *ResolvedPipelineRunTask) checkParentsDone(facts *PipelineRunFacts) bool
 // (4) Pipeline is in stopping state (one of the PipelineTasks failed)
 // Note that this means Skip returns false if a conditionCheck is in progress
 func (t *ResolvedPipelineRunTask) Skip(facts *PipelineRunFacts) bool {
-	// finally tasks are never skipped. If this is a final task, return false
-	if facts.isFinalTask(t.PipelineTask.Name) {
+	if facts.isFinalTask(t.PipelineTask.Name) || t.IsStarted() {
 		return false
 	}
 
-	// it already has TaskRun associated with it - PipelineTask not skipped
-	if t.IsStarted() {
-		return false
+	if t.conditionsSkip() || t.whenExpressionsSkip(facts) || t.parentTasksSkip(facts) || facts.IsStopping() {
+		return true
 	}
 
-	// Check if conditionChecks have failed, if so task is skipped
+	return false
+}
+
+func (t *ResolvedPipelineRunTask) conditionsSkip() bool {
 	if len(t.ResolvedConditionChecks) > 0 {
 		if t.ResolvedConditionChecks.IsDone() && !t.ResolvedConditionChecks.IsSuccess() {
 			return true
 		}
 	}
+	return false
+}
 
-	// Check if the when expressions are false, based on the input's relationship to the values
+func (t *ResolvedPipelineRunTask) whenExpressionsSkip(facts *PipelineRunFacts) bool {
 	if t.checkParentsDone(facts) {
 		if len(t.PipelineTask.WhenExpressions) > 0 {
 			if !t.PipelineTask.WhenExpressions.HaveVariables() {
@@ -167,15 +170,11 @@ func (t *ResolvedPipelineRunTask) Skip(facts *PipelineRunFacts) bool {
 			}
 		}
 	}
+	return false
+}
 
-	// Skip the PipelineTask if pipeline is in stopping state
-	if facts.IsStopping() {
-		return true
-	}
-
+func (t *ResolvedPipelineRunTask) parentTasksSkip(facts *PipelineRunFacts) bool {
 	stateMap := facts.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 := facts.TasksGraph.Nodes[t.PipelineTask.Name]
 	for _, p := range node.Prev {
 		if stateMap[p.Task.HashKey()].Skip(facts) {