Add support for allowing skipped checks in has_successful_status

README.md

@@ -322,13 +322,24 @@ if:
     deletions: "> 100"
     total: "> 200"
 
+  # DEPRECATED: Use "has_status" below instead, which is more flexible.
   # "has_successful_status" is satisfied if the status checks that are specified
   # are marked successful on the head commit of the pull request.
   has_successful_status:
     - "status-name-1"
     - "status-name-2"
     - "status-name-3"
 
+  # "has_status" is satisfied if the status checks that are specified are
+  # finished and concluded with one of the conclusions specified.
+  # "conclusions" is optional and defaults to ["success"].
+  has_status:
+    conclusions: ["success", "skipped"]
+    statuses:
+      - "status-name-1"
+      - "status-name-2"
+      - "status-name-3"
+
   # "has_labels" is satisfied if the pull request has the specified labels
   # applied
   has_labels:
@@ -510,16 +521,17 @@ requires:
   # count as approved. If present, conditions are an additional requirement
   # beyond the approvals required by "count".
   #
-  # For example, if "count" is 1 and "conditions" contains the "has_successful_status"
+  # For example, if "count" is 1 and "conditions" contains the "has_status"
   # condition with the "build" status, the rule is only approved once the
   # "build" status check passes and one authorized reviewer leaves a review.
   conditions:
-    # The "conditions" block accepts all of the keys documented as part
-    # of the "if" block for predicates. The "has_successful_status" key is
-    # shown here as an example of one type of condition.
-    has_successful_status:
-      - "build"
-      - "vulnerability scan"
+    # The "conditions" block accepts all of the keys documented as part of the
+    # "if" block for predicates. The "has_status" key is shown here as an
+    # example of one type of condition.
+    has_status:
+      statuses:
+        - "build"
+        - "vulnerability scan"
 ```
 
 ### Approval Policies

policy/approval/approve_test.go

@@ -676,7 +676,7 @@ func TestIsApproved(t *testing.T) {
 		r := &Rule{
 			Requires: Requires{
 				Conditions: predicate.Predicates{
-					HasSuccessfulStatus: &predicate.HasSuccessfulStatus{"deploy"},
+					HasStatus: &predicate.HasStatus{Statuses: []string{"deploy"}},
 				},
 			},
 		}
@@ -688,7 +688,7 @@ func TestIsApproved(t *testing.T) {
 		r := &Rule{
 			Requires: Requires{
 				Conditions: predicate.Predicates{
-					HasSuccessfulStatus: &predicate.HasSuccessfulStatus{"build"},
+					HasStatus: &predicate.HasStatus{Statuses: []string{"build"}},
 				},
 			},
 		}
@@ -701,7 +701,7 @@ func TestIsApproved(t *testing.T) {
 			Requires: Requires{
 				Count: 1,
 				Conditions: predicate.Predicates{
-					HasSuccessfulStatus: &predicate.HasSuccessfulStatus{"build"},
+					HasStatus: &predicate.HasStatus{Statuses: []string{"build"}},
 				},
 			},
 		}
@@ -717,7 +717,7 @@ func TestIsApproved(t *testing.T) {
 					Organizations: []string{"everyone"},
 				},
 				Conditions: predicate.Predicates{
-					HasSuccessfulStatus: &predicate.HasSuccessfulStatus{"build"},
+					HasStatus: &predicate.HasStatus{Statuses: []string{"build"}},
 				},
 			},
 		}
@@ -825,7 +825,7 @@ func TestTrigger(t *testing.T) {
 		r := &Rule{
 			Requires: Requires{
 				Conditions: predicate.Predicates{
-					HasSuccessfulStatus: &predicate.HasSuccessfulStatus{"build"},
+					HasStatus: predicate.NewHasStatus([]string{"status1"}, []string{"skipped", "success"}),
 				},
 			},
 		}

policy/approval/parse_test.go

@@ -35,6 +35,9 @@ func TestParsePolicy(t *testing.T) {
 - and:
   - rule6
   - rule7
+  - or:
+    - rule8
+    - rule9
 `
 
 	ruleText := `
@@ -67,6 +70,15 @@ func TestParsePolicy(t *testing.T) {
       enabled: true
   requires:
     admins: true
+- name: rule8
+  if:
+    has_successful_status:
+      - status1
+- name: rule9
+  if:
+    has_status:
+      conclusions: ["success", "skipped"]
+      statuses: [status2, status3]
 `
 
 	var policy Policy
@@ -119,6 +131,16 @@ func TestParsePolicy(t *testing.T) {
 						&RuleRequirement{
 							rule: rules[6],
 						},
+						&OrRequirement{
+							requirements: []common.Evaluator{
+								&RuleRequirement{
+									rule: rules[7],
+								},
+								&RuleRequirement{
+									rule: rules[8],
+								},
+							},
+						},
 					},
 				},
 			},

policy/predicate/predicates.go

