fix(pr): report checks like gh, exit 8 while they are pending

tea pr checks printed relative links, "Error: pull request checks have
not all succeeded" with exit 1 for checks that were only pending, and
nothing at all without checks. It now heads the table with a colored
verdict and "0 failing, 1 successful, and 2 pending checks", prints a
status icon and absolute links, and exits 8 while checks are pending
and 1 when one failed, without an Error line (D21). clierr.Quiet
carries such an exit code past main without a message. Without checks
it says "no checks reported for pull request alice/x#8".

--watch redraws the summary in place on a terminal, and an expired
--timeout reads "timed out after 3s waiting for 2 pending checks". The
pull request argument is optional, and --web opens the pull request.

Signed-off-by: bircni <bircni@icloud.com>

internal/app/app_test.go

@@ -457,7 +457,6 @@ const (
 	pr attachment upload
 	pr attachment view
 	pr checkout
-	pr checks
 	pr clean
 	pr close
 	pr comment create

internal/app/checks.go

@@ -7,6 +7,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
+	"strings"
 	"time"
 
 	"gitea.dev/tea/internal/clierr"
@@ -16,53 +18,169 @@ import (
 )
 
 func (a *application) pullChecksCommand() *cobra.Command {
-	var watch bool
+	var watch, web bool
 	var interval, timeout time.Duration
-	cmd := &cobra.Command{Use: "checks <number>", Short: "Show the latest commit-status checks for a pull request", Args: argsExact(1), RunE: func(cmd *cobra.Command, args []string) error {
+	cmd := &cobra.Command{Use: "checks [<number> | <url> | <branch>]", Short: "Show the status checks of a pull request", Long: `Show the latest status of each check of a pull request's head commit.
+
+tea exits with 8 while checks are pending and with 1 when a check failed, like gh. With
+--watch, it waits until every check finished, or one failed.`, Annotations: pullArgumentsAnnotation(nil), Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error {
 		if interval <= 0 || timeout <= 0 {
 			return clierr.Wrap(clierr.Usage, errors.New("--interval and --timeout must be positive"))
 		}
-		n, err := number(args[0])
+		t, n, err := a.pullSelector(cmd, args)
 		if err != nil {
 			return err
 		}
-		target, err := a.resolve(cmd.Context(), true)
+		service := services.Pulls{Client: t.client, Repository: t.Repo}
-		if err != nil {
+		if web {
-			return err
+			item, err := service.View(cmd.Context(), n)
+			if err != nil {
+				return err
+			}
+			return a.openBrowser(cmd, item.URL)
 		}
-		service := services.Pulls{Client: target.client, Repository: target.Repo}
 		var items []services.Check
 		if watch {
 			ctx, cancel := context.WithTimeout(cmd.Context(), timeout)
 			defer cancel()
-			items, err = service.WatchChecks(ctx, n, interval, func(items []services.Check) error {
+			redraw := a.checksRedraw(cmd, interval)
-				pending := 0
+			var latest []services.Check
-				for _, item := range items {
+			items, err = service.WatchChecks(ctx, n, interval, func(current []services.Check) error {
-					if item.Pending() {
+				latest = current
-						pending++
+				return redraw(current)
-					}
-				}
-				_, err := fmt.Fprintf(cmd.ErrOrStderr(), "checks: %d total, %d pending\n", len(items), pending)
-				return err
 			})
-		} else {
+			if err != nil {
-			items, err = service.Checks(cmd.Context(), n)
+				waiting := "checks to be reported"
-		}
+				if len(latest) > 0 {
-		if err != nil {
+					waiting = plural(pendingChecks(latest), "pending check")
-			return err
+				}
-		}
+				return timeoutError(err, timeout, "waiting for %s", waiting)
-		if err := writeList(a, cmd, items, func(item services.Check) []terminal.Cell { return terminal.Row(item.Name, item.State, item.URL) }); err != nil {
-			return err
-		}
-		for _, item := range items {
-			if !item.Passed() {
-				return errors.New("pull request checks have not all succeeded")
 			}
+			if a.deps.Streams.OutTTY && a.options.json == "" {
+				return checksOutcome(items)
+			}
+		} else if items, err = service.Checks(cmd.Context(), n); err != nil {
+			return err
 		}
-		return nil
+		if a.options.json != "" {
+			if err := a.writeJSON(cmd, items); err != nil {
+				return err
+			}
+			return checksOutcome(items)
+		}
+		if len(items) == 0 {
+			if a.deps.Streams.OutTTY {
+				_, err = fmt.Fprintf(cmd.ErrOrStderr(), "no checks reported for pull request %s#%d\n", t.Repo, n)
+			}
+			return err
+		}
+		if err := a.writeHuman(cmd, a.checksText(items)); err != nil {
+			return err
+		}
+		return checksOutcome(items)
 	}}
-	cmd.Flags().BoolVar(&watch, "watch", false, "Wait for checks to finish, failing immediately on unsuccessful status")
+	cmd.Flags().BoolVar(&watch, "watch", false, "Wait for the checks to finish, stopping at the first failure")
-	cmd.Flags().DurationVar(&interval, "interval", 5*time.Second, "Check polling interval")
+	cmd.Flags().DurationVar(&interval, "interval", 5*time.Second, "Time between refreshes with --watch")
-	cmd.Flags().DurationVar(&timeout, "timeout", time.Hour, "Maximum wait duration")
+	cmd.Flags().DurationVar(&timeout, "timeout", time.Hour, "Longest time to wait with --watch")
+	cmd.Flags().BoolVarP(&web, "web", "w", false, "Open the pull request in the browser")
+	cmd.ValidArgsFunction = a.completePulls("open")
+	cmd.Example = `# Show the checks of the current branch's pull request
+$ tea pr checks
+
+# Wait until the checks of #13 finish, then merge it
+$ tea pr checks 13 --watch && tea pr merge 13`
 	return cmd
 }
+
+// checksOutcome is the exit status of checks: 1 when one failed, 8 while any is pending.
+func checksOutcome(items []services.Check) error {
+	pending := false
+	for _, item := range items {
+		switch {
+		case item.Pending():
+			pending = true
+		case !item.Passed():
+			return clierr.Quiet(clierr.Failure)
+		}
+	}
+	if pending {
+		return clierr.Quiet(clierr.Pending)
+	}
+	return nil
+}
+
+// pendingChecks counts the checks that have not finished.
+func pendingChecks(items []services.Check) int {
+	pending := 0
+	for _, item := range items {
+		if item.Pending() {
+			pending++
+		}
+	}
+	return pending
+}
+
+// checkCounts words how the checks stand: "0 failing, 1 successful, and 2 pending checks".
+func checkCounts(items []services.Check) string {
+	var failing, passed, pending int
+	for _, item := range items {
+		switch {
+		case item.Pending():
+			pending++
+		case item.Passed():
+			passed++
+		default:
+			failing++
+		}
+	}
+	return fmt.Sprintf("%d failing, %d successful, and %d pending checks", failing, passed, pending)
+}
+
+// checksText is the human summary of checks: a colored verdict, the counts and a table.
+func (a *application) checksText(items []services.Check) string {
+	d := a.display()
+	verdict, color := "All checks were successful", "green"
+	switch err := checksOutcome(items); {
+	case clierr.ExitCode(err) == clierr.Failure:
+		verdict, color = "Some checks were not successful", "red"
+	case err != nil:
+		verdict, color = "Some checks are still pending", "yellow"
+	}
+	rows := make([][]terminal.Cell, len(items))
+	for i, item := range items {
+		rows[i] = []terminal.Cell{terminal.StatusIcon(item.State, ""), {Text: item.Name}, {Text: item.State}, {Text: item.Description}, {Text: item.URL}}
+	}
+	columns := []terminal.Column{{TTYOnly: true}, {Header: "Name", Flex: true, Min: 16}, {Header: "Status"}, {Header: "Description", Flex: true}, {Header: "Link"}}
+	table := terminal.Table(columns, rows, d)
+	if !d.TTY {
+		return table
+	}
+	return terminal.Colorize(color, verdict, d.Color) + "\n" + checkCounts(items) + "\n\n" + table
+}
+
+// checksRedraw reports each change of the checks while --watch waits: on a terminal it redraws the
+// summary in place; otherwise it writes a line of counts to stderr.
+func (a *application) checksRedraw(cmd *cobra.Command, interval time.Duration) func([]services.Check) error {
+	if !a.deps.Streams.OutTTY || a.options.json != "" {
+		return func(items []services.Check) error {
+			_, err := fmt.Fprintf(cmd.ErrOrStderr(), "checks: %s\n", checkCounts(items))
+			return err
+		}
+	}
+	lines := 0
+	return func(items []services.Check) error {
+		text := "Refreshing every " + shortDuration(interval) + ". Press Ctrl+C to quit.\n\n"
+		if len(items) == 0 {
+			text += "Waiting for checks to be reported…\n"
+		} else {
+			text += a.checksText(items)
+		}
+		clear := ""
+		if lines > 0 {
+			clear = fmt.Sprintf("\x1b[%dA\x1b[J", lines)
+		}
+		lines = strings.Count(text, "\n")
+		_, err := io.WriteString(cmd.OutOrStdout(), clear+text)
+		return err
+	}
+}

internal/app/checks_test.go

@@ -6,6 +6,7 @@ package app
 import (
 	"context"
 	"net/http"
+	"strings"
 	"testing"
 
 	"gitea.dev/tea/internal/clierr"
@@ -38,7 +39,8 @@ func TestChecksUseLatestContextAcrossPagesAndReturnFailureWithCleanJSON(t *testi
 	out.Reset()
 	state = "pending"
 	err := Execute(context.Background(), deps, args)
-	require.ErrorContains(t, err, "not all succeeded")
+	require.Equal(t, clierr.Pending, clierr.ExitCode(err))
+	require.True(t, clierr.IsQuiet(err))
 	require.JSONEq(t, `[{"id":2,"name":"build","state":"pending"}]`, out.String())
 	out.Reset()
 	state = "skipped"
@@ -84,3 +86,69 @@ func TestChecksWatchWaitsForRegistrationAndEmitsOneJSONResult(t *testing.T) {
 	require.ErrorIs(t, err, context.Canceled)
 	require.Equal(t, clierr.Canceled, clierr.ExitCode(err))
 }
+
+func TestChecksSummaryAndExitCodes(t *testing.T) {
+	for _, tc := range []struct {
+		name, statuses, want string
+		code                 int
+	}{
+		{
+			name:     "pending",
+			statuses: `[{"id":1,"context":"ci / test (pull_request)","status":"pending","description":"Waiting to run","target_url":"/alice/x/actions/runs/18/jobs/18"}]`,
+			want: "Some checks are still pending\n0 failing, 0 successful, and 1 pending checks\n\n" +
+				"   NAME                      STATUS   DESCRIPTION     LINK\n" +
+				"*  ci / test (pull_request)  pending  Waiting to run  SERVER/alice/x/actions/runs/18/jobs/18\n",
+			code: clierr.Pending,
+		},
+		{
+			name:     "failing",
+			statuses: `[{"id":1,"context":"lint","status":"failure"},{"id":2,"context":"test","status":"success"}]`,
+			want:     "Some checks were not successful\n1 failing, 1 successful, and 0 pending checks\n\n   NAME  STATUS   DESCRIPTION  LINK\nX  lint  failure\n✓  test  success\n",
+			code:     clierr.Failure,
+		},
+		{name: "none", statuses: `[]`},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				switch {
+				case r.URL.Path == "/api/v1/repos/alice/x/pulls/8":
+					_, _ = w.Write([]byte(`{"number":8,"head":{"sha":"abc"}}`))
+				case r.URL.Query().Get("page") == "1":
+					_, _ = w.Write([]byte(tc.statuses))
+				default:
+					_, _ = w.Write([]byte(`[]`))
+				}
+			}))
+			defer server.Close()
+			deps, out, diagnostics := serverDeps(t, server.URL)
+			plainTTY(&deps, server.URL)
+			err := Execute(context.Background(), deps, []string{"pr", "checks", "8", "-R", "alice/x"})
+			require.Equal(t, tc.code, clierr.ExitCode(err))
+			if tc.code != 0 {
+				require.True(t, clierr.IsQuiet(err), "the summary already reports the outcome")
+			}
+			require.Equal(t, strings.ReplaceAll(tc.want, "SERVER", server.URL), out.String())
+			if tc.want == "" {
+				require.Equal(t, "no checks reported for pull request alice/x#8\n", diagnostics.String())
+			}
+		})
+	}
+}
+
+func TestChecksWatchTimeoutCountsPendingChecks(t *testing.T) {
+	server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.URL.Path == "/api/v1/repos/alice/x/pulls/8":
+			_, _ = w.Write([]byte(`{"number":8,"head":{"sha":"abc"}}`))
+		case r.URL.Query().Get("page") == "1":
+			_, _ = w.Write([]byte(`[{"id":1,"context":"a","status":"pending"},{"id":2,"context":"b","status":"pending"}]`))
+		default:
+			_, _ = w.Write([]byte(`[]`))
+		}
+	}))
+	defer server.Close()
+	deps, _, _ := serverDeps(t, server.URL)
+	err := Execute(context.Background(), deps, []string{"pr", "checks", "8", "-R", "alice/x", "--watch", "--interval=1ms", "--timeout=30ms"})
+	require.EqualError(t, err, "timed out after 30ms waiting for 2 pending checks")
+	require.ErrorIs(t, err, context.DeadlineExceeded)
+}

