From f0e4efd84b8426a287ab642159d2553160dca5ae Mon Sep 17 00:00:00 2001 From: Jerop Date: Wed, 11 Aug 2021 11:10:18 -0400 Subject: [PATCH] (cleanup) use skipping reason instead of existence of variables 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 https://github.com/tektoncd/pipeline/pull/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 --- pkg/apis/pipeline/v1beta1/when_types.go | 18 ------ pkg/apis/pipeline/v1beta1/when_types_test.go | 60 ------------------- .../pipelinerun/resources/pipelinerunstate.go | 2 +- 3 files changed, 1 insertion(+), 79 deletions(-) diff --git a/pkg/apis/pipeline/v1beta1/when_types.go b/pkg/apis/pipeline/v1beta1/when_types.go index 08ac1a8f8a5..929ae8b438d 100644 --- a/pkg/apis/pipeline/v1beta1/when_types.go +++ b/pkg/apis/pipeline/v1beta1/when_types.go @@ -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) @@ -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 { diff --git a/pkg/apis/pipeline/v1beta1/when_types_test.go b/pkg/apis/pipeline/v1beta1/when_types_test.go index 439efcc1e3a..5f9dbbe195e 100644 --- a/pkg/apis/pipeline/v1beta1/when_types_test.go +++ b/pkg/apis/pipeline/v1beta1/when_types_test.go @@ -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 diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go index 078b52fe262..e95c291b1ca 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go @@ -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)