Skip to content

Commit

Permalink
(cleanup) use skipping reason instead of existence of variables
Browse files Browse the repository at this point in the history
today, we existence of variables to determine whether to add
`when` expressions to the skipped `tasks` status for finally tasks
(we only add them when they are resolved, have no variables)

we recently added skipping reason in #4085

now that we have skipping reason, we can use it to check that the reason
for skipping is that `when` expressions evaluated to false before
including the resolved `when` expressions to the skipped `tasks` status

we plan to explore adding skipping reason to the skipped `tasks` status
in the future, but for now this change reuses this new functionality
  • Loading branch information
jerop committed Aug 11, 2021
1 parent d9dd80e commit f0e4efd
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 79 deletions.
18 changes: 0 additions & 18 deletions pkg/apis/pipeline/v1beta1/when_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,6 @@ func (we *WhenExpression) isTrue() bool {
return !we.isInputInValues()
}

func (we *WhenExpression) hasVariable() bool {
if _, hasVariable := we.GetVarSubstitutionExpressions(); hasVariable {
return true
}
return false
}

func (we *WhenExpression) applyReplacements(replacements map[string]string, arrayReplacements map[string][]string) WhenExpression {
replacedInput := substitution.ApplyReplacements(we.Input, replacements)

Expand Down Expand Up @@ -105,17 +98,6 @@ func (wes WhenExpressions) AllowsExecution() bool {
return true
}

// HaveVariables indicates whether When Expressions contains variables, such as Parameters
// or Results in the Inputs or Values.
func (wes WhenExpressions) HaveVariables() bool {
for _, we := range wes {
if we.hasVariable() {
return true
}
}
return false
}

// ReplaceWhenExpressionsVariables interpolates variables, such as Parameters and Results, in
// the Input and Values.
func (wes WhenExpressions) ReplaceWhenExpressionsVariables(replacements map[string]string, arrayReplacements map[string][]string) WhenExpressions {
Expand Down
60 changes: 0 additions & 60 deletions pkg/apis/pipeline/v1beta1/when_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,66 +88,6 @@ func TestAllowsExecution(t *testing.T) {
}
}

func TestHaveVariables(t *testing.T) {
tests := []struct {
name string
whenExpressions WhenExpressions
expected bool
}{{
name: "doesn't have variable",
whenExpressions: WhenExpressions{
{
Input: "foo",
Operator: selection.In,
Values: []string{"foo", "bar"},
},
},
expected: false,
}, {
name: "have variable - input",
whenExpressions: WhenExpressions{
{
Input: "$(params.foobar)",
Operator: selection.NotIn,
Values: []string{"foobar"},
},
},
expected: true,
}, {
name: "have variable - values",
whenExpressions: WhenExpressions{
{
Input: "foobar",
Operator: selection.In,
Values: []string{"$(params.foobar)"},
},
},
expected: true,
}, {
name: "have variable - multiple when expressions",
whenExpressions: WhenExpressions{
{
Input: "foo",
Operator: selection.NotIn,
Values: []string{"bar"},
}, {
Input: "foobar",
Operator: selection.In,
Values: []string{"$(params.foobar)"},
},
},
expected: true,
}}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := tc.whenExpressions.HaveVariables()
if d := cmp.Diff(tc.expected, got); d != "" {
t.Errorf("Error evaluating HaveVariables() for When Expressions in test case %s", diff.PrintWantGot(d))
}
})
}
}

func TestReplaceWhenExpressionsVariables(t *testing.T) {
tests := []struct {
name string
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/resources/pipelinerunstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ func (facts *PipelineRunFacts) GetSkippedTasks() []v1beta1.SkippedTask {
}
// include the when expressions only when the finally task was skipped because
// its when expressions evaluated to false (not because results variables were missing)
if !rprt.PipelineTask.WhenExpressions.HaveVariables() {
if rprt.IsFinallySkipped(facts).SkippingReason == WhenExpressionsSkip {
skippedTask.WhenExpressions = rprt.PipelineTask.WhenExpressions
}
skipped = append(skipped, skippedTask)
Expand Down

0 comments on commit f0e4efd

Please sign in to comment.