Skip to content

Commit

Permalink
Fix pr checks command for GHES versions older than 3.9 (#7725)
Browse files Browse the repository at this point in the history
  • Loading branch information
samcoe committed Jul 19, 2023
1 parent 7f3196f commit ad283cf
Show file tree
Hide file tree
Showing 7 changed files with 272 additions and 20 deletions.
10 changes: 7 additions & 3 deletions api/query_builder.go
Expand Up @@ -205,11 +205,15 @@ func StatusCheckRollupGraphQLWithoutCountByState(after string) string {
}`), afterClause)
}

func RequiredStatusCheckRollupGraphQL(prID, after string) string {
func RequiredStatusCheckRollupGraphQL(prID, after string, includeEvent bool) string {
var afterClause string
if after != "" {
afterClause = ",after:" + after
}
eventField := "event,"
if !includeEvent {
eventField = ""
}
return fmt.Sprintf(shortenQuery(`
statusCheckRollup: commits(last: 1) {
nodes {
Expand All @@ -228,7 +232,7 @@ func RequiredStatusCheckRollupGraphQL(prID, after string) string {
},
...on CheckRun {
name,
checkSuite{workflowRun{event,workflow{name}}},
checkSuite{workflowRun{%[3]sworkflow{name}}},
status,
conclusion,
startedAt,
Expand All @@ -242,7 +246,7 @@ func RequiredStatusCheckRollupGraphQL(prID, after string) string {
}
}
}
}`), afterClause, prID)
}`), afterClause, prID, eventField)
}

var IssueFields = []string{
Expand Down
29 changes: 28 additions & 1 deletion internal/featuredetection/feature_detection.go
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghinstance"
"golang.org/x/sync/errgroup"
)

type Detector interface {
Expand All @@ -27,11 +28,13 @@ type PullRequestFeatures struct {
// the checkRunCount, checkRunCountsByState, statusContextCount and stausContextCountsByState
// fields on the StatusCheckRollupContextConnection
CheckRunAndStatusContextCounts bool
CheckRunEvent bool
}

var allPullRequestFeatures = PullRequestFeatures{
MergeQueue: true,
CheckRunAndStatusContextCounts: true,
CheckRunEvent: true,
}

type RepositoryFeatures struct {
Expand Down Expand Up @@ -111,8 +114,26 @@ func (d *detector) PullRequestFeatures() (PullRequestFeatures, error) {
} `graphql:"StatusCheckRollupContextConnection: __type(name: \"StatusCheckRollupContextConnection\")"`
}

// Break feature detection down into two separate queries because the platform
// only supports two `__type` expressions in one query.
var pullRequestFeatureDetection2 struct {
WorkflowRun struct {
Fields []struct {
Name string
} `graphql:"fields(includeDeprecated: true)"`
} `graphql:"WorkflowRun: __type(name: \"WorkflowRun\")"`
}

gql := api.NewClientFromHTTP(d.httpClient)
if err := gql.Query(d.host, "PullRequest_fields", &pullRequestFeatureDetection, nil); err != nil {

var wg errgroup.Group
wg.Go(func() error {
return gql.Query(d.host, "PullRequest_fields", &pullRequestFeatureDetection, nil)
})
wg.Go(func() error {
return gql.Query(d.host, "PullRequest_fields2", &pullRequestFeatureDetection2, nil)
})
if err := wg.Wait(); err != nil {
return PullRequestFeatures{}, err
}

Expand All @@ -132,6 +153,12 @@ func (d *detector) PullRequestFeatures() (PullRequestFeatures, error) {
}
}

for _, field := range pullRequestFeatureDetection2.WorkflowRun.Fields {
if field.Name == "event" {
features.CheckRunEvent = true
}
}

return features, nil
}

