From ad9ab8fdd172234193346cc2fa1ad76e1e543b1c Mon Sep 17 00:00:00 2001 From: Jacob McCann Date: Wed, 26 Feb 2020 09:58:00 -0600 Subject: [PATCH 1/3] feat: ability to filter on status context existence --- README.md | 1 + check.go | 7 ++++++- check_test.go | 43 +++++++++++++++++++++++++++++++------------ github.go | 36 ++++++++++++++++++++---------------- in_test.go | 14 ++++++++------ models.go | 15 +++++++++++++++ out_test.go | 22 +++++++++++----------- 7 files changed, 92 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index d46077ff..3622701a 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,7 @@ Make sure to check out [#migrating](#migrating) to learn more. | `labels` | No | `["bug", "enhancement"]` | The labels on the PR. The pipeline will only trigger on pull requests having at least one of the specified labels. | | `disable_git_lfs` | No | `true` | Disable Git LFS, skipping an attempt to convert pointers of files tracked into their corresponding objects when checked out into a working copy. | | `states` | No | `["OPEN", "MERGED"]` | The PR states to select (`OPEN`, `MERGED` or `CLOSED`). The pipeline will only trigger on pull requests matching one of the specified states. Default is ["OPEN"]. | +| `status_context` | No | `concourse-ci/build` | Filter out PRs that contain the status context on their latest SHA | Notes: - If `v3_endpoint` is set, `v4_endpoint` must also be set (and the other way around). diff --git a/check.go b/check.go index 64df9e24..a103aea1 100644 --- a/check.go +++ b/check.go @@ -45,7 +45,12 @@ Loop: } // Filter out commits that are too old. - if !p.UpdatedDate().Time.After(request.Version.CommittedDate) { + if request.Source.StatusContext == "" && !p.Tip.CommittedDate.Time.After(request.Version.CommittedDate) { + continue + } + + // Filter out commits that already have a build status + if request.Source.StatusContext != "" && p.HasStatus { continue } diff --git a/check_test.go b/check_test.go index 8c422914..c5ad037f 100644 --- a/check_test.go +++ b/check_test.go @@ -11,18 +11,18 @@ import ( var ( testPullRequests = []*resource.PullRequest{ - createTestPR(1, "master", true, false, 0, nil, false, githubv4.PullRequestStateOpen), - createTestPR(2, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), - createTestPR(3, "master", false, false, 0, nil, true, githubv4.PullRequestStateOpen), - createTestPR(4, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), - createTestPR(5, "master", false, true, 0, nil, false, githubv4.PullRequestStateOpen), - createTestPR(6, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), - createTestPR(7, "develop", false, false, 0, []string{"enhancement"}, false, githubv4.PullRequestStateOpen), - createTestPR(8, "master", false, false, 1, []string{"wontfix"}, false, githubv4.PullRequestStateOpen), - createTestPR(9, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), - createTestPR(10, "master", false, false, 0, nil, false, githubv4.PullRequestStateClosed), - createTestPR(11, "master", false, false, 0, nil, false, githubv4.PullRequestStateMerged), - createTestPR(12, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + createTestPR(1, "master", true, false, 0, nil, false, githubv4.PullRequestStateOpen, false), + createTestPR(2, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, true), + createTestPR(3, "master", false, false, 0, nil, true, githubv4.PullRequestStateOpen, false), + createTestPR(4, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, true), + createTestPR(5, "master", false, true, 0, nil, false, githubv4.PullRequestStateOpen, false), + createTestPR(6, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), + createTestPR(7, "develop", false, false, 0, []string{"enhancement"}, false, githubv4.PullRequestStateOpen, true), + createTestPR(8, "master", false, false, 1, []string{"wontfix"}, false, githubv4.PullRequestStateOpen, true), + createTestPR(9, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), + createTestPR(10, "master", false, false, 0, nil, false, githubv4.PullRequestStateClosed, false), + createTestPR(11, "master", false, false, 0, nil, false, githubv4.PullRequestStateMerged, false), + createTestPR(12, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), } ) @@ -262,6 +262,25 @@ func TestCheck(t *testing.T) { resource.NewVersion(testPullRequests[10]), }, }, + + { + description: "check returns versions with no status", + source: resource.Source{ + Repository: "itsdalmo/test-repository", + AccessToken: "oauthtoken", + StatusContext: "some-status", + }, + version: resource.NewVersion(testPullRequests[11]), + pullRequests: testPullRequests, + files: [][]string{}, + expected: resource.CheckResponse{ + resource.NewVersion(testPullRequests[11]), + resource.NewVersion(testPullRequests[8]), + resource.NewVersion(testPullRequests[5]), + resource.NewVersion(testPullRequests[4]), + resource.NewVersion(testPullRequests[2]), + }, + }, } for _, tc := range tests { diff --git a/github.go b/github.go index ab10cbdc..f9840f48 100644 --- a/github.go +++ b/github.go @@ -31,10 +31,11 @@ type Github interface { // GithubClient for handling requests to the Github V3 and V4 APIs. type GithubClient struct { - V3 *github.Client - V4 *githubv4.Client - Repository string - Owner string + V3 *github.Client + V4 *githubv4.Client + Repository string + Owner string + StatusContext string } // NewGithubClient ... @@ -90,10 +91,11 @@ func NewGithubClient(s *Source) (*GithubClient, error) { } return &GithubClient{ - V3: v3, - V4: v4, - Owner: owner, - Repository: repository, + V3: v3, + V4: v4, + Owner: owner, + Repository: repository, + StatusContext: s.StatusContext, }, nil } @@ -133,14 +135,15 @@ func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState) ([ } vars := map[string]interface{}{ - "repositoryOwner": githubv4.String(m.Owner), - "repositoryName": githubv4.String(m.Repository), - "prFirst": githubv4.Int(100), - "prStates": prStates, - "prCursor": (*githubv4.String)(nil), - "commitsLast": githubv4.Int(1), - "prReviewStates": []githubv4.PullRequestReviewState{githubv4.PullRequestReviewStateApproved}, - "labelsFirst": githubv4.Int(100), + "repositoryOwner": githubv4.String(m.Owner), + "repositoryName": githubv4.String(m.Repository), + "statusContextName": githubv4.String(m.StatusContext), + "prFirst": githubv4.Int(100), + "prStates": prStates, + "prCursor": (*githubv4.String)(nil), + "commitsLast": githubv4.Int(1), + "prReviewStates": []githubv4.PullRequestReviewState{githubv4.PullRequestReviewStateApproved}, + "labelsFirst": githubv4.Int(100), } var response []*PullRequest @@ -160,6 +163,7 @@ func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState) ([ Tip: c.Node.Commit, ApprovedReviewCount: p.Node.Reviews.TotalCount, Labels: labels, + HasStatus: c.Node.Commit.StatusObject.StatusContextObject.Context == (*githubv4.String)(nil), }) } } diff --git a/in_test.go b/in_test.go index 17e7abfc..2b7aa5fd 100644 --- a/in_test.go +++ b/in_test.go @@ -42,7 +42,7 @@ func TestGet(t *testing.T) { State: githubv4.PullRequestStateOpen, }, parameters: resource.GetParameters{}, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, @@ -61,7 +61,7 @@ func TestGet(t *testing.T) { State: githubv4.PullRequestStateOpen, }, parameters: resource.GetParameters{}, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, @@ -81,7 +81,7 @@ func TestGet(t *testing.T) { parameters: resource.GetParameters{ IntegrationTool: "rebase", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, @@ -101,7 +101,7 @@ func TestGet(t *testing.T) { parameters: resource.GetParameters{ IntegrationTool: "checkout", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, @@ -121,7 +121,7 @@ func TestGet(t *testing.T) { parameters: resource.GetParameters{ GitDepth: 2, }, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, @@ -141,7 +141,7 @@ func TestGet(t *testing.T) { parameters: resource.GetParameters{ ListChangedFiles: true, }, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), files: []resource.ChangedFileObject{ { Path: "README.md", @@ -327,6 +327,7 @@ func createTestPR( labels []string, isDraft bool, state githubv4.PullRequestState, + hasStatus bool, ) *resource.PullRequest { n := strconv.Itoa(count) d := time.Now().AddDate(0, 0, -count) @@ -379,6 +380,7 @@ func createTestPR( }, ApprovedReviewCount: approvedCount, Labels: labelObjects, + HasStatus: hasStatus, } } diff --git a/models.go b/models.go index 9e4e7b1c..5a75f94a 100644 --- a/models.go +++ b/models.go @@ -27,6 +27,7 @@ type Source struct { RequiredReviewApprovals int `json:"required_review_approvals"` Labels []string `json:"labels"` States []githubv4.PullRequestState `json:"states"` + StatusContext string `json:"status_context"` } // Validate the source configuration. @@ -95,6 +96,7 @@ type PullRequest struct { Tip CommitObject ApprovedReviewCount int Labels []LabelObject + HasStatus bool } // PullRequestObject represents the GraphQL commit node. @@ -142,6 +144,19 @@ type CommitObject struct { } Email string } + StatusObject +} + +// StatusObject represents the GraphQL status object. +// https://developer.github.com/v4/object/commit/ +type StatusObject struct { + StatusContextObject `graphql:"context(name:$statusContextName)"` +} + +// StatusContextObject represents the GraphQL status context object. +// https://developer.github.com/v4/object/statuscontext/ +type StatusContextObject struct { + Context *githubv4.String } // ChangedFileObject represents the GraphQL FilesChanged node. diff --git a/out_test.go b/out_test.go index 4d430c7b..b6fb9079 100644 --- a/out_test.go +++ b/out_test.go @@ -34,7 +34,7 @@ func TestPut(t *testing.T) { CommittedDate: time.Time{}, }, parameters: resource.PutParameters{}, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), }, { @@ -51,7 +51,7 @@ func TestPut(t *testing.T) { parameters: resource.PutParameters{ Status: "success", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), }, { @@ -69,7 +69,7 @@ func TestPut(t *testing.T) { Status: "failure", Context: "build", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), }, { @@ -88,7 +88,7 @@ func TestPut(t *testing.T) { BaseContext: "concourse-ci-custom", Context: "build", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), }, { @@ -106,7 +106,7 @@ func TestPut(t *testing.T) { Status: "failure", TargetURL: "https://targeturl.com/concourse", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), }, { @@ -124,7 +124,7 @@ func TestPut(t *testing.T) { Status: "failure", Description: "Concourse CI build", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), }, { @@ -141,7 +141,7 @@ func TestPut(t *testing.T) { parameters: resource.PutParameters{ Comment: "comment", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), }, { @@ -158,7 +158,7 @@ func TestPut(t *testing.T) { parameters: resource.PutParameters{ DeletePreviousComments: true, }, - pullRequest: createTestPR(1, "master", false, false, 0, []string{}, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, []string{}, false, githubv4.PullRequestStateOpen, false), }, } @@ -251,7 +251,7 @@ func TestVariableSubstitution(t *testing.T) { Comment: fmt.Sprintf("$%s", variableName), }, expectedComment: variableValue, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), }, { @@ -270,7 +270,7 @@ func TestVariableSubstitution(t *testing.T) { TargetURL: fmt.Sprintf("%s$%s", variableURL, variableName), }, expectedTargetURL: fmt.Sprintf("%s%s", variableURL, variableValue), - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), }, { @@ -288,7 +288,7 @@ func TestVariableSubstitution(t *testing.T) { Comment: "$THIS_IS_NOT_SUBSTITUTED", }, expectedComment: "$THIS_IS_NOT_SUBSTITUTED", - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, false), }, } From 450aa986b4b58d204a0d3c0d99766b90dc9108eb Mon Sep 17 00:00:00 2001 From: Jacob McCann Date: Wed, 26 Feb 2020 11:17:37 -0600 Subject: [PATCH 2/3] fix: commit status query and HasStatus comparison --- github.go | 2 +- models.go | 18 +++++------------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/github.go b/github.go index f9840f48..5916053a 100644 --- a/github.go +++ b/github.go @@ -163,7 +163,7 @@ func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState) ([ Tip: c.Node.Commit, ApprovedReviewCount: p.Node.Reviews.TotalCount, Labels: labels, - HasStatus: c.Node.Commit.StatusObject.StatusContextObject.Context == (*githubv4.String)(nil), + HasStatus: !(c.Node.Commit.Status.Context.Context == nil), }) } } diff --git a/models.go b/models.go index 5a75f94a..752b151d 100644 --- a/models.go +++ b/models.go @@ -144,19 +144,11 @@ type CommitObject struct { } Email string } - StatusObject -} - -// StatusObject represents the GraphQL status object. -// https://developer.github.com/v4/object/commit/ -type StatusObject struct { - StatusContextObject `graphql:"context(name:$statusContextName)"` -} - -// StatusContextObject represents the GraphQL status context object. -// https://developer.github.com/v4/object/statuscontext/ -type StatusContextObject struct { - Context *githubv4.String + Status struct { + Context struct { + Context *githubv4.String + } `graphql:"context(name:$statusContextName)"` + } } // ChangedFileObject represents the GraphQL FilesChanged node. From 67d75f998e5f21419e376d3f42c5169e51c95f2d Mon Sep 17 00:00:00 2001 From: Jacob McCann Date: Wed, 26 Feb 2020 12:44:54 -0600 Subject: [PATCH 3/3] fix: get resource by removing status from commitobject --- github.go | 7 +++++-- models.go | 13 ++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/github.go b/github.go index 5916053a..1a6303f7 100644 --- a/github.go +++ b/github.go @@ -113,7 +113,10 @@ func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState) ([ Commits struct { Edges []struct { Node struct { - Commit CommitObject + Commit struct { + CommitObject + Status StatusObject + } } } } `graphql:"commits(last:$commitsLast)"` @@ -160,7 +163,7 @@ func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState) ([ for _, c := range p.Node.Commits.Edges { response = append(response, &PullRequest{ PullRequestObject: p.Node.PullRequestObject, - Tip: c.Node.Commit, + Tip: c.Node.Commit.CommitObject, ApprovedReviewCount: p.Node.Reviews.TotalCount, Labels: labels, HasStatus: !(c.Node.Commit.Status.Context.Context == nil), diff --git a/models.go b/models.go index 752b151d..2865283c 100644 --- a/models.go +++ b/models.go @@ -144,11 +144,14 @@ type CommitObject struct { } Email string } - Status struct { - Context struct { - Context *githubv4.String - } `graphql:"context(name:$statusContextName)"` - } +} + +// StatusObject represents the GraphQL FilesChanged node. +// https://developer.github.com/v4/object/status/ +type StatusObject struct { + Context struct { + Context *githubv4.String + } `graphql:"context(name:$statusContextName)"` } // ChangedFileObject represents the GraphQL FilesChanged node.