@@ -29,6 +29,10 @@ type Predicates struct {
 
 	ModifiedLines *ModifiedLines `yaml:"modified_lines"`
 
+	HasStatus *HasStatus `yaml:"has_status"`
+	// `has_successful_status` is a deprecated field that is kept for backwards
+	// compatibility.  `has_status` replaces it, and can accept any conclusion
+	// rather than just "success".
 	HasSuccessfulStatus *HasSuccessfulStatus `yaml:"has_successful_status"`
 
 	HasLabels *HasLabels `yaml:"has_labels"`
@@ -78,6 +82,10 @@ func (p *Predicates) Predicates() []Predicate {
 		ps = append(ps, Predicate(p.ModifiedLines))
 	}
 
+	if p.HasStatus != nil {
+		ps = append(ps, Predicate(p.HasStatus))
+	}
+
 	if p.HasSuccessfulStatus != nil {
 		ps = append(ps, Predicate(p.HasSuccessfulStatus))
 	}

policy/predicate/status.go

@@ -16,37 +16,55 @@ package predicate
 
 import (
 	"context"
+	"fmt"
+	"slices"
 	"strings"
 
 	"github.com/palantir/policy-bot/policy/common"
 	"github.com/palantir/policy-bot/pull"
 	"github.com/pkg/errors"
 )
 
-type HasSuccessfulStatus []string
-
-var _ Predicate = HasSuccessfulStatus([]string{})
+type allowedConclusions []string
 
-func (pred HasSuccessfulStatus) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
-	statuses, err := prctx.LatestStatuses()
+type HasStatus struct {
+	Conclusions allowedConclusions `yaml:"conclusions"`
+	Statuses    []string           `yaml:"statuses"`
+}
 
-	predicateResult := common.PredicateResult{
-		ValuePhrase:     "status checks",
-		ConditionPhrase: "exist and pass",
+func NewHasStatus(statuses []string, conclusions []string) *HasStatus {
+	return &HasStatus{
+		Conclusions: conclusions,
+		Statuses:    statuses,
 	}
+}
 
+var _ Predicate = HasStatus{}
+
+func (pred HasStatus) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
+	statuses, err := prctx.LatestStatuses()
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to list commit statuses")
 	}
 
+	conclusions := pred.Conclusions
+	if len(conclusions) == 0 {
+		conclusions = allowedConclusions{"success"}
+	}
+
+	predicateResult := common.PredicateResult{
+		ValuePhrase:     "status checks",
+		ConditionPhrase: fmt.Sprintf("exist and have conclusion %s", conclusions.joinWithOr()),
+	}
+
 	var missingResults []string
 	var failingStatuses []string
-	for _, status := range pred {
+	for _, status := range pred.Statuses {
 		result, ok := statuses[status]
 		if !ok {
 			missingResults = append(missingResults, status)
 		}
-		if result != "success" {
+		if !slices.Contains(conclusions, result) {
 			failingStatuses = append(failingStatuses, status)
 		}
 	}
@@ -60,16 +78,58 @@ func (pred HasSuccessfulStatus) Evaluate(ctx context.Context, prctx pull.Context
 
 	if len(failingStatuses) > 0 {
 		predicateResult.Values = failingStatuses
-		predicateResult.Description = "One or more statuses has not passed: " + strings.Join(failingStatuses, ",")
+		predicateResult.Description = fmt.Sprintf("One or more statuses has not concluded with %s: %s", pred.Conclusions.joinWithOr(), strings.Join(failingStatuses, ","))
 		predicateResult.Satisfied = false
 		return &predicateResult, nil
 	}
 
-	predicateResult.Values = pred
+	predicateResult.Values = pred.Statuses
 	predicateResult.Satisfied = true
 	return &predicateResult, nil
 }
 
+func (pred HasStatus) Trigger() common.Trigger {
+	return common.TriggerStatus
+}
+
+// HasSuccessfulStatus checks that the specified statuses have a successful
+// conclusion.
+//
+// Deprecated: use the more flexible `HasStatus` with `conclusions: ["success"]`
+// instead.
+type HasSuccessfulStatus []string
+
+var _ Predicate = HasSuccessfulStatus{}
+
+func (pred HasSuccessfulStatus) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
+	return HasStatus{
+		Statuses: pred,
+	}.Evaluate(ctx, prctx)
+}
+
 func (pred HasSuccessfulStatus) Trigger() common.Trigger {
 	return common.TriggerStatus
 }
+
+// joinWithOr returns a string that represents the allowed conclusions in a
+// format that can be used in a sentence. For example, if the allowed
+// conclusions are "success" and "failure", this will return "success or
+// failure". If there are more than two conclusions, the first n-1 will be
+// separated by commas.
+func (c allowedConclusions) joinWithOr() string {
+	slices.Sort(c)
+
+	length := len(c)
+	switch length {
+	case 0:
+		return ""
+	case 1:
+		return c[0]
+	case 2:
+		return c[0] + " or " + c[1]
+	}
+
+	head, tail := c[:length-1], c[length-1]
+
+	return strings.Join(head, ", ") + ", or " + tail
+}