Skip to content

Commit

Permalink
Only use the most recent check runs on a commit (#823)
Browse files Browse the repository at this point in the history
This fixes an issue where we would iterate over all check runs on a
commit and keep the last one. Because GitHub returns check runs in
reverse chronological order, this meant we used the oldest result for
each run in the event of duplicates instead of the most recent.
  • Loading branch information
bluekeyes authored Aug 15, 2024
1 parent 38f1378 commit 232ce6d
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 1 deletion.
11 changes: 10 additions & 1 deletion pull/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -819,9 +819,18 @@ func (ghc *GitHubContext) getCheckStatuses() (map[string]string, error) {
if err != nil {
return nil, errors.Wrapf(err, "failed to get check runs for page %d", opt.Page)
}

// Check runs are ordered from most to least recent. In some cases,
// like when a commit is included in multiple PRs or when users re-run
// checks, there may be multiple runs with the same name. We only want
// to keep the first (most recent) result for each name.
for _, checkRun := range checkRuns.CheckRuns {
statuses[checkRun.GetName()] = checkRun.GetConclusion()
name := checkRun.GetName()
if _, exists := statuses[name]; !exists {
statuses[name] = checkRun.GetConclusion()
}
}

if resp.NextPage == 0 {
break
}
Expand Down
24 changes: 24 additions & 0 deletions pull/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,30 @@ func TestLatestWorkflowRuns(t *testing.T) {
assert.Equal(t, 2, runsRule.Count, "incorrect http request count")
}

func TestLatestStatuses(t *testing.T) {
pr := defaultTestPR()

rp := &ResponsePlayer{}
rp.AddRule(
ExactPathMatcher("/repos/testorg/testrepo/commits/"+pr.Head.GetSHA()+"/status"),
"testdata/responses/combined_status_for_ref.yml",
)
rp.AddRule(
ExactPathMatcher("/repos/testorg/testrepo/commits/"+pr.Head.GetSHA()+"/check-runs"),
"testdata/responses/check_runs_for_ref.yml",
)

ctx := makeContext(t, rp, pr, nil)
statuses, err := ctx.LatestStatuses()
require.NoError(t, err)

assert.Len(t, statuses, 4, "incorrect number of statuses")
assert.Equal(t, statuses["commit-status-a"], "success", "incorrect conclusion for 'commit-status-a' status")
assert.Equal(t, statuses["commit-status-b"], "pending", "incorrect conclusion for 'commit-status-a' status")
assert.Equal(t, statuses["check-run-a"], "success", "incorrect conclusion for 'check-run-a' status")
assert.Equal(t, statuses["check-run-b"], "failure", "incorrect conclusion for 'check-run-b' status")
}

func makeContext(t *testing.T, rp *ResponsePlayer, pr *github.PullRequest, gc GlobalCache) Context {
ctx := context.Background()
client := github.NewClient(&http.Client{Transport: rp})
Expand Down
28 changes: 28 additions & 0 deletions pull/testdata/responses/check_runs_for_ref.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
- status: 200
body: |
{
"total_count": 3,
"check_runs": [
{
"status": "completed",
"conclusion": "success",
"started_at": "2024-08-14T12:12:45Z",
"completed_at": "2024-08-14T12:13:14Z",
"name": "check-run-a"
},
{
"status": "completed",
"conclusion": "failure",
"started_at": "2024-08-14T12:10:00Z",
"completed_at": "2024-08-14T12:10:36Z",
"name": "check-run-b"
},
{
"status": "completed",
"conclusion": "failure",
"started_at": "2024-08-14T12:10:00Z",
"completed_at": "2024-08-14T12:10:21Z",
"name": "check-run-a"
}
]
}
19 changes: 19 additions & 0 deletions pull/testdata/responses/combined_status_for_ref.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
- status: 200
body: |
{
"state": "pending",
"statuses": [
{
"state": "pending",
"context": "commit-status-b",
"created_at": "2024-08-14T12:13:14Z",
"updated_at": "2024-08-14T12:13:14Z"
},
{
"state": "success",
"context": "commit-status-a",
"created_at": "2024-08-14T12:12:00Z",
"updated_at": "2024-08-14T12:12:00Z"
}
]
}

0 comments on commit 232ce6d

Please sign in to comment.