(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 tektoncd#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

@@ -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 {

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

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)