Skip to content

Commit 2961c65

Browse files
committed
review feedback
1 parent 8dd93e1 commit 2961c65

File tree

3 files changed

+50
-25
lines changed

3 files changed

+50
-25
lines changed

api/queries_pr.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
284284
state
285285
}
286286
...on CheckRun {
287-
name
288287
status
289-
conclusion
290288
}
291289
}
292290
}

pkg/cmd/pr/checks/checks.go

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -89,19 +89,21 @@ func checksRun(opts *ChecksOptions) error {
8989
pending := 0
9090

9191
type output struct {
92-
mark string
93-
bucket string
94-
name string
95-
elapsed string
96-
link string
92+
mark string
93+
bucket string
94+
name string
95+
elapsed string
96+
link string
97+
markColor func(string) string
9798
}
9899

99100
outputs := []output{}
100101

101102
for _, c := range pr.Commits.Nodes[0].Commit.StatusCheckRollup.Contexts.Nodes {
102-
mark := ""
103-
bucket := ""
103+
mark := ""
104+
bucket := "pass"
104105
state := c.State
106+
markColor := utils.Green
105107
if state == "" {
106108
if c.Status == "COMPLETED" {
107109
state = c.Conclusion
@@ -111,15 +113,15 @@ func checksRun(opts *ChecksOptions) error {
111113
}
112114
switch state {
113115
case "SUCCESS", "NEUTRAL", "SKIPPED":
114-
mark = utils.GreenCheck()
115116
passing++
116-
bucket = "pass"
117117
case "ERROR", "FAILURE", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED":
118-
mark = utils.RedX()
118+
mark = "X"
119+
markColor = utils.Red
119120
failing++
120121
bucket = "fail"
121122
case "EXPECTED", "REQUESTED", "QUEUED", "PENDING", "IN_PROGRESS", "STALE":
122-
mark = utils.YellowDash()
123+
mark = "-"
124+
markColor = utils.Yellow
123125
pending++
124126
bucket = "pending"
125127
default:
@@ -146,28 +148,33 @@ func checksRun(opts *ChecksOptions) error {
146148
name = c.Context
147149
}
148150

149-
outputs = append(outputs, output{mark, bucket, name, elapsed, link})
151+
outputs = append(outputs, output{mark, bucket, name, elapsed, link, markColor})
150152
}
151153

152154
sort.Slice(outputs, func(i, j int) bool {
153-
if outputs[i].bucket == outputs[j].bucket {
154-
return outputs[i].name < outputs[j].name
155-
} else {
156-
if outputs[i].bucket == "fail" {
157-
return true
158-
} else if outputs[i].bucket == "pending" && outputs[j].bucket == "success" {
159-
return true
155+
b0 := outputs[i].bucket
156+
n0 := outputs[i].name
157+
l0 := outputs[i].link
158+
b1 := outputs[j].bucket
159+
n1 := outputs[j].name
160+
l1 := outputs[j].link
161+
162+
if b0 == b1 {
163+
if n0 == n1 {
164+
return l0 < l1
165+
} else {
166+
return n0 < n1
160167
}
161168
}
162169

163-
return false
170+
return (b0 == "fail") || (b0 == "pending" && b1 == "success")
164171
})
165172

166173
tp := utils.NewTablePrinter(opts.IO)
167174

168175
for _, o := range outputs {
169176
if opts.IO.IsStdoutTTY() {
170-
tp.AddField(o.mark, nil, nil)
177+
tp.AddField(o.mark, nil, o.markColor)
171178
tp.AddField(o.name, nil, nil)
172179
tp.AddField(o.elapsed, nil, nil)
173180
tp.AddField(o.link, nil, nil)
@@ -207,5 +214,14 @@ func checksRun(opts *ChecksOptions) error {
207214
fmt.Fprintln(opts.IO.Out)
208215
}
209216

210-
return tp.Render()
217+
err = tp.Render()
218+
if err != nil {
219+
return err
220+
}
221+
222+
if failing+pending > 0 {
223+
return cmdutil.SilentError
224+
}
225+
226+
return nil
211227
}

pkg/cmd/pr/checks/checks_test.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ func Test_checksRun(t *testing.T) {
6868
stubs func(*httpmock.Registry)
6969
wantOut string
7070
nontty bool
71+
wantErr bool
7172
}{
7273
{
7374
name: "no commits",
@@ -96,11 +97,13 @@ func Test_checksRun(t *testing.T) {
9697
name: "some failing",
9798
fixture: "./fixtures/someFailing.json",
9899
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",
100+
wantErr: true,
99101
},
100102
{
101103
name: "some pending",
102104
fixture: "./fixtures/somePending.json",
103105
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",
106+
wantErr: true,
104107
},
105108
{
106109
name: "all passing",
@@ -111,6 +114,7 @@ func Test_checksRun(t *testing.T) {
111114
name: "with statuses",
112115
fixture: "./fixtures/withStatuses.json",
113116
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",
117+
wantErr: true,
114118
},
115119
{
116120
name: "no commits",
@@ -142,12 +146,14 @@ func Test_checksRun(t *testing.T) {
142146
nontty: true,
143147
fixture: "./fixtures/someFailing.json",
144148
wantOut: "sad tests\tfail\t1m26s\tsweet link\ncool tests\tpass\t1m26s\tsweet link\nslow tests\tpending\t1m26s\tsweet link\n",
149+
wantErr: true,
145150
},
146151
{
147152
name: "some pending",
148153
nontty: true,
149154
fixture: "./fixtures/somePending.json",
150155
wantOut: "cool tests\tpass\t1m26s\tsweet link\nrad tests\tpass\t1m26s\tsweet link\nslow tests\tpending\t1m26s\tsweet link\n",
156+
wantErr: true,
151157
},
152158
{
153159
name: "all passing",
@@ -160,6 +166,7 @@ func Test_checksRun(t *testing.T) {
160166
nontty: true,
161167
fixture: "./fixtures/withStatuses.json",
162168
wantOut: "a status\tfail\t0\tsweet link\ncool tests\tpass\t1m26s\tsweet link\nrad tests\tpass\t1m26s\tsweet link\n",
169+
wantErr: true,
163170
},
164171
}
165172

@@ -191,7 +198,11 @@ func Test_checksRun(t *testing.T) {
191198
}
192199

193200
err := checksRun(opts)
194-
assert.NoError(t, err)
201+
if tt.wantErr {
202+
assert.Equal(t, "SilentError", err.Error())
203+
} else {
204+
assert.NoError(t, err)
205+
}
195206

196207
assert.Equal(t, tt.wantOut, stdout.String())
197208
reg.Verify(t)

0 commit comments

Comments
 (0)