diff --git a/api/query_builder.go b/api/query_builder.go index b74f9519b..bd2d2ece5 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -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 { @@ -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, @@ -242,7 +246,7 @@ func RequiredStatusCheckRollupGraphQL(prID, after string) string { } } } - }`), afterClause, prID) + }`), afterClause, prID, eventField) } var IssueFields = []string{ diff --git a/internal/featuredetection/feature_detection.go b/internal/featuredetection/feature_detection.go index 983784804..d289d025f 100644 --- a/internal/featuredetection/feature_detection.go +++ b/internal/featuredetection/feature_detection.go @@ -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 { @@ -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 { @@ -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 } @@ -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 } diff --git a/internal/featuredetection/feature_detection_test.go b/internal/featuredetection/feature_detection_test.go index dda6fe6a5..2a32e9a2d 100644 --- a/internal/featuredetection/feature_detection_test.go +++ b/internal/featuredetection/feature_detection_test.go @@ -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(` @@ -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, }, @@ -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(` @@ -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(` @@ -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, }, diff --git a/pkg/cmd/pr/checks/checks.go b/pkg/cmd/pr/checks/checks.go index fb629d497..6cd47cc98 100644 --- a/pkg/cmd/pr/checks/checks.go +++ b/pkg/cmd/pr/checks/checks.go @@ -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" @@ -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 @@ -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 } @@ -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 } @@ -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 { @@ -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, diff --git a/pkg/cmd/pr/checks/checks_test.go b/pkg/cmd/pr/checks/checks_test.go index a043176aa..e73e15731 100644 --- a/pkg/cmd/pr/checks/checks_test.go +++ b/pkg/cmd/pr/checks/checks_test.go @@ -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" @@ -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", @@ -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 { @@ -407,7 +457,14 @@ 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 @@ -415,6 +472,7 @@ func Test_checksRun(t *testing.T) { IO: ios, SelectorArg: "123", Finder: shared.NewMockFinder("123", response, ghrepo.New("OWNER", "REPO")), + Detector: detector, Watch: tt.watch, FailFast: tt.failFast, Required: tt.required, diff --git a/pkg/cmd/pr/checks/fixtures/withEvents.json b/pkg/cmd/pr/checks/fixtures/withEvents.json new file mode 100644 index 000000000..b617e22d3 --- /dev/null +++ b/pkg/cmd/pr/checks/fixtures/withEvents.json @@ -0,0 +1,55 @@ +{ + "data": { + "node": { + "statusCheckRollup": { + "nodes": [ + { + "commit": { + "oid": "abc", + "statusCheckRollup": { + "contexts": { + "nodes": [ + { + "conclusion": "SUCCESS", + "status": "COMPLETED", + "name": "cool tests", + "description": "cool description", + "completedAt": "2020-08-27T19:00:12Z", + "startedAt": "2020-08-27T18:58:46Z", + "detailsUrl": "sweet link", + "checkSuite": { + "workflowRun": { + "event": "pull_request", + "workflow": { + "name": "tests" + } + } + } + }, + { + "conclusion": "SUCCESS", + "status": "COMPLETED", + "name": "cool tests", + "description": "cool description", + "completedAt": "2020-08-27T19:00:12Z", + "startedAt": "2020-08-27T18:58:46Z", + "detailsUrl": "sweet link", + "checkSuite": { + "workflowRun": { + "event": "push", + "workflow": { + "name": "tests" + } + } + } + } + ] + } + } + } + } + ] + } + } + } +} diff --git a/pkg/cmd/pr/checks/fixtures/withoutEvents.json b/pkg/cmd/pr/checks/fixtures/withoutEvents.json new file mode 100644 index 000000000..f12f82aa8 --- /dev/null +++ b/pkg/cmd/pr/checks/fixtures/withoutEvents.json @@ -0,0 +1,53 @@ +{ + "data": { + "node": { + "statusCheckRollup": { + "nodes": [ + { + "commit": { + "oid": "abc", + "statusCheckRollup": { + "contexts": { + "nodes": [ + { + "conclusion": "SUCCESS", + "status": "COMPLETED", + "name": "cool tests", + "description": "cool description", + "completedAt": "2020-08-27T19:00:12Z", + "startedAt": "2020-08-27T18:58:46Z", + "detailsUrl": "sweet link", + "checkSuite": { + "workflowRun": { + "workflow": { + "name": "tests" + } + } + } + }, + { + "conclusion": "SUCCESS", + "status": "COMPLETED", + "name": "cool tests", + "description": "cool description", + "completedAt": "2020-08-27T19:00:12Z", + "startedAt": "2020-08-27T18:58:46Z", + "detailsUrl": "sweet link", + "checkSuite": { + "workflowRun": { + "workflow": { + "name": "tests" + } + } + } + } + ] + } + } + } + } + ] + } + } + } +}