⑂ main

run logs: --job and --failed, so one red job is not six jobs of noise (#8) #2

Merged@ogirardot wants to merge feat/8-run-logs-job-filter into main

Merged as 1f3fca34d9b6.

4 files changed

modified README.md +5 -1 Viewed

@@ -183,7 +183,11 @@ rickub pr delete-comment <number> <comment-id> [-R owner/repo]
183	183
184	184	rickub run list [-R owner/repo]
185	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	191	rickub run rerun <number> [-R owner/repo]
188	192	rickub run cancel <number> [-R owner/repo]
189	193	rickub run dispatch [-R owner/repo] [--ref B]

  rickub run list     [-R owner/repo]
  rickub run view     <number> [-R owner/repo]
--rickub run logs     <number> [-R owner/repo]
++rickub run logs     <number> [-R owner/repo] [--job NAME | --failed]
++                    # --job prints one job (case-insensitive; an unknown name is an
++                    # error listing the run's real jobs, never the whole log);
++                    # --failed prints every job whose status is not success.
++                    # Both compose with --json.
  rickub run rerun    <number> [-R owner/repo]
  rickub run cancel   <number> [-R owner/repo]
  rickub run dispatch [-R owner/repo] [--ref B]

modified cmd/repo_test.go +2 -0 Viewed

@@ -39,6 +39,8 @@ func execute(t *testing.T, args ...string) (string, string, error) {
39	39	runWatchInterval = "2s"
40	40	runWatchTimeout = "30m"
41	41	runWatchLogs = false
	42	+ runLogsJob = ""
	43	+ runLogsFailed = false
42	44	authLoginScope = "all"
43	45	authLoginNoBrowser = false
44	46	prRepo = ""

  	runWatchInterval = "2s"
  	runWatchTimeout = "30m"
  	runWatchLogs = false
++	runLogsJob = ""
++	runLogsFailed = false
  	authLoginScope = "all"
  	authLoginNoBrowser = false
  	prRepo = ""

modified cmd/run.go +116 -6 Viewed

@@ -17,6 +17,8 @@ var (
17	17	runWatchInterval string
18	18	runWatchTimeout string
19	19	runWatchLogs bool
	20	+ runLogsJob string
	21	+ runLogsFailed bool
20	22	)
21	23
22	24	func init() {
@@ -43,10 +45,24 @@ func init() {
43	45
44	46	logsCmd := &cobra.Command{
45	47	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")
50	66
51	67	rerunCmd := &cobra.Command{
52	68	Use: "rerun <number>",
@@ -179,22 +195,116 @@ func runRunLogs(cmd *cobra.Command, args []string) error {
179	195	if err != nil {
180	196	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	207	logs, err := client.GetRunLogs(cmd.Context(), owner, repo, n)
183	208	if err != nil {
184	209	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	220	if flagJSON {
187	221	return printJSON(cmd.OutOrStdout(), logs)
188	222	}
189	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	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	245	fmt.Fprintln(out, j.Log)
194	246	}
195	247	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	308	func runRunRerun(cmd *cobra.Command, args []string) error {
199	309	client, err := newClient()
200	310	if err != nil {

  	runWatchInterval string
  	runWatchTimeout  string
  	runWatchLogs     bool
++	runLogsJob       string
++	runLogsFailed    bool
  )
  func init() {
  	logsCmd := &cobra.Command{
  		Use:   "logs <number>",
--		Short: "Print a run's accumulated logs",
++		Short: "Print a run's accumulated logs, optionally one job's or only the ones that failed",
--		Args:  cobra.ExactArgs(1),
++		Long: "Print a run's accumulated logs.\n\n" +
--		RunE:  runRunLogs,
++			"With no flags this prints every job, which on a six-job run means five jobs of\n" +
--	}
++			"noise around the one you care about. --job narrows it to one; --failed narrows it\n" +
++			"to every job whose status is not success, which is the actual question most of\n" +
++			"the time. Both compose with --json.\n\n" +
++			"A --job name that matches nothing is an ERROR listing the run's real job names:\n" +
++			"a typo that quietly printed the whole log would look like it worked, which is\n" +
++			"worse than having no flag at all.",
++		Example: "  rickub run logs 30\n" +
++			"  rickub run logs 30 --job ssh\n" +
++			"  rickub run logs 30 --failed\n" +
++			"  rickub run logs 30 --job web --json",
++		Args: cobra.ExactArgs(1),
++		RunE: runRunLogs,
++	}
++	logsCmd.Flags().StringVar(&runLogsJob, "job", "", "print only this job's logs (case-insensitive)")
++	logsCmd.Flags().BoolVar(&runLogsFailed, "failed", false, "print only jobs whose status is not success")
  	rerunCmd := &cobra.Command{
  		Use:   "rerun <number>",
  	if err != nil {
  		return err
  	}
++	// Whether --job was PASSED, not whether it is non-empty: `--job "$JOB"` with an
++	// unset variable would otherwise disable the filter and print everything with
++	// exit 0 — the very "looks like it worked" failure the flag exists to prevent.
++	jobSet := cmd.Flags().Changed("job")
++	if jobSet && runLogsFailed {
++		// Intersecting them would print nothing whenever the named job passed,
++		// which is indistinguishable from the typo case --job exists to rule out.
++		return fmt.Errorf("--job and --failed select different things: pass one or the other")
++	}
  	logs, err := client.GetRunLogs(cmd.Context(), owner, repo, n)
  	if err != nil {
  		return err
  	}
++	// Filter BEFORE the --json branch: a filter that only applied to human output
++	// would make `--job X --json` silently return the whole run.
++	total := len(logs.Jobs)
++	jobs, err := selectRunLogJobs(logs.Jobs, runLogsJob, jobSet, runLogsFailed)
++	if err != nil {
++		return err
++	}
++	logs.Jobs = jobs
++
  	if flagJSON {
  		return printJSON(cmd.OutOrStdout(), logs)
  	}
  	out := cmd.OutOrStdout()
--	fmt.Fprintf(out, "Run #%d [%s]\n", logs.Number, logs.Status)
++	fmt.Fprintf(out, "Run #%d [%s]\n", logs.Number, clean(logs.Status))
++	if len(logs.Jobs) == 0 {
++		// Never end on silence: an empty result and a broken filter look identical
++		// otherwise, and the whole point of the flag is to be trustworthy.
++		//
++		// The distinction is the PRE-filter count, not the flag: a run that failed
++		// before any job started (bad workflow YAML, a dispatch error) has nothing
++		// to filter, and telling its author "every job succeeded" would assert the
++		// opposite of what happened.
++		if total == 0 {
++			fmt.Fprintf(out, "\nThis run has no jobs.\n")
++		} else {
++			fmt.Fprintf(out, "\nEvery job succeeded — nothing to show.\n")
++		}
++		return nil
++	}
  	for _, j := range logs.Jobs {
--		fmt.Fprintf(out, "\n===== job: %s [%s] =====\n", j.Name, j.Status)
++		// Job names and statuses are server-controlled strings on their way to a
++		// terminal: clean() them like every other one (b7e1d1f), or a job named
++		// with an escape sequence repaints the screen of anyone reading its logs.
++		fmt.Fprintf(out, "\n===== job: %s [%s] =====\n", clean(j.Name), clean(j.Status))
  		fmt.Fprintln(out, j.Log)
  	}
  	return nil
  }
++// selectRunLogJobs narrows a run's per-job logs to what the flags asked for.
++//
++// The contract that matters is the failure one: an unknown --job is an error that
++// names the jobs the run actually has, never a silent fall-through to everything.
++// A typo that returns the full log looks like it worked, and a filter you cannot
++// trust is worse than no filter — you stop believing the short output and re-run
++// without it, which is the cost the flag was added to remove (issue #8).
++func selectRunLogJobs(jobs []api.RunLogsJob, name string, jobSet, failedOnly bool) ([]api.RunLogsJob, error) {
++	switch {
++	case jobSet:
++		if strings.TrimSpace(name) == "" {
++			// An empty --job is a broken invocation, not "no filter". Falling
++			// through to the whole log here is the same trap as a typo.
++			return nil, fmt.Errorf("--job needs a job name; this run has: %s", clean(strings.Join(runLogJobNames(jobs), ", ")))
++		}
++		// Every match, not the first: a matrix leg or a workflow called twice can
++		// produce two jobs sharing a name, and quietly printing one of them is the
++		// short-output-you-cannot-trust outcome this flag is meant to avoid.
++		var matched []api.RunLogsJob
++		for _, j := range jobs {
++			// Case-insensitive: job names come from workflow YAML, and matching
++			// `SSH` against `ssh` is a typo worth forgiving rather than reporting.
++			if strings.EqualFold(j.Name, name) {
++				matched = append(matched, j)
++			}
++		}
++		if len(matched) > 0 {
++			return matched, nil
++		}
++		if len(jobs) == 0 {
++			return nil, fmt.Errorf("no job named %q: this run has no jobs", name)
++		}
++		return nil, fmt.Errorf("no job named %q; this run has: %s", name, clean(strings.Join(runLogJobNames(jobs), ", ")))
++	case failedOnly:
++		// "not success" is deliberately wider than "failure": a cancelled job, and
++		// one still running when the logs were fetched, are both things you want to
++		// see when asking what is not green.
++		out := make([]api.RunLogsJob, 0, len(jobs))
++		for _, j := range jobs {
++			if j.Status != "success" {
++				out = append(out, j)
++			}
++		}
++		return out, nil
++	default:
++		return jobs, nil
++	}
++}
++
++// runLogJobNames lists a run's job names, for the unknown-job error.
++func runLogJobNames(jobs []api.RunLogsJob) []string {
++	names := make([]string, 0, len(jobs))
++	for _, j := range jobs {
++		names = append(names, j.Name)
++	}
++	return names
++}
++
  func runRunRerun(cmd *cobra.Command, args []string) error {
  	client, err := newClient()
  	if err != nil {

added cmd/run_logs_filter_test.go +264 -0 Viewed

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	+}

++package cmd
++
++import (
++	"encoding/json"
++	"net/http"
++	"net/http/httptest"
++	"strings"
++	"testing"
++
++	"rickub.com/rickub/cli/internal/api"
++)
++
++// sixJobRun is the shape issue #8 is about: one red job among five that are noise.
++const sixJobRun = `{"number":30,"status":"failure","jobs":[
++  {"name":"engine","status":"success","log":"engine ok\n"},
++  {"name":"registry","status":"success","log":"registry ok\n"},
++  {"name":"web","status":"success","log":"web ok\n"},
++  {"name":"ssh","status":"failure","log":"##[error]getaddrinfo EAI_AGAIN golang.org\n"},
++  {"name":"lint","status":"cancelled","log":"lint cancelled\n"},
++  {"name":"coverage","status":"success","log":"coverage ok\n"}
++]}`
++
++func runLogsServer(t *testing.T, body string) *httptest.Server {
++	t.Helper()
++	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
++		w.Header().Set("Content-Type", "application/json")
++		_, _ = w.Write([]byte(body))
++	}))
++	t.Cleanup(srv.Close)
++	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
++	t.Setenv("RICKUB_HOST", srv.URL)
++	t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
++	return srv
++}
++
++// TestRunLogsJobPrintsOnlyThatJob is issue #8's acceptance criterion: the five
++// other jobs must not appear at all.
++func TestRunLogsJobPrintsOnlyThatJob(t *testing.T) {
++	runLogsServer(t, sixJobRun)
++
++	out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "ssh")
++	if err != nil {
++		t.Fatalf("execute: %v", err)
++	}
++	if !strings.Contains(out, "EAI_AGAIN") || !strings.Contains(out, "job: ssh") {
++		t.Errorf("the requested job is missing:\n%s", out)
++	}
++	for _, other := range []string{"engine ok", "registry ok", "web ok", "coverage ok", "lint cancelled"} {
++		if strings.Contains(out, other) {
++			t.Errorf("output still contains another job's log (%q):\n%s", other, out)
++		}
++	}
++}
++
++// TestRunLogsUnknownJobErrorsAndNamesTheRealOnes: a typo must NOT fall through to
++// the full log. That would look like it worked, which is worse than no flag.
++func TestRunLogsUnknownJobErrorsAndNamesTheRealOnes(t *testing.T) {
++	runLogsServer(t, sixJobRun)
++
++	out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "shh")
++	if err == nil {
++		t.Fatal("an unknown job name was accepted")
++	}
++	if strings.Contains(out, "engine ok") || strings.Contains(out, "EAI_AGAIN") {
++		t.Errorf("a bad --job printed logs anyway:\n%s", out)
++	}
++	for _, name := range []string{"engine", "registry", "web", "ssh", "lint", "coverage"} {
++		if !strings.Contains(err.Error(), name) {
++			t.Errorf("error should list the run's real job names, missing %q: %v", name, err)
++		}
++	}
++}
++
++// TestRunLogsJobIsCaseInsensitive: matching `SSH` against `ssh` is a typo worth
++// forgiving, not reporting.
++func TestRunLogsJobIsCaseInsensitive(t *testing.T) {
++	runLogsServer(t, sixJobRun)
++
++	out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "SSH")
++	if err != nil {
++		t.Fatalf("execute: %v", err)
++	}
++	if !strings.Contains(out, "EAI_AGAIN") {
++		t.Errorf("case-insensitive match failed:\n%s", out)
++	}
++}
++
++// TestRunLogsFailedSelectsEverythingNotGreen: "not success" is wider than
++// "failure" on purpose — a cancelled job is also not green.
++func TestRunLogsFailedSelectsEverythingNotGreen(t *testing.T) {
++	runLogsServer(t, sixJobRun)
++
++	out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--failed")
++	if err != nil {
++		t.Fatalf("execute: %v", err)
++	}
++	if !strings.Contains(out, "EAI_AGAIN") || !strings.Contains(out, "lint cancelled") {
++		t.Errorf("--failed should show the failed AND the cancelled job:\n%s", out)
++	}
++	for _, green := range []string{"engine ok", "registry ok", "web ok", "coverage ok"} {
++		if strings.Contains(out, green) {
++			t.Errorf("--failed leaked a successful job (%q):\n%s", green, out)
++		}
++	}
++}
++
++// TestRunLogsFailedOnAGreenRunSaysSo: never end on silence — an empty result and
++// a broken filter look identical otherwise.
++func TestRunLogsFailedOnAGreenRunSaysSo(t *testing.T) {
++	runLogsServer(t, `{"number":31,"status":"success","jobs":[{"name":"check","status":"success","log":"ok\n"}]}`)
++
++	out, _, err := execute(t, "run", "logs", "31", "-R", "o/r", "--failed")
++	if err != nil {
++		t.Fatalf("execute: %v", err)
++	}
++	if !strings.Contains(out, "Every job succeeded") {
++		t.Errorf("an empty --failed result should say so:\n%s", out)
++	}
++}
++
++// TestRunLogsJobComposesWithJSON: the filter must apply BEFORE the --json branch,
++// or `--job X --json` silently returns the whole run.
++func TestRunLogsJobComposesWithJSON(t *testing.T) {
++	runLogsServer(t, sixJobRun)
++
++	out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "ssh", "--json")
++	if err != nil {
++		t.Fatalf("execute: %v", err)
++	}
++	var decoded api.RunLogs
++	if err := json.Unmarshal([]byte(out), &decoded); err != nil {
++		t.Fatalf("output is not JSON: %v\n%s", err, out)
++	}
++	if len(decoded.Jobs) != 1 || decoded.Jobs[0].Name != "ssh" {
++		t.Errorf("--json should carry only the selected job, got %+v", decoded.Jobs)
++	}
++	if decoded.Number != 30 || decoded.Status != "failure" {
++		t.Errorf("the run's own fields should survive the filter: %+v", decoded)
++	}
++}
++
++// TestRunLogsJobAndFailedAreRefused: intersecting them prints nothing whenever the
++// named job passed, which is indistinguishable from the typo case --job exists to
++// rule out.
++func TestRunLogsJobAndFailedAreRefused(t *testing.T) {
++	runLogsServer(t, sixJobRun)
++
++	if _, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "ssh", "--failed"); err == nil {
++		t.Fatal("--job with --failed was accepted")
++	}
++}
++
++// TestRunLogsUnfilteredIsUnchanged: the no-flag behaviour every existing script
++// depends on still prints every job.
++func TestRunLogsUnfilteredIsUnchanged(t *testing.T) {
++	runLogsServer(t, sixJobRun)
++
++	out, _, err := execute(t, "run", "logs", "30", "-R", "o/r")
++	if err != nil {
++		t.Fatalf("execute: %v", err)
++	}
++	for _, name := range []string{"engine", "registry", "web", "ssh", "lint", "coverage"} {
++		if !strings.Contains(out, "job: "+name) {
++			t.Errorf("unfiltered output is missing job %q:\n%s", name, out)
++		}
++	}
++}
++
++// TestRunLogsEmptyJobIsRefused: `--job "$JOB"` with an unset variable must not
++// quietly print the whole run. Keying on "the flag was passed" rather than "the
++// value is non-empty" is what makes the filter trustworthy.
++func TestRunLogsEmptyJobIsRefused(t *testing.T) {
++	runLogsServer(t, sixJobRun)
++
++	out, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "")
++	if err == nil {
++		t.Fatal("an empty --job was accepted")
++	}
++	if strings.Contains(out, "engine ok") {
++		t.Errorf("an empty --job printed the whole run:\n%s", out)
++	}
++
++	// It must not defeat the mutual exclusion either.
++	if _, _, err := execute(t, "run", "logs", "30", "-R", "o/r", "--job", "", "--failed"); err == nil {
++		t.Fatal(`--job "" --failed was accepted`)
++	}
++}
++
++// TestRunLogsJoblessRunDoesNotClaimSuccess: a run that failed before any job
++// started has nothing to filter, and "every job succeeded" would assert the
++// opposite of what happened.
++func TestRunLogsJoblessRunDoesNotClaimSuccess(t *testing.T) {
++	runLogsServer(t, `{"number":32,"status":"failure","jobs":[]}`)
++
++	out, _, err := execute(t, "run", "logs", "32", "-R", "o/r", "--failed")
++	if err != nil {
++		t.Fatalf("execute: %v", err)
++	}
++	if strings.Contains(out, "Every job succeeded") {
++		t.Errorf("a jobless failed run was reported as all-green:\n%s", out)
++	}
++	if !strings.Contains(out, "no jobs") {
++		t.Errorf("expected a no-jobs message:\n%s", out)
++	}
++}
++
++// TestRunLogsJobPrintsEveryMatch: a matrix leg or a twice-called workflow can emit
++// two jobs sharing a name; printing one of them silently is the short output you
++// cannot trust.
++func TestRunLogsJobPrintsEveryMatch(t *testing.T) {
++	runLogsServer(t, `{"number":33,"status":"failure","jobs":[
++	  {"name":"test","status":"success","log":"leg one\n"},
++	  {"name":"other","status":"success","log":"unrelated\n"},
++	  {"name":"test","status":"failure","log":"leg two\n"}
++	]}`)
++
++	out, _, err := execute(t, "run", "logs", "33", "-R", "o/r", "--job", "test")
++	if err != nil {
++		t.Fatalf("execute: %v", err)
++	}
++	if !strings.Contains(out, "leg one") || !strings.Contains(out, "leg two") {
++		t.Errorf("both jobs sharing the name should be printed:\n%s", out)
++	}
++	if strings.Contains(out, "unrelated") {
++		t.Errorf("a non-matching job leaked:\n%s", out)
++	}
++}
++
++// TestRunLogsCleansServerJobNames: job names are server-controlled strings on
++// their way to a terminal. An escape sequence in one must not survive to the
++// screen — neither in the job header nor in the unknown-job error, which reaches
++// the user through main's bare "rickub: %s" with no cleaning of its own.
++func TestRunLogsCleansServerJobNames(t *testing.T) {
++	const esc = "\x1b"
++	// The job name carries ESC [2K (erase-line) between "ssh" and "masked".
++	evil := "ssh" + esc + "[2Kmasked"
++	body, err := json.Marshal(api.RunLogs{
++		Number: 34, Status: "failure",
++		Jobs: []api.RunLogsJob{{Name: evil, Status: "failure", Log: "boom\n"}},
++	})
++	if err != nil {
++		t.Fatalf("marshal: %v", err)
++	}
++	runLogsServer(t, string(body))
++
++	out, _, err := execute(t, "run", "logs", "34", "-R", "o/r")
++	if err != nil {
++		t.Fatalf("execute: %v", err)
++	}
++	if strings.Contains(out, esc) {
++		t.Errorf("an escape sequence survived into the job header: %q", out)
++	}
++
++	_, _, err = execute(t, "run", "logs", "34", "-R", "o/r", "--job", "nope")
++	if err == nil {
++		t.Fatal("expected an unknown-job error")
++	}
++	if strings.Contains(err.Error(), esc) {
++		t.Errorf("an escape sequence survived into the error: %q", err.Error())
++	}
++	if !strings.Contains(err.Error(), "ssh[2Kmasked") {
++		t.Errorf("the cleaned name should still be listed: %q", err.Error())
++	}
++}