From 2c5e005bcf7339d62da9a74216781016a37bfb28 Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 11 Oct 2026 23:30:27 +0200 Subject: [PATCH] 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 --- internal/app/app_test.go | 1 - internal/app/checks.go | 182 +++++++++++++++++++++++++++------ internal/app/checks_test.go | 70 ++++++++++++- internal/clierr/errors.go | 18 +++- internal/clierr/errors_test.go | 2 + main.go | 2 +- 6 files changed, 239 insertions(+), 36 deletions(-) diff --git a/internal/app/app_test.go b/internal/app/app_test.go index adaddb48..79918dd7 100644 --- a/internal/app/app_test.go +++ b/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 diff --git a/internal/app/checks.go b/internal/app/checks.go index ea1cba29..f037f63d 100644 --- a/internal/app/checks.go +++ b/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 ", 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 [ | | ]", 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) - if err != nil { - return err + service := services.Pulls{Client: t.client, Repository: t.Repo} + if web { + 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 { - pending := 0 - for _, item := range items { - if item.Pending() { - pending++ - } - } - _, err := fmt.Fprintf(cmd.ErrOrStderr(), "checks: %d total, %d pending\n", len(items), pending) - return err + redraw := a.checksRedraw(cmd, interval) + var latest []services.Check + items, err = service.WatchChecks(ctx, n, interval, func(current []services.Check) error { + latest = current + return redraw(current) }) - } else { - items, err = service.Checks(cmd.Context(), n) - } - if err != nil { - return err - } - 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 err != nil { + waiting := "checks to be reported" + if len(latest) > 0 { + waiting = plural(pendingChecks(latest), "pending check") + } + return timeoutError(err, timeout, "waiting for %s", waiting) } + 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().DurationVar(&interval, "interval", 5*time.Second, "Check polling interval") - cmd.Flags().DurationVar(&timeout, "timeout", time.Hour, "Maximum wait duration") + 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, "Time between refreshes with --watch") + 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 + } +} diff --git a/internal/app/checks_test.go b/internal/app/checks_test.go index 43611b1b..4c6fafe4 100644 --- a/internal/app/checks_test.go +++ b/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) +} diff --git a/internal/clierr/errors.go b/internal/clierr/errors.go index 1a6c0367..51ed8b66 100644 --- a/internal/clierr/errors.go +++ b/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 diff --git a/internal/clierr/errors_test.go b/internal/clierr/errors_test.go index eb76b24a..2cbd670c 100644 --- a/internal/clierr/errors_test.go +++ b/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) } diff --git a/main.go b/main.go index 9fd6a8f6..c4393305 100644 --- a/main.go +++ b/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: