From 07dd9ad91057b56a9ac19ba4cefda98cd0f93696 Mon Sep 17 00:00:00 2001 From: Florian Braun <5863788+FloThinksPi@users.noreply.github.com> Date: Wed, 26 Feb 2025 10:07:57 +0100 Subject: [PATCH 1/5] Draft enable status/workflow predicates for use in if condition --- policy/approval/approve_test.go | 35 ++- policy/predicate/predicates.go | 22 +- policy/predicate/status.go | 74 ++---- policy/predicate/status_check.go | 118 +++++++++ policy/predicate/status_check_test.go | 273 +++++++++++++++++++ policy/predicate/status_test.go | 94 ++++--- policy/predicate/workflow.go | 67 +++-- policy/predicate/workflow_test.go | 79 +++--- policy/predicate/workflowresult.go | 51 ++++ policy/predicate/workflowresult_test.go | 333 ++++++++++++++++++++++++ pull/context.go | 11 +- pull/github.go | 51 +--- pull/github_test.go | 65 ++++- pull/pulltest/context.go | 20 +- server/handler/check_run.go | 4 - 15 files changed, 1078 insertions(+), 219 deletions(-) create mode 100644 policy/predicate/status_check.go create mode 100644 policy/predicate/status_check_test.go create mode 100644 policy/predicate/workflowresult.go create mode 100644 policy/predicate/workflowresult_test.go diff --git a/policy/approval/approve_test.go b/policy/approval/approve_test.go index 2cb4f88af..1fa4ae368 100644 --- a/policy/approval/approve_test.go +++ b/policy/approval/approve_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "github.com/google/go-github/v69/github" "github.com/palantir/policy-bot/policy/common" "github.com/palantir/policy-bot/policy/predicate" "github.com/palantir/policy-bot/pull" @@ -139,10 +140,12 @@ func TestIsApproved(t *testing.T) { "comment-approver": {"everyone", "cool-org"}, "review-approver": {"everyone", "even-cooler-org"}, }, - LatestStatusesValue: map[string]string{ - "build": "success", - "deploy": "pending", - "scan": "success", + LatestRepoStatusesValue: map[string]*github.RepoStatus{ + "build": {State: github.Ptr("success")}, + "deploy": {State: github.Ptr("pending")}, + "scan": {State: github.Ptr("success")}, + "flaky": {State: github.Ptr("failure")}, + "kermit": {State: github.Ptr("error")}, }, } } @@ -689,6 +692,30 @@ func TestIsApproved(t *testing.T) { assertPending(t, prctx, r, "0/1 required conditions") }) + t.Run("conditionsRequiredStatusFailure", func(t *testing.T) { + prctx := basePullContext() + r := &Rule{ + Requires: Requires{ + Conditions: predicate.Predicates{ + HasStatus: &predicate.HasStatus{Statuses: []string{"flaky"}}, + }, + }, + } + assertPending(t, prctx, r, "0/1 required conditions") + }) + + t.Run("conditionsRequiredStatusError", func(t *testing.T) { + prctx := basePullContext() + r := &Rule{ + Requires: Requires{ + Conditions: predicate.Predicates{ + HasStatus: &predicate.HasStatus{Statuses: []string{"kermit"}}, + }, + }, + } + assertPending(t, prctx, r, "0/1 required conditions") + }) + t.Run("conditionsRequiredStatusSuccess", func(t *testing.T) { prctx := basePullContext() r := &Rule{ diff --git a/policy/predicate/predicates.go b/policy/predicate/predicates.go index de28c2926..97337acd5 100644 --- a/policy/predicate/predicates.go +++ b/policy/predicate/predicates.go @@ -33,13 +33,9 @@ type Predicates struct { ModifiedLines *ModifiedLines `yaml:"modified_lines,omitempty"` - HasStatus *HasStatus `yaml:"has_status,omitempty"` - // `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,omitempty"` + HasStatusCheck *HasStatusCheck `yaml:"has_status_check,omitempty"` - HasWorkflowResult *HasWorkflowResult `yaml:"has_workflow_result,omitempty"` + HasWorkflow *HasWorkflow `yaml:"has_workflow,omitempty"` HasLabels *HasLabels `yaml:"has_labels,omitempty"` @@ -49,6 +45,12 @@ type Predicates struct { HasValidSignatures *HasValidSignatures `yaml:"has_valid_signatures,omitempty"` HasValidSignaturesBy *HasValidSignaturesBy `yaml:"has_valid_signatures_by,omitempty"` HasValidSignaturesByKeys *HasValidSignaturesByKeys `yaml:"has_valid_signatures_by_keys,omitempty"` + + // `has_successful_status`, `has_workflow_result` and `has_status` are deprecated fields that are kept for backwards + // compatibility. `has_status_check` and `has_workflow` replaces it, and can accept any conclusion and status. + HasStatus *HasStatus `yaml:"has_status,omitempty"` + HasSuccessfulStatus *HasSuccessfulStatus `yaml:"has_successful_status,omitempty"` + HasWorkflowResult *HasWorkflowResult `yaml:"has_workflow_result,omitempty"` } func (p Predicates) IsZero() bool { @@ -108,6 +110,10 @@ func (p *Predicates) Predicates() []Predicate { ps = append(ps, Predicate(p.HasStatus)) } + if p.HasStatusCheck != nil { + ps = append(ps, Predicate(p.HasStatusCheck)) + } + if p.HasSuccessfulStatus != nil { ps = append(ps, Predicate(p.HasSuccessfulStatus)) } @@ -116,6 +122,10 @@ func (p *Predicates) Predicates() []Predicate { ps = append(ps, Predicate(p.HasWorkflowResult)) } + if p.HasWorkflow != nil { + ps = append(ps, Predicate(p.HasWorkflow)) + } + if p.HasLabels != nil { ps = append(ps, Predicate(p.HasLabels)) } diff --git a/policy/predicate/status.go b/policy/predicate/status.go index 5cdbe459e..d771f9a97 100644 --- a/policy/predicate/status.go +++ b/policy/predicate/status.go @@ -16,17 +16,19 @@ 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 AllowedConclusions []string +type AllowedStatuses []string +// HasStatus checks that the specified statuses have a completed status with configurable conclusions. +// +// Deprecated: use the more flexible `HasStatusCheck` instead. type HasStatus struct { Conclusions AllowedConclusions `yaml:"conclusions,omitempty"` Statuses []string `yaml:"statuses,omitempty"` @@ -42,50 +44,7 @@ func NewHasStatus(statuses []string, conclusions []string) *HasStatus { 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.Statuses { - result, ok := statuses[status] - if !ok { - missingResults = append(missingResults, status) - } - if !slices.Contains(conclusions, result) { - failingStatuses = append(failingStatuses, status) - } - } - - if len(missingResults) > 0 { - predicateResult.Values = missingResults - predicateResult.Description = "One or more statuses is missing: " + strings.Join(missingResults, ", ") - predicateResult.Satisfied = false - return &predicateResult, nil - } - - if len(failingStatuses) > 0 { - predicateResult.Values = 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.Statuses - predicateResult.Satisfied = true - return &predicateResult, nil + return HasStatusCheck{Checks: pred.Statuses, Conclusions: pred.Conclusions}.Evaluate(ctx, prctx) } func (pred HasStatus) Trigger() common.Trigger { @@ -95,16 +54,14 @@ func (pred HasStatus) Trigger() common.Trigger { // HasSuccessfulStatus checks that the specified statuses have a successful // conclusion. // -// Deprecated: use the more flexible `HasStatus` with `conclusions: ["success"]` +// Deprecated: use the more flexible `HasStatusCheck` 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) + return HasStatusCheck{Checks: pred}.Evaluate(ctx, prctx) } func (pred HasSuccessfulStatus) Trigger() common.Trigger { @@ -133,3 +90,20 @@ func (c AllowedConclusions) joinWithOr() string { return strings.Join(head, ", ") + ", or " + tail } +func (c AllowedStatuses) 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 +} diff --git a/policy/predicate/status_check.go b/policy/predicate/status_check.go new file mode 100644 index 000000000..d87b3b284 --- /dev/null +++ b/policy/predicate/status_check.go @@ -0,0 +1,118 @@ +// Copyright 2018 Palantir Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +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 HasStatusCheck struct { + Conclusions AllowedConclusions `yaml:"conclusions"` + Statuses AllowedStatuses `yaml:"statuses"` + Checks []string `yaml:"checks,omitempty"` +} + +func NewHasStatusCheck(checks []string, statuses []string, conclusions []string) *HasStatusCheck { + return &HasStatusCheck{ + Conclusions: conclusions, + Statuses: statuses, + Checks: checks, + } +} + +var _ Predicate = HasStatus{} + +func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) { + allowedConclusions := pred.Conclusions + if len(allowedConclusions) == 0 { + allowedConclusions = AllowedConclusions{"success"} + } else if slices.Contains(allowedConclusions, "any") { + allowedConclusions = AllowedConclusions{"action_required", "cancelled", "failure", "neutral", "skipped", "stale", "success", "timed_out"} + } + + allowedStatuses := pred.Statuses + // success and error are statuses that only apply to the repo commit statuses. + // pending and failure are statuses that apply to both repo commit statuses and check runs. + // all other statuses are only applicable to check runs. + if len(allowedStatuses) == 0 { + allowedStatuses = AllowedStatuses{"completed", "success"} + } else if slices.Contains(allowedStatuses, "any") { + allowedStatuses = AllowedStatuses{"completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting"} // "error", "success" + } + + checkStatuses, err := prctx.LatestCheckStatuses() + if err != nil { + return nil, errors.Wrap(err, "failed to list commit statuses") + } + + repoStatuses, err := prctx.LatestRepoStatuses() + if err != nil { + return nil, errors.Wrap(err, "failed to list commit statuses") + } + + predicateResult := common.PredicateResult{ + ValuePhrase: "status checks", + ConditionPhrase: fmt.Sprintf("exist and have conclusion %s", allowedConclusions.joinWithOr()), + } + + var missingResults []string + var failingStatuses []string + for _, check := range pred.Checks { + if checkResult, ok := checkStatuses[check]; ok { + isInvalidConclusion := !slices.Contains(allowedConclusions, *checkResult.Conclusion) + if (checkResult.Status == nil || !slices.Contains(allowedStatuses, *checkResult.Status)) || + (slices.Contains(allowedStatuses, "completed") && checkResult.Conclusion != nil && isInvalidConclusion) { + failingStatuses = append(failingStatuses, check) + } + } else if repoStatusResult, ok := repoStatuses[check]; ok { + if repoStatusResult == nil { + missingResults = append(missingResults, check) + } else if repoStatusResult.State == nil || !slices.Contains(allowedStatuses, *repoStatusResult.State) { + failingStatuses = append(failingStatuses, check) + } + } else { + missingResults = append(missingResults, check) + } + } + + if len(missingResults) > 0 { + predicateResult.Values = missingResults + predicateResult.Description = "One or more statuses is missing: " + strings.Join(missingResults, ", ") + predicateResult.Satisfied = false + return &predicateResult, nil + } + + if len(failingStatuses) > 0 { + predicateResult.Values = failingStatuses + predicateResult.Description = fmt.Sprintf("One or more statuses has not concluded with %s: %s", allowedConclusions.joinWithOr(), strings.Join(failingStatuses, ",")) + predicateResult.Satisfied = false + return &predicateResult, nil + } + + predicateResult.Values = pred.Checks + predicateResult.Satisfied = true + return &predicateResult, nil +} + +func (pred HasStatusCheck) Trigger() common.Trigger { + return common.TriggerStatus +} diff --git a/policy/predicate/status_check_test.go b/policy/predicate/status_check_test.go new file mode 100644 index 000000000..3948e2595 --- /dev/null +++ b/policy/predicate/status_check_test.go @@ -0,0 +1,273 @@ +// Copyright 2019 Palantir Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package predicate + +import ( + "context" + "fmt" + "slices" + "strings" + "testing" + "time" + + "github.com/google/go-github/v69/github" + "github.com/palantir/policy-bot/policy/common" + "github.com/palantir/policy-bot/pull/pulltest" + "github.com/stretchr/testify/assert" +) + +func keysSorted[V any](m map[string]V) []string { + r := make([]string, 0, len(m)) + + for k := range m { + r = append(r, k) + } + + slices.Sort(r) + return r +} + +func mockCheckRun(status string, conclusion string) *github.CheckRun { + id := int64(1) + nodeID := "node-id-123" + headSHA := "abc123" + externalID := "external-id" + url := "https://api.github.com/repos/test/Hello-World/check-runs/1" + htmlURL := "https://github.com/test/Hello-World/check-runs/1" + detailsURL := "https://github.com/test/Hello-World/check-runs/1/details" + startedAt := github.Timestamp{Time: time.Now()} + completedAt := github.Timestamp{Time: time.Now()} + name := "test-check-run" + checkSuite := &github.CheckSuite{ + ID: github.Ptr(int64(1)), + } + app := &github.App{ + ID: github.Ptr(int64(1)), + } + pullRequests := []*github.PullRequest{ + { + ID: github.Ptr(int64(1)), + }, + } + + return &github.CheckRun{ + ID: &id, + NodeID: &nodeID, + HeadSHA: &headSHA, + ExternalID: &externalID, + URL: &url, + HTMLURL: &htmlURL, + DetailsURL: &detailsURL, + Status: &status, + Conclusion: &conclusion, + StartedAt: &startedAt, + CompletedAt: &completedAt, + Name: &name, + CheckSuite: checkSuite, + App: app, + PullRequests: pullRequests, + } +} + +func mockRepoStatus(state string) *github.RepoStatus { + id := int64(1) + nodeID := "node-id-123" + url := "https://api.github.com/repos/octocat/Hello-World/statuses/1" + targetURL := "https://github.com/octocat/Hello-World/statuses/1" + context := "default" + avatarURL := "https://avatars.githubusercontent.com/u/1234?v=4" + createdAt := github.Timestamp{Time: time.Now()} + updatedAt := github.Timestamp{Time: time.Now()} + + return &github.RepoStatus{ + ID: &id, + NodeID: &nodeID, + URL: &url, + State: &state, + TargetURL: &targetURL, + Description: nil, + Context: &context, + AvatarURL: &avatarURL, + CreatedAt: &createdAt, + UpdatedAt: &updatedAt, + } +} + +func TestHasSuccessfulStatusCheck(t *testing.T) { + hasStatusCheck := HasStatusCheck{Checks: []string{"status-name", "status-name-2"}} + hasStatusCheckSkippedOk := HasStatusCheck{ + Checks: []string{"status-name", "status-name-2"}, + Conclusions: AllowedConclusions{"success", "skipped"}, + } + hasSuccessfulStatusCheck := HasSuccessfulStatus{"status-name", "status-name-2"} + + commonTestCases := []StatusTestCase{ + { + "all statuses succeed", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "success"), + }, + }, + &common.PredicateResult{ + Satisfied: true, + }, + }, + { + "a status fails", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "failure"), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + { + "multiple statuses fail", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "failure"), + "status-name-2": mockCheckRun("completed", "failure"), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name", "status-name-2"}, + }, + }, + { + "a status does not exist", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + { + "a status does not exist, the other status is skipped", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name-2": mockCheckRun("completed", "skipped"), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name"}, + }, + }, + { + "multiple statuses do not exist", + &pulltest.Context{}, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name", "status-name-2"}, + }, + }, + } + + okOnlyIfSkippedAllowed := []StatusTestCase{ + { + "a status is skipped", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "skipped"), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + { + "all statuses are skipped", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "skipped"), + "status-name-2": mockCheckRun("completed", "skipped"), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name", "status-name-2"}, + }, + }, + } + + testSuites := []StatusTestSuite{ + {predicate: hasStatusCheck, testCases: commonTestCases}, + {predicate: hasStatusCheck, testCases: okOnlyIfSkippedAllowed}, + {predicate: hasSuccessfulStatusCheck, testCases: commonTestCases}, + {predicate: hasSuccessfulStatusCheck, testCases: okOnlyIfSkippedAllowed}, + { + nameSuffix: "skipped allowed", + predicate: hasStatusCheckSkippedOk, + testCases: commonTestCases, + }, + { + nameSuffix: "skipped allowed", + predicate: hasStatusCheckSkippedOk, + testCases: okOnlyIfSkippedAllowed, + overrideSatisfied: github.Bool(true), + }, + } + + for _, suite := range testSuites { + runStatusTestCase(t, suite.predicate, suite) + } +} + +func runStatusCheckTestCase(t *testing.T, p Predicate, suite StatusTestSuite) { + ctx := context.Background() + + for _, tc := range suite.testCases { + if suite.overrideSatisfied != nil { + tc.ExpectedPredicateResult.Satisfied = *suite.overrideSatisfied + } + + // If the test case expects the predicate to be satisfied, we always + // expect the values to contain all the statuses. Doing this here lets + // us use the same testcases when we allow and don't allow skipped + // statuses. + if tc.ExpectedPredicateResult.Satisfied { + statuses, _ := tc.context.LatestCheckStatuses() + tc.ExpectedPredicateResult.Values = keysSorted(statuses) + } + + // `predicate.HasStatusCheck` -> `HasStatusCheck` + _, predicateType, _ := strings.Cut(fmt.Sprintf("%T", p), ".") + testName := fmt.Sprintf("%s_%s", predicateType, tc.name) + + if suite.nameSuffix != "" { + testName += "_" + suite.nameSuffix + } + + t.Run(testName, func(t *testing.T) { + predicateResult, err := p.Evaluate(ctx, tc.context) + if assert.NoError(t, err, "evaluation failed") { + assertPredicateResult(t, tc.ExpectedPredicateResult, predicateResult) + } + }) + } +} diff --git a/policy/predicate/status_test.go b/policy/predicate/status_test.go index c2ccf09ce..994bae6f7 100644 --- a/policy/predicate/status_test.go +++ b/policy/predicate/status_test.go @@ -17,7 +17,6 @@ package predicate import ( "context" "fmt" - "slices" "strings" "testing" @@ -28,17 +27,6 @@ import ( "github.com/stretchr/testify/assert" ) -func keysSorted[V any](m map[string]V) []string { - r := make([]string, 0, len(m)) - - for k := range m { - r = append(r, k) - } - - slices.Sort(r) - return r -} - func TestHasSuccessfulStatus(t *testing.T) { hasStatus := HasStatus{Statuses: []string{"status-name", "status-name-2"}} hasStatusSkippedOk := HasStatus{ @@ -51,9 +39,9 @@ func TestHasSuccessfulStatus(t *testing.T) { { "all statuses succeed", &pulltest.Context{ - LatestStatusesValue: map[string]string{ - "status-name": "success", - "status-name-2": "success", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "success"), }, }, &common.PredicateResult{ @@ -63,9 +51,9 @@ func TestHasSuccessfulStatus(t *testing.T) { { "a status fails", &pulltest.Context{ - LatestStatusesValue: map[string]string{ - "status-name": "success", - "status-name-2": "failure", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "failure"), }, }, &common.PredicateResult{ @@ -76,9 +64,9 @@ func TestHasSuccessfulStatus(t *testing.T) { { "multiple statuses fail", &pulltest.Context{ - LatestStatusesValue: map[string]string{ - "status-name": "failure", - "status-name-2": "failure", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "failure"), + "status-name-2": mockCheckRun("completed", "failure"), }, }, &common.PredicateResult{ @@ -89,8 +77,8 @@ func TestHasSuccessfulStatus(t *testing.T) { { "a status does not exist", &pulltest.Context{ - LatestStatusesValue: map[string]string{ - "status-name": "success", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), }, }, &common.PredicateResult{ @@ -101,8 +89,8 @@ func TestHasSuccessfulStatus(t *testing.T) { { "a status does not exist, the other status is skipped", &pulltest.Context{ - LatestStatusesValue: map[string]string{ - "status-name-2": "skipped", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name-2": mockCheckRun("completed", "skipped"), }, }, &common.PredicateResult{ @@ -124,9 +112,9 @@ func TestHasSuccessfulStatus(t *testing.T) { { "a status is skipped", &pulltest.Context{ - LatestStatusesValue: map[string]string{ - "status-name": "success", - "status-name-2": "skipped", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "skipped"), }, }, &common.PredicateResult{ @@ -137,9 +125,9 @@ func TestHasSuccessfulStatus(t *testing.T) { { "all statuses are skipped", &pulltest.Context{ - LatestStatusesValue: map[string]string{ - "status-name": "skipped", - "status-name-2": "skipped", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "skipped"), + "status-name-2": mockCheckRun("completed", "skipped"), }, }, &common.PredicateResult{ @@ -198,7 +186,7 @@ func runStatusTestCase(t *testing.T, p Predicate, suite StatusTestSuite) { // us use the same testcases when we allow and don't allow skipped // statuses. if tc.ExpectedPredicateResult.Satisfied { - statuses, _ := tc.context.LatestStatuses() + statuses, _ := tc.context.LatestCheckStatuses() tc.ExpectedPredicateResult.Values = keysSorted(statuses) } @@ -219,7 +207,7 @@ func runStatusTestCase(t *testing.T, p Predicate, suite StatusTestSuite) { } } -func TestJoinWithOr(t *testing.T) { +func TestAllowedConclusionsJoinWithOr(t *testing.T) { testCases := []struct { name string input AllowedConclusions @@ -258,3 +246,43 @@ func TestJoinWithOr(t *testing.T) { }) } } + +func TestAllowedStatuseJoinWithOr(t *testing.T) { + testCases := []struct { + name string + input AllowedStatuses + expected string + }{ + { + "empty", + AllowedStatuses{}, + "", + }, + { + "single", + AllowedStatuses{"a"}, + "a", + }, + { + "two", + AllowedStatuses{"a", "b"}, + "a or b", + }, + { + "three", + AllowedStatuses{"a", "b", "c"}, + "a, b, or c", + }, + { + "conclusions get sorted", + AllowedStatuses{"c", "a", "b"}, + "a, b, or c", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, tc.input.joinWithOr()) + }) + } +} diff --git a/policy/predicate/workflow.go b/policy/predicate/workflow.go index 2597af7de..1c9598444 100644 --- a/policy/predicate/workflow.go +++ b/policy/predicate/workflow.go @@ -25,48 +25,67 @@ import ( "github.com/pkg/errors" ) -type HasWorkflowResult struct { +type HasWorkflow struct { + Statuses AllowedStatuses `yaml:"statuses,omitempty"` Conclusions AllowedConclusions `yaml:"conclusions,omitempty"` - Workflows []string `yaml:"workflows,omitempty"` + Workflows []common.Regexp `yaml:"workflows,omitempty"` } -func NewHasWorkflowResult(workflows []string, conclusions []string) *HasWorkflowResult { - return &HasWorkflowResult{ +func NewHasWorkflow(workflows []common.Regexp, conclusions AllowedConclusions, statuses AllowedStatuses) *HasWorkflow { + return &HasWorkflow{ + Statuses: statuses, Conclusions: conclusions, Workflows: workflows, } } -var _ Predicate = HasWorkflowResult{} - -func (pred HasWorkflowResult) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) { - workflowRuns, err := prctx.LatestWorkflowRuns() - if err != nil { - return nil, errors.Wrap(err, "failed to list latest workflow runs") - } +var _ Predicate = HasWorkflow{} +func (pred HasWorkflow) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) { allowedConclusions := pred.Conclusions if len(allowedConclusions) == 0 { allowedConclusions = AllowedConclusions{"success"} + } else if slices.Contains(allowedConclusions, "any") { + allowedConclusions = AllowedConclusions{"action_required", "cancelled", "failure", "neutral", "skipped", "stale", "success", "timed_out"} + } + + allowedStatuses := pred.Statuses + if len(allowedStatuses) == 0 { + allowedStatuses = AllowedStatuses{"completed"} + } else if slices.Contains(allowedStatuses, "any") { + allowedStatuses = AllowedStatuses{"completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting"} + } + + workflowRuns, err := prctx.LatestWorkflowRuns() + if err != nil { + return nil, errors.Wrap(err, "failed to list latest workflow runs") } predicateResult := common.PredicateResult{ ValuePhrase: "workflow results", - ConditionPhrase: fmt.Sprintf("exist and have conclusion %s", allowedConclusions.joinWithOr()), + ConditionPhrase: fmt.Sprintf("exist and have statuses %s and in case status \"completed\" is required also one of conclusions %s", allowedStatuses.joinWithOr(), allowedConclusions.joinWithOr()), } var missingResults []string var failingWorkflows []string for _, workflow := range pred.Workflows { - conclusions, ok := workflowRuns[workflow] - if !ok { - missingResults = append(missingResults, workflow) - } - for _, conclusion := range conclusions { - if !slices.Contains(allowedConclusions, conclusion) { - failingWorkflows = append(failingWorkflows, workflow) + matched := false + for name, workflowSteps := range workflowRuns { + if workflow.Matches(name) { + matched = true + for _, workflowResult := range workflowSteps { + isStatusAllowed := workflowResult.Status != nil && slices.Contains(allowedStatuses, *workflowResult.Status) + isStatusCompletedAllowed := workflowResult.Status != nil && slices.Contains(allowedStatuses, "completed") + isConclusionAllowed := workflowResult.Conclusion != nil && slices.Contains(allowedConclusions, *workflowResult.Conclusion) + if !isStatusAllowed || (isStatusCompletedAllowed && !isConclusionAllowed) { + failingWorkflows = append(failingWorkflows, name) + } + } } } + if !matched { + missingResults = append(missingResults, workflow.String()) + } } if len(missingResults) > 0 { @@ -78,17 +97,21 @@ func (pred HasWorkflowResult) Evaluate(ctx context.Context, prctx pull.Context) if len(failingWorkflows) > 0 { predicateResult.Values = failingWorkflows - predicateResult.Description = fmt.Sprintf("One or more workflow runs have not concluded with %s: %s", pred.Conclusions.joinWithOr(), strings.Join(failingWorkflows, ",")) + predicateResult.Description = fmt.Sprintf("One or more workflow runs have not concluded with %s (yet): %s", allowedConclusions.joinWithOr(), strings.Join(failingWorkflows, ",")) predicateResult.Satisfied = false return &predicateResult, nil } - predicateResult.Values = pred.Workflows + workflows := make([]string, len(pred.Workflows)) + for i, workflow := range pred.Workflows { + workflows[i] = workflow.String() + } + predicateResult.Values = workflows predicateResult.Satisfied = true return &predicateResult, nil } -func (pred HasWorkflowResult) Trigger() common.Trigger { +func (pred HasWorkflow) Trigger() common.Trigger { return common.TriggerStatus } diff --git a/policy/predicate/workflow_test.go b/policy/predicate/workflow_test.go index 01b78d25b..025c9e317 100644 --- a/policy/predicate/workflow_test.go +++ b/policy/predicate/workflow_test.go @@ -18,25 +18,18 @@ import ( "context" "testing" + "github.com/google/go-github/v69/github" "github.com/palantir/policy-bot/policy/common" "github.com/palantir/policy-bot/pull/pulltest" "github.com/stretchr/testify/assert" ) -type WorkflowTestCase struct { - name string - latestWorkflowRunsValue map[string][]string - latestWorkflowRunsError error - predicate Predicate - ExpectedPredicateResult *common.PredicateResult -} - func TestHasSuccessfulWorkflowRun(t *testing.T) { commonTestCases := []WorkflowTestCase{ { name: "all workflows succeed", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"success"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml"}, @@ -48,9 +41,9 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "multiple workflows succeed", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"success"}, - ".github/workflows/test2.yml": {"success"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "success")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, @@ -62,8 +55,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "a workflow fails", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"failure"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "failure")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml"}, @@ -75,8 +68,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "a workflow fails and succeeds", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"failure", "success"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "failure"), mockWorkflowRun("completed", "success")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml"}, @@ -88,9 +81,9 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "multiple workflows fail", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"failure"}, - ".github/workflows/test2.yml": {"failure"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "failure")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "failure")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, @@ -102,9 +95,9 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "one success, one failure", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"success"}, - ".github/workflows/test2.yml": {"failure"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "failure")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, @@ -116,7 +109,7 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "a workflow is missing", - latestWorkflowRunsValue: map[string][]string{}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{}, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml"}, }, @@ -127,7 +120,7 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "multiple workflow are missing", - latestWorkflowRunsValue: map[string][]string{}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{}, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, }, @@ -138,8 +131,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "a workflow is missing, the other workflow is skipped", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test2.yml": {"skipped"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, @@ -151,8 +144,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "a workflow is skipped, but skipped workflows are allowed", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"skipped"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "skipped")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml"}, @@ -165,9 +158,9 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "a workflow succeeds, the other workflow is skipped, but skipped workflows are allowed", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"success"}, - ".github/workflows/test2.yml": {"skipped"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, @@ -180,8 +173,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "a workflow succeeds and is skipped, but skipped workflows are allowed", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"success", "skipped"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success"), mockWorkflowRun("completed", "skipped")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml"}, @@ -194,9 +187,9 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "a workflow fails, the other workflow is skipped, but skipped workflows are allowed", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"failure"}, - ".github/workflows/test2.yml": {"skipped"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "failure")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, @@ -209,9 +202,9 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "a workflow succeeds, the other workflow is skipped, only skipped workflows are allowed", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"success"}, - ".github/workflows/test2.yml": {"skipped"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, @@ -224,8 +217,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, { name: "a workflow succeeds and is skipped, only skipped workflows are allowed", - latestWorkflowRunsValue: map[string][]string{ - ".github/workflows/test.yml": {"success", "skipped"}, + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success"), mockWorkflowRun("completed", "skipped")}, }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml"}, @@ -248,7 +241,7 @@ func runWorkflowTestCase(t *testing.T, cases []WorkflowTestCase) { t.Run(tc.name, func(t *testing.T) { predicateResult, err := tc.predicate.Evaluate(ctx, &pulltest.Context{ LatestWorkflowRunsValue: tc.latestWorkflowRunsValue, - LatestStatusesError: tc.latestWorkflowRunsError, + LatestWorkflowRunsError: tc.latestWorkflowRunsError, }) if assert.NoError(t, err, "evaluation failed") { assertPredicateResult(t, tc.ExpectedPredicateResult, predicateResult) diff --git a/policy/predicate/workflowresult.go b/policy/predicate/workflowresult.go new file mode 100644 index 000000000..023219b88 --- /dev/null +++ b/policy/predicate/workflowresult.go @@ -0,0 +1,51 @@ +// Copyright 2018 Palantir Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package predicate + +import ( + "context" + + "github.com/palantir/policy-bot/policy/common" + "github.com/palantir/policy-bot/pull" +) + +// HasWorkflowResult checks that the specified workflow has a configurable conclusion. +// +// Deprecated: use the more flexible `HasWorkflow` instead. +type HasWorkflowResult struct { + Conclusions AllowedConclusions `yaml:"conclusions,omitempty"` + Workflows []string `yaml:"workflows,omitempty"` +} + +func NewHasWorkflowResult(workflows []string, conclusions []string) *HasWorkflowResult { + return &HasWorkflowResult{ + Conclusions: conclusions, + Workflows: workflows, + } +} + +var _ Predicate = HasWorkflowResult{} + +func (pred HasWorkflowResult) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) { + regexpWorkflows := make([]common.Regexp, len(pred.Workflows)) + for i, workflow := range pred.Workflows { + regexpWorkflows[i], _ = common.NewRegexp(workflow) + } + return HasWorkflow{Workflows: regexpWorkflows, Conclusions: pred.Conclusions}.Evaluate(ctx, prctx) +} + +func (pred HasWorkflowResult) Trigger() common.Trigger { + return common.TriggerStatus +} diff --git a/policy/predicate/workflowresult_test.go b/policy/predicate/workflowresult_test.go new file mode 100644 index 000000000..1c01710ee --- /dev/null +++ b/policy/predicate/workflowresult_test.go @@ -0,0 +1,333 @@ +// Copyright 2024 Palantir Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package predicate + +import ( + "context" + "testing" + "time" + + "github.com/google/go-github/v69/github" + "github.com/palantir/policy-bot/policy/common" + "github.com/palantir/policy-bot/pull/pulltest" + "github.com/stretchr/testify/assert" +) + +type WorkflowTestCase struct { + name string + latestWorkflowRunsValue map[string][]*github.WorkflowRun + latestWorkflowRunsError error + predicate Predicate + ExpectedPredicateResult *common.PredicateResult +} + +func mockWorkflowRun(status string, conclusion string) *github.WorkflowRun { + id := int64(1) + name := "abc" + nodeID := "MDg6V29ya2Zsb3cx" + headBranch := "main" + headSHA := "abc123" + path := ".github/workflows/test.yml" + runNumber := 1 + runAttempt := 1 + event := "push" + displayTitle := "Test Workflow" + workflowID := int64(123456) + checkSuiteID := int64(654321) + checkSuiteNodeID := "MDg6Q2hlY2tTdWl0ZQ==" + url := "https://api.github.com/repos/octocat/Hello-World/actions/runs/1" + htmlURL := "https://github.com/octocat/Hello-World/actions/runs/1" + createdAt := github.Timestamp{Time: time.Now()} + updatedAt := github.Timestamp{Time: time.Now()} + runStartedAt := github.Timestamp{Time: time.Now()} + jobsURL := "https://api.github.com/repos/octocat/Hello-World/actions/runs/1/jobs" + logsURL := "https://api.github.com/repos/octocat/Hello-World/actions/runs/1/logs" + checkSuiteURL := "https://api.github.com/repos/octocat/Hello-World/check-suites/654321" + artifactsURL := "https://api.github.com/repos/octocat/Hello-World/actions/runs/1/artifacts" + cancelURL := "https://api.github.com/repos/octocat/Hello-World/actions/runs/1/cancel" + rerunURL := "https://api.github.com/repos/octocat/Hello-World/actions/runs/1/rerun" + previousAttemptURL := "https://api.github.com/repos/octocat/Hello-World/actions/runs/1/attempts/1" + headCommit := &github.HeadCommit{ + ID: github.String("abc123"), + Message: github.String("Initial commit"), + } + repository := &github.Repository{ + ID: github.Int64(1296269), + Name: github.String("Hello-World"), + } + actor := &github.User{ + Login: github.String("octocat"), + ID: github.Int64(1), + } + + return &github.WorkflowRun{ + ID: &id, + Name: &name, + NodeID: &nodeID, + HeadBranch: &headBranch, + HeadSHA: &headSHA, + Path: &path, + RunNumber: &runNumber, + RunAttempt: &runAttempt, + Event: &event, + DisplayTitle: &displayTitle, + Status: &status, + Conclusion: &conclusion, + WorkflowID: &workflowID, + CheckSuiteID: &checkSuiteID, + CheckSuiteNodeID: &checkSuiteNodeID, + URL: &url, + HTMLURL: &htmlURL, + CreatedAt: &createdAt, + UpdatedAt: &updatedAt, + RunStartedAt: &runStartedAt, + JobsURL: &jobsURL, + LogsURL: &logsURL, + CheckSuiteURL: &checkSuiteURL, + ArtifactsURL: &artifactsURL, + CancelURL: &cancelURL, + RerunURL: &rerunURL, + PreviousAttemptURL: &previousAttemptURL, + HeadCommit: headCommit, + Repository: repository, + Actor: actor, + } +} + +func TestHasSuccessfulWorkflowResultRun(t *testing.T) { + commonTestCases := []WorkflowTestCase{ + { + name: "all workflows succeed", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "multiple workflows succeed", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "success")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + }, + }, + { + name: "a workflow fails", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "failure")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow fails and succeeds", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success"), mockWorkflowRun("completed", "failure")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "multiple workflows fail", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "failure")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "failure")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + }, + }, + { + name: "one success, one failure", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "failure")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test2.yml"}, + }, + }, + { + name: "a workflow is missing", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{}, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "multiple workflow are missing", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{}, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + }, + }, + { + name: "a workflow is missing, the other workflow is skipped", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow is skipped, but skipped workflows are allowed", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "skipped")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + Conclusions: AllowedConclusions{"skipped"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow succeeds, the other workflow is skipped, but skipped workflows are allowed", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + Conclusions: AllowedConclusions{"skipped", "success"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + }, + }, + { + name: "a workflow succeeds and is skipped, but skipped workflows are allowed", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success"), mockWorkflowRun("completed", "skipped")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + Conclusions: AllowedConclusions{"skipped", "success"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow fails, the other workflow is skipped, but skipped workflows are allowed", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "failure")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "success")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + Conclusions: AllowedConclusions{"skipped", "success"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow succeeds, the other workflow is skipped, only skipped workflows are allowed", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + Conclusions: AllowedConclusions{"skipped"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow succeeds and is skipped, only skipped workflows are allowed", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success"), mockWorkflowRun("completed", "skipped")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + Conclusions: AllowedConclusions{"skipped"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test.yml"}, + }, + }, + } + + runWorkflowResultTestCase(t, commonTestCases) +} + +func runWorkflowResultTestCase(t *testing.T, cases []WorkflowTestCase) { + ctx := context.Background() + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + predicateResult, err := tc.predicate.Evaluate(ctx, &pulltest.Context{ + LatestWorkflowRunsValue: tc.latestWorkflowRunsValue, + LatestWorkflowRunsError: tc.latestWorkflowRunsError, + }) + if assert.NoError(t, err, "evaluation failed") { + assertPredicateResult(t, tc.ExpectedPredicateResult, predicateResult) + } + }) + } +} diff --git a/pull/context.go b/pull/context.go index da866bc75..8df7af3b7 100644 --- a/pull/context.go +++ b/pull/context.go @@ -16,6 +16,8 @@ package pull import ( "time" + + "github.com/google/go-github/v69/github" ) // MembershipContext defines methods to get information @@ -121,13 +123,16 @@ type Context interface { // the pull request. RequestedReviewers() ([]*Reviewer, error) - // LatestStatuses returns a map of status check names to the latest result - LatestStatuses() (map[string]string, error) + // LatestRepoStatuses returns a map of commit status names to the latest result + LatestRepoStatuses() (map[string]*github.RepoStatus, error) + + // LatestStaLatestCheckStatusestuses returns a map of status check names to the latest result + LatestCheckStatuses() (map[string]*github.CheckRun, error) // LatestWorkflowRuns returns the latest GitHub Actions workflow runs for // the pull request. The keys of the map are paths to the workflow files and // the values are the conclusions of the latest runs, one per event type. - LatestWorkflowRuns() (map[string][]string, error) + LatestWorkflowRuns() (map[string][]*github.WorkflowRun, error) // Labels returns a list of labels applied on the Pull Request Labels() ([]string, error) diff --git a/pull/github.go b/pull/github.go index a226f801c..1e5430d2d 100644 --- a/pull/github.go +++ b/pull/github.go @@ -140,10 +140,11 @@ type GitHubContext struct { permissions map[string]Permission teams map[string]Permission membership map[string]bool - statuses map[string]string + repoStatuses map[string]*github.RepoStatus + checkStatuses map[string]*github.CheckRun labels []string pushedAt map[string]time.Time - workflowRuns map[string][]string + workflowRuns map[string][]*github.WorkflowRun } // NewGitHubContext creates a new pull.Context that makes GitHub requests to @@ -767,58 +768,37 @@ func (ghc *GitHubContext) Teams() (map[string]Permission, error) { return ghc.teams, nil } -func (ghc *GitHubContext) LatestStatuses() (map[string]string, error) { - if ghc.statuses == nil { - statuses, err := ghc.getStatuses() - if err != nil { - return nil, err - } - - checkStatuses, err := ghc.getCheckStatuses() - if err != nil { - return nil, err - } - - for k, v := range checkStatuses { - statuses[k] = v - } - - ghc.statuses = statuses - } - - return ghc.statuses, nil -} - -func (ghc *GitHubContext) getStatuses() (map[string]string, error) { +func (ghc *GitHubContext) LatestRepoStatuses() (map[string]*github.RepoStatus, error) { opt := &github.ListOptions{ PerPage: 100, } // get all pages of results - statuses := make(map[string]string) + statuses := make(map[string]*github.RepoStatus) for { combinedStatus, resp, err := ghc.client.Repositories.GetCombinedStatus(ghc.ctx, ghc.owner, ghc.repo, ghc.HeadSHA(), opt) if err != nil { return nil, errors.Wrapf(err, "failed to get statuses for page %d", opt.Page) } for _, s := range combinedStatus.Statuses { - statuses[s.GetContext()] = s.GetState() + statuses[s.GetContext()] = s } if resp.NextPage == 0 { break } opt.Page = resp.NextPage } + ghc.repoStatuses = statuses return statuses, nil } -func (ghc *GitHubContext) getCheckStatuses() (map[string]string, error) { +func (ghc *GitHubContext) LatestCheckStatuses() (map[string]*github.CheckRun, error) { opt := &github.ListCheckRunsOptions{ ListOptions: github.ListOptions{ PerPage: 100, }, } // get all pages of results - statuses := make(map[string]string) + statuses := make(map[string]*github.CheckRun) for { checkRuns, resp, err := ghc.client.Checks.ListCheckRunsForRef(ghc.ctx, ghc.owner, ghc.repo, ghc.HeadSHA(), opt) if err != nil { @@ -832,7 +812,7 @@ func (ghc *GitHubContext) getCheckStatuses() (map[string]string, error) { for _, checkRun := range checkRuns.CheckRuns { name := checkRun.GetName() if _, exists := statuses[name]; !exists { - statuses[name] = checkRun.GetConclusion() + statuses[name] = checkRun } } @@ -841,10 +821,11 @@ func (ghc *GitHubContext) getCheckStatuses() (map[string]string, error) { } opt.Page = resp.NextPage } + ghc.checkStatuses = statuses return statuses, nil } -func (ghc *GitHubContext) LatestWorkflowRuns() (map[string][]string, error) { +func (ghc *GitHubContext) LatestWorkflowRuns() (map[string][]*github.WorkflowRun, error) { if ghc.workflowRuns != nil { return ghc.workflowRuns, nil } @@ -892,10 +873,6 @@ func (ghc *GitHubContext) LatestWorkflowRuns() (map[string][]string, error) { } for _, run := range runs.WorkflowRuns { - if run.GetStatus() != "completed" { - continue - } - eventName := run.GetEvent() previousRuns := runsWithDate[*run.Path] @@ -919,11 +896,11 @@ func (ghc *GitHubContext) LatestWorkflowRuns() (map[string][]string, error) { } opt.Page = resp.NextPage } - workflowRuns := make(map[string][]string, len(runsWithDate)) + workflowRuns := make(map[string][]*github.WorkflowRun, len(runsWithDate)) for path, eventRuns := range runsWithDate { for _, run := range eventRuns { - workflowRuns[path] = append(workflowRuns[path], run.GetConclusion()) + workflowRuns[path] = append(workflowRuns[path], run) } } diff --git a/pull/github_test.go b/pull/github_test.go index 327641137..c7fb8483f 100644 --- a/pull/github_test.go +++ b/pull/github_test.go @@ -658,14 +658,57 @@ func TestLatestWorkflowRuns(t *testing.T) { runs, err := ctx.LatestWorkflowRuns() require.NoError(t, err) - assert.Len(t, runs, 3, "incorrect number of workflow runs") - assert.ElementsMatch(t, runs[".github/workflows/a.yml"], []string{"success", "skipped"}, "incorrect conclusion for workflow run a") - assert.ElementsMatch(t, runs[".github/workflows/b.yml"], []string{"failure"}, "incorrect conclusion for workflow run b") - assert.ElementsMatch(t, runs[".github/workflows/c.yml"], []string{"cancelled"}, "incorrect conclusion for workflow run c") + assert.Len(t, runs, 4, "incorrect number of workflow runs") + assert.IsType(t, []*github.WorkflowRun{}, runs[".github/workflows/a.yml"], "object is not of type *github.WorkflowRun") + assert.IsType(t, []*github.WorkflowRun{}, runs[".github/workflows/b.yml"], "object is not of type *github.WorkflowRun") + assert.IsType(t, []*github.WorkflowRun{}, runs[".github/workflows/c.yml"], "object is not of type *github.WorkflowRun") + assert.IsType(t, []*github.WorkflowRun{}, runs[".github/workflows/d.yml"], "object is not of type *github.WorkflowRun") + for _, run := range runs[".github/workflows/a.yml"] { + assert.Contains(t, []string{"success", "skipped"}, *run.Conclusion, "incorrect conclusion for workflow run a") + assert.Equal(t, "completed", *run.Status, "incorrect status for workflow run a") + } + for _, run := range runs[".github/workflows/b.yml"] { + assert.Equal(t, "failure", *run.Conclusion, "incorrect conclusion for workflow run b") + assert.Equal(t, "completed", *run.Status, "incorrect status for workflow run b") + } + for _, run := range runs[".github/workflows/c.yml"] { + assert.Equal(t, "cancelled", *run.Conclusion, "incorrect conclusion for workflow run c") + assert.Equal(t, "completed", *run.Status, "incorrect status for workflow run c") + } + for _, run := range runs[".github/workflows/d.yml"] { + assert.Empty(t, run.Conclusion, "incorrect conclusion for workflow run d") + assert.Equal(t, "in_progress", *run.Status, "incorrect status for workflow run d") + } assert.Equal(t, 2, runsRule.Count, "incorrect http request count") } -func TestLatestStatuses(t *testing.T) { +func TestLatestCheckStatuses(t *testing.T) { + pr := defaultTestPR() + + rp := &ResponsePlayer{} + rp.AddRule( + ExactPathMatcher("/repos/testorg/testrepo/commits/"+pr.Head.GetSHA()+"/status"), + "testdata/responses/combined_status_for_ref.yml", + ) + rp.AddRule( + ExactPathMatcher("/repos/testorg/testrepo/commits/"+pr.Head.GetSHA()+"/check-runs"), + "testdata/responses/check_runs_for_ref.yml", + ) + + ctx := makeContext(t, rp, pr, nil) + statuses, err := ctx.LatestCheckStatuses() + require.NoError(t, err) + + assert.Len(t, statuses, 2, "incorrect number of statuses") + assert.IsType(t, &github.CheckRun{}, statuses["check-run-a"], "object is not of type *github.CheckRun") + assert.Equal(t, "success", *statuses["check-run-a"].Conclusion, "incorrect conclusion for check run a") + assert.Equal(t, "completed", *statuses["check-run-a"].Status, "incorrect status for check run a") + assert.IsType(t, &github.CheckRun{}, statuses["check-run-b"], "object is not of type *github.CheckRun") + assert.Equal(t, "failure", *statuses["check-run-b"].Conclusion, "incorrect conclusion for check run b") + assert.Equal(t, "completed", *statuses["check-run-b"].Status, "incorrect status for check run b") +} + +func TestLatestRepoStatuses(t *testing.T) { pr := defaultTestPR() rp := &ResponsePlayer{} @@ -679,14 +722,14 @@ func TestLatestStatuses(t *testing.T) { ) ctx := makeContext(t, rp, pr, nil) - statuses, err := ctx.LatestStatuses() + statuses, err := ctx.LatestRepoStatuses() require.NoError(t, err) - assert.Len(t, statuses, 4, "incorrect number of statuses") - assert.Equal(t, statuses["commit-status-a"], "success", "incorrect conclusion for 'commit-status-a' status") - assert.Equal(t, statuses["commit-status-b"], "pending", "incorrect conclusion for 'commit-status-a' status") - assert.Equal(t, statuses["check-run-a"], "success", "incorrect conclusion for 'check-run-a' status") - assert.Equal(t, statuses["check-run-b"], "failure", "incorrect conclusion for 'check-run-b' status") + assert.Len(t, statuses, 2, "incorrect number of statuses") + assert.IsType(t, &github.RepoStatus{}, statuses["commit-status-a"], "object is not of type *github.CheckRun") + assert.Equal(t, "success", *statuses["commit-status-a"].State, "incorrect conclusion for check run a") + assert.IsType(t, &github.RepoStatus{}, statuses["commit-status-b"], "object is not of type *github.CheckRun") + assert.Equal(t, "pending", *statuses["commit-status-b"].State, "incorrect conclusion for check run b") } func makeContext(t *testing.T, rp *ResponsePlayer, pr *github.PullRequest, gc GlobalCache) Context { diff --git a/pull/pulltest/context.go b/pull/pulltest/context.go index 225640bfd..cf8104cb0 100644 --- a/pull/pulltest/context.go +++ b/pull/pulltest/context.go @@ -17,6 +17,7 @@ package pulltest import ( "time" + "github.com/google/go-github/v69/github" "github.com/palantir/policy-bot/pull" ) @@ -68,10 +69,13 @@ type Context struct { RequestedReviewersValue []*pull.Reviewer RequestedReviewersError error - LatestStatusesValue map[string]string - LatestStatusesError error + LatestRepoStatusesValue map[string]*github.RepoStatus + LatestRepoStatusesError error - LatestWorkflowRunsValue map[string][]string + LatestCheckStatusesValue map[string]*github.CheckRun + LatestCheckStatusesError error + + LatestWorkflowRunsValue map[string][]*github.WorkflowRun LatestWorkflowRunsError error LabelsValue []string @@ -253,11 +257,15 @@ func (c *Context) Teams() (map[string]pull.Permission, error) { return c.TeamsValue, c.TeamsError } -func (c *Context) LatestStatuses() (map[string]string, error) { - return c.LatestStatusesValue, c.LatestStatusesError +func (c *Context) LatestRepoStatuses() (map[string]*github.RepoStatus, error) { + return c.LatestRepoStatusesValue, c.LatestRepoStatusesError +} + +func (c *Context) LatestCheckStatuses() (map[string]*github.CheckRun, error) { + return c.LatestCheckStatusesValue, c.LatestCheckStatusesError } -func (c *Context) LatestWorkflowRuns() (map[string][]string, error) { +func (c *Context) LatestWorkflowRuns() (map[string][]*github.WorkflowRun, error) { return c.LatestWorkflowRunsValue, c.LatestWorkflowRunsError } diff --git a/server/handler/check_run.go b/server/handler/check_run.go index d711e766a..9393e5d14 100644 --- a/server/handler/check_run.go +++ b/server/handler/check_run.go @@ -37,10 +37,6 @@ func (h *CheckRun) Handle(ctx context.Context, eventType, deliveryID string, pay return errors.Wrap(err, "failed to parse check_run event payload") } - if event.GetAction() != "completed" || event.GetCheckRun().GetConclusion() != "success" { - return nil - } - repo := event.GetRepo() repoID := repo.GetID() ownerName := repo.GetOwner().GetLogin() From 835799539b62a7dcca65652eb828b5e7b9e3ed0d Mon Sep 17 00:00:00 2001 From: Florian Braun <5863788+FloThinksPi@users.noreply.github.com> Date: Tue, 11 Mar 2025 17:21:08 +0100 Subject: [PATCH 2/5] Make the already existing predicates not use regex --- policy/common/regexp.go | 11 +++++ policy/predicate/status.go | 12 +++++- policy/predicate/status_check.go | 50 +++++++++++++++------- policy/predicate/status_check_test.go | 13 +++--- policy/predicate/workflow.go | 24 +++++++---- policy/predicate/workflow_test.go | 60 +++++++++++++-------------- policy/predicate/workflowresult.go | 12 ++++-- 7 files changed, 118 insertions(+), 64 deletions(-) diff --git a/policy/common/regexp.go b/policy/common/regexp.go index 325f106fb..7ceeec6c8 100644 --- a/policy/common/regexp.go +++ b/policy/common/regexp.go @@ -41,10 +41,21 @@ func NewRegexp(pattern string) (Regexp, error) { return Regexp{r: r}, nil } +func NewMustCompileRegexp(pattern string) Regexp { + return Regexp{r: regexp.MustCompile(pattern)} +} + func NewCompiledRegexp(r *regexp.Regexp) Regexp { return Regexp{r: r} } +// UnquoteMeta reverses the effect of QuoteMeta +func UnquoteMeta(s string) string { + re := regexp.MustCompile(`\\([\\.?])`) // Match escaped characters \\, \., and \? + literalString := re.ReplaceAllString(s, "$1") + return literalString +} + func (r Regexp) Matches(s string) bool { if r.r == nil { return false diff --git a/policy/predicate/status.go b/policy/predicate/status.go index d771f9a97..3344bf2eb 100644 --- a/policy/predicate/status.go +++ b/policy/predicate/status.go @@ -44,7 +44,15 @@ func NewHasStatus(statuses []string, conclusions []string) *HasStatus { var _ Predicate = HasStatus{} func (pred HasStatus) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) { - return HasStatusCheck{Checks: pred.Statuses, Conclusions: pred.Conclusions}.Evaluate(ctx, prctx) + // Convert strings into a regex object to comply with function signature + var checks []common.Regexp + for _, status := range pred.Statuses { + check, err := common.NewRegexp(status) + if err == nil { + checks = append(checks, check) + } + } + return HasStatusCheck{Checks: checks, Conclusions: pred.Conclusions, noRegex: true}.Evaluate(ctx, prctx) } func (pred HasStatus) Trigger() common.Trigger { @@ -61,7 +69,7 @@ type HasSuccessfulStatus []string var _ Predicate = HasSuccessfulStatus{} func (pred HasSuccessfulStatus) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) { - return HasStatusCheck{Checks: pred}.Evaluate(ctx, prctx) + return HasStatus{Statuses: pred}.Evaluate(ctx, prctx) } func (pred HasSuccessfulStatus) Trigger() common.Trigger { diff --git a/policy/predicate/status_check.go b/policy/predicate/status_check.go index d87b3b284..a5b3138d9 100644 --- a/policy/predicate/status_check.go +++ b/policy/predicate/status_check.go @@ -17,6 +17,7 @@ package predicate import ( "context" "fmt" + "regexp" "slices" "strings" @@ -28,14 +29,16 @@ import ( type HasStatusCheck struct { Conclusions AllowedConclusions `yaml:"conclusions"` Statuses AllowedStatuses `yaml:"statuses"` - Checks []string `yaml:"checks,omitempty"` + Checks []common.Regexp `yaml:"checks,omitempty"` + noRegex bool } -func NewHasStatusCheck(checks []string, statuses []string, conclusions []string) *HasStatusCheck { +func NewHasStatusCheck(checks []common.Regexp, statuses []string, conclusions []string) *HasStatusCheck { return &HasStatusCheck{ Conclusions: conclusions, Statuses: statuses, Checks: checks, + noRegex: false, } } @@ -76,21 +79,38 @@ func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*c var missingResults []string var failingStatuses []string + var allChecks []string for _, check := range pred.Checks { - if checkResult, ok := checkStatuses[check]; ok { - isInvalidConclusion := !slices.Contains(allowedConclusions, *checkResult.Conclusion) - if (checkResult.Status == nil || !slices.Contains(allowedStatuses, *checkResult.Status)) || - (slices.Contains(allowedStatuses, "completed") && checkResult.Conclusion != nil && isInvalidConclusion) { - failingStatuses = append(failingStatuses, check) + matched := false + check_to_use := check + if pred.noRegex { + check_to_use, err = common.NewRegexp(fmt.Sprintf("^%s$", regexp.QuoteMeta(check.String()))) + if err != nil { + return nil, errors.Wrapf(err, "failed to create regexp for workflow %s", check.String()) } - } else if repoStatusResult, ok := repoStatuses[check]; ok { - if repoStatusResult == nil { - missingResults = append(missingResults, check) - } else if repoStatusResult.State == nil || !slices.Contains(allowedStatuses, *repoStatusResult.State) { - failingStatuses = append(failingStatuses, check) + } + for checkResultName, checkResult := range checkStatuses { + if check_to_use.Matches(checkResultName) { + matched = true + allChecks = append(allChecks, checkResultName) + isInvalidConclusion := !slices.Contains(allowedConclusions, *checkResult.Conclusion) + if (checkResult.Status == nil || !slices.Contains(allowedStatuses, *checkResult.Status)) || + (slices.Contains(allowedStatuses, "completed") && checkResult.Conclusion != nil && isInvalidConclusion) { + failingStatuses = append(failingStatuses, checkResultName) + } + } + } + for repoStatusName, repoStatusResult := range repoStatuses { + if check_to_use.Matches(repoStatusName) { + matched = true + allChecks = append(allChecks, repoStatusName) + if repoStatusResult == nil || repoStatusResult.State == nil || !slices.Contains(allowedStatuses, *repoStatusResult.State) { + failingStatuses = append(failingStatuses, repoStatusName) + } } - } else { - missingResults = append(missingResults, check) + } + if !matched { + missingResults = append(missingResults, check.String()) } } @@ -108,7 +128,7 @@ func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*c return &predicateResult, nil } - predicateResult.Values = pred.Checks + predicateResult.Values = allChecks predicateResult.Satisfied = true return &predicateResult, nil } diff --git a/policy/predicate/status_check_test.go b/policy/predicate/status_check_test.go index 3948e2595..9de7c1971 100644 --- a/policy/predicate/status_check_test.go +++ b/policy/predicate/status_check_test.go @@ -106,12 +106,17 @@ func mockRepoStatus(state string) *github.RepoStatus { } func TestHasSuccessfulStatusCheck(t *testing.T) { - hasStatusCheck := HasStatusCheck{Checks: []string{"status-name", "status-name-2"}} + hasStatusCheck := HasStatusCheck{Checks: []common.Regexp{ + common.NewMustCompileRegexp("status-name"), + common.NewMustCompileRegexp("status-name-2"), + }} hasStatusCheckSkippedOk := HasStatusCheck{ - Checks: []string{"status-name", "status-name-2"}, + Checks: []common.Regexp{ + common.NewMustCompileRegexp("status-name"), + common.NewMustCompileRegexp("status-name-2"), + }, Conclusions: AllowedConclusions{"success", "skipped"}, } - hasSuccessfulStatusCheck := HasSuccessfulStatus{"status-name", "status-name-2"} commonTestCases := []StatusTestCase{ { @@ -218,8 +223,6 @@ func TestHasSuccessfulStatusCheck(t *testing.T) { testSuites := []StatusTestSuite{ {predicate: hasStatusCheck, testCases: commonTestCases}, {predicate: hasStatusCheck, testCases: okOnlyIfSkippedAllowed}, - {predicate: hasSuccessfulStatusCheck, testCases: commonTestCases}, - {predicate: hasSuccessfulStatusCheck, testCases: okOnlyIfSkippedAllowed}, { nameSuffix: "skipped allowed", predicate: hasStatusCheckSkippedOk, diff --git a/policy/predicate/workflow.go b/policy/predicate/workflow.go index 1c9598444..06d594a59 100644 --- a/policy/predicate/workflow.go +++ b/policy/predicate/workflow.go @@ -17,6 +17,7 @@ package predicate import ( "context" "fmt" + "regexp" "slices" "strings" @@ -29,6 +30,7 @@ type HasWorkflow struct { Statuses AllowedStatuses `yaml:"statuses,omitempty"` Conclusions AllowedConclusions `yaml:"conclusions,omitempty"` Workflows []common.Regexp `yaml:"workflows,omitempty"` + noRegex bool } func NewHasWorkflow(workflows []common.Regexp, conclusions AllowedConclusions, statuses AllowedStatuses) *HasWorkflow { @@ -36,6 +38,7 @@ func NewHasWorkflow(workflows []common.Regexp, conclusions AllowedConclusions, s Statuses: statuses, Conclusions: conclusions, Workflows: workflows, + noRegex: false, } } @@ -68,12 +71,21 @@ func (pred HasWorkflow) Evaluate(ctx context.Context, prctx pull.Context) (*comm var missingResults []string var failingWorkflows []string + var allWorkflows []string for _, workflow := range pred.Workflows { matched := false - for name, workflowSteps := range workflowRuns { - if workflow.Matches(name) { + workflow_to_use := workflow + if pred.noRegex { + workflow_to_use, err = common.NewRegexp(fmt.Sprintf("^%s$", regexp.QuoteMeta(workflow.String()))) + if err != nil { + return nil, errors.Wrapf(err, "failed to create regexp for workflow %s", workflow.String()) + } + } + for name, workflowRun := range workflowRuns { + if workflow_to_use.Matches(name) { matched = true - for _, workflowResult := range workflowSteps { + allWorkflows = append(allWorkflows, name) + for _, workflowResult := range workflowRun { isStatusAllowed := workflowResult.Status != nil && slices.Contains(allowedStatuses, *workflowResult.Status) isStatusCompletedAllowed := workflowResult.Status != nil && slices.Contains(allowedStatuses, "completed") isConclusionAllowed := workflowResult.Conclusion != nil && slices.Contains(allowedConclusions, *workflowResult.Conclusion) @@ -102,11 +114,7 @@ func (pred HasWorkflow) Evaluate(ctx context.Context, prctx pull.Context) (*comm return &predicateResult, nil } - workflows := make([]string, len(pred.Workflows)) - for i, workflow := range pred.Workflows { - workflows[i] = workflow.String() - } - predicateResult.Values = workflows + predicateResult.Values = allWorkflows predicateResult.Satisfied = true return &predicateResult, nil diff --git a/policy/predicate/workflow_test.go b/policy/predicate/workflow_test.go index 025c9e317..767682bfb 100644 --- a/policy/predicate/workflow_test.go +++ b/policy/predicate/workflow_test.go @@ -31,8 +31,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: true, @@ -45,8 +45,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, ".github/workflows/test2.yml": {mockWorkflowRun("completed", "success")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml"), common.NewMustCompileRegexp(".github/workflows/test2.yml")}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: true, @@ -58,8 +58,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ ".github/workflows/test.yml": {mockWorkflowRun("completed", "failure")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, @@ -71,8 +71,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ ".github/workflows/test.yml": {mockWorkflowRun("completed", "failure"), mockWorkflowRun("completed", "success")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, @@ -85,8 +85,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { ".github/workflows/test.yml": {mockWorkflowRun("completed", "failure")}, ".github/workflows/test2.yml": {mockWorkflowRun("completed", "failure")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml"), common.NewMustCompileRegexp(".github/workflows/test2.yml")}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, @@ -99,8 +99,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, ".github/workflows/test2.yml": {mockWorkflowRun("completed", "failure")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml"), common.NewMustCompileRegexp(".github/workflows/test2.yml")}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, @@ -110,8 +110,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { { name: "a workflow is missing", latestWorkflowRunsValue: map[string][]*github.WorkflowRun{}, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, @@ -121,8 +121,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { { name: "multiple workflow are missing", latestWorkflowRunsValue: map[string][]*github.WorkflowRun{}, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml"), common.NewMustCompileRegexp(".github/workflows/test2.yml")}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, @@ -134,8 +134,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml"), common.NewMustCompileRegexp(".github/workflows/test2.yml")}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, @@ -147,8 +147,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ ".github/workflows/test.yml": {mockWorkflowRun("completed", "skipped")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, Conclusions: AllowedConclusions{"skipped"}, }, ExpectedPredicateResult: &common.PredicateResult{ @@ -162,8 +162,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml"), common.NewMustCompileRegexp(".github/workflows/test2.yml")}, Conclusions: AllowedConclusions{"skipped", "success"}, }, ExpectedPredicateResult: &common.PredicateResult{ @@ -176,8 +176,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ ".github/workflows/test.yml": {mockWorkflowRun("completed", "success"), mockWorkflowRun("completed", "skipped")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, Conclusions: AllowedConclusions{"skipped", "success"}, }, ExpectedPredicateResult: &common.PredicateResult{ @@ -191,8 +191,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { ".github/workflows/test.yml": {mockWorkflowRun("completed", "failure")}, ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml"), common.NewMustCompileRegexp(".github/workflows/test2.yml")}, Conclusions: AllowedConclusions{"skipped", "success"}, }, ExpectedPredicateResult: &common.PredicateResult{ @@ -206,8 +206,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml"), common.NewMustCompileRegexp(".github/workflows/test2.yml")}, Conclusions: AllowedConclusions{"skipped"}, }, ExpectedPredicateResult: &common.PredicateResult{ @@ -220,8 +220,8 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ ".github/workflows/test.yml": {mockWorkflowRun("completed", "success"), mockWorkflowRun("completed", "skipped")}, }, - predicate: HasWorkflowResult{ - Workflows: []string{".github/workflows/test.yml"}, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, Conclusions: AllowedConclusions{"skipped"}, }, ExpectedPredicateResult: &common.PredicateResult{ diff --git a/policy/predicate/workflowresult.go b/policy/predicate/workflowresult.go index 023219b88..40bdf1b2c 100644 --- a/policy/predicate/workflowresult.go +++ b/policy/predicate/workflowresult.go @@ -39,11 +39,15 @@ func NewHasWorkflowResult(workflows []string, conclusions []string) *HasWorkflow var _ Predicate = HasWorkflowResult{} func (pred HasWorkflowResult) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) { - regexpWorkflows := make([]common.Regexp, len(pred.Workflows)) - for i, workflow := range pred.Workflows { - regexpWorkflows[i], _ = common.NewRegexp(workflow) + // Convert strings into a regex object to comply with function signature + var workflows []common.Regexp + for _, workflow := range pred.Workflows { + escaped_workflow, err := common.NewRegexp(workflow) + if err == nil { + workflows = append(workflows, escaped_workflow) + } } - return HasWorkflow{Workflows: regexpWorkflows, Conclusions: pred.Conclusions}.Evaluate(ctx, prctx) + return HasWorkflow{Workflows: workflows, Conclusions: pred.Conclusions, noRegex: true}.Evaluate(ctx, prctx) } func (pred HasWorkflowResult) Trigger() common.Trigger { From 33f9a9d5a1f3209c5f7105c7e289a97fef65bb9f Mon Sep 17 00:00:00 2001 From: Florian Braun <5863788+FloThinksPi@users.noreply.github.com> Date: Wed, 12 Mar 2025 14:20:21 +0100 Subject: [PATCH 3/5] Add proper tests and adopt behaviour to testcases --- policy/predicate/status.go | 49 +- policy/predicate/status_check.go | 93 +++- policy/predicate/status_check_test.go | 711 ++++++++++++++++++++---- policy/predicate/status_test.go | 226 +++++--- policy/predicate/workflow.go | 51 +- policy/predicate/workflow_test.go | 493 +++++++++++++++- policy/predicate/workflowresult.go | 4 +- policy/predicate/workflowresult_test.go | 138 ++++- 8 files changed, 1466 insertions(+), 299 deletions(-) diff --git a/policy/predicate/status.go b/policy/predicate/status.go index 3344bf2eb..13e01047b 100644 --- a/policy/predicate/status.go +++ b/policy/predicate/status.go @@ -16,22 +16,17 @@ package predicate import ( "context" - "slices" - "strings" "github.com/palantir/policy-bot/policy/common" "github.com/palantir/policy-bot/pull" ) -type AllowedConclusions []string -type AllowedStatuses []string - // HasStatus checks that the specified statuses have a completed status with configurable conclusions. // // Deprecated: use the more flexible `HasStatusCheck` instead. type HasStatus struct { - Conclusions AllowedConclusions `yaml:"conclusions,omitempty"` - Statuses []string `yaml:"statuses,omitempty"` + Conclusions []string `yaml:"conclusions,omitempty"` + Statuses []string `yaml:"statuses,omitempty"` } func NewHasStatus(statuses []string, conclusions []string) *HasStatus { @@ -75,43 +70,3 @@ func (pred HasSuccessfulStatus) Evaluate(ctx context.Context, prctx pull.Context 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 -} -func (c AllowedStatuses) 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 -} diff --git a/policy/predicate/status_check.go b/policy/predicate/status_check.go index a5b3138d9..d4debf6f7 100644 --- a/policy/predicate/status_check.go +++ b/policy/predicate/status_check.go @@ -17,6 +17,7 @@ package predicate import ( "context" "fmt" + "maps" "regexp" "slices" "strings" @@ -27,9 +28,9 @@ import ( ) type HasStatusCheck struct { - Conclusions AllowedConclusions `yaml:"conclusions"` - Statuses AllowedStatuses `yaml:"statuses"` - Checks []common.Regexp `yaml:"checks,omitempty"` + Conclusions []string `yaml:"conclusions"` + Statuses []string `yaml:"statuses"` + Checks []common.Regexp `yaml:"checks,omitempty"` noRegex bool } @@ -47,9 +48,9 @@ var _ Predicate = HasStatus{} func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) { allowedConclusions := pred.Conclusions if len(allowedConclusions) == 0 { - allowedConclusions = AllowedConclusions{"success"} + allowedConclusions = []string{"success"} } else if slices.Contains(allowedConclusions, "any") { - allowedConclusions = AllowedConclusions{"action_required", "cancelled", "failure", "neutral", "skipped", "stale", "success", "timed_out"} + allowedConclusions = []string{"action_required", "cancelled", "failure", "neutral", "skipped", "stale", "success", "timed_out"} } allowedStatuses := pred.Statuses @@ -57,9 +58,9 @@ func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*c // pending and failure are statuses that apply to both repo commit statuses and check runs. // all other statuses are only applicable to check runs. if len(allowedStatuses) == 0 { - allowedStatuses = AllowedStatuses{"completed", "success"} + allowedStatuses = []string{"completed", "success"} } else if slices.Contains(allowedStatuses, "any") { - allowedStatuses = AllowedStatuses{"completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting"} // "error", "success" + allowedStatuses = []string{"completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting", "error", "success"} } checkStatuses, err := prctx.LatestCheckStatuses() @@ -72,14 +73,9 @@ func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*c return nil, errors.Wrap(err, "failed to list commit statuses") } - predicateResult := common.PredicateResult{ - ValuePhrase: "status checks", - ConditionPhrase: fmt.Sprintf("exist and have conclusion %s", allowedConclusions.joinWithOr()), - } - - var missingResults []string - var failingStatuses []string - var allChecks []string + var missingResults = make(map[string]string) + var failingStatuses = make(map[string]string) + var allChecks = make(map[string]string) for _, check := range pred.Checks { matched := false check_to_use := check @@ -89,46 +85,60 @@ func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*c return nil, errors.Wrapf(err, "failed to create regexp for workflow %s", check.String()) } } - for checkResultName, checkResult := range checkStatuses { + for _, checkResultName := range slices.Sorted(maps.Keys(checkStatuses)) { if check_to_use.Matches(checkResultName) { matched = true - allChecks = append(allChecks, checkResultName) - isInvalidConclusion := !slices.Contains(allowedConclusions, *checkResult.Conclusion) - if (checkResult.Status == nil || !slices.Contains(allowedStatuses, *checkResult.Status)) || - (slices.Contains(allowedStatuses, "completed") && checkResult.Conclusion != nil && isInvalidConclusion) { - failingStatuses = append(failingStatuses, checkResultName) + allChecks[checkResultName] = checkResultName + checkResult := checkStatuses[checkResultName] + isValidStatus := slices.Contains(allowedStatuses, *checkResult.Status) + isValidConclusion := checkResult.Conclusion != nil && slices.Contains(allowedConclusions, *checkResult.Conclusion) + if (checkResult.Status == nil || !isValidStatus) || + (*checkResult.Status == "completed" && !isValidConclusion) { + failingStatuses[checkResultName] = checkResultName } } } - for repoStatusName, repoStatusResult := range repoStatuses { + for _, repoStatusName := range slices.Sorted(maps.Keys(repoStatuses)) { if check_to_use.Matches(repoStatusName) { matched = true - allChecks = append(allChecks, repoStatusName) + allChecks[repoStatusName] = repoStatusName + repoStatusResult := repoStatuses[repoStatusName] if repoStatusResult == nil || repoStatusResult.State == nil || !slices.Contains(allowedStatuses, *repoStatusResult.State) { - failingStatuses = append(failingStatuses, repoStatusName) + failingStatuses[repoStatusName] = repoStatusName } } } if !matched { - missingResults = append(missingResults, check.String()) + missingResults[check.String()] = check.String() } } + allChecksList := slices.Sorted(maps.Keys(allChecks)) + predicateResult := common.PredicateResult{ + ValuePhrase: "status checks and repo statuses", + ConditionPhrase: fmt.Sprintf( + "exist and have statuses %s and have conclusion(in case status is completed) %s: %s", + joinElementsWithOr(allowedStatuses), + joinElementsWithOr(allowedConclusions), + allChecksList, + ), + } + if len(missingResults) > 0 { - predicateResult.Values = missingResults - predicateResult.Description = "One or more statuses is missing: " + strings.Join(missingResults, ", ") + predicateResult.Values = slices.Sorted(maps.Keys(missingResults)) + predicateResult.Description = fmt.Sprintf("One or more status checks or repo statuses are missing: %s", predicateResult.Values) predicateResult.Satisfied = false return &predicateResult, nil } if len(failingStatuses) > 0 { - predicateResult.Values = failingStatuses - predicateResult.Description = fmt.Sprintf("One or more statuses has not concluded with %s: %s", allowedConclusions.joinWithOr(), strings.Join(failingStatuses, ",")) + predicateResult.Values = slices.Sorted(maps.Keys(failingStatuses)) + predicateResult.Description = fmt.Sprintf("One or more status checks or repo statuses have not concluded with %s: %s", joinElementsWithOr(allowedConclusions), failingStatuses) predicateResult.Satisfied = false return &predicateResult, nil } - predicateResult.Values = allChecks + predicateResult.Values = allChecksList predicateResult.Satisfied = true return &predicateResult, nil } @@ -136,3 +146,26 @@ func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*c func (pred HasStatusCheck) 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/statuses are "success" and "failure", this will return "success or +// failure". If there are more than two conclusions/statuses, the first n-1 will be +// separated by commas. +func joinElementsWithOr(c []string) 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 +} diff --git a/policy/predicate/status_check_test.go b/policy/predicate/status_check_test.go index 9de7c1971..5c6ba5d55 100644 --- a/policy/predicate/status_check_test.go +++ b/policy/predicate/status_check_test.go @@ -16,9 +16,7 @@ package predicate import ( "context" - "fmt" "slices" - "strings" "testing" "time" @@ -105,172 +103,669 @@ func mockRepoStatus(state string) *github.RepoStatus { } } -func TestHasSuccessfulStatusCheck(t *testing.T) { - hasStatusCheck := HasStatusCheck{Checks: []common.Regexp{ - common.NewMustCompileRegexp("status-name"), - common.NewMustCompileRegexp("status-name-2"), - }} - hasStatusCheckSkippedOk := HasStatusCheck{ - Checks: []common.Regexp{ - common.NewMustCompileRegexp("status-name"), - common.NewMustCompileRegexp("status-name-2"), - }, - Conclusions: AllowedConclusions{"success", "skipped"}, +type StatusCheckTestCase struct { + name string + LatestCheckStatusesValue map[string]*github.CheckRun + LatestRepoStatusesValue map[string]*github.RepoStatus + predicate Predicate + ExpectedPredicateResult *common.PredicateResult +} + +func runStatusCheckTestCase(t *testing.T, cases []StatusCheckTestCase) { + ctx := context.Background() + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + predicateResult, err := tc.predicate.Evaluate(ctx, &pulltest.Context{ + LatestCheckStatusesValue: tc.LatestCheckStatusesValue, + LatestRepoStatusesValue: tc.LatestRepoStatusesValue, + }) + if assert.NoError(t, err, "evaluation failed") { + assertPredicateResult(t, tc.ExpectedPredicateResult, predicateResult) + } + }) } +} - commonTestCases := []StatusTestCase{ +func TestHasStatusCheck(t *testing.T) { + defaultChecksTestCases := []StatusCheckTestCase{ { - "all statuses succeed", - &pulltest.Context{ - LatestCheckStatusesValue: map[string]*github.CheckRun{ - "status-name": mockCheckRun("completed", "success"), - "status-name-2": mockCheckRun("completed", "success"), - }, + name: "all status check succeed", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "success"), }, - &common.PredicateResult{ + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ Satisfied: true, + Values: []string{"status-name", "status-name-2"}, }, }, { - "a status fails", - &pulltest.Context{ - LatestCheckStatusesValue: map[string]*github.CheckRun{ - "status-name": mockCheckRun("completed", "success"), - "status-name-2": mockCheckRun("completed", "failure"), - }, + name: "a status check fails", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "failure"), }, - &common.PredicateResult{ + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, Values: []string{"status-name-2"}, }, }, { - "multiple statuses fail", - &pulltest.Context{ - LatestCheckStatusesValue: map[string]*github.CheckRun{ - "status-name": mockCheckRun("completed", "failure"), - "status-name-2": mockCheckRun("completed", "failure"), - }, + name: "multiple status checks fail", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "failure"), + "status-name-2": mockCheckRun("completed", "failure"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, }, - &common.PredicateResult{ + ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, Values: []string{"status-name", "status-name-2"}, }, }, { - "a status does not exist", - &pulltest.Context{ - LatestCheckStatusesValue: map[string]*github.CheckRun{ - "status-name": mockCheckRun("completed", "success"), - }, + name: "a status check does not exist", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp("test.*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test.*"}, + }, + }, + { + name: "a status check does not exist, the other status check is skipped", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "test": mockCheckRun("completed", "skipped"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, }, - &common.PredicateResult{ + ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, - Values: []string{"status-name-2"}, + Values: []string{"test"}, }, }, { - "a status does not exist, the other status is skipped", - &pulltest.Context{ - LatestCheckStatusesValue: map[string]*github.CheckRun{ - "status-name-2": mockCheckRun("completed", "skipped"), - }, + name: "multiple status checks do not exist", + LatestCheckStatusesValue: map[string]*github.CheckRun{}, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, }, - &common.PredicateResult{ + ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, - Values: []string{"status-name"}, + Values: []string{".*"}, }, }, { - "multiple statuses do not exist", - &pulltest.Context{}, - &common.PredicateResult{ + name: "a status check has non completed status", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "test": mockCheckRun("completed", "success"), + "test1": mockCheckRun("expected", ""), + "test2": mockCheckRun("failure", ""), + "test3": mockCheckRun("in_progress", ""), + "test4": mockCheckRun("pending", ""), + "test5": mockCheckRun("queued", ""), + "test6": mockCheckRun("requested", ""), + "test7": mockCheckRun("startup_failure", ""), + "test8": mockCheckRun("waiting", ""), + "test9": mockCheckRun("invalid_state", ""), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, - Values: []string{"status-name", "status-name-2"}, + Values: []string{"test1", "test2", "test3", "test4", "test5", "test6", "test7", "test8", "test9"}, + }, + }, + { + name: "status checks is have status completed but non successfull", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "test": mockCheckRun("completed", "success"), + "test1": mockCheckRun("completed", "action_required"), + "test2": mockCheckRun("completed", "cancelled"), + "test3": mockCheckRun("completed", "failure"), + "test4": mockCheckRun("completed", "neutral"), + "test5": mockCheckRun("completed", "skipped"), + "test6": mockCheckRun("completed", "stale"), + "test7": mockCheckRun("completed", "timed_out"), + "test8": mockCheckRun("completed", "invalid_conclusion"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test1", "test2", "test3", "test4", "test5", "test6", "test7", "test8"}, }, }, } - okOnlyIfSkippedAllowed := []StatusTestCase{ + allConclusionChecksValue := map[string]*github.CheckRun{ + "test": mockCheckRun("completed", "success"), + "test1": mockCheckRun("completed", "action_required"), + "test2": mockCheckRun("completed", "cancelled"), + "test3": mockCheckRun("completed", "failure"), + "test4": mockCheckRun("completed", "neutral"), + "test5": mockCheckRun("completed", "skipped"), + "test6": mockCheckRun("completed", "stale"), + "test7": mockCheckRun("completed", "timed_out"), + "test8": mockCheckRun("completed", "invalid_conclusion"), + } + customConclusionChecksTestCases := []StatusCheckTestCase{ { - "a status is skipped", - &pulltest.Context{ - LatestCheckStatusesValue: map[string]*github.CheckRun{ - "status-name": mockCheckRun("completed", "success"), - "status-name-2": mockCheckRun("completed", "skipped"), - }, + name: "status checks is have status completed but conclusion action_required is allowed", + LatestCheckStatusesValue: allConclusionChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Conclusions: []string{"action_required"}, }, - &common.PredicateResult{ + ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, - Values: []string{"status-name-2"}, + Values: []string{"test", "test2", "test3", "test4", "test5", "test6", "test7", "test8"}, }, }, { - "all statuses are skipped", - &pulltest.Context{ - LatestCheckStatusesValue: map[string]*github.CheckRun{ - "status-name": mockCheckRun("completed", "skipped"), - "status-name-2": mockCheckRun("completed", "skipped"), - }, + name: "status checks is have status completed but conclusion cancelled is allowed", + LatestCheckStatusesValue: allConclusionChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Conclusions: []string{"cancelled"}, }, - &common.PredicateResult{ + ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, - Values: []string{"status-name", "status-name-2"}, + Values: []string{"test", "test1", "test3", "test4", "test5", "test6", "test7", "test8"}, + }, + }, + { + name: "status checks is have status completed but conclusion failure is allowed", + LatestCheckStatusesValue: allConclusionChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Conclusions: []string{"failure"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test2", "test4", "test5", "test6", "test7", "test8"}, + }, + }, + { + name: "status checks is have status completed but conclusion neutral is allowed", + LatestCheckStatusesValue: allConclusionChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Conclusions: []string{"neutral"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test2", "test3", "test5", "test6", "test7", "test8"}, + }, + }, + { + name: "status checks is have status completed but conclusion skipped is allowed", + LatestCheckStatusesValue: allConclusionChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Conclusions: []string{"skipped"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test2", "test3", "test4", "test6", "test7", "test8"}, + }, + }, + { + name: "status checks is have status completed but conclusion stale is allowed", + LatestCheckStatusesValue: allConclusionChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Conclusions: []string{"stale"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test2", "test3", "test4", "test5", "test7", "test8"}, + }, + }, + { + name: "status checks is have status completed but conclusion timed_out is allowed", + LatestCheckStatusesValue: allConclusionChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Conclusions: []string{"timed_out"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test2", "test3", "test4", "test5", "test6", "test8"}, }, }, } - testSuites := []StatusTestSuite{ - {predicate: hasStatusCheck, testCases: commonTestCases}, - {predicate: hasStatusCheck, testCases: okOnlyIfSkippedAllowed}, + allStatusesChecksValue := map[string]*github.CheckRun{ + "test": mockCheckRun("completed", "success"), + "test1": mockCheckRun("expected", ""), + "test2": mockCheckRun("failure", ""), + "test3": mockCheckRun("in_progress", ""), + "test4": mockCheckRun("pending", ""), + "test5": mockCheckRun("queued", ""), + "test6": mockCheckRun("requested", ""), + "test7": mockCheckRun("startup_failure", ""), + "test8": mockCheckRun("waiting", ""), + } + customStatusCheckTestCases := []StatusCheckTestCase{ + { + name: "status checks exist with all possible states but status completed is allowed", + LatestCheckStatusesValue: allStatusesChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"completed"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test1", "test2", "test3", "test4", "test5", "test6", "test7", "test8"}, + }, + }, + { + name: "status checks exist with all possible states but status expected is allowed", + LatestCheckStatusesValue: allStatusesChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"expected"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test2", "test3", "test4", "test5", "test6", "test7", "test8"}, + }, + }, + { + name: "status checks exist with all possible states but status failure is allowed", + LatestCheckStatusesValue: allStatusesChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"failure"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test3", "test4", "test5", "test6", "test7", "test8"}, + }, + }, + { + name: "status checks exist with all possible states but status in_progress is allowed", + LatestCheckStatusesValue: allStatusesChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"in_progress"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test2", "test4", "test5", "test6", "test7", "test8"}, + }, + }, + { + name: "status checks exist with all possible states but status pending is allowed", + LatestCheckStatusesValue: allStatusesChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"pending"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test2", "test3", "test5", "test6", "test7", "test8"}, + }, + }, + { + name: "status checks exist with all possible states but status queued is allowed", + LatestCheckStatusesValue: allStatusesChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"queued"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test2", "test3", "test4", "test6", "test7", "test8"}, + }, + }, + { + name: "status checks exist with all possible states but requested is allowed", + LatestCheckStatusesValue: allStatusesChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"requested"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test2", "test3", "test4", "test5", "test7", "test8"}, + }, + }, { - nameSuffix: "skipped allowed", - predicate: hasStatusCheckSkippedOk, - testCases: commonTestCases, + name: "status checks exist with all possible states but status startup_failure is allowed", + LatestCheckStatusesValue: allStatusesChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"startup_failure"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test2", "test3", "test4", "test5", "test6", "test8"}, + }, }, { - nameSuffix: "skipped allowed", - predicate: hasStatusCheckSkippedOk, - testCases: okOnlyIfSkippedAllowed, - overrideSatisfied: github.Bool(true), + name: "status checks exist with all possible states but status waiting is allowed", + LatestCheckStatusesValue: allStatusesChecksValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"waiting"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test", "test1", "test2", "test3", "test4", "test5", "test6", "test7"}, + }, }, } - for _, suite := range testSuites { - runStatusTestCase(t, suite.predicate, suite) + allStatusesRepoStatusValue := map[string]*github.RepoStatus{ + "test1": mockRepoStatus("success"), + "test2": mockRepoStatus("error"), + "test3": mockRepoStatus("pending"), + "test4": mockRepoStatus("failure"), + } + defaultRepoStatusTestCases := []StatusCheckTestCase{ + { + name: "all repo statuse succeed", + LatestRepoStatusesValue: map[string]*github.RepoStatus{ + "status-name": mockRepoStatus("success"), + "status-name-2": mockRepoStatus("success"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{"status-name", "status-name-2"}, + }, + }, + { + name: "a repo statuse fails", + LatestRepoStatusesValue: map[string]*github.RepoStatus{ + "status-name": mockRepoStatus("success"), + "status-name-2": mockRepoStatus("failure"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + { + name: "multiple repo statuses fail", + LatestRepoStatusesValue: map[string]*github.RepoStatus{ + "status-name": mockRepoStatus("failure"), + "status-name-2": mockRepoStatus("failure"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name", "status-name-2"}, + }, + }, + { + name: "a repo status does not exist", + LatestRepoStatusesValue: map[string]*github.RepoStatus{ + "status-name": mockRepoStatus("success"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp("test.*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test.*"}, + }, + }, + { + name: "a repo status does not exist, the other repo status is pending", + LatestRepoStatusesValue: map[string]*github.RepoStatus{ + "test": mockRepoStatus("pending"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test"}, + }, + }, + { + name: "a repo status does not exist, the other repo status is error", + LatestRepoStatusesValue: map[string]*github.RepoStatus{ + "test": mockRepoStatus("error"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test"}, + }, + }, + { + name: "multiple repo statuses do not exist", + LatestRepoStatusesValue: map[string]*github.RepoStatus{}, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".*"}, + }, + }, + { + name: "a repo status has non success status", + LatestRepoStatusesValue: allStatusesRepoStatusValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test2", "test3", "test4"}, + }, + }, } -} -func runStatusCheckTestCase(t *testing.T, p Predicate, suite StatusTestSuite) { - ctx := context.Background() + customStatusRepoStatusTestCases := []StatusCheckTestCase{ + { + name: "repo statuses exist with all possible states but status success is allowed", + LatestRepoStatusesValue: allStatusesRepoStatusValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"success"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test2", "test3", "test4"}, + }, + }, + { + name: "repo statuses exist with all possible states but status error is allowed", + LatestRepoStatusesValue: allStatusesRepoStatusValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"error"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test1", "test3", "test4"}, + }, + }, + { + name: "repo statuses exist with all possible states but status pending is allowed", + LatestRepoStatusesValue: allStatusesRepoStatusValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"pending"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test1", "test2", "test4"}, + }, + }, + { + name: "repo statuses exist with all possible states but status failure is allowed", + LatestRepoStatusesValue: allStatusesRepoStatusValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"failure"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"test1", "test2", "test3"}, + }, + }, + } - for _, tc := range suite.testCases { - if suite.overrideSatisfied != nil { - tc.ExpectedPredicateResult.Satisfied = *suite.overrideSatisfied - } + regexTestCases := []StatusCheckTestCase{ + { + name: "a predicate check is threaded as regex", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "success"), + "abc": mockCheckRun("completed", "failure"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp("status-name.*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{"status-name", "status-name-2"}, + }, + }, + { + name: "a predicate check is threaded as literal string if noRegex is set negative test", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "success"), + "abc": mockCheckRun("completed", "failure"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp("status-name.*")}, + noRegex: true, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name.*"}, + }, + }, + { + name: "a predicate check is threaded as literal string if noRegex is set positive test", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "success"), + "abc": mockCheckRun("completed", "failure"), + }, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp("status-name")}, + noRegex: true, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{"status-name"}, + }, + }, + } - // If the test case expects the predicate to be satisfied, we always - // expect the values to contain all the statuses. Doing this here lets - // us use the same testcases when we allow and don't allow skipped - // statuses. - if tc.ExpectedPredicateResult.Satisfied { - statuses, _ := tc.context.LatestCheckStatuses() - tc.ExpectedPredicateResult.Values = keysSorted(statuses) - } + checkAndRepoStatusMixedTestCases := []StatusCheckTestCase{ + { + name: "A regex only matching statuses returns only matching statuses", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "success"), + "abc": mockCheckRun("completed", "failure"), + }, + LatestRepoStatusesValue: allStatusesRepoStatusValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp("status-name.*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{"status-name", "status-name-2"}, + }, + }, + { + name: "A regex matching everything returns compleded checks and successful repo statuses by default", + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("completed", "success"), + "abc": mockCheckRun("completed", "failure"), + }, + LatestRepoStatusesValue: allStatusesRepoStatusValue, + predicate: HasStatusCheck{ + Checks: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{"abc", "test2", "test3", "test4"}, + }, + }, + } - // `predicate.HasStatusCheck` -> `HasStatusCheck` - _, predicateType, _ := strings.Cut(fmt.Sprintf("%T", p), ".") - testName := fmt.Sprintf("%s_%s", predicateType, tc.name) + runStatusCheckTestCase(t, defaultChecksTestCases) + runStatusCheckTestCase(t, customConclusionChecksTestCases) + runStatusCheckTestCase(t, customStatusCheckTestCases) + runStatusCheckTestCase(t, defaultRepoStatusTestCases) + runStatusCheckTestCase(t, customStatusRepoStatusTestCases) + runStatusCheckTestCase(t, checkAndRepoStatusMixedTestCases) + runStatusCheckTestCase(t, regexTestCases) +} - if suite.nameSuffix != "" { - testName += "_" + suite.nameSuffix - } +func TestjoinElementsWithOr(t *testing.T) { + testCases := []struct { + name string + input []string + expected string + }{ + { + "empty", + []string{}, + "", + }, + { + "single", + []string{"a"}, + "a", + }, + { + "two", + []string{"a", "b"}, + "a or b", + }, + { + "three", + []string{"a", "b", "c"}, + "a, b, or c", + }, + { + "conclusions get sorted", + []string{"c", "a", "b"}, + "a, b, or c", + }, + } - t.Run(testName, func(t *testing.T) { - predicateResult, err := p.Evaluate(ctx, tc.context) - if assert.NoError(t, err, "evaluation failed") { - assertPredicateResult(t, tc.ExpectedPredicateResult, predicateResult) - } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, joinElementsWithOr(tc.input)) }) } } diff --git a/policy/predicate/status_test.go b/policy/predicate/status_test.go index 994bae6f7..fc22b934c 100644 --- a/policy/predicate/status_test.go +++ b/policy/predicate/status_test.go @@ -29,11 +29,13 @@ import ( func TestHasSuccessfulStatus(t *testing.T) { hasStatus := HasStatus{Statuses: []string{"status-name", "status-name-2"}} + hasRegexStatus := HasStatus{Statuses: []string{".*"}} hasStatusSkippedOk := HasStatus{ Statuses: []string{"status-name", "status-name-2"}, - Conclusions: AllowedConclusions{"success", "skipped"}, + Conclusions: []string{"success", "skipped"}, } hasSuccessfulStatus := HasSuccessfulStatus{"status-name", "status-name-2"} + hasSuccessfulRegexStatus := HasSuccessfulStatus{".*"} commonTestCases := []StatusTestCase{ { @@ -137,11 +139,153 @@ func TestHasSuccessfulStatus(t *testing.T) { }, } + nonCompleteStatusesTestCases := []StatusTestCase{ + { + "a workflow in state 'expected', it should be threaded as a failure", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("expected", ""), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + { + "a workflow in state 'failure', it should be threaded as a failure", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("failure", ""), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + { + "a workflow in state 'in_progress', it should be threaded as a failure", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("in_progress", ""), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + { + "a workflow in state 'queued', it should be threaded as a failure", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("queued", ""), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + { + "a workflow in state 'pending', it should be threaded as a failure", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("pending", ""), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + { + "a workflow in state 'waiting', it should be threaded as a failure", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("waiting", ""), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + { + "a workflow in state 'requested', it should be threaded as a failure", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("requested", ""), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + { + "a workflow in state 'startup_failure', it should be threaded as a failure", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("startup_failure", ""), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{"status-name-2"}, + }, + }, + } + + regexStatusTestCases := []StatusTestCase{ + { + "a predicate with regex syntax is treated as a literal string and not found", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("startup_failure", ""), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{".*"}, + }, + }, + } + + regexSuccessfullStatusTestCases := []StatusTestCase{ + { + "a predicate with regex syntax is treated as a literal string and not found", + &pulltest.Context{ + LatestCheckStatusesValue: map[string]*github.CheckRun{ + "status-name": mockCheckRun("completed", "success"), + "status-name-2": mockCheckRun("startup_failure", ""), + }, + }, + &common.PredicateResult{ + Satisfied: false, + Values: []string{".*"}, + }, + }, + } + testSuites := []StatusTestSuite{ {predicate: hasStatus, testCases: commonTestCases}, {predicate: hasStatus, testCases: okOnlyIfSkippedAllowed}, {predicate: hasSuccessfulStatus, testCases: commonTestCases}, {predicate: hasSuccessfulStatus, testCases: okOnlyIfSkippedAllowed}, + {predicate: hasStatus, testCases: nonCompleteStatusesTestCases}, + {predicate: hasRegexStatus, testCases: regexStatusTestCases}, + {predicate: hasSuccessfulRegexStatus, testCases: regexSuccessfullStatusTestCases}, { nameSuffix: "skipped allowed", predicate: hasStatusSkippedOk, @@ -206,83 +350,3 @@ func runStatusTestCase(t *testing.T, p Predicate, suite StatusTestSuite) { }) } } - -func TestAllowedConclusionsJoinWithOr(t *testing.T) { - testCases := []struct { - name string - input AllowedConclusions - expected string - }{ - { - "empty", - AllowedConclusions{}, - "", - }, - { - "single", - AllowedConclusions{"a"}, - "a", - }, - { - "two", - AllowedConclusions{"a", "b"}, - "a or b", - }, - { - "three", - AllowedConclusions{"a", "b", "c"}, - "a, b, or c", - }, - { - "conclusions get sorted", - AllowedConclusions{"c", "a", "b"}, - "a, b, or c", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.expected, tc.input.joinWithOr()) - }) - } -} - -func TestAllowedStatuseJoinWithOr(t *testing.T) { - testCases := []struct { - name string - input AllowedStatuses - expected string - }{ - { - "empty", - AllowedStatuses{}, - "", - }, - { - "single", - AllowedStatuses{"a"}, - "a", - }, - { - "two", - AllowedStatuses{"a", "b"}, - "a or b", - }, - { - "three", - AllowedStatuses{"a", "b", "c"}, - "a, b, or c", - }, - { - "conclusions get sorted", - AllowedStatuses{"c", "a", "b"}, - "a, b, or c", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.expected, tc.input.joinWithOr()) - }) - } -} diff --git a/policy/predicate/workflow.go b/policy/predicate/workflow.go index 06d594a59..28119611b 100644 --- a/policy/predicate/workflow.go +++ b/policy/predicate/workflow.go @@ -17,9 +17,9 @@ package predicate import ( "context" "fmt" + "maps" "regexp" "slices" - "strings" "github.com/palantir/policy-bot/policy/common" "github.com/palantir/policy-bot/pull" @@ -27,13 +27,13 @@ import ( ) type HasWorkflow struct { - Statuses AllowedStatuses `yaml:"statuses,omitempty"` - Conclusions AllowedConclusions `yaml:"conclusions,omitempty"` - Workflows []common.Regexp `yaml:"workflows,omitempty"` + Statuses []string `yaml:"statuses,omitempty"` + Conclusions []string `yaml:"conclusions,omitempty"` + Workflows []common.Regexp `yaml:"workflows,omitempty"` noRegex bool } -func NewHasWorkflow(workflows []common.Regexp, conclusions AllowedConclusions, statuses AllowedStatuses) *HasWorkflow { +func NewHasWorkflow(workflows []common.Regexp, conclusions []string, statuses []string) *HasWorkflow { return &HasWorkflow{ Statuses: statuses, Conclusions: conclusions, @@ -47,16 +47,16 @@ var _ Predicate = HasWorkflow{} func (pred HasWorkflow) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) { allowedConclusions := pred.Conclusions if len(allowedConclusions) == 0 { - allowedConclusions = AllowedConclusions{"success"} + allowedConclusions = []string{"success"} } else if slices.Contains(allowedConclusions, "any") { - allowedConclusions = AllowedConclusions{"action_required", "cancelled", "failure", "neutral", "skipped", "stale", "success", "timed_out"} + allowedConclusions = []string{"action_required", "cancelled", "failure", "neutral", "skipped", "stale", "success", "timed_out"} } allowedStatuses := pred.Statuses if len(allowedStatuses) == 0 { - allowedStatuses = AllowedStatuses{"completed"} + allowedStatuses = []string{"completed"} } else if slices.Contains(allowedStatuses, "any") { - allowedStatuses = AllowedStatuses{"completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting"} + allowedStatuses = []string{"completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting"} } workflowRuns, err := prctx.LatestWorkflowRuns() @@ -64,11 +64,6 @@ func (pred HasWorkflow) Evaluate(ctx context.Context, prctx pull.Context) (*comm return nil, errors.Wrap(err, "failed to list latest workflow runs") } - predicateResult := common.PredicateResult{ - ValuePhrase: "workflow results", - ConditionPhrase: fmt.Sprintf("exist and have statuses %s and in case status \"completed\" is required also one of conclusions %s", allowedStatuses.joinWithOr(), allowedConclusions.joinWithOr()), - } - var missingResults []string var failingWorkflows []string var allWorkflows []string @@ -81,13 +76,13 @@ func (pred HasWorkflow) Evaluate(ctx context.Context, prctx pull.Context) (*comm return nil, errors.Wrapf(err, "failed to create regexp for workflow %s", workflow.String()) } } - for name, workflowRun := range workflowRuns { + for _, name := range slices.Sorted(maps.Keys(workflowRuns)) { if workflow_to_use.Matches(name) { matched = true allWorkflows = append(allWorkflows, name) - for _, workflowResult := range workflowRun { + for _, workflowResult := range workflowRuns[name] { isStatusAllowed := workflowResult.Status != nil && slices.Contains(allowedStatuses, *workflowResult.Status) - isStatusCompletedAllowed := workflowResult.Status != nil && slices.Contains(allowedStatuses, "completed") + isStatusCompletedAllowed := workflowResult.Status != nil && *workflowResult.Status == "completed" && slices.Contains(allowedStatuses, "completed") isConclusionAllowed := workflowResult.Conclusion != nil && slices.Contains(allowedConclusions, *workflowResult.Conclusion) if !isStatusAllowed || (isStatusCompletedAllowed && !isConclusionAllowed) { failingWorkflows = append(failingWorkflows, name) @@ -100,21 +95,39 @@ func (pred HasWorkflow) Evaluate(ctx context.Context, prctx pull.Context) (*comm } } + predicateResult := common.PredicateResult{ + ValuePhrase: "workflow results", + ConditionPhrase: fmt.Sprintf( + "exist and have statuses %s and have conclusion(in case status is completed) %s: %s", + joinElementsWithOr(allowedStatuses), + joinElementsWithOr(allowedConclusions), + allWorkflows, + ), + } + if len(missingResults) > 0 { predicateResult.Values = missingResults - predicateResult.Description = "One or more workflow runs are missing: " + strings.Join(missingResults, ", ") + slices.Sort(missingResults) + predicateResult.Description = fmt.Sprintf("One or more workflow runs are missing: %s", predicateResult.Values) predicateResult.Satisfied = false return &predicateResult, nil } if len(failingWorkflows) > 0 { predicateResult.Values = failingWorkflows - predicateResult.Description = fmt.Sprintf("One or more workflow runs have not concluded with %s (yet): %s", allowedConclusions.joinWithOr(), strings.Join(failingWorkflows, ",")) + slices.Sort(failingWorkflows) + predicateResult.Description = fmt.Sprintf( + "One or more workflow runs have currently not status %s and/or conclusion(in case status is completed) %s: %s", + joinElementsWithOr(allowedStatuses), + joinElementsWithOr(allowedConclusions), + failingWorkflows, + ) predicateResult.Satisfied = false return &predicateResult, nil } predicateResult.Values = allWorkflows + slices.Sort(allWorkflows) predicateResult.Satisfied = true return &predicateResult, nil diff --git a/policy/predicate/workflow_test.go b/policy/predicate/workflow_test.go index 767682bfb..7f19a4b07 100644 --- a/policy/predicate/workflow_test.go +++ b/policy/predicate/workflow_test.go @@ -149,7 +149,7 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, predicate: HasWorkflow{ Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, - Conclusions: AllowedConclusions{"skipped"}, + Conclusions: []string{"skipped"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: true, @@ -164,7 +164,7 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, predicate: HasWorkflow{ Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml"), common.NewMustCompileRegexp(".github/workflows/test2.yml")}, - Conclusions: AllowedConclusions{"skipped", "success"}, + Conclusions: []string{"skipped", "success"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: true, @@ -178,7 +178,7 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, predicate: HasWorkflow{ Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, - Conclusions: AllowedConclusions{"skipped", "success"}, + Conclusions: []string{"skipped", "success"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: true, @@ -193,7 +193,7 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, predicate: HasWorkflow{ Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml"), common.NewMustCompileRegexp(".github/workflows/test2.yml")}, - Conclusions: AllowedConclusions{"skipped", "success"}, + Conclusions: []string{"skipped", "success"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, @@ -208,7 +208,7 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, predicate: HasWorkflow{ Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml"), common.NewMustCompileRegexp(".github/workflows/test2.yml")}, - Conclusions: AllowedConclusions{"skipped"}, + Conclusions: []string{"skipped"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, @@ -222,13 +222,494 @@ func TestHasSuccessfulWorkflowRun(t *testing.T) { }, predicate: HasWorkflow{ Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, - Conclusions: AllowedConclusions{"skipped"}, + Conclusions: []string{"skipped"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, Values: []string{".github/workflows/test.yml"}, }, }, + { + name: "a workflow in state 'expected', it should be ignored by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("expected", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'failure', it should be ignored by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("failure", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'in_progress', it should be ignored by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("in_progress", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'queued', it should be ignored by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("queued", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'pending', it should be ignored by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("pending", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'waiting', it should be ignored by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("waiting", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'requested', it should be ignored by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("requested", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'startup_failure', it should be ignored by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("startup_failure", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".github/workflows/test.yml")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'expected' and this state is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test2.yml": {mockWorkflowRun("expected", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"expected"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test2.yml"}, + }, + }, + { + name: "a workflow in state 'failure' and this state is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test2.yml": {mockWorkflowRun("failure", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"failure"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test2.yml"}, + }, + }, + { + name: "a workflow in state 'in_progress' and this state is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test2.yml": {mockWorkflowRun("in_progress", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"in_progress"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test2.yml"}, + }, + }, + { + name: "a workflow in state 'queued' and this state is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test2.yml": {mockWorkflowRun("queued", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"queued"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test2.yml"}, + }, + }, + { + name: "a workflow in state 'pending' and this state is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test2.yml": {mockWorkflowRun("pending", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"pending"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test2.yml"}, + }, + }, + { + name: "a workflow in state 'waiting' and this state is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test2.yml": {mockWorkflowRun("waiting", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"waiting"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test2.yml"}, + }, + }, + { + name: "a workflow in state 'requested' and this state is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test2.yml": {mockWorkflowRun("requested", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"requested"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test2.yml"}, + }, + }, + { + name: "a workflow in state 'startup_failure' and this state is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test2.yml": {mockWorkflowRun("startup_failure", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"startup_failure"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test2.yml"}, + }, + }, + { + name: "workflows in state 'expected' and 'completed' but only 'expected' is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test1.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("expected", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"expected"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test1.yml"}, + }, + }, + { + name: "workflows in state 'failure' and 'completed' but only 'expected' is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test1.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("failure", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"failure"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test1.yml"}, + }, + }, + { + name: "workflows in state 'in_progress' and 'completed' but only 'expected' is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test1.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("in_progress", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"in_progress"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test1.yml"}, + }, + }, + { + name: "workflows in state 'queued' and 'completed' but only 'expected' is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test1.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("queued", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"queued"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test1.yml"}, + }, + }, + { + name: "workflows in state 'pending' and 'completed' but only 'expected' is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test1.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("pending", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"pending"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test1.yml"}, + }, + }, + { + name: "workflows in state 'waiting' and 'completed' but only 'expected' is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test1.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("waiting", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"waiting"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test1.yml"}, + }, + }, + { + name: "workflows in state 'requested' and 'completed' but only 'expected' is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test1.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("requested", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"requested"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test1.yml"}, + }, + }, + { + name: "workflows in state 'startup_failure' and 'completed' but only 'expected' is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test1.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("startup_failure", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"startup_failure"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test1.yml"}, + }, + }, + { + name: "a non existing Status is required, it should not match", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"complete"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a non existing conclusion is required, it should not match", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Conclusions: []string{"succes"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a only state completed is allowed, it should only match the allowed state", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("requested", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test2.yml"}, + }, + }, + { + name: "a any state is allowed, it should match all the states and conclusion 'success' by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("expected", "")}, + ".github/workflows/test3.yml": {mockWorkflowRun("failure", "")}, + ".github/workflows/test4.yml": {mockWorkflowRun("in_progress", "")}, + ".github/workflows/test5.yml": {mockWorkflowRun("queued", "")}, + ".github/workflows/test6.yml": {mockWorkflowRun("pending", "")}, + ".github/workflows/test7.yml": {mockWorkflowRun("waiting", "")}, + ".github/workflows/test8.yml": {mockWorkflowRun("requested", "")}, + ".github/workflows/test9.yml": {mockWorkflowRun("startup_failure", "")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"any"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{ + ".github/workflows/test.yml", + ".github/workflows/test2.yml", + ".github/workflows/test3.yml", + ".github/workflows/test4.yml", + ".github/workflows/test5.yml", + ".github/workflows/test6.yml", + ".github/workflows/test7.yml", + ".github/workflows/test8.yml", + ".github/workflows/test9.yml", + }, + }, + }, + { + name: "a any state is allowed, it should match only successful conclusions by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "skipped")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"any"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".github/workflows/test2.yml"}, + }, + }, + { + name: "a any state is allowed and all workflows succeded, it should statisfy by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("completed", "success")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*")}, + Statuses: []string{"any"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, + }, + }, + { + name: "a predicate with regex syntax is treated as regex by default", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/abctestabc.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test3.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/abc.yml": {mockWorkflowRun("in_progress", "success")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*test.*")}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/abctestabc.yml", ".github/workflows/test.yml", ".github/workflows/test3.yml"}, + }, + }, + { + name: "a predicate with regex syntax is treated as literal when noRegex is set", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/abctestabc.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test3.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/abc.yml": {mockWorkflowRun("in_progress", "success")}, + }, + predicate: HasWorkflow{ + Workflows: []common.Regexp{common.NewMustCompileRegexp(".*test.*")}, + noRegex: true, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".*test.*"}, + }, + }, } runWorkflowTestCase(t, commonTestCases) diff --git a/policy/predicate/workflowresult.go b/policy/predicate/workflowresult.go index 40bdf1b2c..e9ae55faf 100644 --- a/policy/predicate/workflowresult.go +++ b/policy/predicate/workflowresult.go @@ -25,8 +25,8 @@ import ( // // Deprecated: use the more flexible `HasWorkflow` instead. type HasWorkflowResult struct { - Conclusions AllowedConclusions `yaml:"conclusions,omitempty"` - Workflows []string `yaml:"workflows,omitempty"` + Conclusions []string `yaml:"conclusions,omitempty"` + Workflows []string `yaml:"workflows,omitempty"` } func NewHasWorkflowResult(workflows []string, conclusions []string) *HasWorkflowResult { diff --git a/policy/predicate/workflowresult_test.go b/policy/predicate/workflowresult_test.go index 1c01710ee..ece2be19e 100644 --- a/policy/predicate/workflowresult_test.go +++ b/policy/predicate/workflowresult_test.go @@ -231,7 +231,7 @@ func TestHasSuccessfulWorkflowResultRun(t *testing.T) { }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml"}, - Conclusions: AllowedConclusions{"skipped"}, + Conclusions: []string{"skipped"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: true, @@ -246,7 +246,7 @@ func TestHasSuccessfulWorkflowResultRun(t *testing.T) { }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, - Conclusions: AllowedConclusions{"skipped", "success"}, + Conclusions: []string{"skipped", "success"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: true, @@ -260,7 +260,7 @@ func TestHasSuccessfulWorkflowResultRun(t *testing.T) { }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml"}, - Conclusions: AllowedConclusions{"skipped", "success"}, + Conclusions: []string{"skipped", "success"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: true, @@ -275,7 +275,7 @@ func TestHasSuccessfulWorkflowResultRun(t *testing.T) { }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, - Conclusions: AllowedConclusions{"skipped", "success"}, + Conclusions: []string{"skipped", "success"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, @@ -290,7 +290,7 @@ func TestHasSuccessfulWorkflowResultRun(t *testing.T) { }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml", ".github/workflows/test2.yml"}, - Conclusions: AllowedConclusions{"skipped"}, + Conclusions: []string{"skipped"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, @@ -304,13 +304,139 @@ func TestHasSuccessfulWorkflowResultRun(t *testing.T) { }, predicate: HasWorkflowResult{ Workflows: []string{".github/workflows/test.yml"}, - Conclusions: AllowedConclusions{"skipped"}, + Conclusions: []string{"skipped"}, }, ExpectedPredicateResult: &common.PredicateResult{ Satisfied: false, Values: []string{".github/workflows/test.yml"}, }, }, + { + name: "a workflow in state 'expected', it should be ignored", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("expected", "")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'failure', it should be ignored", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("failure", "")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'in_progress', it should be ignored", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("in_progress", "")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'queued', it should be ignored", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("queued", "")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'pending', it should be ignored", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("pending", "")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'waiting', it should be ignored", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("waiting", "")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'requested', it should be ignored", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("requested", "")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a workflow in state 'startup_failure', it should be ignored", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("startup_failure", "")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".github/workflows/test.yml"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: true, + Values: []string{".github/workflows/test.yml"}, + }, + }, + { + name: "a predicate with regex syntax is treated as a literal string", + latestWorkflowRunsValue: map[string][]*github.WorkflowRun{ + ".github/workflows/test.yml": {mockWorkflowRun("completed", "success")}, + ".github/workflows/test2.yml": {mockWorkflowRun("startup_failure", "")}, + }, + predicate: HasWorkflowResult{ + Workflows: []string{".*test.*"}, + }, + ExpectedPredicateResult: &common.PredicateResult{ + Satisfied: false, + Values: []string{".*test.*"}, + }, + }, } runWorkflowResultTestCase(t, commonTestCases) From ce4c3d5a3770de0575ab058ce6398f4905d35a83 Mon Sep 17 00:00:00 2001 From: Florian Braun <5863788+FloThinksPi@users.noreply.github.com> Date: Thu, 13 Mar 2025 08:53:21 +0100 Subject: [PATCH 4/5] Add documentation for the new predicates --- README.md | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b96999f2a..40dc0cbdc 100644 --- a/README.md +++ b/README.md @@ -372,7 +372,7 @@ if: deletions: "> 100" total: "> 200" - # DEPRECATED: Use "has_status" below instead, which is more flexible. + # DEPRECATED: Use "has_status_check" 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: @@ -380,6 +380,7 @@ if: - "status-name-2" - "status-name-3" + # DEPRECATED: Use "has_status_check" below instead, which is more flexible. # "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"]. @@ -390,6 +391,29 @@ if: - "status-name-2" - "status-name-3" + # "has_status_check" is satisfied if the status checks that are specified are + # in one of the statuses specified. If statuses includes "completed" and the status check + # is in that state it must also concluded with one of the conclusions specified additionally to statify has_status_check. + # > "checks" are required and are evaluated as regular expressions. + # > "statuses" is optional and defaults to ["completed", "success"]. + # Possible values are: ["completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting", "error", "success"]. + # If the value "any" is included in "statuses", the statuses will be expanded to all possible values listed above. + # While ["completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting] + # represent possible statuses for a 'check', + # the statuses ["error", "failing", "pending", "success"] represent possible statuses for 'commit statuses' + # see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks#types-of-status-checks-on-github + # > "conclusions" is optional and defaults to ["success"]. + # Possible values are ["action_required", "cancelled", "failure", "neutral", "skipped", "stale", "success", "timed_out"]. + # If the value "any" is included in "conclusions", the statuses will be expanded to all possible values listed above. + # Conclustions are only evaluated if the status is in the "completed" state so this must be included in the "statuses" list. + # Also it has no effect on 'commit statuses' since they do not have conclusions. + has_status_check: + statuses: ["any"] + conclusions: ["any"] + checks: + - status-name-[0-9] + + # DEPRECATED: Use "has_workflow" below instead, which is more flexible. # "has_workflow_result" is satisfied if the GitHub Actions workflow runs that # are specified all finished and concluded with one of the conclusions # specified. "conclusions" is optional and defaults to ["success"]. @@ -403,6 +427,24 @@ if: - ".github/workflows/a.yml" - ".github/workflows/b.yml" + # "has_workflow" is satisfied if the workflows that are specified are + # in one of the statuses specified. If statuses includes "completed" and the workflow + # is in that state it must also concluded with one of the conclusions specified additionally to statify has_workflow. + # "workflows" are required and are evaluated as regular expressions. + # "statuses" is optional and defaults to ["completed"]. + # Possible values are: ["completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting"]. + # If the value "any" is included in "statuses", the statuses will be expanded to all possible values listed above. + # "conclusions" is optional and defaults to ["success"]. + # Possible values are ["action_required", "cancelled", "failure", "neutral", "skipped", "stale", "success", "timed_out"]. + # If the value "any" is included in "conclusions", the statuses will be expanded to all possible values listed above. + # Conclustions are only evaluated if the status is in the "completed" state so this must be included in the "statuses" list. + has_workflow: + statuses: ["any"] + conclusions: ["any"] + workflows: + - '\.github/workflows/test-dependencies\.yml' + - 'check.*\.yml' + # "has_labels" is satisfied if the pull request has the specified labels # applied has_labels: From 27dc1fa024d7d575076a6e4a303a188d83af17d0 Mon Sep 17 00:00:00 2001 From: Florian Braun <5863788+FloThinksPi@users.noreply.github.com> Date: Wed, 9 Apr 2025 10:07:22 +0200 Subject: [PATCH 5/5] Adoptions after Rebase Updated everything to github v70 Removed unused test function --- policy/approval/approve_test.go | 2 +- policy/predicate/status_check_test.go | 42 +------------------------ policy/predicate/workflow_test.go | 2 +- policy/predicate/workflowresult_test.go | 2 +- pull/context.go | 2 +- pull/pulltest/context.go | 2 +- 6 files changed, 6 insertions(+), 46 deletions(-) diff --git a/policy/approval/approve_test.go b/policy/approval/approve_test.go index 1fa4ae368..9572301cb 100644 --- a/policy/approval/approve_test.go +++ b/policy/approval/approve_test.go @@ -22,7 +22,7 @@ import ( "testing" "time" - "github.com/google/go-github/v69/github" + "github.com/google/go-github/v70/github" "github.com/palantir/policy-bot/policy/common" "github.com/palantir/policy-bot/policy/predicate" "github.com/palantir/policy-bot/pull" diff --git a/policy/predicate/status_check_test.go b/policy/predicate/status_check_test.go index 5c6ba5d55..5e76a7acb 100644 --- a/policy/predicate/status_check_test.go +++ b/policy/predicate/status_check_test.go @@ -20,7 +20,7 @@ import ( "testing" "time" - "github.com/google/go-github/v69/github" + "github.com/google/go-github/v70/github" "github.com/palantir/policy-bot/policy/common" "github.com/palantir/policy-bot/pull/pulltest" "github.com/stretchr/testify/assert" @@ -729,43 +729,3 @@ func TestHasStatusCheck(t *testing.T) { runStatusCheckTestCase(t, checkAndRepoStatusMixedTestCases) runStatusCheckTestCase(t, regexTestCases) } - -func TestjoinElementsWithOr(t *testing.T) { - testCases := []struct { - name string - input []string - expected string - }{ - { - "empty", - []string{}, - "", - }, - { - "single", - []string{"a"}, - "a", - }, - { - "two", - []string{"a", "b"}, - "a or b", - }, - { - "three", - []string{"a", "b", "c"}, - "a, b, or c", - }, - { - "conclusions get sorted", - []string{"c", "a", "b"}, - "a, b, or c", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.expected, joinElementsWithOr(tc.input)) - }) - } -} diff --git a/policy/predicate/workflow_test.go b/policy/predicate/workflow_test.go index 7f19a4b07..6de52243c 100644 --- a/policy/predicate/workflow_test.go +++ b/policy/predicate/workflow_test.go @@ -18,7 +18,7 @@ import ( "context" "testing" - "github.com/google/go-github/v69/github" + "github.com/google/go-github/v70/github" "github.com/palantir/policy-bot/policy/common" "github.com/palantir/policy-bot/pull/pulltest" "github.com/stretchr/testify/assert" diff --git a/policy/predicate/workflowresult_test.go b/policy/predicate/workflowresult_test.go index ece2be19e..d11e1a268 100644 --- a/policy/predicate/workflowresult_test.go +++ b/policy/predicate/workflowresult_test.go @@ -19,7 +19,7 @@ import ( "testing" "time" - "github.com/google/go-github/v69/github" + "github.com/google/go-github/v70/github" "github.com/palantir/policy-bot/policy/common" "github.com/palantir/policy-bot/pull/pulltest" "github.com/stretchr/testify/assert" diff --git a/pull/context.go b/pull/context.go index 8df7af3b7..4a4bca15f 100644 --- a/pull/context.go +++ b/pull/context.go @@ -17,7 +17,7 @@ package pull import ( "time" - "github.com/google/go-github/v69/github" + "github.com/google/go-github/v70/github" ) // MembershipContext defines methods to get information diff --git a/pull/pulltest/context.go b/pull/pulltest/context.go index cf8104cb0..4ba7d60aa 100644 --- a/pull/pulltest/context.go +++ b/pull/pulltest/context.go @@ -17,7 +17,7 @@ package pulltest import ( "time" - "github.com/google/go-github/v69/github" + "github.com/google/go-github/v70/github" "github.com/palantir/policy-bot/pull" )