diff --git a/internal/app/action_logs.go b/internal/app/action_logs.go index bd1ea60f..786e279a 100644 --- a/internal/app/action_logs.go +++ b/internal/app/action_logs.go @@ -4,6 +4,7 @@ package app import ( + "cmp" "context" "errors" "fmt" @@ -86,11 +87,22 @@ func (a *application) runLogsCommand() *cobra.Command { }) } for _, job := range jobs { - if _, err := fmt.Fprintf(cmd.ErrOrStderr(), "Job %d: %s\n", job.ID, terminal.SafeLine(job.Name)); err != nil { + label := fmt.Sprintf("%s (job %d)", terminal.SafeLine(job.Name), job.ID) + var logs io.ReadCloser + if job.Started() { + logs, err = service.JobLogs(cmd.Context(), job.ID) + } + if !job.Started() || errors.Is(err, services.ErrJobNotStarted) { + if _, err := fmt.Fprintf(cmd.ErrOrStderr(), "%s: %s\n", label, unstarted(job)); err != nil { + return err + } + continue + } + if err != nil { return err } - logs, err := service.JobLogs(cmd.Context(), job.ID) - if err != nil { + if _, err := fmt.Fprintln(cmd.ErrOrStderr(), label); err != nil { + _ = logs.Close() return err } _, copyErr := io.Copy(out, logs) @@ -111,3 +123,12 @@ func (a *application) runLogsCommand() *cobra.Command { cmd.Flags().DurationVar(&timeout, "timeout", time.Hour, "Maximum follow duration") return cmd } + +// unstarted explains why a job has no logs: it is waiting for a runner, or it completed, such +// as by cancellation, before one started it. +func unstarted(job services.Job) string { + if job.CompletedAt == nil { + return "not started yet" + } + return "never started (" + cmp.Or(job.Conclusion, job.Status) + ")" +} diff --git a/internal/app/action_logs_test.go b/internal/app/action_logs_test.go index 901852bc..6cde8b04 100644 --- a/internal/app/action_logs_test.go +++ b/internal/app/action_logs_test.go @@ -20,7 +20,7 @@ func TestRunLogsStreamWithoutMixingDiagnosticsAndVerifyJobMembership(t *testing. switch r.URL.Path { case "/api/v1/repos/owner/repo/actions/runs/7/jobs": if r.URL.Query().Get("page") == "1" { - _, _ = w.Write([]byte(`{"jobs":[{"id":9,"name":"Build"}]}`)) + _, _ = w.Write([]byte(`{"jobs":[{"id":9,"name":"Build","started_at":"2028-01-01T00:00:00Z"}]}`)) } else { _, _ = w.Write([]byte(`{"jobs":[]}`)) } @@ -39,7 +39,7 @@ func TestRunLogsStreamWithoutMixingDiagnosticsAndVerifyJobMembership(t *testing. args := []string{"run", "logs", "7", "-R", "owner/repo", "--job=9", "--no-input"} require.NoError(t, Execute(context.Background(), deps, args)) require.Equal(t, "exact log\n", out.String()) - require.Equal(t, "Job 9: Build\n", diagnostics.String()) + require.Equal(t, "Build (job 9)\n", diagnostics.String()) require.Equal(t, 1, downloads) out.Reset() diagnostics.Reset() @@ -71,7 +71,7 @@ func TestRunLogsKeepOnlyColorsOnATerminal(t *testing.T) { switch r.URL.Path { case "/api/v1/repos/owner/repo/actions/runs/7/jobs": if r.URL.Query().Get("page") == "1" { - _, _ = w.Write([]byte(`{"jobs":[{"id":9,"name":"Build\u001b]0;PWNED\u0007"}]}`)) + _, _ = w.Write([]byte(`{"jobs":[{"id":9,"name":"Build\u001b]0;PWNED\u0007","started_at":"2028-01-01T00:00:00Z"}]}`)) } else { _, _ = w.Write([]byte(`{"jobs":[]}`)) } @@ -88,6 +88,36 @@ func TestRunLogsKeepOnlyColorsOnATerminal(t *testing.T) { deps.Streams.OutTTY = tty require.NoError(t, Execute(context.Background(), deps, []string{"run", "logs", "7", "-R", "owner/repo", "--job=9", "--no-input"})) require.Equal(t, want, out.String(), "tty=%t", tty) - require.Equal(t, "Job 9: Build]0;PWNED\n", diagnostics.String()) + require.Equal(t, "Build]0;PWNED (job 9)\n", diagnostics.String()) } } + +func TestRunLogsContinuePastJobsThatHaveNotStarted(t *testing.T) { + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/repos/owner/repo/actions/runs/7/jobs": + if r.URL.Query().Get("page") != "1" { + _, _ = w.Write([]byte(`{"jobs":[]}`)) + return + } + _, _ = w.Write([]byte(`{"jobs":[ + {"id":8,"name":"lint","status":"completed","conclusion":"skipped","started_at":"1970-01-01T00:00:00Z","completed_at":"2028-01-01T00:01:00Z"}, + {"id":9,"name":"build","status":"queued","started_at":"1970-01-01T00:00:00Z","completed_at":"1970-01-01T00:00:00Z"}, + {"id":10,"name":"test","status":"in_progress","started_at":"2028-01-01T00:00:00Z"}, + {"id":11,"name":"deploy","status":"in_progress","started_at":"2028-01-01T00:00:00Z"}]}`)) + case "/api/v1/repos/owner/repo/actions/jobs/10/logs": + _, _ = w.Write([]byte("test output\n")) + case "/api/v1/repos/owner/repo/actions/jobs/11/logs": + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"job not started"}`)) + default: + t.Errorf("unexpected endpoint %s", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + deps, out, diagnostics := serverDeps(t, server.URL) + require.NoError(t, Execute(context.Background(), deps, []string{"run", "logs", "7", "-R", "owner/repo", "--no-input"})) + require.Equal(t, "test output\n", out.String()) + require.Equal(t, "lint (job 8): never started (skipped)\nbuild (job 9): not started yet\ntest (job 10)\ndeploy (job 11): not started yet\n", diagnostics.String()) +} diff --git a/internal/services/action_logs.go b/internal/services/action_logs.go index 48f86041..414796ef 100644 --- a/internal/services/action_logs.go +++ b/internal/services/action_logs.go @@ -11,19 +11,23 @@ import ( "fmt" "io" "net/http" + "strings" "time" gitea "gitea.dev/sdk" "gitea.dev/tea/internal/transport" ) -// Job identifies one workflow job without exposing server-specific metadata. +// Job identifies one workflow job without exposing server-specific metadata. StartedAt and +// CompletedAt are nil until the job starts and completes. type Job struct { - RunAttempt int64 `json:"run_attempt"` - ID int64 `json:"id"` - Name string `json:"name"` - Status string `json:"status"` - Conclusion string `json:"conclusion"` + RunAttempt int64 `json:"run_attempt"` + ID int64 `json:"id"` + Name string `json:"name"` + Status string `json:"status"` + Conclusion string `json:"conclusion"` + StartedAt *string `json:"started_at"` + CompletedAt *string `json:"completed_at"` } // Jobs lists every page of jobs in the selected run. @@ -66,13 +70,35 @@ func (s Actions) jobsPage(ctx context.Context, run int64, limit, page int) ([]Jo } func jobFrom(job *gitea.ActionsWorkflowJob) Job { - return Job{RunAttempt: job.RunAttempt, ID: job.ID, Name: job.Name, Status: job.Status, Conclusion: job.Conclusion} + return Job{ + RunAttempt: job.RunAttempt, ID: job.ID, Name: job.Name, Status: job.Status, Conclusion: job.Conclusion, + StartedAt: actionTime(job.StartedAt), CompletedAt: actionTime(job.CompletedAt), + } } -// JobLogs streams a job's plain-text logs within the selected instance. +// actionTime is runTime for SDK times. +func actionTime(t time.Time) *string { + if t.Unix() <= 0 { + return nil + } + value := timestamp(t) + return &value +} + +// Started reports whether the job has started, so that it has logs. +func (j Job) Started() bool { return j.StartedAt != nil } + +// ErrJobNotStarted reports a job without logs because no runner has started it. +var ErrJobNotStarted = errors.New("job not started") + +// JobLogs streams a job's plain-text logs within the selected instance. A job that has not +// started fails with ErrJobNotStarted. func (s Actions) JobLogs(ctx context.Context, id int64) (io.ReadCloser, error) { // TODO(sdk): GetRepoRunJobLogs buffers the whole log; a reader variant would allow streaming. response, err := s.Client.Do(ctx, http.MethodGet, fmt.Sprintf("%sjobs/%d/logs", s.endpoint(), id), nil, nil) + if api, ok := errors.AsType[*transport.APIError](err); ok && api.Status == http.StatusNotFound && strings.Contains(api.Message, "not started") { + return nil, fmt.Errorf("job %d: %w", id, ErrJobNotStarted) + } if err != nil { return nil, err } @@ -109,7 +135,8 @@ func (s Actions) FollowJobLogs(ctx context.Context, run, id int64, interval time attempt = current.RunAttempt logs, err := s.JobLogs(ctx, id) if err != nil { - if api, ok := errors.AsType[*transport.APIError](err); ok && api.Status == http.StatusNotFound && offset == 0 && (current.Status == "queued" || current.Status == "waiting" || current.Status == "pending") { + api, ok := errors.AsType[*transport.APIError](err) + if offset == 0 && !completedActionStatus(current.Status) && (errors.Is(err, ErrJobNotStarted) || ok && api.Status == http.StatusNotFound) { if err := sleepCtx(ctx, interval); err != nil { return err }