rickub
rickub/clipublic Fork
⑂ 1f3fca3
Branches
feat/10-repo-rename-transfer feat/7-edit-title-and-body feat/8-run-logs-job-filter feat/staleness-notice main test/create-body-dash-stdin
Tags
v0.1.0 v0.1.0-rc.11 v0.1.0-rc.14 v0.1.0-rc.16 v0.1.0-rc.20 v0.1.0-rc.26 v0.1.0-rc.36 v0.1.0-rc.43 v0.1.0-rc.50 v0.1.0-rc.64 v0.2.0 View all branches → View all tags →
Commits
⬇ Clone ▾
git clone https://git.rickub.com/rickub/cli.git
git clone ssh://git@rickub.com/rickub/cli.git

Host key fingerprint (ed25519): SHA256:iycHnxEyq0Q7uyVpB7JlznP0G7JrTPXLYRcAU5CSLhc — verify it before your first connect.

Download ZIP

Merge feat/8-run-logs-job-filter into main

rickub committed 2026-09-07T22:48:59Z Browse files
1f3fca3 parent: 8e1b39c
modified README.md +5 -1
@@ -183,7 +183,11 @@ rickub pr delete-comment <number> <comment-id> [-R owner/repo]
183183
184184 rickub run list [-R owner/repo]
185185 rickub run view <number> [-R owner/repo]
186-rickub run logs <number> [-R owner/repo]
186+rickub run logs <number> [-R owner/repo] [--job NAME | --failed]
187+ # --job prints one job (case-insensitive; an unknown name is an
188+ # error listing the run's real jobs, never the whole log);
189+ # --failed prints every job whose status is not success.
190+ # Both compose with --json.
187191 rickub run rerun <number> [-R owner/repo]
188192 rickub run cancel <number> [-R owner/repo]
189193 rickub run dispatch [-R owner/repo] [--ref B]
@@ -183,7 +183,11 @@ rickub pr delete-comment <number> <comment-id> [-R owner/repo]
183 183
184 rickub run list [-R owner/repo]184 rickub run list [-R owner/repo]
185 rickub run view <number> [-R owner/repo]185 rickub run view <number> [-R owner/repo]
186-rickub run logs <number> [-R owner/repo]186+rickub run logs <number> [-R owner/repo] [--job NAME | --failed]
187+ # --job prints one job (case-insensitive; an unknown name is an
188+ # error listing the run's real jobs, never the whole log);
189+ # --failed prints every job whose status is not success.
190+ # Both compose with --json.
187 rickub run rerun <number> [-R owner/repo]191 rickub run rerun <number> [-R owner/repo]
188 rickub run cancel <number> [-R owner/repo]192 rickub run cancel <number> [-R owner/repo]
189 rickub run dispatch [-R owner/repo] [--ref B]193 rickub run dispatch [-R owner/repo] [--ref B]
modified cmd/repo_test.go +2 -0
@@ -39,6 +39,8 @@ func execute(t *testing.T, args ...string) (string, string, error) {
3939 runWatchInterval = "2s"
4040 runWatchTimeout = "30m"
4141 runWatchLogs = false
42+ runLogsJob = ""
43+ runLogsFailed = false
4244 authLoginScope = "all"
4345 authLoginNoBrowser = false
4446 prRepo = ""
@@ -39,6 +39,8 @@ func execute(t *testing.T, args ...string) (string, string, error) {
39 runWatchInterval = "2s"39 runWatchInterval = "2s"
40 runWatchTimeout = "30m"40 runWatchTimeout = "30m"
41 runWatchLogs = false41 runWatchLogs = false
42+ runLogsJob = ""
43+ runLogsFailed = false
42 authLoginScope = "all"44 authLoginScope = "all"
43 authLoginNoBrowser = false45 authLoginNoBrowser = false
44 prRepo = ""46 prRepo = ""
modified cmd/run.go +116 -6
@@ -17,6 +17,8 @@ var (
1717 runWatchInterval string
1818 runWatchTimeout string
1919 runWatchLogs bool
20+ runLogsJob string
21+ runLogsFailed bool
2022 )
2123
2224 func init() {
@@ -43,10 +45,24 @@ func init() {
4345
4446 logsCmd := &cobra.Command{
4547 Use: "logs <number>",
46- Short: "Print a run's accumulated logs",
47- Args: cobra.ExactArgs(1),
48- RunE: runRunLogs,
49- }
48+ Short: "Print a run's accumulated logs, optionally one job's or only the ones that failed",
49+ Long: "Print a run's accumulated logs.\n\n" +
50+ "With no flags this prints every job, which on a six-job run means five jobs of\n" +
51+ "noise around the one you care about. --job narrows it to one; --failed narrows it\n" +
52+ "to every job whose status is not success, which is the actual question most of\n" +
53+ "the time. Both compose with --json.\n\n" +
54+ "A --job name that matches nothing is an ERROR listing the run's real job names:\n" +
55+ "a typo that quietly printed the whole log would look like it worked, which is\n" +
56+ "worse than having no flag at all.",
57+ Example: " rickub run logs 30\n" +
58+ " rickub run logs 30 --job ssh\n" +
59+ " rickub run logs 30 --failed\n" +
60+ " rickub run logs 30 --job web --json",
61+ Args: cobra.ExactArgs(1),
62+ RunE: runRunLogs,
63+ }
64+ logsCmd.Flags().StringVar(&runLogsJob, "job", "", "print only this job's logs (case-insensitive)")
65+ logsCmd.Flags().BoolVar(&runLogsFailed, "failed", false, "print only jobs whose status is not success")
5066
5167 rerunCmd := &cobra.Command{
5268 Use: "rerun <number>",
@@ -179,22 +195,116 @@ func runRunLogs(cmd *cobra.Command, args []string) error {
179195 if err != nil {
180196 return err
181197 }
198+ // Whether --job was PASSED, not whether it is non-empty: `--job "$JOB"` with an
199+ // unset variable would otherwise disable the filter and print everything with
200+ // exit 0 — the very "looks like it worked" failure the flag exists to prevent.
201+ jobSet := cmd.Flags().Changed("job")
202+ if jobSet && runLogsFailed {
203+ // Intersecting them would print nothing whenever the named job passed,
204+ // which is indistinguishable from the typo case --job exists to rule out.
205+ return fmt.Errorf("--job and --failed select different things: pass one or the other")
206+ }
182207 logs, err := client.GetRunLogs(cmd.Context(), owner, repo, n)
183208 if err != nil {
184209 return err
185210 }
211+ // Filter BEFORE the --json branch: a filter that only applied to human output
212+ // would make `--job X --json` silently return the whole run.
213+ total := len(logs.Jobs)
214+ jobs, err := selectRunLogJobs(logs.Jobs, runLogsJob, jobSet, runLogsFailed)
215+ if err != nil {
216+ return err
217+ }
218+ logs.Jobs = jobs
219+
186220 if flagJSON {
187221 return printJSON(cmd.OutOrStdout(), logs)
188222 }
189223 out := cmd.OutOrStdout()
190- fmt.Fprintf(out, "Run #%d [%s]\n", logs.Number, logs.Status)
224+ fmt.Fprintf(out, "Run #%d [%s]\n", logs.Number, clean(logs.Status))
225+ if len(logs.Jobs) == 0 {
226+ // Never end on silence: an empty result and a broken filter look identical
227+ // otherwise, and the whole point of the flag is to be trustworthy.
228+ //
229+ // The distinction is the PRE-filter count, not the flag: a run that failed
230+ // before any job started (bad workflow YAML, a dispatch error) has nothing
231+ // to filter, and telling its author "every job succeeded" would assert the
232+ // opposite of what happened.
233+ if total == 0 {
234+ fmt.Fprintf(out, "\nThis run has no jobs.\n")
235+ } else {
236+ fmt.Fprintf(out, "\nEvery job succeeded — nothing to show.\n")
237+ }
238+ return nil
239+ }
191240 for _, j := range logs.Jobs {
192- fmt.Fprintf(out, "\n===== job: %s [%s] =====\n", j.Name, j.Status)
241+ // Job names and statuses are server-controlled strings on their way to a
242+ // terminal: clean() them like every other one (b7e1d1f), or a job named
243+ // with an escape sequence repaints the screen of anyone reading its logs.
244+ fmt.Fprintf(out, "\n===== job: %s [%s] =====\n", clean(j.Name), clean(j.Status))
193245 fmt.Fprintln(out, j.Log)
194246 }
195247 return nil
196248 }
197249
250+// selectRunLogJobs narrows a run's per-job logs to what the flags asked for.
251+//
252+// The contract that matters is the failure one: an unknown --job is an error that
253+// names the jobs the run actually has, never a silent fall-through to everything.
254+// A typo that returns the full log looks like it worked, and a filter you cannot
255+// trust is worse than no filter — you stop believing the short output and re-run
256+// without it, which is the cost the flag was added to remove (issue #8).
257+func selectRunLogJobs(jobs []api.RunLogsJob, name string, jobSet, failedOnly bool) ([]api.RunLogsJob, error) {
258+ switch {
259+ case jobSet:
260+ if strings.TrimSpace(name) == "" {
261+ // An empty --job is a broken invocation, not "no filter". Falling
262+ // through to the whole log here is the same trap as a typo.
263+ return nil, fmt.Errorf("--job needs a job name; this run has: %s", clean(strings.Join(runLogJobNames(jobs), ", ")))
264+ }
265+ // Every match, not the first: a matrix leg or a workflow called twice can
266+ // produce two jobs sharing a name, and quietly printing one of them is the
267+ // short-output-you-cannot-trust outcome this flag is meant to avoid.
268+ var matched []api.RunLogsJob
269+ for _, j := range jobs {
270+ // Case-insensitive: job names come from workflow YAML, and matching
271+ // `SSH` against `ssh` is a typo worth forgiving rather than reporting.
272+ if strings.EqualFold(j.Name, name) {
273+ matched = append(matched, j)
274+ }
275+ }
276+ if len(matched) > 0 {
277+ return matched, nil
278+ }
279+ if len(jobs) == 0 {
280+ return nil, fmt.Errorf("no job named %q: this run has no jobs", name)
281+ }
282+ return nil, fmt.Errorf("no job named %q; this run has: %s", name, clean(strings.Join(runLogJobNames(jobs), ", ")))
283+ case failedOnly:
284+ // "not success" is deliberately wider than "failure": a cancelled job, and
285+ // one still running when the logs were fetched, are both things you want to
286+ // see when asking what is not green.
287+ out := make([]api.RunLogsJob, 0, len(jobs))
288+ for _, j := range jobs {
289+ if j.Status != "success" {
290+ out = append(out, j)
291+ }
292+ }
293+ return out, nil
294+ default:
295+ return jobs, nil
296+ }
297+}
298+
299+// runLogJobNames lists a run's job names, for the unknown-job error.
300+func runLogJobNames(jobs []api.RunLogsJob) []string {
301+ names := make([]string, 0, len(jobs))
302+ for _, j := range jobs {
303+ names = append(names, j.Name)
304+ }
305+ return names
306+}
307+
198308 func runRunRerun(cmd *cobra.Command, args []string) error {
199309 client, err := newClient()
200310 if err != nil {
@@ -17,6 +17,8 @@ var (
17 runWatchInterval string17 runWatchInterval string
18 runWatchTimeout string18 runWatchTimeout string
19 runWatchLogs bool19 runWatchLogs bool
20+ runLogsJob string
21+ runLogsFailed bool
20 )22 )
21 23
22 func init() {24 func init() {
@@ -43,10 +45,24 @@ func init() {
43 45
44 logsCmd := &cobra.Command{46 logsCmd := &cobra.Command{
45 Use: "logs <number>",47 Use: "logs <number>",
46- Short: "Print a run's accumulated logs",48+ Short: "Print a run's accumulated logs, optionally one job's or only the ones that failed",
47- Args: cobra.ExactArgs(1),49+ Long: "Print a run's accumulated logs.\n\n" +
48- RunE: runRunLogs,50+ "With no flags this prints every job, which on a six-job run means five jobs of\n" +
49- }51+ "noise around the one you care about. --job narrows it to one; --failed narrows it\n" +
52+ "to every job whose status is not success, which is the actual question most of\n" +
53+ "the time. Both compose with --json.\n\n" +
54+ "A --job name that matches nothing is an ERROR listing the run's real job names:\n" +
55+ "a typo that quietly printed the whole log would look like it worked, which is\n" +
56+ "worse than having no flag at all.",
57+ Example: " rickub run logs 30\n" +
58+ " rickub run logs 30 --job ssh\n" +
59+ " rickub run logs 30 --failed\n" +
60+ " rickub run logs 30 --job web --json",
61+ Args: cobra.ExactArgs(1),
62+ RunE: runRunLogs,
63+ }
64+ logsCmd.Flags().StringVar(&runLogsJob, "job", "", "print only this job's logs (case-insensitive)")
65+ logsCmd.Flags().BoolVar(&runLogsFailed, "failed", false, "print only jobs whose status is not success")
50 66
51 rerunCmd := &cobra.Command{67 rerunCmd := &cobra.Command{
52 Use: "rerun <number>",68 Use: "rerun <number>",
@@ -179,22 +195,116 @@ func runRunLogs(cmd *cobra.Command, args []string) error {
179 if err != nil {195 if err != nil {
180 return err196 return err
181 }197 }
198+ // Whether --job was PASSED, not whether it is non-empty: `--job "$JOB"` with an
199+ // unset variable would otherwise disable the filter and print everything with
200+ // exit 0 — the very "looks like it worked" failure the flag exists to prevent.
201+ jobSet := cmd.Flags().Changed("job")
202+ if jobSet && runLogsFailed {
203+ // Intersecting them would print nothing whenever the named job passed,
204+ // which is indistinguishable from the typo case --job exists to rule out.
205+ return fmt.Errorf("--job and --failed select different things: pass one or the other")
206+ }
182 logs, err := client.GetRunLogs(cmd.Context(), owner, repo, n)207 logs, err := client.GetRunLogs(cmd.Context(), owner, repo, n)
183 if err != nil {208 if err != nil {
184 return err209 return err
185 }210 }
211+ // Filter BEFORE the --json branch: a filter that only applied to human output
212+ // would make `--job X --json` silently return the whole run.
213+ total := len(logs.Jobs)
214+ jobs, err := selectRunLogJobs(logs.Jobs, runLogsJob, jobSet, runLogsFailed)
215+ if err != nil {
216+ return err
217+ }
218+ logs.Jobs = jobs
219+
186 if flagJSON {220 if flagJSON {
187 return printJSON(cmd.OutOrStdout(), logs)221 return printJSON(cmd.OutOrStdout(), logs)
188 }222 }
189 out := cmd.OutOrStdout()223 out := cmd.OutOrStdout()
190- fmt.Fprintf(out, "Run #%d [%s]\n", logs.Number, logs.Status)224+ fmt.Fprintf(out, "Run #%d [%s]\n", logs.Number, clean(logs.Status))
225+ if len(logs.Jobs) == 0 {
226+ // Never end on silence: an empty result and a broken filter look identical
227+ // otherwise, and the whole point of the flag is to be trustworthy.
228+ //
229+ // The distinction is the PRE-filter count, not the flag: a run that failed
230+ // before any job started (bad workflow YAML, a dispatch error) has nothing
231+ // to filter, and telling its author "every job succeeded" would assert the
232+ // opposite of what happened.
233+ if total == 0 {
234+ fmt.Fprintf(out, "\nThis run has no jobs.\n")
235+ } else {
236+ fmt.Fprintf(out, "\nEvery job succeeded — nothing to show.\n")
237+ }
238+ return nil
239+ }
191 for _, j := range logs.Jobs {240 for _, j := range logs.Jobs {
192- fmt.Fprintf(out, "\n===== job: %s [%s] =====\n", j.Name, j.Status)241+ // Job names and statuses are server-controlled strings on their way to a
242+ // terminal: clean() them like every other one (b7e1d1f), or a job named
243+ // with an escape sequence repaints the screen of anyone reading its logs.
244+ fmt.Fprintf(out, "\n===== job: %s [%s] =====\n", clean(j.Name), clean(j.Status))
193 fmt.Fprintln(out, j.Log)245 fmt.Fprintln(out, j.Log)
194 }246 }
195 return nil247 return nil
196 }248 }
197 249
250+// selectRunLogJobs narrows a run's per-job logs to what the flags asked for.
251+//
252+// The contract that matters is the failure one: an unknown --job is an error that
253+// names the jobs the run actually has, never a silent fall-through to everything.
254+// A typo that returns the full log looks like it worked, and a filter you cannot
255+// trust is worse than no filter — you stop believing the short output and re-run
256+// without it, which is the cost the flag was added to remove (issue #8).
257+func selectRunLogJobs(jobs []api.RunLogsJob, name string, jobSet, failedOnly bool) ([]api.RunLogsJob, error) {
258+ switch {
259+ case jobSet:
260+ if strings.TrimSpace(name) == "" {
261+ // An empty --job is a broken invocation, not "no filter". Falling
262+ // through to the whole log here is the same trap as a typo.
263+ return nil, fmt.Errorf("--job needs a job name; this run has: %s", clean(strings.Join(runLogJobNames(jobs), ", ")))
264+ }
265+ // Every match, not the first: a matrix leg or a workflow called twice can
266+ // produce two jobs sharing a name, and quietly printing one of them is the
267+ // short-output-you-cannot-trust outcome this flag is meant to avoid.
268+ var matched []api.RunLogsJob
269+ for _, j := range jobs {
270+ // Case-insensitive: job names come from workflow YAML, and matching
271+ // `SSH` against `ssh` is a typo worth forgiving rather than reporting.
272+ if strings.EqualFold(j.Name, name) {
273+ matched = append(matched, j)
274+ }
275+ }
276+ if len(matched) > 0 {
277+ return matched, nil
278+ }
279+ if len(jobs) == 0 {
280+ return nil, fmt.Errorf("no job named %q: this run has no jobs", name)
281+ }
282+ return nil, fmt.Errorf("no job named %q; this run has: %s", name, clean(strings.Join(runLogJobNames(jobs), ", ")))
283+ case failedOnly:
284+ // "not success" is deliberately wider than "failure": a cancelled job, and
285+ // one still running when the logs were fetched, are both things you want to
286+ // see when asking what is not green.
287+ out := make([]api.RunLogsJob, 0, len(jobs))
288+ for _, j := range jobs {
289+ if j.Status != "success" {
290+ out = append(out, j)
291+ }
292+ }
293+ return out, nil
294+ default:
295+ return jobs, nil
296+ }
297+}
298+
299+// runLogJobNames lists a run's job names, for the unknown-job error.
300+func runLogJobNames(jobs []api.RunLogsJob) []string {
301+ names := make([]string, 0, len(jobs))
302+ for _, j := range jobs {
303+ names = append(names, j.Name)
304+ }
305+ return names
306+}
307+
198 func runRunRerun(cmd *cobra.Command, args []string) error {308 func runRunRerun(cmd *cobra.Command, args []string) error {
199 client, err := newClient()309 client, err := newClient()
200 if err != nil {310 if err != nil {
added cmd/run_logs_filter_test.go +264 -0
new file mode 100644
@@ -0,0 +1,264 @@
1+package cmd
2+
3+import (
4+ "encoding/json"
5+ "net/http"
6+ "net/http/httptest"
7+ "strings"
8+ "testing"
9+
10+ "rickub.com/rickub/cli/internal/api"
11+)
12+
13+// sixJobRun is the shape issue #8 is about: one red job among five that are noise.
14+const sixJobRun = `{"number":30,"status":"failure","jobs":[
15+ {"name":"engine","status":"success","log":"engine ok\n"},
16+ {"name":"registry","status":"success","log":"registry ok\n"},
17+ {"name":"web","status":"success","log":"web ok\n"},
18+ {"name":"ssh","status":"failure","log":"##[error]getaddrinfo EAI_AGAIN golang.org\n"},
19+ {"name":"lint","status":"cancelled","log":"lint cancelled\n"},
20+ {"name":"coverage","status":"success","log":"coverage ok\n"}
21+]}`
22+
23+func runLogsServer(t *testing.T, body string) *httptest.Server {
24+ t.Helper()
25+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
26+ w.Header().Set("Content-Type", "application/json")
27+ _, _ = w.Write([]byte(body))
28+ }))
29+ t.Cleanup(srv.Close)
30+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
31+ t.Setenv("RICKUB_HOST", srv.URL)
32+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
33+ return srv
34+}
35+
36+// TestRunLogsJobPrintsOnlyThatJob is issue #8's acceptance criterion: the five
37+// other jobs must not appear at all.
38+func TestRunLogsJobPrintsOnlyThatJob(t *testing.T) {
39+ runLogsServer(t, sixJobRun)
40+
41+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "ssh")
42+ if err != nil {
43+ t.Fatalf("execute: %v", err)
44+ }
45+ if !strings.Contains(out, "EAI_AGAIN") || !strings.Contains(out, "job: ssh") {
46+ t.Errorf("the requested job is missing:\n%s", out)
47+ }
48+ for _, other := range []string{"engine ok", "registry ok", "web ok", "coverage ok", "lint cancelled"} {
49+ if strings.Contains(out, other) {
50+ t.Errorf("output still contains another job's log (%q):\n%s", other, out)
51+ }
52+ }
53+}
54+
55+// TestRunLogsUnknownJobErrorsAndNamesTheRealOnes: a typo must NOT fall through to
56+// the full log. That would look like it worked, which is worse than no flag.
57+func TestRunLogsUnknownJobErrorsAndNamesTheRealOnes(t *testing.T) {
58+ runLogsServer(t, sixJobRun)
59+
60+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "shh")
61+ if err == nil {
62+ t.Fatal("an unknown job name was accepted")
63+ }
64+ if strings.Contains(out, "engine ok") || strings.Contains(out, "EAI_AGAIN") {
65+ t.Errorf("a bad --job printed logs anyway:\n%s", out)
66+ }
67+ for _, name := range []string{"engine", "registry", "web", "ssh", "lint", "coverage"} {
68+ if !strings.Contains(err.Error(), name) {
69+ t.Errorf("error should list the run's real job names, missing %q: %v", name, err)
70+ }
71+ }
72+}
73+
74+// TestRunLogsJobIsCaseInsensitive: matching `SSH` against `ssh` is a typo worth
75+// forgiving, not reporting.
76+func TestRunLogsJobIsCaseInsensitive(t *testing.T) {
77+ runLogsServer(t, sixJobRun)
78+
79+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "SSH")
80+ if err != nil {
81+ t.Fatalf("execute: %v", err)
82+ }
83+ if !strings.Contains(out, "EAI_AGAIN") {
84+ t.Errorf("case-insensitive match failed:\n%s", out)
85+ }
86+}
87+
88+// TestRunLogsFailedSelectsEverythingNotGreen: "not success" is wider than
89+// "failure" on purpose — a cancelled job is also not green.
90+func TestRunLogsFailedSelectsEverythingNotGreen(t *testing.T) {
91+ runLogsServer(t, sixJobRun)
92+
93+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--failed")
94+ if err != nil {
95+ t.Fatalf("execute: %v", err)
96+ }
97+ if !strings.Contains(out, "EAI_AGAIN") || !strings.Contains(out, "lint cancelled") {
98+ t.Errorf("--failed should show the failed AND the cancelled job:\n%s", out)
99+ }
100+ for _, green := range []string{"engine ok", "registry ok", "web ok", "coverage ok"} {
101+ if strings.Contains(out, green) {
102+ t.Errorf("--failed leaked a successful job (%q):\n%s", green, out)
103+ }
104+ }
105+}
106+
107+// TestRunLogsFailedOnAGreenRunSaysSo: never end on silence — an empty result and
108+// a broken filter look identical otherwise.
109+func TestRunLogsFailedOnAGreenRunSaysSo(t *testing.T) {
110+ runLogsServer(t, `{"number":31,"status":"success","jobs":[{"name":"check","status":"success","log":"ok\n"}]}`)
111+
112+ out, _, err := execute(t, "run", "logs", "31", "-R", "o/r", "--failed")
113+ if err != nil {
114+ t.Fatalf("execute: %v", err)
115+ }
116+ if !strings.Contains(out, "Every job succeeded") {
117+ t.Errorf("an empty --failed result should say so:\n%s", out)
118+ }
119+}
120+
121+// TestRunLogsJobComposesWithJSON: the filter must apply BEFORE the --json branch,
122+// or `--job X --json` silently returns the whole run.
123+func TestRunLogsJobComposesWithJSON(t *testing.T) {
124+ runLogsServer(t, sixJobRun)
125+
126+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "ssh", "--json")
127+ if err != nil {
128+ t.Fatalf("execute: %v", err)
129+ }
130+ var decoded api.RunLogs
131+ if err := json.Unmarshal([]byte(out), &decoded); err != nil {
132+ t.Fatalf("output is not JSON: %v\n%s", err, out)
133+ }
134+ if len(decoded.Jobs) != 1 || decoded.Jobs[0].Name != "ssh" {
135+ t.Errorf("--json should carry only the selected job, got %+v", decoded.Jobs)
136+ }
137+ if decoded.Number != 30 || decoded.Status != "failure" {
138+ t.Errorf("the run's own fields should survive the filter: %+v", decoded)
139+ }
140+}
141+
142+// TestRunLogsJobAndFailedAreRefused: intersecting them prints nothing whenever the
143+// named job passed, which is indistinguishable from the typo case --job exists to
144+// rule out.
145+func TestRunLogsJobAndFailedAreRefused(t *testing.T) {
146+ runLogsServer(t, sixJobRun)
147+
148+ if _, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "ssh", "--failed"); err == nil {
149+ t.Fatal("--job with --failed was accepted")
150+ }
151+}
152+
153+// TestRunLogsUnfilteredIsUnchanged: the no-flag behaviour every existing script
154+// depends on still prints every job.
155+func TestRunLogsUnfilteredIsUnchanged(t *testing.T) {
156+ runLogsServer(t, sixJobRun)
157+
158+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r")
159+ if err != nil {
160+ t.Fatalf("execute: %v", err)
161+ }
162+ for _, name := range []string{"engine", "registry", "web", "ssh", "lint", "coverage"} {
163+ if !strings.Contains(out, "job: "+name) {
164+ t.Errorf("unfiltered output is missing job %q:\n%s", name, out)
165+ }
166+ }
167+}
168+
169+// TestRunLogsEmptyJobIsRefused: `--job "$JOB"` with an unset variable must not
170+// quietly print the whole run. Keying on "the flag was passed" rather than "the
171+// value is non-empty" is what makes the filter trustworthy.
172+func TestRunLogsEmptyJobIsRefused(t *testing.T) {
173+ runLogsServer(t, sixJobRun)
174+
175+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "")
176+ if err == nil {
177+ t.Fatal("an empty --job was accepted")
178+ }
179+ if strings.Contains(out, "engine ok") {
180+ t.Errorf("an empty --job printed the whole run:\n%s", out)
181+ }
182+
183+ // It must not defeat the mutual exclusion either.
184+ if _, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "", "--failed"); err == nil {
185+ t.Fatal(`--job "" --failed was accepted`)
186+ }
187+}
188+
189+// TestRunLogsJoblessRunDoesNotClaimSuccess: a run that failed before any job
190+// started has nothing to filter, and "every job succeeded" would assert the
191+// opposite of what happened.
192+func TestRunLogsJoblessRunDoesNotClaimSuccess(t *testing.T) {
193+ runLogsServer(t, `{"number":32,"status":"failure","jobs":[]}`)
194+
195+ out, _, err := execute(t, "run", "logs", "32", "-R", "o/r", "--failed")
196+ if err != nil {
197+ t.Fatalf("execute: %v", err)
198+ }
199+ if strings.Contains(out, "Every job succeeded") {
200+ t.Errorf("a jobless failed run was reported as all-green:\n%s", out)
201+ }
202+ if !strings.Contains(out, "no jobs") {
203+ t.Errorf("expected a no-jobs message:\n%s", out)
204+ }
205+}
206+
207+// TestRunLogsJobPrintsEveryMatch: a matrix leg or a twice-called workflow can emit
208+// two jobs sharing a name; printing one of them silently is the short output you
209+// cannot trust.
210+func TestRunLogsJobPrintsEveryMatch(t *testing.T) {
211+ runLogsServer(t, `{"number":33,"status":"failure","jobs":[
212+ {"name":"test","status":"success","log":"leg one\n"},
213+ {"name":"other","status":"success","log":"unrelated\n"},
214+ {"name":"test","status":"failure","log":"leg two\n"}
215+ ]}`)
216+
217+ out, _, err := execute(t, "run", "logs", "33", "-R", "o/r", "--job", "test")
218+ if err != nil {
219+ t.Fatalf("execute: %v", err)
220+ }
221+ if !strings.Contains(out, "leg one") || !strings.Contains(out, "leg two") {
222+ t.Errorf("both jobs sharing the name should be printed:\n%s", out)
223+ }
224+ if strings.Contains(out, "unrelated") {
225+ t.Errorf("a non-matching job leaked:\n%s", out)
226+ }
227+}
228+
229+// TestRunLogsCleansServerJobNames: job names are server-controlled strings on
230+// their way to a terminal. An escape sequence in one must not survive to the
231+// screen — neither in the job header nor in the unknown-job error, which reaches
232+// the user through main's bare "rickub: %s" with no cleaning of its own.
233+func TestRunLogsCleansServerJobNames(t *testing.T) {
234+ const esc = "\x1b"
235+ // The job name carries ESC [2K (erase-line) between "ssh" and "masked".
236+ evil := "ssh" + esc + "[2Kmasked"
237+ body, err := json.Marshal(api.RunLogs{
238+ Number: 34, Status: "failure",
239+ Jobs: []api.RunLogsJob{{Name: evil, Status: "failure", Log: "boom\n"}},
240+ })
241+ if err != nil {
242+ t.Fatalf("marshal: %v", err)
243+ }
244+ runLogsServer(t, string(body))
245+
246+ out, _, err := execute(t, "run", "logs", "34", "-R", "o/r")
247+ if err != nil {
248+ t.Fatalf("execute: %v", err)
249+ }
250+ if strings.Contains(out, esc) {
251+ t.Errorf("an escape sequence survived into the job header: %q", out)
252+ }
253+
254+ _, _, err = execute(t, "run", "logs", "34", "-R", "o/r", "--job", "nope")
255+ if err == nil {
256+ t.Fatal("expected an unknown-job error")
257+ }
258+ if strings.Contains(err.Error(), esc) {
259+ t.Errorf("an escape sequence survived into the error: %q", err.Error())
260+ }
261+ if !strings.Contains(err.Error(), "ssh[2Kmasked") {
262+ t.Errorf("the cleaned name should still be listed: %q", err.Error())
263+ }
264+}
new file mode 100644
@@ -0,0 +1,264 @@
1+package cmd
2+
3+import (
4+ "encoding/json"
5+ "net/http"
6+ "net/http/httptest"
7+ "strings"
8+ "testing"
9+
10+ "rickub.com/rickub/cli/internal/api"
11+)
12+
13+// sixJobRun is the shape issue #8 is about: one red job among five that are noise.
14+const sixJobRun = `{"number":30,"status":"failure","jobs":[
15+ {"name":"engine","status":"success","log":"engine ok\n"},
16+ {"name":"registry","status":"success","log":"registry ok\n"},
17+ {"name":"web","status":"success","log":"web ok\n"},
18+ {"name":"ssh","status":"failure","log":"##[error]getaddrinfo EAI_AGAIN golang.org\n"},
19+ {"name":"lint","status":"cancelled","log":"lint cancelled\n"},
20+ {"name":"coverage","status":"success","log":"coverage ok\n"}
21+]}`
22+
23+func runLogsServer(t *testing.T, body string) *httptest.Server {
24+ t.Helper()
25+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
26+ w.Header().Set("Content-Type", "application/json")
27+ _, _ = w.Write([]byte(body))
28+ }))
29+ t.Cleanup(srv.Close)
30+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
31+ t.Setenv("RICKUB_HOST", srv.URL)
32+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
33+ return srv
34+}
35+
36+// TestRunLogsJobPrintsOnlyThatJob is issue #8's acceptance criterion: the five
37+// other jobs must not appear at all.
38+func TestRunLogsJobPrintsOnlyThatJob(t *testing.T) {
39+ runLogsServer(t, sixJobRun)
40+
41+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "ssh")
42+ if err != nil {
43+ t.Fatalf("execute: %v", err)
44+ }
45+ if !strings.Contains(out, "EAI_AGAIN") || !strings.Contains(out, "job: ssh") {
46+ t.Errorf("the requested job is missing:\n%s", out)
47+ }
48+ for _, other := range []string{"engine ok", "registry ok", "web ok", "coverage ok", "lint cancelled"} {
49+ if strings.Contains(out, other) {
50+ t.Errorf("output still contains another job's log (%q):\n%s", other, out)
51+ }
52+ }
53+}
54+
55+// TestRunLogsUnknownJobErrorsAndNamesTheRealOnes: a typo must NOT fall through to
56+// the full log. That would look like it worked, which is worse than no flag.
57+func TestRunLogsUnknownJobErrorsAndNamesTheRealOnes(t *testing.T) {
58+ runLogsServer(t, sixJobRun)
59+
60+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "shh")
61+ if err == nil {
62+ t.Fatal("an unknown job name was accepted")
63+ }
64+ if strings.Contains(out, "engine ok") || strings.Contains(out, "EAI_AGAIN") {
65+ t.Errorf("a bad --job printed logs anyway:\n%s", out)
66+ }
67+ for _, name := range []string{"engine", "registry", "web", "ssh", "lint", "coverage"} {
68+ if !strings.Contains(err.Error(), name) {
69+ t.Errorf("error should list the run's real job names, missing %q: %v", name, err)
70+ }
71+ }
72+}
73+
74+// TestRunLogsJobIsCaseInsensitive: matching `SSH` against `ssh` is a typo worth
75+// forgiving, not reporting.
76+func TestRunLogsJobIsCaseInsensitive(t *testing.T) {
77+ runLogsServer(t, sixJobRun)
78+
79+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "SSH")
80+ if err != nil {
81+ t.Fatalf("execute: %v", err)
82+ }
83+ if !strings.Contains(out, "EAI_AGAIN") {
84+ t.Errorf("case-insensitive match failed:\n%s", out)
85+ }
86+}
87+
88+// TestRunLogsFailedSelectsEverythingNotGreen: "not success" is wider than
89+// "failure" on purpose — a cancelled job is also not green.
90+func TestRunLogsFailedSelectsEverythingNotGreen(t *testing.T) {
91+ runLogsServer(t, sixJobRun)
92+
93+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--failed")
94+ if err != nil {
95+ t.Fatalf("execute: %v", err)
96+ }
97+ if !strings.Contains(out, "EAI_AGAIN") || !strings.Contains(out, "lint cancelled") {
98+ t.Errorf("--failed should show the failed AND the cancelled job:\n%s", out)
99+ }
100+ for _, green := range []string{"engine ok", "registry ok", "web ok", "coverage ok"} {
101+ if strings.Contains(out, green) {
102+ t.Errorf("--failed leaked a successful job (%q):\n%s", green, out)
103+ }
104+ }
105+}
106+
107+// TestRunLogsFailedOnAGreenRunSaysSo: never end on silence — an empty result and
108+// a broken filter look identical otherwise.
109+func TestRunLogsFailedOnAGreenRunSaysSo(t *testing.T) {
110+ runLogsServer(t, `{"number":31,"status":"success","jobs":[{"name":"check","status":"success","log":"ok\n"}]}`)
111+
112+ out, _, err := execute(t, "run", "logs", "31", "-R", "o/r", "--failed")
113+ if err != nil {
114+ t.Fatalf("execute: %v", err)
115+ }
116+ if !strings.Contains(out, "Every job succeeded") {
117+ t.Errorf("an empty --failed result should say so:\n%s", out)
118+ }
119+}
120+
121+// TestRunLogsJobComposesWithJSON: the filter must apply BEFORE the --json branch,
122+// or `--job X --json` silently returns the whole run.
123+func TestRunLogsJobComposesWithJSON(t *testing.T) {
124+ runLogsServer(t, sixJobRun)
125+
126+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "ssh", "--json")
127+ if err != nil {
128+ t.Fatalf("execute: %v", err)
129+ }
130+ var decoded api.RunLogs
131+ if err := json.Unmarshal([]byte(out), &decoded); err != nil {
132+ t.Fatalf("output is not JSON: %v\n%s", err, out)
133+ }
134+ if len(decoded.Jobs) != 1 || decoded.Jobs[0].Name != "ssh" {
135+ t.Errorf("--json should carry only the selected job, got %+v", decoded.Jobs)
136+ }
137+ if decoded.Number != 30 || decoded.Status != "failure" {
138+ t.Errorf("the run's own fields should survive the filter: %+v", decoded)
139+ }
140+}
141+
142+// TestRunLogsJobAndFailedAreRefused: intersecting them prints nothing whenever the
143+// named job passed, which is indistinguishable from the typo case --job exists to
144+// rule out.
145+func TestRunLogsJobAndFailedAreRefused(t *testing.T) {
146+ runLogsServer(t, sixJobRun)
147+
148+ if _, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "ssh", "--failed"); err == nil {
149+ t.Fatal("--job with --failed was accepted")
150+ }
151+}
152+
153+// TestRunLogsUnfilteredIsUnchanged: the no-flag behaviour every existing script
154+// depends on still prints every job.
155+func TestRunLogsUnfilteredIsUnchanged(t *testing.T) {
156+ runLogsServer(t, sixJobRun)
157+
158+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r")
159+ if err != nil {
160+ t.Fatalf("execute: %v", err)
161+ }
162+ for _, name := range []string{"engine", "registry", "web", "ssh", "lint", "coverage"} {
163+ if !strings.Contains(out, "job: "+name) {
164+ t.Errorf("unfiltered output is missing job %q:\n%s", name, out)
165+ }
166+ }
167+}
168+
169+// TestRunLogsEmptyJobIsRefused: `--job "$JOB"` with an unset variable must not
170+// quietly print the whole run. Keying on "the flag was passed" rather than "the
171+// value is non-empty" is what makes the filter trustworthy.
172+func TestRunLogsEmptyJobIsRefused(t *testing.T) {
173+ runLogsServer(t, sixJobRun)
174+
175+ out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "")
176+ if err == nil {
177+ t.Fatal("an empty --job was accepted")
178+ }
179+ if strings.Contains(out, "engine ok") {
180+ t.Errorf("an empty --job printed the whole run:\n%s", out)
181+ }
182+
183+ // It must not defeat the mutual exclusion either.
184+ if _, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "", "--failed"); err == nil {
185+ t.Fatal(`--job "" --failed was accepted`)
186+ }
187+}
188+
189+// TestRunLogsJoblessRunDoesNotClaimSuccess: a run that failed before any job
190+// started has nothing to filter, and "every job succeeded" would assert the
191+// opposite of what happened.
192+func TestRunLogsJoblessRunDoesNotClaimSuccess(t *testing.T) {
193+ runLogsServer(t, `{"number":32,"status":"failure","jobs":[]}`)
194+
195+ out, _, err := execute(t, "run", "logs", "32", "-R", "o/r", "--failed")
196+ if err != nil {
197+ t.Fatalf("execute: %v", err)
198+ }
199+ if strings.Contains(out, "Every job succeeded") {
200+ t.Errorf("a jobless failed run was reported as all-green:\n%s", out)
201+ }
202+ if !strings.Contains(out, "no jobs") {
203+ t.Errorf("expected a no-jobs message:\n%s", out)
204+ }
205+}
206+
207+// TestRunLogsJobPrintsEveryMatch: a matrix leg or a twice-called workflow can emit
208+// two jobs sharing a name; printing one of them silently is the short output you
209+// cannot trust.
210+func TestRunLogsJobPrintsEveryMatch(t *testing.T) {
211+ runLogsServer(t, `{"number":33,"status":"failure","jobs":[
212+ {"name":"test","status":"success","log":"leg one\n"},
213+ {"name":"other","status":"success","log":"unrelated\n"},
214+ {"name":"test","status":"failure","log":"leg two\n"}
215+ ]}`)
216+
217+ out, _, err := execute(t, "run", "logs", "33", "-R", "o/r", "--job", "test")
218+ if err != nil {
219+ t.Fatalf("execute: %v", err)
220+ }
221+ if !strings.Contains(out, "leg one") || !strings.Contains(out, "leg two") {
222+ t.Errorf("both jobs sharing the name should be printed:\n%s", out)
223+ }
224+ if strings.Contains(out, "unrelated") {
225+ t.Errorf("a non-matching job leaked:\n%s", out)
226+ }
227+}
228+
229+// TestRunLogsCleansServerJobNames: job names are server-controlled strings on
230+// their way to a terminal. An escape sequence in one must not survive to the
231+// screen — neither in the job header nor in the unknown-job error, which reaches
232+// the user through main's bare "rickub: %s" with no cleaning of its own.
233+func TestRunLogsCleansServerJobNames(t *testing.T) {
234+ const esc = "\x1b"
235+ // The job name carries ESC [2K (erase-line) between "ssh" and "masked".
236+ evil := "ssh" + esc + "[2Kmasked"
237+ body, err := json.Marshal(api.RunLogs{
238+ Number: 34, Status: "failure",
239+ Jobs: []api.RunLogsJob{{Name: evil, Status: "failure", Log: "boom\n"}},
240+ })
241+ if err != nil {
242+ t.Fatalf("marshal: %v", err)
243+ }
244+ runLogsServer(t, string(body))
245+
246+ out, _, err := execute(t, "run", "logs", "34", "-R", "o/r")
247+ if err != nil {
248+ t.Fatalf("execute: %v", err)
249+ }
250+ if strings.Contains(out, esc) {
251+ t.Errorf("an escape sequence survived into the job header: %q", out)
252+ }
253+
254+ _, _, err = execute(t, "run", "logs", "34", "-R", "o/r", "--job", "nope")
255+ if err == nil {
256+ t.Fatal("expected an unknown-job error")
257+ }
258+ if strings.Contains(err.Error(), esc) {
259+ t.Errorf("an escape sequence survived into the error: %q", err.Error())
260+ }
261+ if !strings.Contains(err.Error(), "ssh[2Kmasked") {
262+ t.Errorf("the cleaned name should still be listed: %q", err.Error())
263+ }
264+}