From b705b3d6ba0589ed8079e29b6fe5d90d8da76f68 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 5 Apr 2021 14:11:06 -0500 Subject: [PATCH 1/8] make ExitStatus reflect focused job --- pkg/cmd/run/view/view.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 7be430b98..694b3713e 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -70,7 +70,6 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman # Exit non-zero if a run failed $ gh run view 0451 -e && echo "run pending or passed" `), - // TODO should exit status respect only a selected job if --job is passed? RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo @@ -213,7 +212,7 @@ func runView(opts *ViewOptions) error { return fmt.Errorf("failed to read log: %w", err) } - if opts.ExitStatus && shared.IsFailureState(run.Conclusion) { + if opts.ExitStatus && shared.IsFailureState(selectedJob.Conclusion) { return cmdutil.SilentError } @@ -296,15 +295,18 @@ func runView(opts *ViewOptions) error { fmt.Fprintln(out, "For more information about a job, try: gh run view --job=") // TODO note about run view --log when that exists 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) // TODO this does not exist yet 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(run.Conclusion) { - return cmdutil.SilentError + if opts.ExitStatus && shared.IsFailureState(selectedJob.Conclusion) { + return cmdutil.SilentError + } } return nil From b5fc794b7843f148d7ea75f49dbf7d942af4d0bb Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 5 Apr 2021 14:37:21 -0500 Subject: [PATCH 2/8] support --log for runs --- pkg/cmd/run/view/view.go | 67 +++++++++++++++++++--- pkg/cmd/run/view/view_test.go | 104 ++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 694b3713e..4ab8b2ed9 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -5,6 +5,8 @@ import ( "fmt" "io" "net/http" + "os" + "path" "time" "github.com/AlecAivazis/survey/v2" @@ -39,7 +41,8 @@ type ViewOptions struct { Prompt bool - Now func() time.Time + Now func() time.Time + CreateFile func(string) (io.Writer, error) } func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { @@ -48,6 +51,9 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman HttpClient: f.HttpClient, Now: time.Now, Browser: f.Browser, + CreateFile: func(fullPath string) (io.Writer, error) { + return os.Create(fullPath) + }, } cmd := &cobra.Command{ Use: "view []", @@ -196,6 +202,10 @@ func runView(opts *ViewOptions) error { opts.IO.StartProgressIndicator() if opts.Log && selectedJob != nil { + if selectedJob.Status != shared.Completed { + return fmt.Errorf("job %d is still in progress; logs will be available when it is complete", selectedJob.ID) + } + r, err := jobLog(httpClient, repo, selectedJob.ID) if err != nil { return err @@ -219,7 +229,40 @@ func runView(opts *ViewOptions) error { return nil } - // TODO support --log without selectedJob + if opts.Log { + if run.Status != shared.Completed { + return fmt.Errorf("run %d is still in progress; logs will be available when it is complete", run.ID) + } + + filename := fmt.Sprintf("gh-run-log-%d.zip", run.ID) + dir := os.TempDir() + fullpath := path.Join(dir, filename) + f, err := opts.CreateFile(fullpath) + if err != nil { + return fmt.Errorf("failed to open %s: %w", fullpath, err) + } + + r, err := runLog(httpClient, repo, run.ID) + if err != nil { + return fmt.Errorf("failed to get run log: %w", err) + } + + if _, err := io.Copy(f, r); err != nil { + return fmt.Errorf("failed to download log: %w", err) + } + + opts.IO.StopProgressIndicator() + if opts.IO.IsStdoutTTY() { + cs := opts.IO.ColorScheme() + fmt.Fprintf(opts.IO.Out, "%s Downloaded logs to %s\n", cs.SuccessIcon(), fullpath) + } + + if opts.ExitStatus && shared.IsFailureState(run.Conclusion) { + return cmdutil.SilentError + } + + return nil + } if selectedJob == nil && len(jobs) == 0 { jobs, err = shared.GetJobs(client, repo, *run) @@ -324,10 +367,8 @@ func getJob(client *api.Client, repo ghrepo.Interface, jobID string) (*shared.Jo return &result, nil } -func jobLog(httpClient *http.Client, repo ghrepo.Interface, jobID int) (io.ReadCloser, error) { - url := fmt.Sprintf("%srepos/%s/actions/jobs/%d/logs", - ghinstance.RESTPrefix(repo.RepoHost()), ghrepo.FullName(repo), jobID) - req, err := http.NewRequest("GET", url, nil) +func getLog(httpClient *http.Client, logURL string) (io.ReadCloser, error) { + req, err := http.NewRequest("GET", logURL, nil) if err != nil { return nil, err } @@ -338,7 +379,7 @@ func jobLog(httpClient *http.Client, repo ghrepo.Interface, jobID int) (io.ReadC } if resp.StatusCode == 404 { - return nil, errors.New("job not found") + return nil, errors.New("log not found") } else if resp.StatusCode != 200 { return nil, api.HandleHTTPError(resp) } @@ -346,6 +387,18 @@ func jobLog(httpClient *http.Client, repo ghrepo.Interface, jobID int) (io.ReadC return resp.Body, nil } +func runLog(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 jobLog(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 { diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index 49eff0fa9..34c4cd56e 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" + "io" "io/ioutil" "net/http" "testing" @@ -158,6 +159,8 @@ func TestViewRun(t *testing.T) { wantErr bool wantOut string browsedURL string + wantWrite string + errMsg string }{ { name: "associate with PR", @@ -368,6 +371,96 @@ func TestViewRun(t *testing.T) { }, wantOut: "it's a log\nfor this job\nbeautiful log\n", }, + { + name: "interactive with run log", + tty: true, + opts: &ViewOptions{ + Prompt: true, + Log: 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/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.StringResponse("pretend these bytes constitute a zip file")) + }, + askStubs: func(as *prompt.AskStubber) { + as.StubOne(2) + as.StubOne(0) + }, + wantOut: "✓ Downloaded logs to /tmp/gh-run-log-3.zip\n", + wantWrite: "pretend these bytes constitute a zip file", + }, + { + name: "noninteractive with run log", + tty: true, + opts: &ViewOptions{ + RunID: "3", + Log: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), + httpmock.StringResponse("pretend these bytes constitute a zip file")) + }, + wantOut: "✓ Downloaded logs to /tmp/gh-run-log-3.zip\n", + wantWrite: "pretend these bytes constitute a zip file", + }, + { + name: "run log but run is not done", + tty: true, + opts: &ViewOptions{ + RunID: "2", + Log: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/2"), + httpmock.JSONResponse(shared.TestRun("in progress", 2, shared.InProgress, ""))) + }, + wantErr: true, + errMsg: "run 2 is still in progress; logs will be available when it is complete", + }, + { + name: "job log but job is not done", + tty: true, + opts: &ViewOptions{ + JobID: "20", + Log: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/20"), + httpmock.JSONResponse(shared.Job{ + ID: 20, + Status: shared.InProgress, + RunID: 2, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/2"), + httpmock.JSONResponse(shared.TestRun("in progress", 2, shared.InProgress, ""))) + }, + wantErr: true, + errMsg: "job 20 is still in progress; logs will be available when it is complete", + }, { name: "noninteractive with job", opts: &ViewOptions{ @@ -502,6 +595,11 @@ func TestViewRun(t *testing.T) { return notnow } + fileBuff := bytes.Buffer{} + tt.opts.CreateFile = func(fullPath string) (io.Writer, error) { + return &fileBuff, nil + } + io, _, stdout, _ := iostreams.Test() io.SetStdoutTTY(tt.tty) tt.opts.IO = io @@ -522,6 +620,9 @@ func TestViewRun(t *testing.T) { err := runView(tt.opts) if tt.wantErr { assert.Error(t, err) + if tt.errMsg != "" { + assert.Equal(t, tt.errMsg, err.Error()) + } if !tt.opts.ExitStatus { return } @@ -533,6 +634,9 @@ func TestViewRun(t *testing.T) { if tt.browsedURL != "" { assert.Equal(t, tt.browsedURL, browser.BrowsedURL()) } + if tt.wantWrite != "" { + assert.Equal(t, tt.wantWrite, fileBuff.String()) + } reg.Verify(t) }) } From 97b15fea2d269cc579ea299a9af33e7c61a685ba Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 5 Apr 2021 15:33:31 -0500 Subject: [PATCH 3/8] xplatform oops --- pkg/cmd/run/view/view_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index 34c4cd56e..ffa46ad06 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -5,6 +5,8 @@ import ( "io" "io/ioutil" "net/http" + "os" + "path" "testing" "time" @@ -403,7 +405,7 @@ func TestViewRun(t *testing.T) { as.StubOne(2) as.StubOne(0) }, - wantOut: "✓ Downloaded logs to /tmp/gh-run-log-3.zip\n", + wantOut: "✓ Downloaded logs to " + path.Join(os.TempDir(), "gh-run-log-3.zip") + "\n", wantWrite: "pretend these bytes constitute a zip file", }, { From c8d1d6e8b4635a6d07b78e8daad83716762e5413 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 5 Apr 2021 15:39:14 -0500 Subject: [PATCH 4/8] sigh --- pkg/cmd/run/view/view_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index ffa46ad06..1cbd7fda6 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -423,7 +423,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), httpmock.StringResponse("pretend these bytes constitute a zip file")) }, - wantOut: "✓ Downloaded logs to /tmp/gh-run-log-3.zip\n", + wantOut: "✓ Downloaded logs to " + path.Join(os.TempDir(), "gh-run-log-3.zip") + "\n", wantWrite: "pretend these bytes constitute a zip file", }, { From 67e45f1bceddb3dc532da047994a08be0e2bd1ab Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 6 Apr 2021 11:36:29 -0700 Subject: [PATCH 5/8] Display all run logs --- pkg/cmd/run/view/fixtures/run_log.zip | Bin 0 -> 1018 bytes pkg/cmd/run/view/view.go | 162 +++++++++++++++++++++----- pkg/cmd/run/view/view_test.go | 41 ++++--- 3 files changed, 155 insertions(+), 48 deletions(-) create mode 100644 pkg/cmd/run/view/fixtures/run_log.zip diff --git a/pkg/cmd/run/view/fixtures/run_log.zip b/pkg/cmd/run/view/fixtures/run_log.zip new file mode 100644 index 0000000000000000000000000000000000000000..d30f088931b8e8706225779f10fe75f0e1948d8d GIT binary patch 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+)]", Short: "View a summary of a workflow run", @@ -234,34 +250,19 @@ func runView(opts *ViewOptions) error { return fmt.Errorf("run %d is still in progress; logs will be available when it is complete", run.ID) } - filename := fmt.Sprintf("gh-run-log-%d.zip", run.ID) - dir := os.TempDir() - fullpath := path.Join(dir, filename) - f, err := opts.CreateFile(fullpath) - if err != nil { - return fmt.Errorf("failed to open %s: %w", fullpath, err) - } - - r, err := runLog(httpClient, repo, run.ID) + runLogZip, err := runLog(httpClient, repo, run.ID) if err != nil { return fmt.Errorf("failed to get run log: %w", err) } - - if _, err := io.Copy(f, r); err != nil { - return fmt.Errorf("failed to download log: %w", err) - } - opts.IO.StopProgressIndicator() - if opts.IO.IsStdoutTTY() { - cs := opts.IO.ColorScheme() - fmt.Fprintf(opts.IO.Out, "%s Downloaded logs to %s\n", cs.SuccessIcon(), fullpath) + + logs, err := readLogsFromZip(runLogZip) + if err != nil { + return err } - if opts.ExitStatus && shared.IsFailureState(run.Conclusion) { - return cmdutil.SilentError - } - - return nil + err = displayLogs(opts.IO, logs) + return err } if selectedJob == nil && len(jobs) == 0 { @@ -423,3 +424,106 @@ func promptForJob(cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, er // User wants to see all jobs return nil, nil } + +// Structure of log zip file +// zip/ +// ├── jobname1/ +// │ ├── 1_stepname.txt +// │ ├── 2_anotherstepname.txt +// │ ├── 3_stepstepname.txt +// │ └── 4_laststepname.txt +// └── jobname2/ +// ├── 1_stepname.txt +// └── 2_somestepname.txt +func readLogsFromZip(lz io.ReadCloser) (logs, error) { + ls := make(logs) + defer lz.Close() + z, err := ioutil.ReadAll(lz) + if err != nil { + return ls, err + } + + zipReader, err := zip.NewReader(bytes.NewReader(z), int64(len(z))) + if err != nil { + return ls, 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 ls, 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 ls, errors.New("invalid step log filename") + } + + stepLogs, err := readZipFile(zipFile) + if err != nil { + return ls, err + } + + st := step{ + order: stepOrder, + name: stepName, + logs: string(stepLogs), + } + + if j, ok := ls[jobName]; !ok { + ls[jobName] = &job{name: jobName, steps: []step{st}} + } else { + j.steps = append(j.steps, st) + } + } + } + + return ls, nil +} + +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 displayLogs(io *iostreams.IOStreams, ls logs) error { + err := io.StartPager() + if err != nil { + return err + } + defer io.StopPager() + + var jobNames []string + for name := range ls { + jobNames = append(jobNames, name) + } + sort.Strings(jobNames) + + for _, name := range jobNames { + job := ls[name] + steps := job.steps + sort.Slice(steps, func(i, j int) bool { + return steps[i].order < steps[j].order + }) + for _, step := range steps { + prefix := fmt.Sprintf("%s\t%s\t", job.name, step.name) + scanner := bufio.NewScanner(strings.NewReader(step.logs)) + for scanner.Scan() { + fmt.Fprintf(io.Out, "%s%s\n", prefix, scanner.Text()) + } + } + } + + return nil +} diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index 1cbd7fda6..d665aa033 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -2,14 +2,12 @@ package view import ( "bytes" - "io" "io/ioutil" "net/http" - "os" - "path" "testing" "time" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/cmd/run/shared" "github.com/cli/cli/pkg/cmdutil" @@ -151,7 +149,6 @@ func TestNewCmdView(t *testing.T) { } func TestViewRun(t *testing.T) { - tests := []struct { name string httpStubs func(*httpmock.Registry) @@ -161,7 +158,6 @@ func TestViewRun(t *testing.T) { wantErr bool wantOut string browsedURL string - wantWrite string errMsg string }{ { @@ -399,14 +395,13 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.StringResponse("pretend these bytes constitute a zip file")) + httpmock.FileResponse("./fixtures/run_log.zip")) }, askStubs: func(as *prompt.AskStubber) { as.StubOne(2) as.StubOne(0) }, - wantOut: "✓ Downloaded logs to " + path.Join(os.TempDir(), "gh-run-log-3.zip") + "\n", - wantWrite: "pretend these bytes constitute a zip file", + wantOut: runLogOutput(), }, { name: "noninteractive with run log", @@ -421,10 +416,9 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse(shared.SuccessfulRun)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.StringResponse("pretend these bytes constitute a zip file")) + httpmock.FileResponse("./fixtures/run_log.zip")) }, - wantOut: "✓ Downloaded logs to " + path.Join(os.TempDir(), "gh-run-log-3.zip") + "\n", - wantWrite: "pretend these bytes constitute a zip file", + wantOut: runLogOutput(), }, { name: "run log but run is not done", @@ -597,11 +591,6 @@ func TestViewRun(t *testing.T) { return notnow } - fileBuff := bytes.Buffer{} - tt.opts.CreateFile = func(fullPath string) (io.Writer, error) { - return &fileBuff, nil - } - io, _, stdout, _ := iostreams.Test() io.SetStdoutTTY(tt.tty) tt.opts.IO = io @@ -636,10 +625,24 @@ func TestViewRun(t *testing.T) { if tt.browsedURL != "" { assert.Equal(t, tt.browsedURL, browser.BrowsedURL()) } - if tt.wantWrite != "" { - assert.Equal(t, tt.wantWrite, fileBuff.String()) - } reg.Verify(t) }) } } + +func runLogOutput() string { + return 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 +`) +} From 5cb4ece75420b22c566bca6117fb86e188dce06d Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 7 Apr 2021 10:10:11 -0700 Subject: [PATCH 6/8] Renaming and cleanup --- pkg/cmd/run/view/view.go | 44 +++++++++++++++++------------------ pkg/cmd/run/view/view_test.go | 8 +++---- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index f0b223824..af3194ba8 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -32,7 +32,7 @@ type browser interface { Browse(string) error } -type logs map[string]*job +type runLog map[string]*job type job struct { name string @@ -222,7 +222,7 @@ func runView(opts *ViewOptions) error { return fmt.Errorf("job %d is still in progress; logs will be available when it is complete", selectedJob.ID) } - r, err := jobLog(httpClient, repo, selectedJob.ID) + r, err := getJobLog(httpClient, repo, selectedJob.ID) if err != nil { return err } @@ -250,18 +250,18 @@ func runView(opts *ViewOptions) error { return fmt.Errorf("run %d is still in progress; logs will be available when it is complete", run.ID) } - runLogZip, err := runLog(httpClient, repo, run.ID) + runLogZip, err := getRunLog(httpClient, repo, run.ID) if err != nil { return fmt.Errorf("failed to get run log: %w", err) } opts.IO.StopProgressIndicator() - logs, err := readLogsFromZip(runLogZip) + runLog, err := readRunLog(runLogZip) if err != nil { return err } - err = displayLogs(opts.IO, logs) + err = displayRunLog(opts.IO, runLog) return err } @@ -388,13 +388,13 @@ func getLog(httpClient *http.Client, logURL string) (io.ReadCloser, error) { return resp.Body, nil } -func runLog(httpClient *http.Client, repo ghrepo.Interface, runID int) (io.ReadCloser, error) { +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 jobLog(httpClient *http.Client, repo ghrepo.Interface, jobID int) (io.ReadCloser, error) { +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) @@ -435,17 +435,17 @@ func promptForJob(cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, er // └── jobname2/ // ├── 1_stepname.txt // └── 2_somestepname.txt -func readLogsFromZip(lz io.ReadCloser) (logs, error) { - ls := make(logs) - defer lz.Close() - z, err := ioutil.ReadAll(lz) +func readRunLog(rlz io.ReadCloser) (runLog, error) { + rl := make(runLog) + defer rlz.Close() + z, err := ioutil.ReadAll(rlz) if err != nil { - return ls, err + return rl, err } zipReader, err := zip.NewReader(bytes.NewReader(z), int64(len(z))) if err != nil { - return ls, err + return rl, err } for _, zipFile := range zipReader.File { @@ -456,19 +456,19 @@ func readLogsFromZip(lz io.ReadCloser) (logs, error) { if dir != "" && ext == ".txt" { split := strings.Split(file, "_") if len(split) != 2 { - return ls, errors.New("invalid step log filename") + 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 ls, errors.New("invalid step log filename") + return rl, errors.New("invalid step log filename") } stepLogs, err := readZipFile(zipFile) if err != nil { - return ls, err + return rl, err } st := step{ @@ -477,15 +477,15 @@ func readLogsFromZip(lz io.ReadCloser) (logs, error) { logs: string(stepLogs), } - if j, ok := ls[jobName]; !ok { - ls[jobName] = &job{name: jobName, steps: []step{st}} + if j, ok := rl[jobName]; !ok { + rl[jobName] = &job{name: jobName, steps: []step{st}} } else { j.steps = append(j.steps, st) } } } - return ls, nil + return rl, nil } func readZipFile(zf *zip.File) ([]byte, error) { @@ -497,7 +497,7 @@ func readZipFile(zf *zip.File) ([]byte, error) { return ioutil.ReadAll(f) } -func displayLogs(io *iostreams.IOStreams, ls logs) error { +func displayRunLog(io *iostreams.IOStreams, rl runLog) error { err := io.StartPager() if err != nil { return err @@ -505,13 +505,13 @@ func displayLogs(io *iostreams.IOStreams, ls logs) error { defer io.StopPager() var jobNames []string - for name := range ls { + for name := range rl { jobNames = append(jobNames, name) } sort.Strings(jobNames) for _, name := range jobNames { - job := ls[name] + job := rl[name] steps := job.steps sort.Slice(steps, func(i, j int) bool { return steps[i].order < steps[j].order diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index d665aa033..cec86fd24 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -401,7 +401,7 @@ func TestViewRun(t *testing.T) { as.StubOne(2) as.StubOne(0) }, - wantOut: runLogOutput(), + wantOut: expectedRunLogOutput, }, { name: "noninteractive with run log", @@ -418,7 +418,7 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), httpmock.FileResponse("./fixtures/run_log.zip")) }, - wantOut: runLogOutput(), + wantOut: expectedRunLogOutput, }, { name: "run log but run is not done", @@ -630,8 +630,7 @@ func TestViewRun(t *testing.T) { } } -func runLogOutput() string { - return heredoc.Doc(` +var expectedRunLogOutput = heredoc.Doc(` job1 step1 log line 1 job1 step1 log line 2 job1 step1 log line 3 @@ -645,4 +644,3 @@ job2 step2 log line 1 job2 step2 log line 2 job2 step2 log line 3 `) -} From c2375205f4ca23c79c59b72a3814cf0ed706d8ae Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Wed, 7 Apr 2021 19:55:44 -0500 Subject: [PATCH 7/8] golf --- pkg/cmd/run/view/view.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index af3194ba8..66fe17384 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -261,8 +261,7 @@ func runView(opts *ViewOptions) error { return err } - err = displayRunLog(opts.IO, runLog) - return err + return displayRunLog(opts.IO, runLog) } if selectedJob == nil && len(jobs) == 0 { From 7f021413644f7630e604f512c868f85e39ace1fb Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Wed, 7 Apr 2021 19:55:57 -0500 Subject: [PATCH 8/8] obsolete TODO --- pkg/cmd/run/view/view.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 66fe17384..0b393248b 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -343,7 +343,6 @@ func runView(opts *ViewOptions) error { } } else { fmt.Fprintln(out) - // TODO this does not exist yet 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)