Expand Down
48 changes: 45 additions & 3 deletions internal/featuredetection/feature_detection_test.go
Expand Up @@ -82,7 +82,7 @@ func TestPullRequestFeatures(t *testing.T) {
wantErr bool
}{
{
name: "github.com with merge queue and status check counts by state",
name: "github.com with all features",
hostname: "github.com",
queryResponse: map[string]string{
`query PullRequest_fields\b`: heredoc.Doc(`
Expand All @@ -104,10 +104,21 @@ func TestPullRequestFeatures(t *testing.T) {
}
}
}`),
`query PullRequest_fields2\b`: heredoc.Doc(`
{
"data": {
"WorkflowRun": {
"fields": [
{"name": "event"}
]
}
}
}`),
},
wantFeatures: PullRequestFeatures{
MergeQueue: true,
CheckRunAndStatusContextCounts: true,
CheckRunEvent: true,
},
wantErr: false,
},
Expand All @@ -131,15 +142,26 @@ func TestPullRequestFeatures(t *testing.T) {
}
}
}`),
`query PullRequest_fields2\b`: heredoc.Doc(`
{
"data": {
"WorkflowRun": {
"fields": [
{"name": "event"}
]
}
}
}`),
},
wantFeatures: PullRequestFeatures{
MergeQueue: false,
CheckRunAndStatusContextCounts: true,
CheckRunEvent: true,
},
wantErr: false,
},
{
name: "GHE with merge queue and status check counts by state",
name: "GHE with all features",
hostname: "git.my.org",
queryResponse: map[string]string{
`query PullRequest_fields\b`: heredoc.Doc(`
Expand All @@ -161,15 +183,26 @@ func TestPullRequestFeatures(t *testing.T) {
}
}
}`),
`query PullRequest_fields2\b`: heredoc.Doc(`
{
"data": {
"WorkflowRun": {
"fields": [
{"name": "event"}
]
}
}
}`),
},
wantFeatures: PullRequestFeatures{
MergeQueue: true,
CheckRunAndStatusContextCounts: true,
CheckRunEvent: true,
},
wantErr: false,
},
{
name: "GHE without merge queue and status check counts by state",
name: "GHE with no features",
hostname: "git.my.org",
queryResponse: map[string]string{
`query PullRequest_fields\b`: heredoc.Doc(`
Expand All @@ -183,10 +216,19 @@ func TestPullRequestFeatures(t *testing.T) {
}
}
}`),
`query PullRequest_fields2\b`: heredoc.Doc(`
{
"data": {
"WorkflowRun": {
"fields": []
}
}
}`),
},
wantFeatures: PullRequestFeatures{
MergeQueue: false,
CheckRunAndStatusContextCounts: false,
CheckRunEvent: false,
},
wantErr: false,
},
Expand Down
23 changes: 18 additions & 5 deletions pkg/cmd/pr/checks/checks.go
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/browser"
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/text"
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
Expand All @@ -24,7 +25,8 @@ type ChecksOptions struct {
IO *iostreams.IOStreams
Browser browser.Browser

Finder shared.PRFinder
Finder shared.PRFinder
Detector fd.Detector

SelectorArg string
WebMode bool
Expand Down Expand Up @@ -142,8 +144,19 @@ func checksRun(opts *ChecksOptions) error {
var checks []check
var counts checkCounts
var err error
var includeEvent bool

checks, counts, err = populateStatusChecks(client, repo, pr, opts.Required)
if opts.Detector == nil {
cachedClient := api.NewCachedHTTPClient(client, time.Hour*24)
opts.Detector = fd.NewDetector(cachedClient, repo.RepoHost())
}
if features, featuresErr := opts.Detector.PullRequestFeatures(); featuresErr != nil {
return featuresErr
} else {
includeEvent = features.CheckRunEvent
}

checks, counts, err = populateStatusChecks(client, repo, pr, opts.Required, includeEvent)
if err != nil {
return err
}
Expand Down Expand Up @@ -183,7 +196,7 @@ func checksRun(opts *ChecksOptions) error {

time.Sleep(opts.Interval)

checks, counts, err = populateStatusChecks(client, repo, pr, opts.Required)
checks, counts, err = populateStatusChecks(client, repo, pr, opts.Required, includeEvent)
if err != nil {
break
}
Expand All @@ -210,7 +223,7 @@ func checksRun(opts *ChecksOptions) error {
return nil
}

func populateStatusChecks(client *http.Client, repo ghrepo.Interface, pr *api.PullRequest, requiredChecks bool) ([]check, checkCounts, error) {
func populateStatusChecks(client *http.Client, repo ghrepo.Interface, pr *api.PullRequest, requiredChecks bool, includeEvent bool) ([]check, checkCounts, error) {
apiClient := api.NewClientFromHTTP(client)

type response struct {
Expand All @@ -224,7 +237,7 @@ func populateStatusChecks(client *http.Client, repo ghrepo.Interface, pr *api.Pu
%s
}
}
}`, api.RequiredStatusCheckRollupGraphQL("$id", "$endCursor"))
}`, api.RequiredStatusCheckRollupGraphQL("$id", "$endCursor", includeEvent))

variables := map[string]interface{}{
"id": pr.ID,
Expand Down
74 changes: 66 additions & 8 deletions pkg/cmd/pr/checks/checks_test.go
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/browser"
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/run"
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
Expand Down Expand Up @@ -123,14 +124,15 @@ func TestNewCmdChecks(t *testing.T) {

func Test_checksRun(t *testing.T) {
tests := []struct {
name string
tty bool
watch bool
failFast bool
required bool
httpStubs func(*httpmock.Registry)
wantOut string
wantErr string
name string
tty bool
watch bool
failFast bool
required bool
disableDetector bool
httpStubs func(*httpmock.Registry)
wantOut string
wantErr string
}{
{
name: "no commits tty",
Expand Down Expand Up @@ -393,6 +395,54 @@ func Test_checksRun(t *testing.T) {
wantOut: "awesome tests\tpass\t1m26s\tsweet link\tawesome description\ncool tests\tpass\t1m26s\tsweet link\tcool description\nrad tests\tpass\t1m26s\tsweet link\trad description\n",
wantErr: "",
},
{
name: "events tty",
tty: true,
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query PullRequestStatusChecks\b`),
httpmock.FileResponse("./fixtures/withEvents.json"),
)
},
wantOut: "All checks were successful\n0 failing, 2 successful, 0 skipped, and 0 pending checks\n\n✓ tests/cool tests (pull_request) cool description 1m26s sweet link\n✓ tests/cool tests (push) cool description 1m26s sweet link\n",
wantErr: "",
},
{
name: "events not supported tty",
tty: true,
disableDetector: true,
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query PullRequestStatusChecks\b`),
httpmock.FileResponse("./fixtures/withoutEvents.json"),
)
},
wantOut: "All checks were successful\n0 failing, 1 successful, 0 skipped, and 0 pending checks\n\n✓ tests/cool tests cool description 1m26s sweet link\n",
wantErr: "",
},
{
name: "events",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query PullRequestStatusChecks\b`),
httpmock.FileResponse("./fixtures/withEvents.json"),
)
},
wantOut: "cool tests\tpass\t1m26s\tsweet link\tcool description\ncool tests\tpass\t1m26s\tsweet link\tcool description\n",
wantErr: "",
},
{
name: "events not supported",
disableDetector: true,
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query PullRequestStatusChecks\b`),
httpmock.FileResponse("./fixtures/withoutEvents.json"),
)
},
wantOut: "cool tests\tpass\t1m26s\tsweet link\tcool description\n",
wantErr: "",
},
}

for _, tt := range tests {
Expand All @@ -407,14 +457,22 @@ func Test_checksRun(t *testing.T) {
tt.httpStubs(reg)
}

var detector fd.Detector
detector = &fd.EnabledDetectorMock{}
if tt.disableDetector {
detector = &fd.DisabledDetectorMock{}
}

response := &api.PullRequest{Number: 123, HeadRefName: "trunk"}

opts := &ChecksOptions{
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
IO: ios,
SelectorArg: "123",
Finder: shared.NewMockFinder("123", response, ghrepo.New("OWNER", "REPO")),
Detector: detector,
Watch: tt.watch,
FailFast: tt.failFast,
Required: tt.required,
Expand Down

0 comments on commit ad283cf

Please sign in to comment.