Merge pull request #7454 from cli/wm/pull-request-checks-status-confidence
Get some additional confidence around PullRequest ChecksStatus
This commit is contained in:
William Martin
GitHub
commit
5ec494f00d
2 changed files with 293 additions and 30 deletions
|
|
@ -2,42 +2,295 @@ package api
|
|||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestPullRequest_ChecksStatus(t *testing.T) {
|
||||
pr := PullRequest{}
|
||||
func TestChecksStatus_NoCheckRunsOrStatusContexts(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
payload := `
|
||||
{ "statusCheckRollup": { "nodes": [] } }
|
||||
`
|
||||
var pr PullRequest
|
||||
require.NoError(t, json.Unmarshal([]byte(payload), &pr))
|
||||
|
||||
expectedChecksStatus := PullRequestChecksStatus{
|
||||
Pending: 0,
|
||||
Failing: 0,
|
||||
Passing: 0,
|
||||
Total: 0,
|
||||
}
|
||||
require.Equal(t, expectedChecksStatus, pr.ChecksStatus())
|
||||
}
|
||||
|
||||
func TestChecksStatus_SummarisingCheckRuns(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
payload string
|
||||
expectedChecksStatus PullRequestChecksStatus
|
||||
}{
|
||||
{
|
||||
name: "QUEUED is treated as Pending",
|
||||
payload: singleCheckRunWithStatus("QUEUED"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "IN_PROGRESS is treated as Pending",
|
||||
payload: singleCheckRunWithStatus("IN_PROGRESS"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "WAITING is treated as Pending",
|
||||
payload: singleCheckRunWithStatus("WAITING"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "PENDING is treated as Pending",
|
||||
payload: singleCheckRunWithStatus("PENDING"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "REQUESTED is treated as Pending",
|
||||
payload: singleCheckRunWithStatus("REQUESTED"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "COMPLETED / STARTUP_FAILURE is treated as Pending",
|
||||
payload: singleCompletedCheckRunWithConclusion("STARTUP_FAILURE"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "COMPLETED / STALE is treated as Pending",
|
||||
payload: singleCompletedCheckRunWithConclusion("STALE"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "COMPLETED / SUCCESS is treated as Passing",
|
||||
payload: singleCompletedCheckRunWithConclusion("SUCCESS"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Passing: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "COMPLETED / NEUTRAL is treated as Passing",
|
||||
payload: singleCompletedCheckRunWithConclusion("NEUTRAL"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Passing: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "COMPLETED / SKIPPED is treated as Passing",
|
||||
payload: singleCompletedCheckRunWithConclusion("SKIPPED"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Passing: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "COMPLETED / ACTION_REQUIRED is treated as Failing",
|
||||
payload: singleCompletedCheckRunWithConclusion("ACTION_REQUIRED"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Failing: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "COMPLETED / TIMED_OUT is treated as Failing",
|
||||
payload: singleCompletedCheckRunWithConclusion("TIMED_OUT"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Failing: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "COMPLETED / CANCELLED is treated as Failing",
|
||||
payload: singleCompletedCheckRunWithConclusion("CANCELLED"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Failing: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "COMPLETED / CANCELLED is treated as Failing",
|
||||
payload: singleCompletedCheckRunWithConclusion("CANCELLED"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Failing: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "COMPLETED / FAILURE is treated as Failing",
|
||||
payload: singleCompletedCheckRunWithConclusion("FAILURE"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Failing: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "Unrecognized Status are treated as Pending",
|
||||
payload: singleCheckRunWithStatus("AnUnrecognizedStatusJustForThisTest"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "Unrecognized Conclusions are treated as Pending",
|
||||
payload: singleCompletedCheckRunWithConclusion("AnUnrecognizedConclusionJustForThisTest"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
tt := tt
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var pr PullRequest
|
||||
require.NoError(t, json.Unmarshal([]byte(tt.payload), &pr))
|
||||
|
||||
require.Equal(t, tt.expectedChecksStatus, pr.ChecksStatus())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestChecksStatus_SummarisingStatusContexts(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
payload string
|
||||
expectedChecksStatus PullRequestChecksStatus
|
||||
}{
|
||||
{
|
||||
name: "EXPECTED is treated as Pending",
|
||||
payload: singleStatusContextWithState("EXPECTED"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "PENDING is treated as Pending",
|
||||
payload: singleStatusContextWithState("PENDING"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "SUCCESS is treated as Passing",
|
||||
payload: singleStatusContextWithState("SUCCESS"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Passing: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "ERROR is treated as Failing",
|
||||
payload: singleStatusContextWithState("ERROR"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Failing: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "FAILURE is treated as Failing",
|
||||
payload: singleStatusContextWithState("FAILURE"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Failing: 1, Total: 1},
|
||||
},
|
||||
{
|
||||
name: "Unrecognized States are treated as Pending",
|
||||
payload: singleStatusContextWithState("AnUnrecognizedStateJustForThisTest"),
|
||||
expectedChecksStatus: PullRequestChecksStatus{Pending: 1, Total: 1},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
tt := tt
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var pr PullRequest
|
||||
require.NoError(t, json.Unmarshal([]byte(tt.payload), &pr))
|
||||
|
||||
require.Equal(t, tt.expectedChecksStatus, pr.ChecksStatus())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestChecksStatus_SummarisingCheckRunsAndStatusContexts(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// This might look a bit intimidating, but we're just inserting three nodes
|
||||
// into the rollup, two completed check run nodes and one status context node.
|
||||
payload := fmt.Sprintf(`
|
||||
{ "statusCheckRollup": { "nodes": [{ "commit": {
|
||||
"statusCheckRollup": {
|
||||
"contexts": {
|
||||
"nodes": [
|
||||
{ "state": "SUCCESS" },
|
||||
{ "state": "PENDING" },
|
||||
{ "state": "FAILURE" },
|
||||
{ "status": "IN_PROGRESS",
|
||||
"conclusion": null },
|
||||
{ "status": "COMPLETED",
|
||||
"conclusion": "SUCCESS" },
|
||||
{ "status": "COMPLETED",
|
||||
"conclusion": "FAILURE" },
|
||||
{ "status": "COMPLETED",
|
||||
"conclusion": "ACTION_REQUIRED" },
|
||||
{ "status": "COMPLETED",
|
||||
"conclusion": "STALE" }
|
||||
%s,
|
||||
%s,
|
||||
%s
|
||||
]
|
||||
}
|
||||
}
|
||||
} }] } }
|
||||
`
|
||||
err := json.Unmarshal([]byte(payload), &pr)
|
||||
assert.NoError(t, err)
|
||||
`,
|
||||
completedCheckRunNode("SUCCESS"),
|
||||
statusContextNode("PENDING"),
|
||||
completedCheckRunNode("FAILURE"),
|
||||
)
|
||||
|
||||
checks := pr.ChecksStatus()
|
||||
assert.Equal(t, 8, checks.Total)
|
||||
assert.Equal(t, 3, checks.Pending)
|
||||
assert.Equal(t, 3, checks.Failing)
|
||||
assert.Equal(t, 2, checks.Passing)
|
||||
var pr PullRequest
|
||||
require.NoError(t, json.Unmarshal([]byte(payload), &pr))
|
||||
|
||||
expectedChecksStatus := PullRequestChecksStatus{
|
||||
Pending: 1,
|
||||
Failing: 1,
|
||||
Passing: 1,
|
||||
Total: 3,
|
||||
}
|
||||
require.Equal(t, expectedChecksStatus, pr.ChecksStatus())
|
||||
}
|
||||
|
||||
// Note that it would be incorrect to provide a status of COMPLETED here
|
||||
// as the conclusion is always set to null. If you want a COMPLETED status,
|
||||
// use `singleCompletedCheckRunWithConclusion`.
|
||||
func singleCheckRunWithStatus(status string) string {
|
||||
return fmt.Sprintf(`
|
||||
{ "statusCheckRollup": { "nodes": [{ "commit": {
|
||||
"statusCheckRollup": {
|
||||
"contexts": {
|
||||
"nodes": [
|
||||
{
|
||||
"__typename": "CheckRun",
|
||||
"status": "%s",
|
||||
"conclusion": null
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
} }] } }
|
||||
`, status)
|
||||
}
|
||||
|
||||
func singleCompletedCheckRunWithConclusion(conclusion string) string {
|
||||
return fmt.Sprintf(`
|
||||
{ "statusCheckRollup": { "nodes": [{ "commit": {
|
||||
"statusCheckRollup": {
|
||||
"contexts": {
|
||||
"nodes": [
|
||||
{
|
||||
"__typename": "CheckRun",
|
||||
"status": "COMPLETED",
|
||||
"conclusion": "%s"
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
} }] } }
|
||||
`, conclusion)
|
||||
}
|
||||
|
||||
func singleStatusContextWithState(state string) string {
|
||||
return fmt.Sprintf(`
|
||||
{ "statusCheckRollup": { "nodes": [{ "commit": {
|
||||
"statusCheckRollup": {
|
||||
"contexts": {
|
||||
"nodes": [
|
||||
{
|
||||
"__typename": "StatusContext",
|
||||
"state": "%s"
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
} }] } }
|
||||
`, state)
|
||||
}
|
||||
|
||||
func completedCheckRunNode(conclusion string) string {
|
||||
return fmt.Sprintf(`
|
||||
{
|
||||
"__typename": "CheckRun",
|
||||
"status": "COMPLETED",
|
||||
"conclusion": "%s"
|
||||
}`, conclusion)
|
||||
}
|
||||
|
||||
func statusContextNode(state string) string {
|
||||
return fmt.Sprintf(`
|
||||
{
|
||||
"__typename": "StatusContext",
|
||||
"state": "%s"
|
||||
}`, state)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -261,15 +261,23 @@ type PullRequestChecksStatus struct {
|
|||
Total int
|
||||
}
|
||||
|
||||
func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) {
|
||||
func (pr *PullRequest) ChecksStatus() PullRequestChecksStatus {
|
||||
var summary PullRequestChecksStatus
|
||||
|
||||
if len(pr.StatusCheckRollup.Nodes) == 0 {
|
||||
return
|
||||
return summary
|
||||
}
|
||||
|
||||
commit := pr.StatusCheckRollup.Nodes[0].Commit
|
||||
for _, c := range commit.StatusCheckRollup.Contexts.Nodes {
|
||||
state := c.State // StatusContext
|
||||
// Nodes are a discriminated union of CheckRun or StatusContext,
|
||||
// but we can use the State field to disambiguate, rather than using the TypeName.
|
||||
// First we try to get the State, as if we have a StatusContext
|
||||
state := c.State
|
||||
// But if the State is empty, then we assume we actually have a CheckRun
|
||||
if state == "" {
|
||||
// CheckRun
|
||||
// If the Status of a CheckRun is COMPLETED, then we want to look more closely
|
||||
// at the Conclusion, as that contains the relevant information.
|
||||
if c.Status == "COMPLETED" {
|
||||
state = c.Conclusion
|
||||
} else {
|
||||
|
|
@ -281,13 +289,15 @@ func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) {
|
|||
summary.Passing++
|
||||
case "ERROR", "FAILURE", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED":
|
||||
summary.Failing++
|
||||
default: // "EXPECTED", "REQUESTED", "WAITING", "QUEUED", "PENDING", "IN_PROGRESS", "STALE"
|
||||
// We treat anything that isn't Passing or Failing as Pending, which included any future unknown states
|
||||
// we might get back from the API.
|
||||
default: // "EXPECTED", "REQUESTED", "WAITING", "QUEUED", "PENDING", "IN_PROGRESS", "STALE", "STARTUP_FAILURE"
|
||||
summary.Pending++
|
||||
}
|
||||
summary.Total++
|
||||
}
|
||||
|
||||
return
|
||||
return summary
|
||||
}
|
||||
|
||||
func (pr *PullRequest) DisplayableReviews() PullRequestReviews {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue