From dc63480cf6b5e53303b69a62a4e895ca55db4277 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Thu, 8 Apr 2021 11:29:55 -0700 Subject: [PATCH 1/5] Add flag log-failed to display only logs of failed steps --- pkg/cmd/run/shared/shared.go | 9 +- pkg/cmd/run/view/fixtures/run_log.zip | Bin 1018 -> 1120 bytes pkg/cmd/run/view/view.go | 202 +++++++++--------------- pkg/cmd/run/view/view_test.go | 211 ++++++++++++++++++++++---- 4 files changed, 261 insertions(+), 161 deletions(-) diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index 7cfc822f6..bd82f623f 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -81,7 +81,7 @@ type Job struct { Status Status Conclusion Conclusion Name string - Steps []Step + Steps Steps StartedAt time.Time `json:"started_at"` CompletedAt time.Time `json:"completed_at"` URL string `json:"html_url"` @@ -93,8 +93,15 @@ type Step struct { Status Status Conclusion Conclusion Number int + Log string } +type Steps []Step + +func (s Steps) Len() int { return len(s) } +func (s Steps) Less(i, j int) bool { return s[i].Number < s[j].Number } +func (s Steps) Swap(i, j int) { s[i], s[j] = s[j], s[i] } + type Annotation struct { JobName string Message string diff --git a/pkg/cmd/run/view/fixtures/run_log.zip b/pkg/cmd/run/view/fixtures/run_log.zip index d30f088931b8e8706225779f10fe75f0e1948d8d..270310e374c4d3dad97c93984ef8d832cbda6ff4 100644 GIT binary patch literal 1120 zcmWIWW@h1H0D+6u9YJ6Ql;C8LVMxx;&r!(APtp$!;bdUuY{^dm;nE6j21b^zj0_Ac zB0$9fKqEjj2ZK^xN07jyl57x95s0OcjWCQ)%TH1$$w*a5N-V0 z=;@RG;U`X>@eVb3a;nj=SHLG(56L1)d=?qSgDe2s1hf%ukrB`$7UC^}1WEwVBA`d$ zfCK2G;=~k0n9Km0Iv0p>h6%_7^e};$U=&|iT3P`MlQg(fL54nP#xN9rnE2wc$Pg4H zX<&;$Ho`5MM2ba>OmfV)l8OX0gajDgI)a!;35OMua4-T0WE`$!12GPmz!;V^IwKp0 zEfE1N03{+k7GNe9WDC|HTYxR`04)F|9;_DNN+!qwA`0{fEXqSefTn@s7prNI@W4zm x$j%K%G7W12B5VO>QbD%h0Fnh*6AsV7yxCj_Y(jB literal 1018 zcmWIWW@h1H0D-xl?LlA$lwf6$VaUo)GSm+Z;bdUux|N#%!lf1542&#a85tN@M1Tqd zfZ9Pc2g6LDc7aDF*&v=G5DUPx8^#xxq!t+Jl~j~~O=1F?!iH(m>68B9Cr+O64mEgk zs?o4lz$aM`$#jt8u$pcJHr)trI>?PY#G4NBdJlH58$rDO;%08bJ0QmC^$zTgGtx)* z`hB1&4=_!_?{$#luz1}_AKmL9cfKUnbVepQW?bfEAJoFoFSO z6s|M?F$$Pk7?w2J!;C@+7NDu16oJQ7%(zE3buP?Ql;FVUR$Ot8?CswWQ$Z;JXcQ=9 z@i-7O{*fJM1@t{I1kq9h&{R-rz+)") - // TODO note about run view --log when that exists - fmt.Fprintf(out, cs.Gray("view this run on GitHub: %s\n"), run.URL) + if shared.IsFailureState(run.Conclusion) { + fmt.Fprintf(out, "To see what failed, try: gh run view %d --log-failed\n", run.ID) + } else { + fmt.Fprintln(out, "For more information about a job, try: gh run view --job=") + } + fmt.Fprintf(out, cs.Gray("View this run on GitHub: %s\n"), run.URL) + if opts.ExitStatus && shared.IsFailureState(run.Conclusion) { return cmdutil.SilentError } } else { fmt.Fprintln(out) - fmt.Fprintf(out, "To see the full job log, try: gh run view --log --job=%d\n", selectedJob.ID) - fmt.Fprintf(out, cs.Gray("view this run on GitHub: %s\n"), run.URL) + if shared.IsFailureState(selectedJob.Conclusion) { + fmt.Fprintf(out, "To see the log of steps that failed, try: gh run view --log-failed --job=%d\n", selectedJob.ID) + } else { + fmt.Fprintf(out, "To see the full job log, try: gh run view --log --job=%d\n", selectedJob.ID) + } + fmt.Fprintf(out, cs.Gray("View this run on GitHub: %s\n"), run.URL) if opts.ExitStatus && shared.IsFailureState(selectedJob.Conclusion) { return cmdutil.SilentError @@ -412,12 +390,6 @@ func getRunLog(httpClient *http.Client, repo ghrepo.Interface, runID int) (io.Re return getLog(httpClient, logURL) } -func getJobLog(httpClient *http.Client, repo ghrepo.Interface, jobID int) (io.ReadCloser, error) { - logURL := fmt.Sprintf("%srepos/%s/actions/jobs/%d/logs", - ghinstance.RESTPrefix(repo.RepoHost()), ghrepo.FullName(repo), jobID) - return getLog(httpClient, logURL) -} - func promptForJob(cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, error) { candidates := []string{"View all jobs in this run"} for _, job := range jobs { @@ -443,7 +415,8 @@ func promptForJob(cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, er return nil, nil } -// Structure of log zip file +// This function takes a zip file of logs and a list of jobs. +// Structure of zip file // zip/ // ├── jobname1/ // │ ├── 1_stepname.txt @@ -453,57 +426,38 @@ func promptForJob(cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, er // └── jobname2/ // ├── 1_stepname.txt // └── 2_somestepname.txt -func readRunLog(rlz io.ReadCloser) (runLog, error) { - rl := make(runLog) +// It iterates through the list of jobs and trys to find the matching +// log in the zip file. If the matching log is found it is attached +// to the job. +func readRunLog(rlz io.ReadCloser, jobs []shared.Job) error { defer rlz.Close() z, err := ioutil.ReadAll(rlz) if err != nil { - return rl, err + return err } zipReader, err := zip.NewReader(bytes.NewReader(z), int64(len(z))) if err != nil { - return rl, err + return err } - for _, zipFile := range zipReader.File { - dir, file := filepath.Split(zipFile.Name) - ext := filepath.Ext(zipFile.Name) - - // Skip all top level files and non-text files - if dir != "" && ext == ".txt" { - split := strings.Split(file, "_") - if len(split) != 2 { - return rl, errors.New("invalid step log filename") - } - - jobName := strings.TrimSuffix(dir, "/") - stepName := strings.TrimSuffix(split[1], ".txt") - stepOrder, err := strconv.Atoi(split[0]) - if err != nil { - return rl, errors.New("invalid step log filename") - } - - stepLogs, err := readZipFile(zipFile) - if err != nil { - return rl, err - } - - st := step{ - order: stepOrder, - name: stepName, - logs: string(stepLogs), - } - - if j, ok := rl[jobName]; !ok { - rl[jobName] = &job{name: jobName, steps: []step{st}} - } else { - j.steps = append(j.steps, st) + for i, job := range jobs { + for j, step := range job.Steps { + filename := fmt.Sprintf("%s/%d_%s.txt", job.Name, step.Number, step.Name) + for _, file := range zipReader.File { + if file.Name == filename { + log, err := readZipFile(file) + if err != nil { + return err + } + jobs[i].Steps[j].Log = string(log) + break + } } } } - return rl, nil + return nil } func readZipFile(zf *zip.File) ([]byte, error) { @@ -515,28 +469,22 @@ func readZipFile(zf *zip.File) ([]byte, error) { return ioutil.ReadAll(f) } -func displayRunLog(io *iostreams.IOStreams, rl runLog) error { +func displayRunLog(io *iostreams.IOStreams, jobs []shared.Job, failed bool) error { err := io.StartPager() if err != nil { return err } defer io.StopPager() - var jobNames []string - for name := range rl { - jobNames = append(jobNames, name) - } - sort.Strings(jobNames) - - for _, name := range jobNames { - job := rl[name] - steps := job.steps - sort.Slice(steps, func(i, j int) bool { - return steps[i].order < steps[j].order - }) + for _, job := range jobs { + steps := job.Steps + sort.Sort(steps) for _, step := range steps { - prefix := fmt.Sprintf("%s\t%s\t", job.name, step.name) - scanner := bufio.NewScanner(strings.NewReader(step.logs)) + if failed && !shared.IsFailureState(step.Conclusion) { + continue + } + prefix := fmt.Sprintf("%s\t%s\t", job.Name, step.Name) + scanner := bufio.NewScanner(strings.NewReader(step.Log)) for scanner.Scan() { fmt.Fprintf(io.Out, "%s%s\n", prefix, scanner.Text()) } diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index 067e318ea..cbaeb0b06 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -2,6 +2,7 @@ package view import ( "bytes" + "fmt" "io/ioutil" "net/http" "testing" @@ -60,6 +61,12 @@ func TestNewCmdView(t *testing.T) { cli: "-w --log", wantsErr: true, }, + { + name: "disallow log and log-failed", + tty: true, + cli: "--log --log-failed", + wantsErr: true, + }, { name: "exit status", cli: "--exit-status 1234", @@ -198,7 +205,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/check-runs/10/annotations"), httpmock.JSONResponse([]shared.Annotation{})) }, - wantOut: "\n✓ trunk successful #2898 · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nview this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful #2898 · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: runs/3\n", }, { name: "exit status, failed run", @@ -227,7 +234,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/check-runs/20/annotations"), httpmock.JSONResponse(shared.FailedJobAnnotations)) }, - wantOut: "\nX trunk failed · 1234\nTriggered via push about 59 minutes ago\n\nJOBS\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nFor more information about a job, try: gh run view --job=\nview this run on GitHub: runs/1234\n", + wantOut: "\nX trunk failed · 1234\nTriggered via push about 59 minutes ago\n\nJOBS\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nTo see what failed, try: gh run view 1234 --log-failed\nView this run on GitHub: runs/1234\n", wantErr: true, }, { @@ -256,20 +263,20 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse(shared.JobsPayload{})) }, wantOut: heredoc.Doc(` - + ✓ trunk successful · 3 Triggered via push about 59 minutes ago - + JOBS - - + + ARTIFACTS artifact-1 artifact-2 (expired) artifact-3 - + For more information about a job, try: gh run view --job= - view this run on GitHub: runs/3 + View this run on GitHub: runs/3 `), }, { @@ -299,7 +306,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/check-runs/10/annotations"), httpmock.JSONResponse([]shared.Annotation{})) }, - wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nview this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: runs/3\n", }, { name: "verbose", @@ -334,7 +341,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/check-runs/20/annotations"), httpmock.JSONResponse(shared.FailedJobAnnotations)) }, - wantOut: "\nX trunk failed · 1234\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nFor more information about a job, try: gh run view --job=\nview this run on GitHub: runs/1234\n", + wantOut: "\nX trunk failed · 1234\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nTo see what failed, try: gh run view 1234 --log-failed\nView this run on GitHub: runs/1234\n", }, { name: "prompts for choice, one job", @@ -371,7 +378,7 @@ func TestViewRun(t *testing.T) { opts: &ViewOptions{ Prompt: true, }, - wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nview this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: runs/3\n", }, { name: "interactive with log", @@ -398,14 +405,14 @@ func TestViewRun(t *testing.T) { }, })) reg.Register( - httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/10/logs"), - httpmock.StringResponse("it's a log\nfor this job\nbeautiful log\n")) + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), + httpmock.FileResponse("./fixtures/run_log.zip")) }, askStubs: func(as *prompt.AskStubber) { as.StubOne(2) as.StubOne(1) }, - wantOut: "it's a log\nfor this job\nbeautiful log\n", + wantOut: coolJobRunLogOutput, }, { name: "noninteractive with log", @@ -421,10 +428,10 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), httpmock.JSONResponse(shared.SuccessfulRun)) reg.Register( - httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/10/logs"), - httpmock.StringResponse("it's a log\nfor this job\nbeautiful log\n")) + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), + httpmock.FileResponse("./fixtures/run_log.zip")) }, - wantOut: "it's a log\nfor this job\nbeautiful log\n", + wantOut: coolJobRunLogOutput, }, { name: "interactive with run log", @@ -471,12 +478,132 @@ func TestViewRun(t *testing.T) { reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "runs/3/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + shared.FailedJob, + }, + })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), httpmock.FileResponse("./fixtures/run_log.zip")) }, wantOut: expectedRunLogOutput, }, + { + name: "interactive with log-failed", + tty: true, + opts: &ViewOptions{ + Prompt: true, + LogFailed: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs"), + httpmock.JSONResponse(shared.RunsPayload{ + WorkflowRuns: shared.TestRuns, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.JSONResponse(shared.FailedRun)) + reg.Register( + httpmock.REST("GET", "runs/1234/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + shared.FailedJob, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), + httpmock.FileResponse("./fixtures/run_log.zip")) + }, + askStubs: func(as *prompt.AskStubber) { + as.StubOne(4) + as.StubOne(2) + }, + wantOut: quuxTheBarfLogOutput, + }, + { + name: "noninteractive with log-failed", + opts: &ViewOptions{ + JobID: "20", + LogFailed: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/20"), + httpmock.JSONResponse(shared.FailedJob)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.JSONResponse(shared.FailedRun)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), + httpmock.FileResponse("./fixtures/run_log.zip")) + }, + wantOut: quuxTheBarfLogOutput, + }, + { + name: "interactive with run log-failed", + tty: true, + opts: &ViewOptions{ + Prompt: true, + LogFailed: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs"), + httpmock.JSONResponse(shared.RunsPayload{ + WorkflowRuns: shared.TestRuns, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.JSONResponse(shared.FailedRun)) + reg.Register( + httpmock.REST("GET", "runs/1234/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + shared.FailedJob, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), + httpmock.FileResponse("./fixtures/run_log.zip")) + }, + askStubs: func(as *prompt.AskStubber) { + as.StubOne(4) + as.StubOne(0) + }, + wantOut: quuxTheBarfLogOutput, + }, + { + name: "noninteractive with run log-failed", + tty: true, + opts: &ViewOptions{ + RunID: "1234", + LogFailed: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.JSONResponse(shared.FailedRun)) + reg.Register( + httpmock.REST("GET", "runs/1234/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + shared.FailedJob, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), + httpmock.FileResponse("./fixtures/run_log.zip")) + }, + wantOut: quuxTheBarfLogOutput, + }, { name: "run log but run is not done", tty: true, @@ -488,6 +615,11 @@ func TestViewRun(t *testing.T) { reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/2"), httpmock.JSONResponse(shared.TestRun("in progress", 2, shared.InProgress, ""))) + reg.Register( + httpmock.REST("GET", "runs/2/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{}, + })) }, wantErr: true, errMsg: "run 2 is still in progress; logs will be available when it is complete", @@ -530,7 +662,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/check-runs/10/annotations"), httpmock.JSONResponse([]shared.Annotation{})) }, - wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\nTo see the full job log, try: gh run view --log --job=10\nview this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\nTo see the full job log, try: gh run view --log --job=10\nView this run on GitHub: runs/3\n", }, { name: "interactive, multiple jobs, choose all jobs", @@ -569,7 +701,7 @@ func TestViewRun(t *testing.T) { as.StubOne(2) as.StubOne(0) }, - wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nFor more information about a job, try: gh run view --job=\nview this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: runs/3\n", }, { name: "interactive, multiple jobs, choose specific jobs", @@ -602,7 +734,7 @@ func TestViewRun(t *testing.T) { as.StubOne(2) as.StubOne(1) }, - wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\nTo see the full job log, try: gh run view --log --job=10\nview this run on GitHub: runs/3\n", + wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\nTo see the full job log, try: gh run view --log --job=10\nView this run on GitHub: runs/3\n", }, { name: "web run", @@ -690,17 +822,30 @@ func TestViewRun(t *testing.T) { } } -var expectedRunLogOutput = heredoc.Doc(` -job1 step1 log line 1 -job1 step1 log line 2 -job1 step1 log line 3 -job1 step2 log line 1 -job1 step2 log line 2 -job1 step2 log line 3 -job2 step1 log line 1 -job2 step1 log line 2 -job2 step1 log line 3 -job2 step2 log line 1 -job2 step2 log line 2 -job2 step2 log line 3 +var barfTheFobLogOutput = heredoc.Doc(` +cool job barz the fob log line 1 +cool job barz the fob log line 2 +cool job barz the fob log line 3 `) + +var fobTheBarzLogOutput = heredoc.Doc(` +cool job fob the barz log line 1 +cool job fob the barz log line 2 +cool job fob the barz log line 3 +`) + +var barfTheQuuxLogOutput = heredoc.Doc(` +sad job barf the quux log line 1 +sad job barf the quux log line 2 +sad job barf the quux log line 3 +`) + +var quuxTheBarfLogOutput = heredoc.Doc(` +sad job quux the barf log line 1 +sad job quux the barf log line 2 +sad job quux the barf log line 3 +`) + +var coolJobRunLogOutput = fmt.Sprintf("%s%s", fobTheBarzLogOutput, barfTheFobLogOutput) +var sadJobRunLogOutput = fmt.Sprintf("%s%s", barfTheQuuxLogOutput, quuxTheBarfLogOutput) +var expectedRunLogOutput = fmt.Sprintf("%s%s", coolJobRunLogOutput, sadJobRunLogOutput) From 6b2b7922411301f786397c715a4223d19f47d9c6 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Thu, 8 Apr 2021 15:57:57 -0700 Subject: [PATCH 2/5] linter --- pkg/cmd/run/view/view.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 375084ca4..158373826 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -31,8 +31,6 @@ type browser interface { Browse(string) error } -type runLog map[string]*shared.Job - type ViewOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams From 4a610f13cf777ef7e5788a411dec322e0b1fe62f Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Fri, 9 Apr 2021 09:19:15 -0700 Subject: [PATCH 3/5] tweak wording --- pkg/cmd/run/view/view.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 158373826..4dce50069 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -120,7 +120,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd.Flags().BoolVar(&opts.ExitStatus, "exit-status", false, "Exit with non-zero status if run failed") cmd.Flags().StringVarP(&opts.JobID, "job", "j", "", "View a specific job ID from a run") cmd.Flags().BoolVar(&opts.Log, "log", false, "View full log for either a run or specific job") - cmd.Flags().BoolVar(&opts.LogFailed, "log-failed", false, "View log of failed steps for either a run or specific job") + cmd.Flags().BoolVar(&opts.LogFailed, "log-failed", false, "View the log for any failed steps in a run or specific job") cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open run in the browser") return cmd @@ -336,7 +336,7 @@ func runView(opts *ViewOptions) error { } else { fmt.Fprintln(out) if shared.IsFailureState(selectedJob.Conclusion) { - fmt.Fprintf(out, "To see the log of steps that failed, try: gh run view --log-failed --job=%d\n", selectedJob.ID) + fmt.Fprintf(out, "To see the logs for the failed steps, try: gh run view --log-failed --job=%d\n", selectedJob.ID) } else { fmt.Fprintf(out, "To see the full job log, try: gh run view --log --job=%d\n", selectedJob.ID) } From 8a4a8dd451b90ca0d6ac6000043f404c91de50df Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Fri, 9 Apr 2021 12:13:01 -0700 Subject: [PATCH 4/5] Moar memory --- pkg/cmd/run/shared/shared.go | 3 +- pkg/cmd/run/view/view.go | 112 ++++++++++++++++++++++------------ pkg/cmd/run/view/view_test.go | 16 +++++ 3 files changed, 90 insertions(+), 41 deletions(-) diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index bd82f623f..90a16071d 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -1,6 +1,7 @@ package shared import ( + "archive/zip" "fmt" "net/url" "strings" @@ -93,7 +94,7 @@ type Step struct { Status Status Conclusion Conclusion Number int - Log string + Log *zip.File } type Steps []Step diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 4dce50069..ce8050a37 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -3,15 +3,15 @@ package view import ( "archive/zip" "bufio" - "bytes" "errors" "fmt" "io" "io/ioutil" "net/http" + "os" + "path/filepath" "sort" "strconv" - "strings" "time" "github.com/AlecAivazis/survey/v2" @@ -31,11 +31,39 @@ type browser interface { Browse(string) error } +type runLogCache interface { + Exists(string) bool + Create(string, io.ReadCloser) error + Open(string) (*zip.ReadCloser, error) +} + +type rlc struct{} + +func (rlc) Exists(path string) bool { + if _, err := os.Stat(path); err != nil { + return false + } + return true +} +func (rlc) Create(path string, content io.ReadCloser) error { + out, err := os.Create(path) + if err != nil { + return err + } + defer out.Close() + _, err = io.Copy(out, content) + return err +} +func (rlc) Open(path string) (*zip.ReadCloser, error) { + return zip.OpenReader(path) +} + type ViewOptions struct { - HttpClient func() (*http.Client, error) - IO *iostreams.IOStreams - BaseRepo func() (ghrepo.Interface, error) - Browser browser + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Browser browser + RunLogCache runLogCache RunID string JobID string @@ -52,10 +80,11 @@ type ViewOptions struct { func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { opts := &ViewOptions{ - IO: f.IOStreams, - HttpClient: f.HttpClient, - Now: time.Now, - Browser: f.Browser, + IO: f.IOStreams, + HttpClient: f.HttpClient, + Now: time.Now, + Browser: f.Browser, + RunLogCache: rlc{}, } cmd := &cobra.Command{ @@ -228,16 +257,14 @@ func runView(opts *ViewOptions) error { } opts.IO.StartProgressIndicator() - runLogZip, err := getRunLog(httpClient, repo, run.ID) + runLogZip, err := getRunLog(opts.RunLogCache, httpClient, repo, run.ID) + defer runLogZip.Close() opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to get run log: %w", err) } - err = readRunLog(runLogZip, jobs) - if err != nil { - return err - } + attachRunLog(runLogZip, jobs) return displayRunLog(opts.IO, jobs, opts.LogFailed) } @@ -382,10 +409,27 @@ func getLog(httpClient *http.Client, logURL string) (io.ReadCloser, error) { return resp.Body, nil } -func getRunLog(httpClient *http.Client, repo ghrepo.Interface, runID int) (io.ReadCloser, error) { - logURL := fmt.Sprintf("%srepos/%s/actions/runs/%d/logs", - ghinstance.RESTPrefix(repo.RepoHost()), ghrepo.FullName(repo), runID) - return getLog(httpClient, logURL) +func getRunLog(cache runLogCache, httpClient *http.Client, repo ghrepo.Interface, runID int) (*zip.ReadCloser, error) { + filename := fmt.Sprintf("run-log-%d.zip", runID) + filepath := filepath.Join(os.TempDir(), "gh-cli-cache", filename) + if !cache.Exists(filepath) { + // Run log does not exist in cache so retrieve and store it + logURL := fmt.Sprintf("%srepos/%s/actions/runs/%d/logs", + ghinstance.RESTPrefix(repo.RepoHost()), ghrepo.FullName(repo), runID) + + resp, err := getLog(httpClient, logURL) + if err != nil { + return nil, err + } + defer resp.Close() + + err = cache.Create(filepath, resp) + if err != nil { + return nil, err + } + } + + return cache.Open(filepath) } func promptForJob(cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, error) { @@ -427,35 +471,18 @@ func promptForJob(cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, er // It iterates through the list of jobs and trys to find the matching // log in the zip file. If the matching log is found it is attached // to the job. -func readRunLog(rlz io.ReadCloser, jobs []shared.Job) error { - defer rlz.Close() - z, err := ioutil.ReadAll(rlz) - if err != nil { - return err - } - - zipReader, err := zip.NewReader(bytes.NewReader(z), int64(len(z))) - if err != nil { - return err - } - +func attachRunLog(rlz *zip.ReadCloser, jobs []shared.Job) { for i, job := range jobs { for j, step := range job.Steps { filename := fmt.Sprintf("%s/%d_%s.txt", job.Name, step.Number, step.Name) - for _, file := range zipReader.File { + for _, file := range rlz.File { if file.Name == filename { - log, err := readZipFile(file) - if err != nil { - return err - } - jobs[i].Steps[j].Log = string(log) + jobs[i].Steps[j].Log = file break } } } } - - return nil } func readZipFile(zf *zip.File) ([]byte, error) { @@ -482,10 +509,15 @@ func displayRunLog(io *iostreams.IOStreams, jobs []shared.Job, failed bool) erro continue } prefix := fmt.Sprintf("%s\t%s\t", job.Name, step.Name) - scanner := bufio.NewScanner(strings.NewReader(step.Log)) + f, err := step.Log.Open() + if err != nil { + return err + } + scanner := bufio.NewScanner(f) for scanner.Scan() { fmt.Fprintf(io.Out, "%s%s\n", prefix, scanner.Text()) } + f.Close() } } diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index cbaeb0b06..355ac3c68 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -1,8 +1,10 @@ package view import ( + "archive/zip" "bytes" "fmt" + "io" "io/ioutil" "net/http" "testing" @@ -798,6 +800,8 @@ func TestViewRun(t *testing.T) { browser := &cmdutil.TestBrowser{} tt.opts.Browser = browser + rlc := testRunLogCache{} + tt.opts.RunLogCache = rlc t.Run(tt.name, func(t *testing.T) { err := runView(tt.opts) @@ -822,6 +826,18 @@ func TestViewRun(t *testing.T) { } } +type testRunLogCache struct{} + +func (testRunLogCache) Exists(path string) bool { + return false +} +func (testRunLogCache) Create(path string, content io.ReadCloser) error { + return nil +} +func (testRunLogCache) Open(path string) (*zip.ReadCloser, error) { + return zip.OpenReader("./fixtures/run_log.zip") +} + var barfTheFobLogOutput = heredoc.Doc(` cool job barz the fob log line 1 cool job barz the fob log line 2 From 33d601746744c3d2714bbbbed9f57007e4fe719a Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Fri, 9 Apr 2021 12:16:31 -0700 Subject: [PATCH 5/5] linter --- pkg/cmd/run/view/view.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index ce8050a37..d4d0db1f6 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "net/http" "os" "path/filepath" @@ -258,11 +257,11 @@ func runView(opts *ViewOptions) error { opts.IO.StartProgressIndicator() runLogZip, err := getRunLog(opts.RunLogCache, httpClient, repo, run.ID) - defer runLogZip.Close() opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to get run log: %w", err) } + defer runLogZip.Close() attachRunLog(runLogZip, jobs) @@ -485,15 +484,6 @@ func attachRunLog(rlz *zip.ReadCloser, jobs []shared.Job) { } } -func readZipFile(zf *zip.File) ([]byte, error) { - f, err := zf.Open() - if err != nil { - return nil, err - } - defer f.Close() - return ioutil.ReadAll(f) -} - func displayRunLog(io *iostreams.IOStreams, jobs []shared.Job, failed bool) error { err := io.StartPager() if err != nil {