From 463ed9a41587367895e09163151f70069ebf4c77 Mon Sep 17 00:00:00 2001 From: Nathan Rowlan Date: Thu, 8 Dec 2022 16:42:04 -0600 Subject: [PATCH] Revert "Close #26: Filter PRs by updated date instead of commit date" --- check.go | 7 ++++- check_test.go | 53 +++++++++---------------------- e2e/e2e_test.go | 63 +++++++++++++++++-------------------- fakes/fake_github.go | 19 +++++------- github.go | 74 +++++++++++++++++++------------------------- 5 files changed, 89 insertions(+), 127 deletions(-) diff --git a/check.go b/check.go index 6407caa8..64df9e24 100644 --- a/check.go +++ b/check.go @@ -20,7 +20,7 @@ func Check(request CheckRequest, manager Github) (CheckResponse, error) { filterStates = request.Source.States } - pulls, err := manager.ListPullRequests(filterStates, request.Version.CommittedDate) + pulls, err := manager.ListPullRequests(filterStates) if err != nil { return nil, fmt.Errorf("failed to get last commits: %s", err) } @@ -44,6 +44,11 @@ Loop: continue } + // Filter out commits that are too old. + if !p.UpdatedDate().Time.After(request.Version.CommittedDate) { + continue + } + // Filter out pull request if it does not contain at least one of the desired labels if len(request.Source.Labels) > 0 { labelFound := false diff --git a/check_test.go b/check_test.go index 956a8728..8c422914 100644 --- a/check_test.go +++ b/check_test.go @@ -50,7 +50,7 @@ func TestCheck(t *testing.T) { }, { - description: "check returns all open PRs if there is a previous", + description: "check returns the previous version when its still latest", source: resource.Source{ Repository: "itsdalmo/test-repository", AccessToken: "oauthtoken", @@ -59,13 +59,20 @@ func TestCheck(t *testing.T) { pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[11]), - resource.NewVersion(testPullRequests[8]), - resource.NewVersion(testPullRequests[7]), - resource.NewVersion(testPullRequests[6]), - resource.NewVersion(testPullRequests[5]), - resource.NewVersion(testPullRequests[4]), - resource.NewVersion(testPullRequests[3]), + resource.NewVersion(testPullRequests[1]), + }, + }, + + { + description: "check returns all new versions since the last", + source: resource.Source{ + Repository: "itsdalmo/test-repository", + AccessToken: "oauthtoken", + }, + version: resource.NewVersion(testPullRequests[3]), + pullRequests: testPullRequests, + files: [][]string{}, + expected: resource.CheckResponse{ resource.NewVersion(testPullRequests[2]), resource.NewVersion(testPullRequests[1]), }, @@ -86,7 +93,6 @@ func TestCheck(t *testing.T) { {"terraform/modules/variables.tf", "travis.yml"}, }, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[3]), resource.NewVersion(testPullRequests[2]), }, }, @@ -106,7 +112,6 @@ func TestCheck(t *testing.T) { {"terraform/modules/variables.tf", "travis.yml"}, }, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[3]), resource.NewVersion(testPullRequests[2]), }, }, @@ -121,15 +126,6 @@ func TestCheck(t *testing.T) { version: resource.NewVersion(testPullRequests[1]), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[11]), - resource.NewVersion(testPullRequests[8]), - resource.NewVersion(testPullRequests[7]), - resource.NewVersion(testPullRequests[6]), - resource.NewVersion(testPullRequests[5]), - resource.NewVersion(testPullRequests[4]), - resource.NewVersion(testPullRequests[3]), - resource.NewVersion(testPullRequests[2]), - resource.NewVersion(testPullRequests[1]), resource.NewVersion(testPullRequests[0]), }, }, @@ -144,13 +140,6 @@ func TestCheck(t *testing.T) { version: resource.NewVersion(testPullRequests[3]), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[11]), - resource.NewVersion(testPullRequests[8]), - resource.NewVersion(testPullRequests[7]), - resource.NewVersion(testPullRequests[6]), - resource.NewVersion(testPullRequests[5]), - resource.NewVersion(testPullRequests[4]), - resource.NewVersion(testPullRequests[3]), resource.NewVersion(testPullRequests[1]), }, }, @@ -165,13 +154,6 @@ func TestCheck(t *testing.T) { version: resource.NewVersion(testPullRequests[3]), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[11]), - resource.NewVersion(testPullRequests[8]), - resource.NewVersion(testPullRequests[7]), - resource.NewVersion(testPullRequests[6]), - resource.NewVersion(testPullRequests[5]), - resource.NewVersion(testPullRequests[4]), - resource.NewVersion(testPullRequests[3]), resource.NewVersion(testPullRequests[2]), resource.NewVersion(testPullRequests[1]), }, @@ -187,11 +169,6 @@ func TestCheck(t *testing.T) { version: resource.NewVersion(testPullRequests[5]), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[11]), - resource.NewVersion(testPullRequests[8]), - resource.NewVersion(testPullRequests[7]), - resource.NewVersion(testPullRequests[6]), - resource.NewVersion(testPullRequests[5]), resource.NewVersion(testPullRequests[3]), resource.NewVersion(testPullRequests[2]), resource.NewVersion(testPullRequests[1]), diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index ee1bcd86..523838c2 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -23,9 +23,6 @@ import ( ) var ( - firstCommitID = "23dc9f552bf989d1a4aeb65ce23351dee0ec9019" - firstPullRequestID = "3" - firstDateTime = time.Date(2018, time.May, 11, 7, 28, 56, 0, time.UTC) targetCommitID = "a5114f6ab89f4b736655642a11e8d15ce363d882" targetPullRequestID = "4" targetDateTime = time.Date(2018, time.May, 11, 8, 43, 48, 0, time.UTC) @@ -52,20 +49,19 @@ func TestCheckE2E(t *testing.T) { }, version: resource.Version{}, expected: resource.CheckResponse{ - resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime, ApprovedReviewCount: "0", State: "OPEN"}, + resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime}, }, }, { - description: "check returns all open PRs if there is a previous version", + description: "check returns the previous version when its still latest", source: resource.Source{ Repository: "itsdalmo/test-repository", AccessToken: os.Getenv("GITHUB_ACCESS_TOKEN"), }, version: resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime}, expected: resource.CheckResponse{ - resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime, ApprovedReviewCount: "1", State: "OPEN"}, - resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime, ApprovedReviewCount: "0", State: "OPEN"}, + resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime}, }, }, @@ -77,8 +73,7 @@ func TestCheckE2E(t *testing.T) { }, version: resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime}, expected: resource.CheckResponse{ - resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime, ApprovedReviewCount: "1", State: "OPEN"}, - resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime, ApprovedReviewCount: "0", State: "OPEN"}, + resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime}, }, }, @@ -91,7 +86,7 @@ func TestCheckE2E(t *testing.T) { }, version: resource.Version{}, expected: resource.CheckResponse{ - resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime, ApprovedReviewCount: "1", State: "OPEN"}, + resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime}, }, }, @@ -104,7 +99,7 @@ func TestCheckE2E(t *testing.T) { }, version: resource.Version{}, expected: resource.CheckResponse{ - resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime, ApprovedReviewCount: "1", State: "OPEN"}, + resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime}, }, }, @@ -118,7 +113,7 @@ func TestCheckE2E(t *testing.T) { }, version: resource.Version{}, expected: resource.CheckResponse{ - resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime, ApprovedReviewCount: "0", State: "OPEN"}, + resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime}, }, }, @@ -134,7 +129,7 @@ func TestCheckE2E(t *testing.T) { }, version: resource.Version{}, expected: resource.CheckResponse{ - resource.Version{PR: developPullRequestID, Commit: developCommitID, CommittedDate: developDateTime, ApprovedReviewCount: "0", State: "OPEN"}, + resource.Version{PR: developPullRequestID, Commit: developCommitID, CommittedDate: developDateTime}, }, }, @@ -149,7 +144,7 @@ func TestCheckE2E(t *testing.T) { }, version: resource.Version{}, expected: resource.CheckResponse{ - resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime, ApprovedReviewCount: "1", State: "OPEN"}, + resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime}, }, }, @@ -175,7 +170,7 @@ func TestCheckE2E(t *testing.T) { }, version: resource.Version{}, expected: resource.CheckResponse{ - resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime, ApprovedReviewCount: "1", State: "OPEN"}, + resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime}, }, }, } @@ -209,7 +204,7 @@ func TestCheckAPICostE2E(t *testing.T) { AccessToken: os.Getenv("GITHUB_ACCESS_TOKEN"), }, version: resource.Version{}, - expected: 1, + expected: 2, }, } @@ -259,8 +254,8 @@ func TestGetAndPutE2E(t *testing.T) { }, getParameters: resource.GetParameters{}, putParameters: resource.PutParameters{}, - versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z","approved_review_count":"","state":""}`, - metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"},{"name":"state","value":"OPEN"}]`, + versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z"}`, + metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"}]`, metadataFiles: map[string]string{ "pr": "4", "url": "https://github.com/itsdalmo/test-repository/pull/4", @@ -272,7 +267,7 @@ func TestGetAndPutE2E(t *testing.T) { "author": "itsdalmo", }, expectedCommitCount: 10, - expectedCommits: []string{"Merge commit 'a5114f6ab89f4b736655642a11e8d15ce363d882' into master"}, + expectedCommits: []string{"Merge commit 'a5114f6ab89f4b736655642a11e8d15ce363d882'"}, }, { description: "get works when rebasing", @@ -291,8 +286,8 @@ func TestGetAndPutE2E(t *testing.T) { IntegrationTool: "rebase", }, putParameters: resource.PutParameters{}, - versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z","approved_review_count":"","state":""}`, - metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"},{"name":"state","value":"OPEN"}]`, + versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z"}`, + metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"}]`, expectedCommitCount: 9, expectedCommits: []string{"Push 2."}, }, @@ -313,8 +308,8 @@ func TestGetAndPutE2E(t *testing.T) { IntegrationTool: "checkout", }, putParameters: resource.PutParameters{}, - versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z","approved_review_count":"","state":""}`, - metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"},{"name":"state","value":"OPEN"}]`, + versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z"}`, + metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"}]`, expectedCommitCount: 7, expectedCommits: []string{ "Push 2.", @@ -341,8 +336,8 @@ func TestGetAndPutE2E(t *testing.T) { }, getParameters: resource.GetParameters{}, putParameters: resource.PutParameters{}, - versionString: `{"pr":"6","commit":"ac771f3b69cbd63b22bbda553f827ab36150c640","committed":"0001-01-01T00:00:00Z","approved_review_count":"","state":""}`, - metadataString: `[{"name":"pr","value":"6"},{"name":"title","value":"[skip ci] Add a PR with a non-master base"},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/6"},{"name":"head_name","value":"test-develop-pr"},{"name":"head_sha","value":"ac771f3b69cbd63b22bbda553f827ab36150c640"},{"name":"base_name","value":"develop"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"[skip ci] Add a PR with a non-master base"},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"},{"name":"state","value":"OPEN"}]`, + versionString: `{"pr":"6","commit":"ac771f3b69cbd63b22bbda553f827ab36150c640","committed":"0001-01-01T00:00:00Z"}`, + metadataString: `[{"name":"pr","value":"6"},{"name":"title","value":"[skip ci] Add a PR with a non-master base"},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/6"},{"name":"head_name","value":"test-develop-pr"},{"name":"head_sha","value":"ac771f3b69cbd63b22bbda553f827ab36150c640"},{"name":"base_name","value":"develop"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"[skip ci] Add a PR with a non-master base"},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"}]`, expectedCommitCount: 5, expectedCommits: []string{"[skip ci] Add a PR with a non-master base"}, // This merge ends up being fast-forwarded }, @@ -362,10 +357,10 @@ func TestGetAndPutE2E(t *testing.T) { }, getParameters: resource.GetParameters{}, putParameters: resource.PutParameters{}, - versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z","approved_review_count":"","state":""}`, - metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"},{"name":"state","value":"OPEN"}]`, + versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z"}`, + metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"}]`, expectedCommitCount: 10, - expectedCommits: []string{"Merge commit 'a5114f6ab89f4b736655642a11e8d15ce363d882' into master"}, + expectedCommits: []string{"Merge commit 'a5114f6ab89f4b736655642a11e8d15ce363d882'"}, }, { description: "get works with git_depth", @@ -380,11 +375,11 @@ func TestGetAndPutE2E(t *testing.T) { }, getParameters: resource.GetParameters{GitDepth: 6}, putParameters: resource.PutParameters{}, - versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z","approved_review_count":"","state":""}`, - metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"},{"name":"state","value":"OPEN"}]`, + versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z"}`, + metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"}]`, expectedCommitCount: 9, expectedCommits: []string{ - "Merge commit 'a5114f6ab89f4b736655642a11e8d15ce363d882' into master", + "Merge commit 'a5114f6ab89f4b736655642a11e8d15ce363d882'", "Push 2.", "Push 1.", "Add another commit to the 2nd PR to verify concourse behaviour.", @@ -410,11 +405,11 @@ func TestGetAndPutE2E(t *testing.T) { ListChangedFiles: true, }, putParameters: resource.PutParameters{}, - versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z","approved_review_count":"","state":""}`, - metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"},{"name":"state","value":"OPEN"}]`, + versionString: `{"pr":"4","commit":"a5114f6ab89f4b736655642a11e8d15ce363d882","committed":"0001-01-01T00:00:00Z"}`, + metadataString: `[{"name":"pr","value":"4"},{"name":"title","value":"Add comment from 2nd pull request."},{"name":"url","value":"https://github.com/itsdalmo/test-repository/pull/4"},{"name":"head_name","value":"my_second_pull"},{"name":"head_sha","value":"a5114f6ab89f4b736655642a11e8d15ce363d882"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"93eeeedb8a16e6662062d1eca5655108977cc59a"},{"name":"message","value":"Push 2."},{"name":"author","value":"itsdalmo"},{"name":"author_email","value":"kristian@doingit.no"}]`, filesString: "README.md\ntest.txt\n", expectedCommitCount: 10, - expectedCommits: []string{"Merge commit 'a5114f6ab89f4b736655642a11e8d15ce363d882' into master"}, + expectedCommits: []string{"Merge commit 'a5114f6ab89f4b736655642a11e8d15ce363d882'"}, }, } diff --git a/fakes/fake_github.go b/fakes/fake_github.go index 151890bf..1847478f 100644 --- a/fakes/fake_github.go +++ b/fakes/fake_github.go @@ -3,7 +3,6 @@ package fakes import ( "sync" - "time" "github.com/shurcooL/githubv4" resource "github.com/telia-oss/github-pr-resource" @@ -62,11 +61,10 @@ type FakeGithub struct { result1 []string result2 error } - ListPullRequestsStub func([]githubv4.PullRequestState, time.Time) ([]*resource.PullRequest, error) + ListPullRequestsStub func([]githubv4.PullRequestState) ([]*resource.PullRequest, error) listPullRequestsMutex sync.RWMutex listPullRequestsArgsForCall []struct { arg1 []githubv4.PullRequestState - arg2 time.Time } listPullRequestsReturns struct { result1 []*resource.PullRequest @@ -359,7 +357,7 @@ func (fake *FakeGithub) ListModifiedFilesReturnsOnCall(i int, result1 []string, }{result1, result2} } -func (fake *FakeGithub) ListPullRequests(arg1 []githubv4.PullRequestState, arg2 time.Time) ([]*resource.PullRequest, error) { +func (fake *FakeGithub) ListPullRequests(arg1 []githubv4.PullRequestState) ([]*resource.PullRequest, error) { var arg1Copy []githubv4.PullRequestState if arg1 != nil { arg1Copy = make([]githubv4.PullRequestState, len(arg1)) @@ -369,12 +367,11 @@ func (fake *FakeGithub) ListPullRequests(arg1 []githubv4.PullRequestState, arg2 ret, specificReturn := fake.listPullRequestsReturnsOnCall[len(fake.listPullRequestsArgsForCall)] fake.listPullRequestsArgsForCall = append(fake.listPullRequestsArgsForCall, struct { arg1 []githubv4.PullRequestState - arg2 time.Time - }{arg1Copy, arg2}) - fake.recordInvocation("ListPullRequests", []interface{}{arg1Copy, arg2}) + }{arg1Copy}) + fake.recordInvocation("ListPullRequests", []interface{}{arg1Copy}) fake.listPullRequestsMutex.Unlock() if fake.ListPullRequestsStub != nil { - return fake.ListPullRequestsStub(arg1, arg2) + return fake.ListPullRequestsStub(arg1) } if specificReturn { return ret.result1, ret.result2 @@ -389,17 +386,17 @@ func (fake *FakeGithub) ListPullRequestsCallCount() int { return len(fake.listPullRequestsArgsForCall) } -func (fake *FakeGithub) ListPullRequestsCalls(stub func([]githubv4.PullRequestState, time.Time) ([]*resource.PullRequest, error)) { +func (fake *FakeGithub) ListPullRequestsCalls(stub func([]githubv4.PullRequestState) ([]*resource.PullRequest, error)) { fake.listPullRequestsMutex.Lock() defer fake.listPullRequestsMutex.Unlock() fake.ListPullRequestsStub = stub } -func (fake *FakeGithub) ListPullRequestsArgsForCall(i int) ([]githubv4.PullRequestState, time.Time) { +func (fake *FakeGithub) ListPullRequestsArgsForCall(i int) []githubv4.PullRequestState { fake.listPullRequestsMutex.RLock() defer fake.listPullRequestsMutex.RUnlock() argsForCall := fake.listPullRequestsArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2 + return argsForCall.arg1 } func (fake *FakeGithub) ListPullRequestsReturns(result1 []*resource.PullRequest, result2 error) { diff --git a/github.go b/github.go index 390e81dd..ab10cbdc 100644 --- a/github.go +++ b/github.go @@ -11,7 +11,6 @@ import ( "path" "strconv" "strings" - "time" "github.com/google/go-github/v28/github" "github.com/shurcooL/githubv4" @@ -21,7 +20,7 @@ import ( // Github for testing purposes. //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -o fakes/fake_github.go . Github type Github interface { - ListPullRequests([]githubv4.PullRequestState, time.Time) ([]*PullRequest, error) + ListPullRequests([]githubv4.PullRequestState) ([]*PullRequest, error) ListModifiedFiles(int) ([]string, error) PostComment(string, string) error GetPullRequest(string, string) (*PullRequest, error) @@ -99,18 +98,16 @@ func NewGithubClient(s *Source) (*GithubClient, error) { } // ListPullRequests gets the last commit on all pull requests with the matching state. -func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState, since time.Time) ([]*PullRequest, error) { - var prSearch struct { - Search struct { - Edges []struct { - Node struct { - PullRequest struct { +func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState) ([]*PullRequest, error) { + var query struct { + Repository struct { + PullRequests struct { + Edges []struct { + Node struct { PullRequestObject - Reviews struct { TotalCount int } `graphql:"reviews(states: $prReviewStates)"` - Commits struct { Edges []struct { Node struct { @@ -118,7 +115,6 @@ func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState, si } } } `graphql:"commits(last:$commitsLast)"` - Labels struct { Edges []struct { Node struct { @@ -126,59 +122,51 @@ func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState, si } } } `graphql:"labels(first:$labelsFirst)"` - } `graphql:"... on PullRequest"` + } } - } - PageInfo struct { - EndCursor githubv4.String - HasNextPage bool - } - } `graphql:"search(query:$query,type:ISSUE,last:$prFirst,after:$prCursor)"` - } - - query := fmt.Sprintf("is:pr repo:%s/%s", m.Owner, m.Repository) - - for _, state := range prStates { - query += strings.ToLower(fmt.Sprintf(" state:%s", state)) - } - - if !since.IsZero() { - query += fmt.Sprintf(" updated:>=%s", since.Format(time.RFC3339)) + PageInfo struct { + EndCursor githubv4.String + HasNextPage bool + } + } `graphql:"pullRequests(first:$prFirst,states:$prStates,after:$prCursor)"` + } `graphql:"repository(owner:$repositoryOwner,name:$repositoryName)"` } vars := map[string]interface{}{ - "prFirst": githubv4.Int(100), - "prCursor": (*githubv4.String)(nil), - "prReviewStates": []githubv4.PullRequestReviewState{githubv4.PullRequestReviewStateApproved}, - "commitsLast": githubv4.Int(1), - "labelsFirst": githubv4.Int(100), - "query": githubv4.String(query), + "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), } var response []*PullRequest for { - if err := m.V4.Query(context.TODO(), &prSearch, vars); err != nil { + if err := m.V4.Query(context.TODO(), &query, vars); err != nil { return nil, err } - for _, p := range prSearch.Search.Edges { - labels := make([]LabelObject, len(p.Node.PullRequest.Labels.Edges)) - for _, l := range p.Node.PullRequest.Labels.Edges { + for _, p := range query.Repository.PullRequests.Edges { + labels := make([]LabelObject, len(p.Node.Labels.Edges)) + for _, l := range p.Node.Labels.Edges { labels = append(labels, l.Node.LabelObject) } - for _, c := range p.Node.PullRequest.Commits.Edges { + for _, c := range p.Node.Commits.Edges { response = append(response, &PullRequest{ - PullRequestObject: p.Node.PullRequest.PullRequestObject, + PullRequestObject: p.Node.PullRequestObject, Tip: c.Node.Commit, - ApprovedReviewCount: p.Node.PullRequest.Reviews.TotalCount, + ApprovedReviewCount: p.Node.Reviews.TotalCount, Labels: labels, }) } } - if !prSearch.Search.PageInfo.HasNextPage { + if !query.Repository.PullRequests.PageInfo.HasNextPage { break } - vars["prCursor"] = prSearch.Search.PageInfo.EndCursor + vars["prCursor"] = query.Repository.PullRequests.PageInfo.EndCursor } return response, nil }