From b6d440b9cf3dc27a7e897a44aa8779db0fecb81b Mon Sep 17 00:00:00 2001 From: JordanGoasdoue Date: Wed, 31 Jan 2024 17:09:58 +0100 Subject: [PATCH] feat: allow to skip draft prs unless /ok-to-test is added --- ...ub-com-jenkins-x-lighthouse-pkg-plugins.md | 1 + docs/install_lighthouse_with_tekton.md | 1 + docs/plugins/Plugins config.md | 1 + pkg/plugins/config.go | 2 + pkg/plugins/trigger/generic-comment.go | 4 +- pkg/plugins/trigger/pull-request.go | 94 ++-- pkg/plugins/trigger/pull-request_test.go | 442 +++++++++++++++++- 7 files changed, 506 insertions(+), 39 deletions(-) diff --git a/docs/config/plugins/github-com-jenkins-x-lighthouse-pkg-plugins.md b/docs/config/plugins/github-com-jenkins-x-lighthouse-pkg-plugins.md index 9ff021c4f..7436d5cc7 100644 --- a/docs/config/plugins/github-com-jenkins-x-lighthouse-pkg-plugins.md +++ b/docs/config/plugins/github-com-jenkins-x-lighthouse-pkg-plugins.md @@ -210,6 +210,7 @@ Trigger specifies a configuration for a single trigger.

The configura | `only_org_members` | bool | No | OnlyOrgMembers requires PRs and/or /ok-to-test comments to come from org members.
By default, trigger also include repo collaborators. | | `ignore_ok_to_test` | bool | No | IgnoreOkToTest makes trigger ignore /ok-to-test comments.
This is a security mitigation to only allow testing from trusted users. | | `elide_skipped_contexts` | bool | No | ElideSkippedContexts makes trigger not post "Skipped" contexts for jobs
that could run but do not run. | +| `skip_draft_pr` | bool | No | SkipDraftPR when enabled, skips triggering pipelines for draft PRs
unless /ok-to-test is added. | ## Welcome diff --git a/docs/install_lighthouse_with_tekton.md b/docs/install_lighthouse_with_tekton.md index 83dbf5073..01cb208a3 100644 --- a/docs/install_lighthouse_with_tekton.md +++ b/docs/install_lighthouse_with_tekton.md @@ -534,6 +534,7 @@ Now we need to add the sample project to the Lighthouse configuration and setup - $bot_user/$sample_repo_name ignore_ok_to_test: false elide_skipped_contexts: false + skip_draft_pr: false plugins: $bot_user/$config_repo_name: - config-updater diff --git a/docs/plugins/Plugins config.md b/docs/plugins/Plugins config.md index 2067225a3..a82eb07e1 100644 --- a/docs/plugins/Plugins config.md +++ b/docs/plugins/Plugins config.md @@ -215,6 +215,7 @@ Trigger specifies a configuration for a single trigger.

