Skip to content

Commit b6a5ad6

Browse files
committed
Close #26: Return all open PRs instead of filtering by date
This resource can inadvertently miss Pull Requests due to out-of-order commits across PRs. If PR#2 is opened after PR#1, but the head commit of PR#2 is older than the head commit of PR#1, the resource will not include PR#2 in the list of new versions provided to Concourse. Rather than attempt to find a different way of tracking which PRs are "new" given an input version, we can remove the date-based filtering and return all open PRs. Concourse can deduplicate versions based on metadata, which means that we will only trigger new jobs for versions that Concourse hasn't seen before. This makes it easier for teams to use this resource to track PRs in Concourse, since they no longer have to ensure that a PR has a later head commit than all currently-opened PRs in order to notify Concourse that their PR exists.
1 parent a2ec386 commit b6a5ad6

File tree

3 files changed

+27
-32
lines changed

3 files changed

+27
-32
lines changed

check.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,6 @@ Loop:
3333
if request.Source.BaseBranch != "" && p.PullRequestObject.BaseRefName != request.Source.BaseBranch {
3434
continue
3535
}
36-
// Filter out commits that are too old.
37-
if !p.Tip.CommittedDate.Time.After(request.Version.CommittedDate) {
38-
continue
39-
}
4036

4137
// Filter out pull request if it does not contain at least one of the desired labels
4238
if len(request.Source.Labels) > 0 {

check_test.go

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func TestCheck(t *testing.T) {
4646
},
4747

4848
{
49-
description: "check returns the previous version when its still latest",
49+
description: "check returns all open PRs if there is a previous",
5050
source: resource.Source{
5151
Repository: "itsdalmo/test-repository",
5252
AccessToken: "oauthtoken",
@@ -55,20 +55,12 @@ func TestCheck(t *testing.T) {
5555
pullRequests: testPullRequests,
5656
files: [][]string{},
5757
expected: resource.CheckResponse{
58-
resource.NewVersion(testPullRequests[1]),
59-
},
60-
},
61-
62-
{
63-
description: "check returns all new versions since the last",
64-
source: resource.Source{
65-
Repository: "itsdalmo/test-repository",
66-
AccessToken: "oauthtoken",
67-
},
68-
version: resource.NewVersion(testPullRequests[3]),
69-
pullRequests: testPullRequests,
70-
files: [][]string{},
71-
expected: resource.CheckResponse{
58+
resource.NewVersion(testPullRequests[8]),
59+
resource.NewVersion(testPullRequests[7]),
60+
resource.NewVersion(testPullRequests[6]),
61+
resource.NewVersion(testPullRequests[5]),
62+
resource.NewVersion(testPullRequests[4]),
63+
resource.NewVersion(testPullRequests[3]),
7264
resource.NewVersion(testPullRequests[2]),
7365
resource.NewVersion(testPullRequests[1]),
7466
},
@@ -89,6 +81,7 @@ func TestCheck(t *testing.T) {
8981
{"terraform/modules/variables.tf", "travis.yml"},
9082
},
9183
expected: resource.CheckResponse{
84+
resource.NewVersion(testPullRequests[3]),
9285
resource.NewVersion(testPullRequests[2]),
9386
},
9487
},
@@ -108,6 +101,7 @@ func TestCheck(t *testing.T) {
108101
{"terraform/modules/variables.tf", "travis.yml"},
109102
},
110103
expected: resource.CheckResponse{
104+
resource.NewVersion(testPullRequests[3]),
111105
resource.NewVersion(testPullRequests[2]),
112106
},
113107
},
@@ -122,6 +116,14 @@ func TestCheck(t *testing.T) {
122116
version: resource.NewVersion(testPullRequests[1]),
123117
pullRequests: testPullRequests,
124118
expected: resource.CheckResponse{
119+
resource.NewVersion(testPullRequests[8]),
120+
resource.NewVersion(testPullRequests[7]),
121+
resource.NewVersion(testPullRequests[6]),
122+
resource.NewVersion(testPullRequests[5]),
123+
resource.NewVersion(testPullRequests[4]),
124+
resource.NewVersion(testPullRequests[3]),
125+
resource.NewVersion(testPullRequests[2]),
126+
resource.NewVersion(testPullRequests[1]),
125127
resource.NewVersion(testPullRequests[0]),
126128
},
127129
},
@@ -136,6 +138,10 @@ func TestCheck(t *testing.T) {
136138
version: resource.NewVersion(testPullRequests[5]),
137139
pullRequests: testPullRequests,
138140
expected: resource.CheckResponse{
141+
resource.NewVersion(testPullRequests[8]),
142+
resource.NewVersion(testPullRequests[7]),
143+
resource.NewVersion(testPullRequests[6]),
144+
resource.NewVersion(testPullRequests[5]),
139145
resource.NewVersion(testPullRequests[3]),
140146
resource.NewVersion(testPullRequests[2]),
141147
resource.NewVersion(testPullRequests[1]),

e2e/e2e_test.go

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import (
2323
)
2424

2525
var (
26+
firstCommitID = "23dc9f552bf989d1a4aeb65ce23351dee0ec9019"
27+
firstPullRequestID = "3"
28+
firstDateTime = time.Date(2018, time.May, 11, 7, 28, 56, 0, time.UTC)
2629
targetCommitID = "a5114f6ab89f4b736655642a11e8d15ce363d882"
2730
targetPullRequestID = "4"
2831
targetDateTime = time.Date(2018, time.May, 11, 8, 43, 48, 0, time.UTC)
@@ -54,25 +57,15 @@ func TestCheckE2E(t *testing.T) {
5457
},
5558

5659
{
57-
description: "check returns the previous version when its still latest",
60+
description: "check returns all open PRs if there is a previous version",
5861
source: resource.Source{
5962
Repository: "itsdalmo/test-repository",
6063
AccessToken: os.Getenv("GITHUB_ACCESS_TOKEN"),
6164
},
6265
version: resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime},
6366
expected: resource.CheckResponse{
64-
resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime},
65-
},
66-
},
67-
68-
{
69-
description: "check returns all new versions since the last",
70-
source: resource.Source{
71-
Repository: "itsdalmo/test-repository",
72-
AccessToken: os.Getenv("GITHUB_ACCESS_TOKEN"),
73-
},
74-
version: resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime},
75-
expected: resource.CheckResponse{
67+
resource.Version{PR: firstPullRequestID, Commit: firstCommitID, CommittedDate: firstDateTime},
68+
resource.Version{PR: targetPullRequestID, Commit: targetCommitID, CommittedDate: targetDateTime},
7669
resource.Version{PR: latestPullRequestID, Commit: latestCommitID, CommittedDate: latestDateTime},
7770
},
7871
},

0 commit comments

Comments
 (0)