diff --git a/pkg/cmd/pr/checks/checks.go b/pkg/cmd/pr/checks/checks.go index f267c9854..3c20f97a0 100644 --- a/pkg/cmd/pr/checks/checks.go +++ b/pkg/cmd/pr/checks/checks.go @@ -76,12 +76,12 @@ func checksRun(opts *ChecksOptions) error { } if len(pr.Commits.Nodes) == 0 { - return nil + return fmt.Errorf("no commit found on the pull request") } rollup := pr.Commits.Nodes[0].Commit.StatusCheckRollup.Contexts.Nodes if len(rollup) == 0 { - return nil + return fmt.Errorf("no checks reported on the '%s' branch", pr.BaseRefName) } passing := 0 diff --git a/pkg/cmd/pr/checks/checks_test.go b/pkg/cmd/pr/checks/checks_test.go index 43dbf8cda..324f8a74f 100644 --- a/pkg/cmd/pr/checks/checks_test.go +++ b/pkg/cmd/pr/checks/checks_test.go @@ -66,69 +66,59 @@ func Test_checksRun(t *testing.T) { name string fixture string stubs func(*httpmock.Registry) - wantOut string nontty bool - wantErr bool + wantOut string + wantErr string }{ { name: "no commits", stubs: func(reg *httpmock.Registry) { reg.Register( httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.JSONResponse( - bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 123 } - } } } - `))) + httpmock.StringResponse(` + { "data": { "repository": { + "pullRequest": { "number": 123 } + } } } + `)) }, + wantOut: "", + wantErr: "no commit found on the pull request", }, { name: "no checks", stubs: func(reg *httpmock.Registry) { reg.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "pullRequest": { "number": 123, "commits": { "nodes": [{"commit": {"oid": "abc"}}]} } + "pullRequest": { "number": 123, "commits": { "nodes": [{"commit": {"oid": "abc"}}]}, "baseRefName": "master" } } } } `)) }, + wantOut: "", + wantErr: "no checks reported on the 'master' branch", }, { name: "some failing", fixture: "./fixtures/someFailing.json", wantOut: "Some checks were not successful\n1 failing, 1 successful, and 1 pending checks\n\nX sad tests 1m26s sweet link\n✓ cool tests 1m26s sweet link\n- slow tests 1m26s sweet link\n", - wantErr: true, + wantErr: "SilentError", }, { name: "some pending", fixture: "./fixtures/somePending.json", wantOut: "Some checks are still pending\n0 failing, 2 successful, and 1 pending checks\n\n✓ cool tests 1m26s sweet link\n✓ rad tests 1m26s sweet link\n- slow tests 1m26s sweet link\n", - wantErr: true, + wantErr: "SilentError", }, { name: "all passing", fixture: "./fixtures/allPassing.json", wantOut: "All checks were successful\n0 failing, 3 successful, and 0 pending checks\n\n✓ awesome tests 1m26s sweet link\n✓ cool tests 1m26s sweet link\n✓ rad tests 1m26s sweet link\n", + wantErr: "", }, { name: "with statuses", fixture: "./fixtures/withStatuses.json", wantOut: "Some checks were not successful\n1 failing, 2 successful, and 0 pending checks\n\nX a status sweet link\n✓ cool tests 1m26s sweet link\n✓ rad tests 1m26s sweet link\n", - wantErr: true, - }, - { - name: "no commits", - nontty: true, - stubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.JSONResponse( - bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 123 } - } } } - `))) - }, + wantErr: "SilentError", }, { name: "no checks", @@ -136,37 +126,40 @@ func Test_checksRun(t *testing.T) { stubs: func(reg *httpmock.Registry) { reg.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "pullRequest": { "number": 123, "commits": { "nodes": [{"commit": {"oid": "abc"}}]} } + "pullRequest": { "number": 123, "commits": { "nodes": [{"commit": {"oid": "abc"}}]}, "baseRefName": "master" } } } } `)) }, + wantOut: "", + wantErr: "no checks reported on the 'master' branch", }, { name: "some failing", nontty: true, fixture: "./fixtures/someFailing.json", wantOut: "sad tests\tfail\t1m26s\tsweet link\ncool tests\tpass\t1m26s\tsweet link\nslow tests\tpending\t1m26s\tsweet link\n", - wantErr: true, + wantErr: "SilentError", }, { name: "some pending", nontty: true, fixture: "./fixtures/somePending.json", wantOut: "cool tests\tpass\t1m26s\tsweet link\nrad tests\tpass\t1m26s\tsweet link\nslow tests\tpending\t1m26s\tsweet link\n", - wantErr: true, + wantErr: "SilentError", }, { name: "all passing", nontty: true, fixture: "./fixtures/allPassing.json", wantOut: "awesome tests\tpass\t1m26s\tsweet link\ncool tests\tpass\t1m26s\tsweet link\nrad tests\tpass\t1m26s\tsweet link\n", + wantErr: "", }, { name: "with statuses", nontty: true, fixture: "./fixtures/withStatuses.json", wantOut: "a status\tfail\t0\tsweet link\ncool tests\tpass\t1m26s\tsweet link\nrad tests\tpass\t1m26s\tsweet link\n", - wantErr: true, + wantErr: "SilentError", }, } @@ -184,13 +177,12 @@ func Test_checksRun(t *testing.T) { } reg := &httpmock.Registry{} + defer reg.Verify(t) + if tt.stubs != nil { tt.stubs(reg) } else if tt.fixture != "" { - reg.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.FileResponse(tt.fixture)) - } else { - panic("need either stubs or fixture key") + reg.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.FileResponse(tt.fixture)) } opts.HttpClient = func() (*http.Client, error) { @@ -198,14 +190,13 @@ func Test_checksRun(t *testing.T) { } err := checksRun(opts) - if tt.wantErr { - assert.Equal(t, "SilentError", err.Error()) - } else { - assert.NoError(t, err) + if err != nil { + assert.Equal(t, tt.wantErr, err.Error()) + } else if tt.wantErr != "" { + t.Errorf("expected %q, got nil error", tt.wantErr) } assert.Equal(t, tt.wantOut, stdout.String()) - reg.Verify(t) }) } }