From 03309930f0ddbb4fc2602965d62557749d4fe9cf Mon Sep 17 00:00:00 2001 From: Irzhy Ranaivoarivony Date: Fri, 23 Oct 2020 23:29:49 +0300 Subject: [PATCH 1/2] add message on `pr checks` returns no CI with non-empty PR --- pkg/cmd/pr/checks/checks.go | 4 +- pkg/cmd/pr/checks/checks_test.go | 93 +++++++++++++++++++------------- 2 files changed, 58 insertions(+), 39 deletions(-) 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..90e1ee249 100644 --- a/pkg/cmd/pr/checks/checks_test.go +++ b/pkg/cmd/pr/checks/checks_test.go @@ -63,12 +63,13 @@ func TestNewCmdChecks(t *testing.T) { func Test_checksRun(t *testing.T) { tests := []struct { - name string - fixture string - stubs func(*httpmock.Registry) - wantOut string - nontty bool - wantErr bool + name string + fixture string + stubs func(*httpmock.Registry) + wantOut string + nontty bool + unsuccessfulChecks bool + wantErr bool }{ { name: "no commits", @@ -82,28 +83,34 @@ func Test_checksRun(t *testing.T) { } } } `))) }, + unsuccessfulChecks: true, + wantErr: true, + wantOut: "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" } } } } `)) }, + unsuccessfulChecks: true, + wantErr: true, + wantOut: "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, + 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", + unsuccessfulChecks: true, }, { - 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, + 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", + unsuccessfulChecks: true, }, { name: "all passing", @@ -111,10 +118,10 @@ func Test_checksRun(t *testing.T) { 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", }, { - 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: "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", + unsuccessfulChecks: true, }, { name: "no commits", @@ -129,6 +136,9 @@ func Test_checksRun(t *testing.T) { } } } `))) }, + unsuccessfulChecks: true, + wantErr: true, + wantOut: "no commit found on the pull request", }, { name: "no checks", @@ -136,24 +146,27 @@ 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" } } } } `)) }, + unsuccessfulChecks: true, + wantErr: true, + wantOut: "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, + 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", + unsuccessfulChecks: true, }, { - 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, + 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", + unsuccessfulChecks: true, }, { name: "all passing", @@ -162,11 +175,11 @@ func Test_checksRun(t *testing.T) { wantOut: "awesome tests\tpass\t1m26s\tsweet link\ncool tests\tpass\t1m26s\tsweet link\nrad tests\tpass\t1m26s\tsweet link\n", }, { - 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, + 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", + unsuccessfulChecks: true, }, } @@ -184,6 +197,8 @@ func Test_checksRun(t *testing.T) { } reg := &httpmock.Registry{} + defer reg.Verify(t) + if tt.stubs != nil { tt.stubs(reg) } else if tt.fixture != "" { @@ -198,14 +213,18 @@ func Test_checksRun(t *testing.T) { } err := checksRun(opts) - if tt.wantErr { + if err != nil && tt.wantErr { + assert.Equal(t, tt.wantOut, err.Error()) + return + } + + if tt.unsuccessfulChecks { assert.Equal(t, "SilentError", err.Error()) } else { assert.NoError(t, err) } assert.Equal(t, tt.wantOut, stdout.String()) - reg.Verify(t) }) } } From 6b953f2913c4d6953f752f63888029fc7de9dfa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 29 Oct 2020 13:05:34 +0100 Subject: [PATCH 2/2] Simplify checks test --- pkg/cmd/pr/checks/checks_test.go | 130 ++++++++++++------------------- 1 file changed, 51 insertions(+), 79 deletions(-) diff --git a/pkg/cmd/pr/checks/checks_test.go b/pkg/cmd/pr/checks/checks_test.go index 90e1ee249..324f8a74f 100644 --- a/pkg/cmd/pr/checks/checks_test.go +++ b/pkg/cmd/pr/checks/checks_test.go @@ -63,29 +63,26 @@ func TestNewCmdChecks(t *testing.T) { func Test_checksRun(t *testing.T) { tests := []struct { - name string - fixture string - stubs func(*httpmock.Registry) - wantOut string - nontty bool - unsuccessfulChecks bool - wantErr bool + name string + fixture string + stubs func(*httpmock.Registry) + nontty 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 } + } } } + `)) }, - unsuccessfulChecks: true, - wantErr: true, - wantOut: "no commit found on the pull request", + wantOut: "", + wantErr: "no commit found on the pull request", }, { name: "no checks", @@ -96,49 +93,32 @@ func Test_checksRun(t *testing.T) { } } } `)) }, - unsuccessfulChecks: true, - wantErr: true, - wantOut: "no checks reported on the 'master' branch", + 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", - unsuccessfulChecks: true, + 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: "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", - unsuccessfulChecks: true, + 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: "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", - unsuccessfulChecks: 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 } - } } } - `))) - }, - unsuccessfulChecks: true, - wantErr: true, - wantOut: "no commit found on the pull request", + 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: "SilentError", }, { name: "no checks", @@ -150,36 +130,36 @@ func Test_checksRun(t *testing.T) { } } } `)) }, - unsuccessfulChecks: true, - wantErr: true, - wantOut: "no checks reported on the 'master' branch", + 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", - unsuccessfulChecks: true, + 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: "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", - unsuccessfulChecks: true, + 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: "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", - unsuccessfulChecks: true, + 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: "SilentError", }, } @@ -202,10 +182,7 @@ func Test_checksRun(t *testing.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) { @@ -213,15 +190,10 @@ func Test_checksRun(t *testing.T) { } err := checksRun(opts) - if err != nil && tt.wantErr { - assert.Equal(t, tt.wantOut, err.Error()) - return - } - - if tt.unsuccessfulChecks { - 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())