The configura | OnlyOrgMembers | `only_org_members` | bool | No | OnlyOrgMembers requires PRs and/or /ok-to-test comments to come from org members.
By default, trigger also include repo collaborators. | | IgnoreOkToTest | `ignore_ok_to_test` | bool | No | IgnoreOkToTest makes trigger ignore /ok-to-test comments.
This is a security mitigation to only allow testing from trusted users. | | ElideSkippedContexts | `elide_skipped_contexts` | bool | No | ElideSkippedContexts makes trigger not post "Skipped" contexts for jobs
that could run but do not run. | +| SkipDraftPR | `skip_draft_pr` | bool | No | SkipDraftPR when enabled, skips triggering pipelines for draft PRs
unless /ok-to-test is added. | ## Welcome diff --git a/pkg/plugins/config.go b/pkg/plugins/config.go index 969a87e98..d3fc6c029 100644 --- a/pkg/plugins/config.go +++ b/pkg/plugins/config.go @@ -309,6 +309,8 @@ type Trigger struct { // ElideSkippedContexts makes trigger not post "Skipped" contexts for jobs // that could run but do not run. ElideSkippedContexts bool `json:"elide_skipped_contexts,omitempty"` + // SkipDraftPR when enabled, skips triggering pipelines for draft PRs, unless /ok-to-test is added. + SkipDraftPR bool `json:"skip_draft_pr,omitempty"` } // Milestone contains the configuration options for the milestone and diff --git a/pkg/plugins/trigger/generic-comment.go b/pkg/plugins/trigger/generic-comment.go index 0f22c05b5..15a57ce18 100644 --- a/pkg/plugins/trigger/generic-comment.go +++ b/pkg/plugins/trigger/generic-comment.go @@ -59,7 +59,7 @@ func handleGenericComment(c Client, trigger *plugins.Trigger, gc scmprovider.Gen var l []*scm.Label if !trusted { // Skip untrusted PRs. - l, trusted, err = TrustedPullRequest(c.SCMProviderClient, trigger, gc.IssueAuthor.Login, org, repo, number, nil) + l, trusted, err = TrustedOrDraftPullRequest(c.SCMProviderClient, trigger, gc.IssueAuthor.Login, org, repo, number, pr.Draft, nil) if err != nil { return err } @@ -71,7 +71,7 @@ func handleGenericComment(c Client, trigger *plugins.Trigger, gc scmprovider.Gen } // At this point we can trust the PR, so we eventually update labels. - // Ensure we have labels before test, because TrustedPullRequest() won't be called + // Ensure we have labels before test, because TrustedOrDraftPullRequest() won't be called // when commentAuthor is trusted. if l == nil { l, err = c.SCMProviderClient.GetIssueLabels(org, repo, number, gc.IsPR) diff --git a/pkg/plugins/trigger/pull-request.go b/pkg/plugins/trigger/pull-request.go index d39b824b8..d47b15f61 100644 --- a/pkg/plugins/trigger/pull-request.go +++ b/pkg/plugins/trigger/pull-request.go @@ -18,6 +18,8 @@ package trigger import ( "fmt" + "net/url" + "github.com/jenkins-x/go-scm/scm" "github.com/jenkins-x/lighthouse/pkg/config" "github.com/jenkins-x/lighthouse/pkg/config/job" @@ -28,7 +30,6 @@ import ( "github.com/jenkins-x/lighthouse/pkg/scmprovider" "github.com/pkg/errors" "github.com/sirupsen/logrus" - "net/url" ) func handlePR(c Client, trigger *plugins.Trigger, pr scm.PullRequestHook) error { @@ -55,6 +56,13 @@ func handlePR(c Client, trigger *plugins.Trigger, pr scm.PullRequestHook) error } return nil } + if trigger.SkipDraftPR && pr.PullRequest.Draft { + c.Logger.Infof("Skipping build for draft PR %d unless /ok-to-test is added.", num) + if err = welcomeMsgForDraftPR(c.SCMProviderClient, trigger, pr.PullRequest); err != nil { + return fmt.Errorf("could not welcome for draft pr %q: %v", author, err) + } + return nil + } if err = infoMsg(c, pr.PullRequest); err != nil { return err @@ -62,22 +70,7 @@ func handlePR(c Client, trigger *plugins.Trigger, pr scm.PullRequestHook) error c.Logger.Infof("Author %q is a member, Starting all jobs for new PR.", author) return buildAll(c, &pr.PullRequest, pr.GUID, trigger.ElideSkippedContexts) case scm.ActionReopen: - // When a PR is reopened, check that the user is in the org or that an org - // member had said "/ok-to-test" before building, resulting in label ok-to-test. - l, trusted, err := TrustedPullRequest(c.SCMProviderClient, trigger, author, org, repo, num, nil) - if err != nil { - return fmt.Errorf("could not validate PR: %s", err) - } else if trusted { - // Eventually remove need-ok-to-test - // Does not work for TrustedUser() == true since labels are not fetched in this case - if scmprovider.HasLabel(labels.NeedsOkToTest, l) { - if err := c.SCMProviderClient.RemoveLabel(org, repo, num, labels.NeedsOkToTest, true); err != nil { - return err - } - } - c.Logger.Info("Starting all jobs for updated PR.") - return buildAll(c, &pr.PullRequest, pr.GUID, trigger.ElideSkippedContexts) - } + return buildAllIfTrustedOrDraft(c, trigger, pr) case scm.ActionEdited, scm.ActionUpdate: // if someone changes the base of their PR, we will get this // event and the changes field will list that the base SHA and @@ -85,14 +78,14 @@ func handlePR(c Client, trigger *plugins.Trigger, pr scm.PullRequestHook) error changes := pr.Changes if changes.Base.Ref.From != "" || changes.Base.Sha.From != "" { // the base of the PR changed and we need to re-test it - return buildAllIfTrusted(c, trigger, pr) + return buildAllIfTrustedOrDraft(c, trigger, pr) } - case scm.ActionSync: - return buildAllIfTrusted(c, trigger, pr) + case scm.ActionSync, scm.ActionReadyForReview, scm.ActionConvertedToDraft: + return buildAllIfTrustedOrDraft(c, trigger, pr) case scm.ActionLabel: // When a PR is LGTMd, if it is untrusted then build it once. if pr.Label.Name == labels.LGTM { - _, trusted, err := TrustedPullRequest(c.SCMProviderClient, trigger, author, org, repo, num, nil) + _, trusted, err := TrustedOrDraftPullRequest(c.SCMProviderClient, trigger, author, org, repo, num, pr.PullRequest.Draft, nil) if err != nil { return fmt.Errorf("could not validate PR: %s", err) } else if !trusted { @@ -115,14 +108,12 @@ func orgRepoAuthor(pr scm.PullRequest) (string, string, login) { return org, repo, login(author) } -func buildAllIfTrusted(c Client, trigger *plugins.Trigger, pr scm.PullRequestHook) error { - // When a PR is updated, check that the user is in the org or that an org - // member has said "/ok-to-test" before building. There's no need to ask - // for "/ok-to-test" because we do that once when the PR is created. +func buildAllIfTrustedOrDraft(c Client, trigger *plugins.Trigger, pr scm.PullRequestHook) error { + // When a PR is updated, check if the user is trusted or if the PR is a draft. org, repo, a := orgRepoAuthor(pr.PullRequest) author := string(a) num := pr.PullRequest.Number - l, trusted, err := TrustedPullRequest(c.SCMProviderClient, trigger, author, org, repo, num, nil) + l, trusted, err := TrustedOrDraftPullRequest(c.SCMProviderClient, trigger, author, org, repo, num, pr.PullRequest.Draft, nil) if err != nil { return fmt.Errorf("could not validate PR: %s", err) } else if trusted { @@ -133,6 +124,13 @@ func buildAllIfTrusted(c Client, trigger *plugins.Trigger, pr scm.PullRequestHoo return err } } + + // Skip pipelines trigger if SkipDraftPR enabled and PR is a draft without OkToTest label + if trigger.SkipDraftPR && pr.PullRequest.Draft && !scmprovider.HasLabel(labels.OkToTest, l) { + c.Logger.Info("Skipping pipelines trigger for draft PR.") + return nil + } + c.Logger.Info("Starting all jobs for updated PR.") return buildAll(c, &pr.PullRequest, pr.GUID, trigger.ElideSkippedContexts) } @@ -242,16 +240,35 @@ I understand the commands that are listed [here](https://jenkins-x.io/v3/develop return nil } -// TrustedPullRequest returns whether or not the given PR should be tested. -// It first checks if the author is in the org, then looks for "ok-to-test" label. -func TrustedPullRequest(spc scmProviderClient, trigger *plugins.Trigger, author, org, repo string, num int, l []*scm.Label) ([]*scm.Label, bool, error) { - // First check if the author is a member of the org. - if orgMember, err := TrustedUser(spc, trigger, author, org, repo); err != nil { - return l, false, fmt.Errorf("error checking %s for trust: %v", author, err) - } else if orgMember { - return l, true, nil +func welcomeMsgForDraftPR(spc scmProviderClient, trigger *plugins.Trigger, pr scm.PullRequest) error { + org, repo, author := orgRepoAuthor(pr) + + comment := fmt.Sprintf("Hi @%s. This is a draft PR. Thanks for your contribution!\n", author) + + var errors []error + + if !trigger.IgnoreOkToTest { + comment += "To trigger tests on this draft PR, please mark it as ready for review by removing the draft status. Collaborators can also trigger tests on draft PRs using /ok-to-test." + + if err := spc.AddLabel(org, repo, pr.Number, labels.NeedsOkToTest, true); err != nil { + errors = append(errors, err) + } + } + + if err := spc.CreateComment(org, repo, pr.Number, true, comment); err != nil { + errors = append(errors, err) + } + + if len(errors) > 0 { + return errorutil.NewAggregate(errors...) } - // Then check if PR has ok-to-test label + return nil +} + +// TrustedOrDraftPullRequest returns whether or not the given PR should be tested. +// It first checks if the author is in the org, then looks for "ok-to-test" label. +func TrustedOrDraftPullRequest(spc scmProviderClient, trigger *plugins.Trigger, author, org, repo string, num int, isDraft bool, l []*scm.Label) ([]*scm.Label, bool, error) { + // First get PR labels if l == nil { var err error l, err = spc.GetIssueLabels(org, repo, num, true) @@ -259,6 +276,13 @@ func TrustedPullRequest(spc scmProviderClient, trigger *plugins.Trigger, author, return l, false, fmt.Errorf("error getting issue labels: %v", err) } } + // Then check if the author is a member of the org and if trigger.SkipDraftPR disabled or PR not in draft + if orgMember, err := TrustedUser(spc, trigger, author, org, repo); err != nil { + return l, false, fmt.Errorf("error checking %s for trust: %v", author, err) + } else if orgMember && (!trigger.SkipDraftPR || !isDraft) { + return l, true, nil + } + // Then check if PR has ok-to-test label (untrusted user or trigger.SkipDraftPR enabled && draft PR) return l, scmprovider.HasLabel(labels.OkToTest, l), nil } diff --git a/pkg/plugins/trigger/pull-request_test.go b/pkg/plugins/trigger/pull-request_test.go index 4aabc1993..77da416de 100644 --- a/pkg/plugins/trigger/pull-request_test.go +++ b/pkg/plugins/trigger/pull-request_test.go @@ -96,7 +96,7 @@ func TestTrusted(t *testing.T) { Name: label, }) } - _, actual, err := TrustedPullRequest(g, trigger, tc.author, "kubernetes-incubator", "random-repo", 1, labels) + _, actual, err := TrustedOrDraftPullRequest(g, trigger, tc.author, "kubernetes-incubator", "random-repo", 1, false, labels) if err != nil { t.Fatalf("Didn't expect error: %s", err) } @@ -117,6 +117,8 @@ func TestHandlePullRequest(t *testing.T) { HasOkToTest bool prLabel string prChanges bool + isDraftPR bool + skipDraftPR bool prAction scm.Action }{ { @@ -126,6 +128,24 @@ func TestHandlePullRequest(t *testing.T) { ShouldBuild: true, prAction: scm.ActionOpen, }, + { + name: "Trusted user open draft PR with SkipDraftPR should not build and should comment", + + Author: "t", + ShouldBuild: false, + ShouldComment: true, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionOpen, + }, + { + name: "Trusted user open draft PR without SkipDraftPR should build", + + Author: "t", + ShouldBuild: true, + isDraftPR: true, + prAction: scm.ActionOpen, + }, { name: "Untrusted user open PR should not build and should comment", @@ -134,6 +154,25 @@ func TestHandlePullRequest(t *testing.T) { ShouldComment: true, prAction: scm.ActionOpen, }, + { + name: "Untrusted user open draft PR with SkipDraftPR should not build and should comment", + + Author: "u", + ShouldBuild: false, + ShouldComment: true, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionOpen, + }, + { + name: "Untrusted user open draft PR without SkipDraftPR should not build and should comment", + + Author: "u", + ShouldBuild: false, + ShouldComment: true, + isDraftPR: true, + prAction: scm.ActionOpen, + }, { name: "Trusted user reopen PR should build", @@ -141,6 +180,42 @@ func TestHandlePullRequest(t *testing.T) { ShouldBuild: true, prAction: scm.ActionReopen, }, + { + name: "Trusted user reopen draft PR with SkipDraftPR with ok-to-test should build", + + Author: "t", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionReopen, + }, + { + name: "Trusted user reopen draft PR without SkipDraftPR with ok-to-test should build", + + Author: "t", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + prAction: scm.ActionReopen, + }, + { + name: "Trusted user reopen draft PR with SkipDraftPR without ok-to-test should not build", + + Author: "t", + ShouldBuild: false, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionReopen, + }, + { + name: "Trusted user reopen draft PR without SkipDraftPR without ok-to-test should build", + + Author: "t", + ShouldBuild: true, + isDraftPR: true, + prAction: scm.ActionReopen, + }, { name: "Untrusted user reopen PR with ok-to-test should build", @@ -156,6 +231,42 @@ func TestHandlePullRequest(t *testing.T) { ShouldBuild: false, prAction: scm.ActionReopen, }, + { + name: "Untrusted user reopen draft PR with SkipDraftPR with ok-to-test should build", + + Author: "u", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionReopen, + }, + { + name: "Untrusted user reopen draft PR without SkipDraftPR with ok-to-test should build", + + Author: "u", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + prAction: scm.ActionReopen, + }, + { + name: "Untrusted user reopen draft PR with SkipDraftPR without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionReopen, + }, + { + name: "Untrusted user reopen draft PR without SkipDraftPR without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + isDraftPR: true, + prAction: scm.ActionReopen, + }, { name: "Trusted user edit PR with changes should build", @@ -171,6 +282,46 @@ func TestHandlePullRequest(t *testing.T) { ShouldBuild: false, prAction: scm.ActionEdited, }, + { + name: "Trusted user edit draft PR with SkipDraftPR with ok-to-test with changes should build", + + Author: "t", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + skipDraftPR: true, + prChanges: true, + prAction: scm.ActionEdited, + }, + { + name: "Trusted user edit draft PR without SkipDraftPR with ok-to-test with changes should build", + + Author: "t", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + prChanges: true, + prAction: scm.ActionEdited, + }, + { + name: "Trusted user edit draft PR with SkipDraftPR without ok-to-test with changes should not build", + + Author: "t", + ShouldBuild: false, + isDraftPR: true, + skipDraftPR: true, + prChanges: true, + prAction: scm.ActionEdited, + }, + { + name: "Trusted user edit draft PR without SkipDraftPR without ok-to-test with changes should build", + + Author: "t", + ShouldBuild: true, + isDraftPR: true, + prChanges: true, + prAction: scm.ActionEdited, + }, { name: "Untrusted user edit PR without changes and without ok-to-test should not build", @@ -178,6 +329,23 @@ func TestHandlePullRequest(t *testing.T) { ShouldBuild: false, prAction: scm.ActionEdited, }, + { + name: "Untrusted user edit draft PR with SkipDraftPR without changes and without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionEdited, + }, + { + name: "Untrusted user edit draft PR without SkipDraftPR without changes and without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + isDraftPR: true, + prAction: scm.ActionEdited, + }, { name: "Untrusted user edit PR with changes and without ok-to-test should not build", @@ -186,6 +354,25 @@ func TestHandlePullRequest(t *testing.T) { prChanges: true, prAction: scm.ActionEdited, }, + { + name: "Untrusted user edit draft PR with SkipDraftPR with changes and without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + prChanges: true, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionEdited, + }, + { + name: "Untrusted user edit draft PR without SkipDraftPR with changes and without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + prChanges: true, + isDraftPR: true, + prAction: scm.ActionEdited, + }, { name: "Untrusted user edit PR without changes and with ok-to-test should not build", @@ -194,6 +381,25 @@ func TestHandlePullRequest(t *testing.T) { HasOkToTest: true, prAction: scm.ActionEdited, }, + { + name: "Untrusted user edit draft PR with SkipDraftPR without changes and with ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + HasOkToTest: true, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionEdited, + }, + { + name: "Untrusted user edit draft PR without SkipDraftPR without changes and with ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + HasOkToTest: true, + isDraftPR: true, + prAction: scm.ActionEdited, + }, { name: "Untrusted user edit PR with changes and with ok-to-test should build", @@ -203,6 +409,27 @@ func TestHandlePullRequest(t *testing.T) { prChanges: true, prAction: scm.ActionEdited, }, + { + name: "Untrusted user edit draft PR with SkipDraftPR with changes and with ok-to-test should build", + + Author: "u", + ShouldBuild: true, + HasOkToTest: true, + prChanges: true, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionEdited, + }, + { + name: "Untrusted user edit draft PR without SkipDraftPR with changes and with ok-to-test should build", + + Author: "u", + ShouldBuild: true, + HasOkToTest: true, + prChanges: true, + isDraftPR: true, + prAction: scm.ActionEdited, + }, { name: "Trusted user sync PR should build", @@ -210,6 +437,50 @@ func TestHandlePullRequest(t *testing.T) { ShouldBuild: true, prAction: scm.ActionSync, }, + { + name: "Trusted user sync draft PR with SkipDraftPR with ok-to-test should build", + + Author: "t", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionSync, + }, + { + name: "Trusted user sync draft PR without SkipDraftPR with ok-to-test should build", + + Author: "t", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + prAction: scm.ActionSync, + }, + { + name: "Trusted user sync draft PR with SkipDraftPR without ok-to-test should not build", + + Author: "t", + ShouldBuild: false, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionSync, + }, + { + name: "Trusted user sync draft PR without SkipDraftPR without ok-to-test should build", + + Author: "t", + ShouldBuild: true, + isDraftPR: true, + prAction: scm.ActionSync, + }, + { + name: "Untrusted user sync PR with ok-to-test should build", + + Author: "u", + ShouldBuild: true, + HasOkToTest: true, + prAction: scm.ActionSync, + }, { name: "Untrusted user sync PR without ok-to-test should not build", @@ -218,11 +489,39 @@ func TestHandlePullRequest(t *testing.T) { prAction: scm.ActionSync, }, { - name: "Untrusted user sync PR with ok-to-test should build", + name: "Untrusted user sync draft PR with SkipDraftPR with ok-to-test should build", Author: "u", ShouldBuild: true, HasOkToTest: true, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionSync, + }, + { + name: "Untrusted user sync draft PR without SkipDraftPR with ok-to-test should build", + + Author: "u", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + prAction: scm.ActionSync, + }, + { + name: "Untrusted user sync draft PR with SkipDraftPR without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionSync, + }, + { + name: "Untrusted user sync draft PR without SkipDraftPR without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + isDraftPR: true, prAction: scm.ActionSync, }, { @@ -256,6 +555,142 @@ func TestHandlePullRequest(t *testing.T) { ShouldBuild: false, prAction: scm.ActionClose, }, + { + name: "Trusted user convert PR to draft with SkipDraftPR with ok-to-test should build", + + Author: "t", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionConvertedToDraft, + }, + { + name: "Trusted user convert PR to draft with SkipDraftPR without ok-to-test should not build", + + Author: "t", + ShouldBuild: false, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionConvertedToDraft, + }, + { + name: "Trusted user convert PR to draft without SkipDraftPR with ok-to-test should build", + + Author: "t", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + prAction: scm.ActionConvertedToDraft, + }, + { + name: "Trusted user convert PR to draft without SkipDraftPR without ok-to-test should build", + + Author: "t", + ShouldBuild: true, + isDraftPR: true, + prAction: scm.ActionConvertedToDraft, + }, + { + name: "Untrusted user convert PR to draft with SkipDraftPR with ok-to-test should build", + + Author: "u", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionConvertedToDraft, + }, + { + name: "Untrusted user convert PR to draft with SkipDraftPR without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + isDraftPR: true, + skipDraftPR: true, + prAction: scm.ActionConvertedToDraft, + }, + { + name: "Untrusted user convert PR to draft without SkipDraftPR with ok-to-test should build", + + Author: "u", + ShouldBuild: true, + HasOkToTest: true, + isDraftPR: true, + prAction: scm.ActionConvertedToDraft, + }, + { + name: "Untrusted user convert PR to draft without SkipDraftPR without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + isDraftPR: true, + prAction: scm.ActionConvertedToDraft, + }, + { + name: "Trusted user convert PR to ready for review with SkipDraftPR with ok-to-test should build", + + Author: "t", + ShouldBuild: true, + HasOkToTest: true, + skipDraftPR: true, + prAction: scm.ActionReadyForReview, + }, + { + name: "Trusted user convert PR to ready for review with SkipDraftPR without ok-to-test should build", + + Author: "t", + ShouldBuild: true, + skipDraftPR: true, + prAction: scm.ActionReadyForReview, + }, + { + name: "Trusted user convert PR to ready for review without SkipDraftPR with ok-to-test should build", + + Author: "t", + ShouldBuild: true, + HasOkToTest: true, + prAction: scm.ActionReadyForReview, + }, + { + name: "Trusted user convert PR to ready for review without SkipDraftPR without ok-to-test should build", + + Author: "t", + ShouldBuild: true, + prAction: scm.ActionReadyForReview, + }, + { + name: "Untrusted user convert PR to ready for review with SkipDraftPR with ok-to-test should build", + + Author: "u", + ShouldBuild: true, + HasOkToTest: true, + skipDraftPR: true, + prAction: scm.ActionReadyForReview, + }, + { + name: "Untrusted user convert PR to ready for review with SkipDraftPR without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + skipDraftPR: true, + prAction: scm.ActionReadyForReview, + }, + { + name: "Untrusted user convert PR to ready for review without SkipDraftPR with ok-to-test should build", + + Author: "u", + ShouldBuild: true, + HasOkToTest: true, + prAction: scm.ActionReadyForReview, + }, + { + name: "Untrusted user convert PR to ready for review without SkipDraftPR without ok-to-test should not build", + + Author: "u", + ShouldBuild: false, + prAction: scm.ActionReadyForReview, + }, } for _, tc := range testcases { t.Logf("running scenario %q", tc.name) @@ -266,6 +701,7 @@ func TestHandlePullRequest(t *testing.T) { PullRequests: map[int]*scm.PullRequest{ 0: { Number: 0, + Draft: tc.isDraftPR, Author: scm.User{Login: tc.Author}, Base: scm.PullRequestBranch{ Ref: "master", @@ -307,6 +743,7 @@ func TestHandlePullRequest(t *testing.T) { Label: scm.Label{Name: tc.prLabel}, PullRequest: scm.PullRequest{ Number: 0, + Draft: tc.isDraftPR, Author: scm.User{Login: tc.Author}, Base: scm.PullRequestBranch{ Ref: "master", @@ -333,6 +770,7 @@ func TestHandlePullRequest(t *testing.T) { trigger := &plugins.Trigger{ TrustedOrg: "org", OnlyOrgMembers: true, + SkipDraftPR: tc.skipDraftPR, } if err := handlePR(c, trigger, pr); err != nil { t.Fatalf("Didn't expect error: %s", err)