internal/clierr/errors.go

@@ -14,9 +14,25 @@ const (
 	Usage          = 2
 	Canceled       = 3
 	Authentication = 4
-	Interrupted    = 130
+	// Pending reports checks that have not finished, like gh pr checks.
+	Pending     = 8
+	Interrupted = 130
 )
 
+// errReported is the message of Quiet errors.
+var errReported = errors.New("the command reported the outcome")
+
+// Quiet exits with code without an error message, for an outcome the command already reported,
+// such as failing checks.
+func Quiet(code int) error {
+	return &Error{Code: code, Err: errReported}
+}
+
+// IsQuiet reports whether err is a Quiet exit code.
+func IsQuiet(err error) bool {
+	return errors.Is(err, errReported)
+}
+
 // Error associates an operation error with a public CLI exit code.
 type Error struct {
 	Code int

internal/clierr/errors_test.go

@@ -27,6 +27,8 @@ func TestExitCodeDistinguishesCancellationAndInterruption(t *testing.T) {
 		{fmt.Errorf("draft preserved: %w", context.DeadlineExceeded), Canceled},
 		{Wrap(Interrupted, context.Canceled), Interrupted},
 		{errors.Join(errors.New("partial failure"), context.Canceled), Canceled},
+		{Quiet(Pending), Pending},
+		{fmt.Errorf("checks: %w", Quiet(Failure)), Failure},
 	} {
 		assert.Equal(t, test.code, ExitCode(test.err), "%v", test.err)
 	}

main.go

@@ -33,7 +33,7 @@ func run(ctx context.Context, deps app.Dependencies, args []string) int {
 	}
 	code := clierr.ExitCode(err)
 	switch {
-	case err == nil, code == clierr.Interrupted:
+	case err == nil, code == clierr.Interrupted, clierr.IsQuiet(err):
 	case code == clierr.Canceled && !errors.Is(err, context.DeadlineExceeded):
 		fmt.Fprintln(deps.Streams.ErrOut, "Canceled.")
 	default: