Skip to content

Commit fd89b79

Browse files
ctreatmabmalehorn
authored andcommitted
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. (cherry picked from commit 50ef79a)
1 parent f819803 commit fd89b79

File tree

3 files changed

+29
-19
lines changed

3 files changed

+29
-19
lines changed

check.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,6 @@ Loop:
6969
}
7070
}
7171

72-
// Filter out commits that are too old.
73-
if !versionTime.After(request.Version.CommittedDate) {
74-
continue
75-
}
76-
7772
// Filter out pull request if it does not contain at least one of the desired labels
7873
if len(request.Source.Labels) > 0 {
7974
labelFound := false

check_test.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func TestCheck(t *testing.T) {
6464
},
6565

6666
{
67-
description: "check returns the previous version when its still latest",
67+
description: "check returns all open PRs if there is a previous",
6868
source: resource.Source{
6969
Repository: "itsdalmo/test-repository",
7070
AccessToken: "oauthtoken",
@@ -73,7 +73,15 @@ func TestCheck(t *testing.T) {
7373
pullRequests: testPullRequests,
7474
files: [][]string{},
7575
expected: resource.CheckResponse{
76+
resource.NewVersion(testPullRequests[8], testPullRequests[8].Tip.CommittedDate.Time),
77+
resource.NewVersion(testPullRequests[7], testPullRequests[7].Tip.CommittedDate.Time),
78+
resource.NewVersion(testPullRequests[6], testPullRequests[6].Tip.CommittedDate.Time),
79+
resource.NewVersion(testPullRequests[5], testPullRequests[5].Tip.CommittedDate.Time),
80+
resource.NewVersion(testPullRequests[4], testPullRequests[4].Tip.CommittedDate.Time),
81+
resource.NewVersion(testPullRequests[3], testPullRequests[3].Tip.CommittedDate.Time),
82+
resource.NewVersion(testPullRequests[2], testPullRequests[2].Tip.CommittedDate.Time),
7683
resource.NewVersion(testPullRequests[1], testPullRequests[1].Tip.CommittedDate.Time),
84+
7785
},
7886
},
7987

@@ -107,6 +115,7 @@ func TestCheck(t *testing.T) {
107115
{"terraform/modules/variables.tf", "travis.yml"},
108116
},
109117
expected: resource.CheckResponse{
118+
resource.NewVersion(testPullRequests[3], testPullRequests[3].Tip.CommittedDate.Time),
110119
resource.NewVersion(testPullRequests[2], testPullRequests[2].Tip.CommittedDate.Time),
111120
},
112121
},
@@ -126,6 +135,7 @@ func TestCheck(t *testing.T) {
126135
{"terraform/modules/variables.tf", "travis.yml"},
127136
},
128137
expected: resource.CheckResponse{
138+
resource.NewVersion(testPullRequests[3], testPullRequests[3].Tip.CommittedDate.Time),
129139
resource.NewVersion(testPullRequests[2], testPullRequests[2].Tip.CommittedDate.Time),
130140
},
131141
},
@@ -140,6 +150,14 @@ func TestCheck(t *testing.T) {
140150
version: resource.NewVersion(testPullRequests[1], testPullRequests[1].Tip.CommittedDate.Time),
141151
pullRequests: testPullRequests,
142152
expected: resource.CheckResponse{
153+
resource.NewVersion(testPullRequests[8], testPullRequests[8].Tip.CommittedDate.Time),
154+
resource.NewVersion(testPullRequests[7], testPullRequests[7].Tip.CommittedDate.Time),
155+
resource.NewVersion(testPullRequests[6], testPullRequests[6].Tip.CommittedDate.Time),
156+
resource.NewVersion(testPullRequests[5], testPullRequests[5].Tip.CommittedDate.Time),
157+
resource.NewVersion(testPullRequests[4], testPullRequests[4].Tip.CommittedDate.Time),
158+
resource.NewVersion(testPullRequests[3], testPullRequests[3].Tip.CommittedDate.Time),
159+
resource.NewVersion(testPullRequests[2], testPullRequests[2].Tip.CommittedDate.Time),
160+
resource.NewVersion(testPullRequests[1], testPullRequests[1].Tip.CommittedDate.Time),
143161
resource.NewVersion(testPullRequests[0], testPullRequests[0].Tip.CommittedDate.Time),
144162
},
145163
},
@@ -183,6 +201,10 @@ func TestCheck(t *testing.T) {
183201
version: resource.NewVersion(testPullRequests[5], testPullRequests[5].Tip.CommittedDate.Time),
184202
pullRequests: testPullRequests,
185203
expected: resource.CheckResponse{
204+
resource.NewVersion(testPullRequests[8], testPullRequests[8].Tip.CommittedDate.Time),
205+
resource.NewVersion(testPullRequests[7], testPullRequests[7].Tip.CommittedDate.Time),
206+
resource.NewVersion(testPullRequests[6], testPullRequests[6].Tip.CommittedDate.Time),
207+
resource.NewVersion(testPullRequests[5], testPullRequests[5].Tip.CommittedDate.Time),
186208
resource.NewVersion(testPullRequests[3], testPullRequests[3].Tip.CommittedDate.Time),
187209
resource.NewVersion(testPullRequests[2], testPullRequests[2].Tip.CommittedDate.Time),
188210
resource.NewVersion(testPullRequests[1], testPullRequests[1].Tip.CommittedDate.Time),

e2e/e2e_test.go

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

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

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

0 commit comments

Comments
 (0)