From 289469565281dd8834621e303f23ec5aaaf00d9e Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 26 Mar 2023 14:11:03 +0800 Subject: [PATCH 01/35] feat: add attempt flag to run view --- pkg/cmd/run/view/view.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 895823ade..43c7b351c 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -74,6 +74,7 @@ type ViewOptions struct { Log bool LogFailed bool Web bool + Attempt uint64 Prompt bool Exporter cmdutil.Exporter @@ -93,7 +94,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "view []", Short: "View a summary of a workflow run", - Args: cobra.MaximumNArgs(1), + Args: cobra.MaximumNArgs(2), Example: heredoc.Doc(` # Interactively select a run to view, optionally selecting a single job $ gh run view @@ -101,6 +102,9 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman # View a specific run $ gh run view 12345 + # View a specific run with specific attempt number + $ gh run view 12345 --attempt 3 + # View a specific job within a run $ gh run view --job 456789 @@ -153,6 +157,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman 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 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") + cmd.Flags().Uint64VarP(&opts.Attempt, "attempt", "a", 0, "The attempt number of the workflow run") cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.SingleRunFields) return cmd @@ -172,6 +177,7 @@ func runView(opts *ViewOptions) error { jobID := opts.JobID runID := opts.RunID + attempt := opts.Attempt var selectedJob *shared.Job var run *shared.Run var jobs []shared.Job @@ -206,7 +212,7 @@ func runView(opts *ViewOptions) error { } opts.IO.StartProgressIndicator() - run, err = shared.GetRun(client, repo, runID) + run, err = shared.GetRun(client, repo, runID, attempt) opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to get run: %w", err) From bd0f535dea139d2586744396e806f65ff5d2c52c Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 26 Mar 2023 14:11:19 +0800 Subject: [PATCH 02/35] feat: pass 0 to fetch latest attempt --- pkg/cmd/run/cancel/cancel.go | 2 +- pkg/cmd/run/rerun/rerun.go | 2 +- pkg/cmd/run/watch/watch.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/run/cancel/cancel.go b/pkg/cmd/run/cancel/cancel.go index 39891986c..ca4521b1d 100644 --- a/pkg/cmd/run/cancel/cancel.go +++ b/pkg/cmd/run/cancel/cancel.go @@ -95,7 +95,7 @@ func runCancel(opts *CancelOptions) error { } } } else { - run, err = shared.GetRun(client, repo, runID) + run, err = shared.GetRun(client, repo, runID, 0) if err != nil { var httpErr api.HTTPError if errors.As(err, &httpErr) { diff --git a/pkg/cmd/run/rerun/rerun.go b/pkg/cmd/run/rerun/rerun.go index f439a2304..2522e632c 100644 --- a/pkg/cmd/run/rerun/rerun.go +++ b/pkg/cmd/run/rerun/rerun.go @@ -139,7 +139,7 @@ func runRerun(opts *RerunOptions) error { } } else { opts.IO.StartProgressIndicator() - run, err := shared.GetRun(client, repo, runID) + run, err := shared.GetRun(client, repo, runID, 0) opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to get run: %w", err) diff --git a/pkg/cmd/run/watch/watch.go b/pkg/cmd/run/watch/watch.go index d56c42405..0d31f8299 100644 --- a/pkg/cmd/run/watch/watch.go +++ b/pkg/cmd/run/watch/watch.go @@ -114,7 +114,7 @@ func watchRun(opts *WatchOptions) error { } } } else { - run, err = shared.GetRun(client, repo, runID) + run, err = shared.GetRun(client, repo, runID, 0) if err != nil { return fmt.Errorf("failed to get run: %w", err) } @@ -201,7 +201,7 @@ func renderRun(out io.Writer, opts WatchOptions, client *api.Client, repo ghrepo var err error - run, err = shared.GetRun(client, repo, fmt.Sprintf("%d", run.ID)) + run, err = shared.GetRun(client, repo, fmt.Sprintf("%d", run.ID), 0) if err != nil { return nil, fmt.Errorf("failed to get run: %w", err) } From 9a0f7916c3387d983e9b23a83c98da48efe40a21 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 26 Mar 2023 14:11:42 +0800 Subject: [PATCH 03/35] feat: update GetRun path based on attempt --- pkg/cmd/run/shared/shared.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index a1a71c4f9..cfee75705 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -449,10 +449,15 @@ func PromptForRun(cs *iostreams.ColorScheme, runs []Run) (string, error) { return fmt.Sprintf("%d", runs[selected].ID), nil } -func GetRun(client *api.Client, repo ghrepo.Interface, runID string) (*Run, error) { +func GetRun(client *api.Client, repo ghrepo.Interface, runID string, attempt uint64) (*Run, error) { var result Run + var path string - path := fmt.Sprintf("repos/%s/actions/runs/%s?exclude_pull_requests=true", ghrepo.FullName(repo), runID) + if attempt == 0 { + path = fmt.Sprintf("repos/%s/actions/runs/%s?exclude_pull_requests=true", ghrepo.FullName(repo), runID) + } else { + path = fmt.Sprintf("repos/%s/actions/runs/%s/attempts/%d?exclude_pull_requests=true", ghrepo.FullName(repo), runID, attempt) + } err := client.REST(repo.RepoHost(), "GET", path, nil, &result) if err != nil { From 343e7300168ed4c008b2b952cba741e37d24a6fd Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 26 Mar 2023 14:38:05 +0800 Subject: [PATCH 04/35] feat: pass attempt to RenderRunHeader --- pkg/cmd/run/view/view.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 43c7b351c..10ed4919f 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -324,7 +324,7 @@ func runView(opts *ViewOptions) error { out := opts.IO.Out fmt.Fprintln(out) - fmt.Fprintln(out, shared.RenderRunHeader(cs, *run, text.FuzzyAgo(opts.Now(), run.StartedTime()), prNumber)) + fmt.Fprintln(out, shared.RenderRunHeader(cs, *run, text.FuzzyAgo(opts.Now(), run.StartedTime()), prNumber, attempt)) fmt.Fprintln(out) if len(jobs) == 0 && run.Conclusion == shared.Failure || run.Conclusion == shared.StartupFailure { From baaa5c3184753ecbd0d15ad75572e322ca72e535 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 26 Mar 2023 14:38:24 +0800 Subject: [PATCH 05/35] feat: add attemptLabel to RenderRunHeader --- pkg/cmd/run/shared/presentation.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/run/shared/presentation.go b/pkg/cmd/run/shared/presentation.go index 900bd07a1..21ee15c25 100644 --- a/pkg/cmd/run/shared/presentation.go +++ b/pkg/cmd/run/shared/presentation.go @@ -7,14 +7,21 @@ import ( "github.com/cli/cli/v2/pkg/iostreams" ) -func RenderRunHeader(cs *iostreams.ColorScheme, run Run, ago, prNumber string) string { +func RenderRunHeader(cs *iostreams.ColorScheme, run Run, ago, prNumber string, attempt uint64) string { title := fmt.Sprintf("%s %s%s", cs.Bold(run.HeadBranch), run.WorkflowName(), prNumber) symbol, symbolColor := Symbol(cs, run.Status, run.Conclusion) id := cs.Cyanf("%d", run.ID) + var attemptLabel string + if attempt == 0 { + attemptLabel = "Latest Attempt" + } else { + attemptLabel = fmt.Sprintf("Attempt #%d", attempt) + } + header := "" - header += fmt.Sprintf("%s %s · %s\n", symbolColor(symbol), title, id) + header += fmt.Sprintf("%s %s · %s (%s)\n", symbolColor(symbol), title, id, attemptLabel) header += fmt.Sprintf("Triggered via %s %s", run.Event, ago) return header From 6c663427425390ab79e60d682d5437993ecf1901 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 26 Mar 2023 14:38:53 +0800 Subject: [PATCH 06/35] feat: pass latest attempt to RenderRunHeader for watch --- pkg/cmd/run/watch/watch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/run/watch/watch.go b/pkg/cmd/run/watch/watch.go index 0d31f8299..05b207693 100644 --- a/pkg/cmd/run/watch/watch.go +++ b/pkg/cmd/run/watch/watch.go @@ -236,7 +236,7 @@ func renderRun(out io.Writer, opts WatchOptions, client *api.Client, repo ghrepo return nil, fmt.Errorf("failed to get annotations: %w", annotationErr) } - fmt.Fprintln(out, shared.RenderRunHeader(cs, *run, text.FuzzyAgo(opts.Now(), run.StartedTime()), prNumber)) + fmt.Fprintln(out, shared.RenderRunHeader(cs, *run, text.FuzzyAgo(opts.Now(), run.StartedTime()), prNumber, 0)) fmt.Fprintln(out) if len(jobs) == 0 { From 3796b6d6e8641170daed89a2acf71678100d8b95 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 26 Mar 2023 16:10:42 +0800 Subject: [PATCH 07/35] feat: append attempt number in returned run url --- pkg/cmd/run/shared/shared.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index cfee75705..90aae6d9d 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -464,6 +464,10 @@ func GetRun(client *api.Client, repo ghrepo.Interface, runID string, attempt uin return nil, err } + if attempt > 0 { + result.URL += fmt.Sprintf("/attempts/%d", attempt) + } + // Set name to workflow name workflow, err := workflowShared.GetWorkflow(client, repo, result.WorkflowID) if err != nil { From 897a2aa53f3d65fd15974c7f652a02f656240693 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 26 Mar 2023 16:20:57 +0800 Subject: [PATCH 08/35] refactor: don't show latest attempt --- pkg/cmd/run/shared/presentation.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/run/shared/presentation.go b/pkg/cmd/run/shared/presentation.go index 21ee15c25..2ec149729 100644 --- a/pkg/cmd/run/shared/presentation.go +++ b/pkg/cmd/run/shared/presentation.go @@ -13,15 +13,13 @@ func RenderRunHeader(cs *iostreams.ColorScheme, run Run, ago, prNumber string, a symbol, symbolColor := Symbol(cs, run.Status, run.Conclusion) id := cs.Cyanf("%d", run.ID) - var attemptLabel string - if attempt == 0 { - attemptLabel = "Latest Attempt" - } else { - attemptLabel = fmt.Sprintf("Attempt #%d", attempt) + attemptLabel := "" + if attempt > 0 { + attemptLabel = fmt.Sprintf(" (Attempt #%d)", attempt) } header := "" - header += fmt.Sprintf("%s %s · %s (%s)\n", symbolColor(symbol), title, id, attemptLabel) + header += fmt.Sprintf("%s %s · %s%s\n", symbolColor(symbol), title, id, attemptLabel) header += fmt.Sprintf("Triggered via %s %s", run.Event, ago) return header From a4c77ffd92489bda7c1505f9055cb230f25f6691 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 26 Mar 2023 19:12:21 +0800 Subject: [PATCH 09/35] feat: include attempt in getRunLog --- pkg/cmd/run/view/view.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 10ed4919f..9470b0001 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -277,7 +277,7 @@ func runView(opts *ViewOptions) error { } opts.IO.StartProgressIndicator() - runLogZip, err := getRunLog(opts.RunLogCache, httpClient, repo, run) + runLogZip, err := getRunLog(opts.RunLogCache, httpClient, repo, run, attempt) opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to get run log: %w", err) @@ -431,13 +431,19 @@ func getLog(httpClient *http.Client, logURL string) (io.ReadCloser, error) { return resp.Body, nil } -func getRunLog(cache runLogCache, httpClient *http.Client, repo ghrepo.Interface, run *shared.Run) (*zip.ReadCloser, error) { +func getRunLog(cache runLogCache, httpClient *http.Client, repo ghrepo.Interface, run *shared.Run, attempt uint64) (*zip.ReadCloser, error) { filename := fmt.Sprintf("run-log-%d-%d.zip", run.ID, run.StartedTime().Unix()) 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), run.ID) + var logURL string + if attempt == 0 { + logURL = fmt.Sprintf("%srepos/%s/actions/runs/%d/logs", + ghinstance.RESTPrefix(repo.RepoHost()), ghrepo.FullName(repo), run.ID) + } else { + logURL = fmt.Sprintf("%srepos/%s/actions/runs/%d/attempts/%d/logs", + ghinstance.RESTPrefix(repo.RepoHost()), ghrepo.FullName(repo), run.ID, attempt) + } resp, err := getLog(httpClient, logURL) if err != nil { From 0fa168b80bcf9dc8cd99c9e410c12cc016f21ece Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 26 Mar 2023 19:12:40 +0800 Subject: [PATCH 10/35] feat: add tests for attempt flag --- pkg/cmd/run/view/view_test.go | 249 ++++++++++++++++++++++++++++++++++ 1 file changed, 249 insertions(+) diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index ee1fca6ee..99a9ef30e 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -117,6 +117,14 @@ func TestNewCmdView(t *testing.T) { JobID: "4567", }, }, + { + name: "run id with attempt", + cli: "1234 --attempt 2", + wants: ViewOptions{ + RunID: "1234", + Attempt: 2, + }, + }, } for _, tt := range tests { @@ -154,6 +162,7 @@ func TestNewCmdView(t *testing.T) { assert.Equal(t, tt.wants.Prompt, gotOpts.Prompt) assert.Equal(t, tt.wants.ExitStatus, gotOpts.ExitStatus) assert.Equal(t, tt.wants.Verbose, gotOpts.Verbose) + assert.Equal(t, tt.wants.Attempt, gotOpts.Attempt) }) } } @@ -213,6 +222,50 @@ func TestViewRun(t *testing.T) { }, wantOut: "\n✓ trunk CI #2898 · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about the job, try: gh run view --job=10\nView this run on GitHub: https://github.com/runs/3\n", }, + { + name: "associate with PR with attempt", + tty: true, + opts: &ViewOptions{ + RunID: "3", + Attempt: 3, + Prompt: false, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/attempts/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/artifacts"), + httpmock.StringResponse(`{}`)) + reg.Register( + httpmock.GraphQL(`query PullRequestForRun`), + httpmock.StringResponse(`{"data": { + "repository": { + "pullRequests": { + "nodes": [ + {"number": 2898, + "headRepository": { + "owner": { + "login": "OWNER" + }, + "name": "REPO"}} + ]}}}}`)) + reg.Register( + httpmock.REST("GET", "runs/3/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/check-runs/10/annotations"), + httpmock.JSONResponse([]shared.Annotation{})) + }, + wantOut: "\n✓ trunk CI #2898 · 3 (Attempt #3)\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about the job, try: gh run view --job=10\nView this run on GitHub: https://github.com/runs/3/attempts/3\n", + }, { name: "exit status, failed run", opts: &ViewOptions{ @@ -291,6 +344,52 @@ func TestViewRun(t *testing.T) { View this run on GitHub: https://github.com/runs/3 `), }, + { + name: "with artifacts and attempt", + opts: &ViewOptions{ + RunID: "3", + Attempt: 3, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/attempts/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/artifacts"), + httpmock.JSONResponse(map[string][]shared.Artifact{ + "artifacts": { + shared.Artifact{Name: "artifact-1", Expired: false}, + shared.Artifact{Name: "artifact-2", Expired: true}, + shared.Artifact{Name: "artifact-3", Expired: false}, + }, + })) + reg.Register( + httpmock.GraphQL(`query PullRequestForRun`), + httpmock.StringResponse(``)) + reg.Register( + httpmock.REST("GET", "runs/3/jobs"), + httpmock.JSONResponse(shared.JobsPayload{})) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + }, + wantOut: heredoc.Doc(` + + ✓ trunk CI · 3 (Attempt #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: https://github.com/runs/3/attempts/3 + `), + }, { name: "exit status, successful run", opts: &ViewOptions{ @@ -323,6 +422,39 @@ func TestViewRun(t *testing.T) { }, wantOut: "\n✓ trunk CI · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about the job, try: gh run view --job=10\nView this run on GitHub: https://github.com/runs/3\n", }, + { + name: "exit status, successful run, with attempt", + opts: &ViewOptions{ + RunID: "3", + Attempt: 3, + ExitStatus: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/attempts/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/artifacts"), + httpmock.StringResponse(`{}`)) + reg.Register( + httpmock.GraphQL(`query PullRequestForRun`), + httpmock.StringResponse(``)) + reg.Register( + httpmock.REST("GET", "runs/3/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/check-runs/10/annotations"), + httpmock.JSONResponse([]shared.Annotation{})) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + }, + wantOut: "\n✓ trunk CI · 3 (Attempt #3)\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n\nFor more information about the job, try: gh run view --job=10\nView this run on GitHub: https://github.com/runs/3/attempts/3\n", + }, { name: "verbose", tty: true, @@ -455,6 +587,53 @@ func TestViewRun(t *testing.T) { }, wantOut: coolJobRunLogOutput, }, + { + name: "interactive with log and attempt", + tty: true, + opts: &ViewOptions{ + Prompt: true, + Attempt: 3, + 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/attempts/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/attempts/3/logs"), + httpmock.FileResponse("./fixtures/run_log.zip")) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"), + httpmock.JSONResponse(workflowShared.WorkflowsPayload{ + Workflows: []workflowShared.Workflow{ + shared.TestWorkflow, + }, + })) + }, + askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(2) + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(1) + }, + wantOut: coolJobRunLogOutput, + }, { name: "noninteractive with log", opts: &ViewOptions{ @@ -477,6 +656,29 @@ func TestViewRun(t *testing.T) { }, wantOut: coolJobRunLogOutput, }, + { + name: "noninteractive with log and attempt", + opts: &ViewOptions{ + JobID: "10", + Attempt: 3, + Log: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/10"), + httpmock.JSONResponse(shared.SuccessfulJob)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/attempts/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/attempts/3/logs"), + httpmock.FileResponse("./fixtures/run_log.zip")) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + }, + wantOut: coolJobRunLogOutput, + }, { name: "interactive with run log", tty: true, @@ -597,6 +799,53 @@ func TestViewRun(t *testing.T) { }, wantOut: quuxTheBarfLogOutput, }, + { + name: "interactive with log-failed with attempt", + tty: true, + opts: &ViewOptions{ + Prompt: true, + Attempt: 3, + 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/attempts/3"), + 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/attempts/3/logs"), + httpmock.FileResponse("./fixtures/run_log.zip")) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"), + httpmock.JSONResponse(workflowShared.WorkflowsPayload{ + Workflows: []workflowShared.Workflow{ + shared.TestWorkflow, + }, + })) + }, + askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(4) + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(2) + }, + wantOut: quuxTheBarfLogOutput, + }, { name: "noninteractive with log-failed", opts: &ViewOptions{ From 5a2c24d3632bed5c2a34e0ceedcbe639eddd74b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D5=A1=C9=A8=D5=BC=C9=A2=D3=84=D5=A1=D6=85=D5=BC=C9=A2?= Date: Mon, 27 Mar 2023 09:18:09 +0800 Subject: [PATCH 11/35] docs: revise GH_CONFIG_DIR content in help_topic.go (#7230) --- pkg/cmd/root/help_topic.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index 42892d76d..4a4d19a51 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -79,8 +79,11 @@ var HelpTopics = map[string]map[string]string{ checks for new releases once every 24 hours and displays an upgrade notice on standard error if a newer version was found. - GH_CONFIG_DIR: the directory where gh will store configuration files. Default: - "$XDG_CONFIG_HOME/gh" or "$HOME/.config/gh". + GH_CONFIG_DIR: the directory where gh will store configuration files. If not specified, + the default value will be one of the following paths (in order of precedence): + - "$XDG_CONFIG_HOME/gh" (if $XDG_CONFIG_HOME is set), + - "$AppData/GitHub CLI" (on Windows if $AppData is set), or + - "$HOME/.config/gh". GH_PROMPT_DISABLED: set to any value to disable interactive prompting in the terminal. From 185ebbe69dcf213356e8b8d75f3c1c08fcd933a4 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Mon, 27 Mar 2023 03:36:30 +0200 Subject: [PATCH 12/35] Add a `--fail-fast` option to `pr checks --watch` (#7203) --- pkg/cmd/pr/checks/checks.go | 10 ++++++++++ pkg/cmd/pr/checks/checks_test.go | 33 +++++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/pr/checks/checks.go b/pkg/cmd/pr/checks/checks.go index 65df87b15..fb629d497 100644 --- a/pkg/cmd/pr/checks/checks.go +++ b/pkg/cmd/pr/checks/checks.go @@ -30,6 +30,7 @@ type ChecksOptions struct { WebMode bool Interval time.Duration Watch bool + FailFast bool Required bool } @@ -59,6 +60,10 @@ func NewCmdChecks(f *cmdutil.Factory, runF func(*ChecksOptions) error) *cobra.Co return cmdutil.FlagErrorf("argument required when using the `--repo` flag") } + if opts.FailFast && !opts.Watch { + return cmdutil.FlagErrorf("cannot use `--fail-fast` flag without `--watch` flag") + } + intervalChanged := cmd.Flags().Changed("interval") if !opts.Watch && intervalChanged { return cmdutil.FlagErrorf("cannot use `--interval` flag without `--watch` flag") @@ -86,6 +91,7 @@ func NewCmdChecks(f *cmdutil.Factory, runF func(*ChecksOptions) error) *cobra.Co cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the web browser to show details about checks") cmd.Flags().BoolVarP(&opts.Watch, "watch", "", false, "Watch checks until they finish") + cmd.Flags().BoolVarP(&opts.FailFast, "fail-fast", "", false, "Exit watch mode on first check failure") cmd.Flags().IntVarP(&interval, "interval", "i", 10, "Refresh interval in seconds when using `--watch` flag") cmd.Flags().BoolVar(&opts.Required, "required", false, "Only show checks that are required") @@ -171,6 +177,10 @@ func checksRun(opts *ChecksOptions) error { break } + if opts.FailFast && counts.Failed > 0 { + break + } + time.Sleep(opts.Interval) checks, counts, err = populateStatusChecks(client, repo, pr, opts.Required) diff --git a/pkg/cmd/pr/checks/checks_test.go b/pkg/cmd/pr/checks/checks_test.go index 3e5c8394a..186c1d324 100644 --- a/pkg/cmd/pr/checks/checks_test.go +++ b/pkg/cmd/pr/checks/checks_test.go @@ -62,6 +62,20 @@ func TestNewCmdChecks(t *testing.T) { cli: "--interval 5", wantsError: "cannot use `--interval` flag without `--watch` flag", }, + { + name: "watch with fail-fast flag", + cli: "--watch --fail-fast", + wants: ChecksOptions{ + Watch: true, + FailFast: true, + Interval: time.Duration(10000000000), + }, + }, + { + name: "fail-fast flag without watch flag", + cli: "--fail-fast", + wantsError: "cannot use `--fail-fast` flag without `--watch` flag", + }, { name: "required flag", cli: "--required", @@ -102,6 +116,7 @@ func TestNewCmdChecks(t *testing.T) { assert.Equal(t, tt.wants.Watch, gotOpts.Watch) assert.Equal(t, tt.wants.Interval, gotOpts.Interval) assert.Equal(t, tt.wants.Required, gotOpts.Required) + assert.Equal(t, tt.wants.FailFast, gotOpts.FailFast) }) } } @@ -111,6 +126,7 @@ func Test_checksRun(t *testing.T) { name string tty bool watch bool + failFast bool required bool httpStubs func(*httpmock.Registry) wantOut string @@ -189,6 +205,20 @@ func Test_checksRun(t *testing.T) { wantOut: "\x1b[?1049hAll checks were successful\n0 failing, 3 successful, 0 skipped, and 0 pending checks\n\n✓ awesome tests 1m26s sweet link\n✓ cool tests 1m26s sweet link\n✓ rad tests 1m26s sweet link\n\x1b[?1049lAll checks were successful\n0 failing, 3 successful, 0 skipped, and 0 pending checks\n\n✓ awesome tests 1m26s sweet link\n✓ cool tests 1m26s sweet link\n✓ rad tests 1m26s sweet link\n", wantErr: "", }, + { + name: "watch some failing with fail fast", + tty: true, + watch: true, + failFast: true, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query PullRequestStatusChecks\b`), + httpmock.FileResponse("./fixtures/someFailing.json"), + ) + }, + wantOut: "\x1b[?1049h\x1b[0;0H\x1b[JRefreshing checks status every 0 seconds. Press Ctrl+C to quit.\n\nSome checks were not successful\n1 failing, 1 successful, 0 skipped, and 1 pending checks\n\nX sad tests 1m26s sweet link\n✓ cool tests 1m26s sweet link\n* slow tests 1m26s sweet link\n\x1b[?1049lSome checks were not successful\n1 failing, 1 successful, 0 skipped, and 1 pending checks\n\nX sad tests 1m26s sweet link\n✓ cool tests 1m26s sweet link\n* slow tests 1m26s sweet link\n", + wantErr: "SilentError", + }, { name: "with statuses", tty: true, @@ -316,6 +346,7 @@ func Test_checksRun(t *testing.T) { SelectorArg: "123", Finder: shared.NewMockFinder("123", response, ghrepo.New("OWNER", "REPO")), Watch: tt.watch, + FailFast: tt.failFast, Required: tt.required, } @@ -381,7 +412,7 @@ func TestChecksRun_web(t *testing.T) { } } -func TestEliminateDupulicates(t *testing.T) { +func TestEliminateDuplicates(t *testing.T) { tests := []struct { name string checkContexts []api.CheckContext From c6a693c459c22b66a2914ba2794e24d22faa8015 Mon Sep 17 00:00:00 2001 From: Federico Guerinoni <41150432+guerinoni@users.noreply.github.com> Date: Mon, 27 Mar 2023 10:24:12 +0200 Subject: [PATCH 13/35] Add `--template` flag for issue create and pr create (#7185) Signed-off-by: Federico Guerinoni Co-authored-by: Alessio Cosenza --- pkg/cmd/issue/create/create.go | 21 +++++- pkg/cmd/issue/create/create_test.go | 33 +++++++++ pkg/cmd/pr/create/create.go | 20 +++++- pkg/cmd/pr/create/create_test.go | 39 ++++++++++ pkg/cmd/pr/shared/templates.go | 19 +++++ pkg/cmd/pr/shared/templates_test.go | 108 ++++++++++++++++++++++++++++ 6 files changed, 234 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 9e14760f5..2414c1807 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -1,6 +1,7 @@ package create import ( + "errors" "fmt" "net/http" @@ -39,6 +40,7 @@ type CreateOptions struct { Labels []string Projects []string Milestone string + Template string } func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { @@ -91,6 +93,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return cmdutil.FlagErrorf("`--recover` only supported when running interactively") } + if opts.Template != "" && bodyProvided { + return errors.New("`--template` is not supported when using `--body` or `--body-file`") + } + opts.Interactive = !(titleProvided && bodyProvided) if opts.Interactive && !opts.IO.CanPrompt() { @@ -113,6 +119,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd.Flags().StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the issue to projects by `name`") cmd.Flags().StringVarP(&opts.Milestone, "milestone", "m", "", "Add the issue to a milestone by `name`") cmd.Flags().StringVar(&opts.RecoverFile, "recover", "", "Recover input from a failed run of create") + cmd.Flags().StringVarP(&opts.Template, "template", "T", "", "Template `name` to use as starting body text") return cmd } @@ -216,9 +223,17 @@ func createRun(opts *CreateOptions) (err error) { if opts.RecoverFile == "" { var template shared.Template - template, err = tpl.Choose() - if err != nil { - return + + if opts.Template != "" { + template, err = tpl.Select(opts.Template) + if err != nil { + return + } + } else { + template, err = tpl.Choose() + if err != nil { + return + } } if template != nil { diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 1b498eebc..dcc4c7618 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -92,6 +92,38 @@ func TestNewCmdCreate(t *testing.T) { Interactive: false, }, }, + { + name: "template from name tty", + tty: true, + cli: `-t mytitle --template "bug report"`, + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "mytitle", + Body: "", + RecoverFile: "", + WebMode: false, + Template: "bug report", + Interactive: true, + }, + }, + { + name: "template from name non-tty", + tty: false, + cli: `-t mytitle --template "bug report"`, + wantsErr: true, + }, + { + name: "template and body", + tty: false, + cli: `-t mytitle --template "bug report" --body "issue body"`, + wantsErr: true, + }, + { + name: "template and body file", + tty: false, + cli: `-t mytitle --template "bug report" --body-file "body_file.md"`, + wantsErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -134,6 +166,7 @@ func TestNewCmdCreate(t *testing.T) { assert.Equal(t, tt.wantsOpts.RecoverFile, opts.RecoverFile) assert.Equal(t, tt.wantsOpts.WebMode, opts.WebMode) assert.Equal(t, tt.wantsOpts.Interactive, opts.Interactive) + assert.Equal(t, tt.wantsOpts.Template, opts.Template) }) } } diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index d8f58638e..c427f4d68 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -60,6 +60,7 @@ type CreateOptions struct { Milestone string MaintainerCanModify bool + Template string } type CreateContext struct { @@ -155,6 +156,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co opts.BodyProvided = true } + if opts.Template != "" && opts.BodyProvided { + return errors.New("`--template` is not supported when using `--body` or `--body-file`") + } + if !opts.IO.CanPrompt() && !opts.WebMode && !opts.Autofill && (!opts.TitleProvided || !opts.BodyProvided) { return cmdutil.FlagErrorf("must provide `--title` and `--body` (or `--fill`) when not running interactively") } @@ -182,6 +187,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl.StringVarP(&opts.Milestone, "milestone", "m", "", "Add the pull request to a milestone by `name`") fl.Bool("no-maintainer-edit", false, "Disable maintainer's ability to modify pull request") fl.StringVar(&opts.RecoverFile, "recover", "", "Recover input from a failed run of create") + fl.StringVarP(&opts.Template, "template", "T", "", "Template `file` to use as starting body text") _ = cmd.RegisterFlagCompletionFunc("reviewer", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { results, err := requestableReviewersForCompletion(opts) @@ -295,9 +301,17 @@ func createRun(opts *CreateOptions) (err error) { if opts.RecoverFile == "" { tpl := shared.NewTemplateManager(client.HTTP(), ctx.BaseRepo, opts.Prompter, opts.RootDirOverride, opts.RepoOverride == "", true) var template shared.Template - template, err = tpl.Choose() - if err != nil { - return + + if opts.Template != "" { + template, err = tpl.Select(opts.Template) + if err != nil { + return + } + } else { + template, err = tpl.Choose() + if err != nil { + return + } } if template != nil { diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 06100c04d..84b5cf6fa 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -130,6 +130,44 @@ func TestNewCmdCreate(t *testing.T) { MaintainerCanModify: true, }, }, + { + name: "template from file name tty", + tty: true, + cli: "-t mytitle --template bug_fix.md", + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "mytitle", + TitleProvided: true, + Body: "", + BodyProvided: false, + Autofill: false, + RecoverFile: "", + WebMode: false, + IsDraft: false, + BaseBranch: "", + HeadBranch: "", + MaintainerCanModify: true, + Template: "bug_fix.md", + }, + }, + { + name: "template from file name non-tty", + tty: false, + cli: "-t mytitle --template bug_fix.md", + wantsErr: true, + }, + { + name: "template and body", + tty: false, + cli: `-t mytitle --template bug_fix.md --body "pr body"`, + wantsErr: true, + }, + { + name: "template and body file", + tty: false, + cli: "-t mytitle --template bug_fix.md --body-file body_file.md", + wantsErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -178,6 +216,7 @@ func TestNewCmdCreate(t *testing.T) { assert.Equal(t, tt.wantsOpts.MaintainerCanModify, opts.MaintainerCanModify) assert.Equal(t, tt.wantsOpts.BaseBranch, opts.BaseBranch) assert.Equal(t, tt.wantsOpts.HeadBranch, opts.HeadBranch) + assert.Equal(t, tt.wantsOpts.Template, opts.Template) }) } } diff --git a/pkg/cmd/pr/shared/templates.go b/pkg/cmd/pr/shared/templates.go index 998952246..f46a9f1d4 100644 --- a/pkg/cmd/pr/shared/templates.go +++ b/pkg/cmd/pr/shared/templates.go @@ -2,6 +2,7 @@ package shared import ( "context" + "errors" "fmt" "net/http" "time" @@ -199,6 +200,24 @@ func (m *templateManager) Choose() (Template, error) { return m.templates[selectedOption], nil } +func (m *templateManager) Select(name string) (Template, error) { + if err := m.memoizedFetch(); err != nil { + return nil, err + } + + if len(m.templates) == 0 { + return nil, errors.New("no templates found") + } + + for _, t := range m.templates { + if t.Name() == name { + return t, nil + } + } + + return nil, fmt.Errorf("template %q not found", name) +} + func (m *templateManager) memoizedFetch() error { if m.didFetch { return m.fetchError diff --git a/pkg/cmd/pr/shared/templates_test.go b/pkg/cmd/pr/shared/templates_test.go index a962bb187..319d9daa5 100644 --- a/pkg/cmd/pr/shared/templates_test.go +++ b/pkg/cmd/pr/shared/templates_test.go @@ -113,3 +113,111 @@ func TestTemplateManager_hasAPI_PullRequest(t *testing.T) { assert.Equal(t, "", tpl.NameForSubmit()) assert.Equal(t, "I fixed a problem", string(tpl.Body())) } + +func TestTemplateManagerSelect(t *testing.T) { + tests := []struct { + name string + isPR bool + templateName string + wantTemplate Template + wantErr bool + errMsg string + httpStubs func(*httpmock.Registry) + }{ + { + name: "no templates found", + templateName: "Bug report", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueTemplates\b`), + httpmock.StringResponse(`{"data":{"repository":{"issueTemplates":[]}}}`), + ) + }, + wantErr: true, + errMsg: "no templates found", + }, + { + name: "no matching templates found", + templateName: "Unknown report", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueTemplates\b`), + httpmock.StringResponse(` + { "data": { "repository": { "issueTemplates": [ + { "name": "Bug report", "body": "I found a problem" }, + { "name": "Feature request", "body": "I need a feature" } + ] } } }`), + ) + }, + wantErr: true, + errMsg: `template "Unknown report" not found`, + }, + { + name: "matching issue template found", + templateName: "Bug report", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueTemplates\b`), + httpmock.StringResponse(` + { "data": { "repository": { "issueTemplates": [ + { "name": "Bug report", "body": "I found a problem" }, + { "name": "Feature request", "body": "I need a feature" } + ] } } }`), + ) + }, + wantTemplate: &issueTemplate{ + Gname: "Bug report", + Gbody: "I found a problem", + }, + }, + { + name: "matching pull request template found", + isPR: true, + templateName: "feature.md", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query PullRequestTemplates\b`), + httpmock.StringResponse(` + { "data": { "repository": { "PullRequestTemplates": [ + { "filename": "bug.md", "body": "I fixed a problem" }, + { "filename": "feature.md", "body": "I made a feature" } + ] } } }`), + ) + }, + wantTemplate: &pullRequestTemplate{ + Gname: "feature.md", + Gbody: "I made a feature", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + if tt.httpStubs != nil { + tt.httpStubs(reg) + } + + m := templateManager{ + repo: ghrepo.NewWithHost("OWNER", "REPO", "example.com"), + allowFS: false, + isPR: tt.isPR, + httpClient: &http.Client{Transport: reg}, + detector: &fd.EnabledDetectorMock{}, + } + + tmpl, err := m.Select(tt.templateName) + + if tt.wantErr { + assert.EqualError(t, err, tt.errMsg) + } else { + assert.NoError(t, err) + } + if tt.wantTemplate != nil { + assert.Equal(t, tt.wantTemplate.Name(), tmpl.Name()) + assert.Equal(t, tt.wantTemplate.Body(), tmpl.Body()) + } + }) + } +} From cc554135e826a1acfe49202c162d457377d946fe Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Tue, 28 Mar 2023 21:16:04 +0800 Subject: [PATCH 14/35] fix: change MaximumNArgs from 2 to 1 in run view.go --- pkg/cmd/run/view/view.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 9470b0001..b276cce77 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -94,7 +94,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "view []", Short: "View a summary of a workflow run", - Args: cobra.MaximumNArgs(2), + Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` # Interactively select a run to view, optionally selecting a single job $ gh run view From 48ed914d585e43ece728125d902d0606aefcae0c Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Tue, 28 Mar 2023 21:21:11 +0800 Subject: [PATCH 15/35] refactor: set the default case first --- pkg/cmd/run/shared/shared.go | 7 +++---- pkg/cmd/run/view/view.go | 9 ++++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index 90aae6d9d..87bbb907c 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -451,11 +451,10 @@ func PromptForRun(cs *iostreams.ColorScheme, runs []Run) (string, error) { func GetRun(client *api.Client, repo ghrepo.Interface, runID string, attempt uint64) (*Run, error) { var result Run - var path string - if attempt == 0 { - path = fmt.Sprintf("repos/%s/actions/runs/%s?exclude_pull_requests=true", ghrepo.FullName(repo), runID) - } else { + path := fmt.Sprintf("repos/%s/actions/runs/%s?exclude_pull_requests=true", ghrepo.FullName(repo), runID) + + if attempt > 0 { path = fmt.Sprintf("repos/%s/actions/runs/%s/attempts/%d?exclude_pull_requests=true", ghrepo.FullName(repo), runID, attempt) } diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index b276cce77..34c5fcbc7 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -436,11 +436,10 @@ func getRunLog(cache runLogCache, httpClient *http.Client, repo ghrepo.Interface 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 - var logURL string - if attempt == 0 { - logURL = fmt.Sprintf("%srepos/%s/actions/runs/%d/logs", - ghinstance.RESTPrefix(repo.RepoHost()), ghrepo.FullName(repo), run.ID) - } else { + logURL := fmt.Sprintf("%srepos/%s/actions/runs/%d/logs", + ghinstance.RESTPrefix(repo.RepoHost()), ghrepo.FullName(repo), run.ID) + + if attempt > 0 { logURL = fmt.Sprintf("%srepos/%s/actions/runs/%d/attempts/%d/logs", ghinstance.RESTPrefix(repo.RepoHost()), ghrepo.FullName(repo), run.ID, attempt) } From 0dd8261d2b12d2e0f11c484fcc28f4053e35e1f3 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Tue, 28 Mar 2023 21:25:42 +0800 Subject: [PATCH 16/35] refactor: use net/url JoinPath --- pkg/cmd/run/shared/shared.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index 87bbb907c..db347eee5 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -464,7 +464,10 @@ func GetRun(client *api.Client, repo ghrepo.Interface, runID string, attempt uin } if attempt > 0 { - result.URL += fmt.Sprintf("/attempts/%d", attempt) + result.URL, err = url.JoinPath(result.URL, fmt.Sprintf("/attempts/%d", attempt)) + if err != nil { + return nil, err + } } // Set name to workflow name From 1f0e53319e7c06401f0a3b5b8a5fbaa493edd459 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D5=A1=C9=A8=D5=BC=C9=A2=D3=84=D5=A1=D6=85=D5=BC=C9=A2?= Date: Wed, 29 Mar 2023 01:48:38 +0800 Subject: [PATCH 17/35] fix: throw error for non-existing org / repo with non-zero status in repo list (#7240) --- pkg/cmd/repo/list/list.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index f9c0ca8ac..e5b031794 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -154,6 +154,10 @@ func listRun(opts *ListOptions) error { return err } + if opts.Owner != "" && listResult.Owner == "" && !listResult.FromSearch { + return fmt.Errorf("the owner handle %q was not recognized as either a GitHub user or an organization", opts.Owner) + } + if err := opts.IO.StartPager(); err != nil { fmt.Fprintf(opts.IO.ErrOut, "error starting pager: %v\n", err) } From 3b2397811400a6bc5d98f9e66cd95d34c377bb0e Mon Sep 17 00:00:00 2001 From: Marco Carvalho Date: Tue, 28 Mar 2023 13:43:31 -0500 Subject: [PATCH 18/35] repo sync: clearer error message with actionable hint (#7110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić --- pkg/cmd/repo/sync/sync.go | 2 +- pkg/cmd/repo/sync/sync_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/repo/sync/sync.go b/pkg/cmd/repo/sync/sync.go index eebec4389..1f734b543 100644 --- a/pkg/cmd/repo/sync/sync.go +++ b/pkg/cmd/repo/sync/sync.go @@ -256,7 +256,7 @@ func executeLocalRepoSync(srcRepo ghrepo.Interface, remote string, opts *SyncOpt } if currentBranch == branch { if isDirty, err := git.IsDirty(); err == nil && isDirty { - return fmt.Errorf("can't sync because there are local changes; please stash them before trying again") + return fmt.Errorf("refusing to sync due to uncommitted/untracked local changes\ntip: use `git stash --all` before retrying the sync and run `git stash pop` afterwards") } else if err != nil { return err } diff --git a/pkg/cmd/repo/sync/sync_test.go b/pkg/cmd/repo/sync/sync_test.go index 9e7e5b5b1..90a216607 100644 --- a/pkg/cmd/repo/sync/sync_test.go +++ b/pkg/cmd/repo/sync/sync_test.go @@ -229,7 +229,7 @@ func Test_SyncRun(t *testing.T) { mgc.On("IsDirty").Return(true, nil).Once() }, wantErr: true, - errMsg: "can't sync because there are local changes; please stash them before trying again", + errMsg: "refusing to sync due to uncommitted/untracked local changes\ntip: use `git stash --all` before retrying the sync and run `git stash pop` afterwards", }, { name: "sync local repo with parent - existing branch, non-current", From 944df28f1606427e0414f5187ff3a56d2d4b8c5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 29 Mar 2023 11:32:19 +0200 Subject: [PATCH 19/35] repo list: add test for invalid owner error --- pkg/cmd/repo/list/list_test.go | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/pkg/cmd/repo/list/list_test.go b/pkg/cmd/repo/list/list_test.go index e6fb918d0..3a02cbb22 100644 --- a/pkg/cmd/repo/list/list_test.go +++ b/pkg/cmd/repo/list/list_test.go @@ -452,3 +452,40 @@ func TestRepoList_noVisibilityField(t *testing.T) { assert.Equal(t, "", stderr.String()) assert.Equal(t, "", stdout.String()) } + +func TestRepoList_invalidOwner(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(false) + ios.SetStdinTTY(false) + ios.SetStderrTTY(false) + + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.GraphQL(`query RepositoryList\b`), + httpmock.StringResponse(`{ "data": { "repositoryOwner": null } }`), + ) + + opts := ListOptions{ + Owner: "nonexist", + IO: ios, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + Now: func() time.Time { + t, _ := time.Parse(time.RFC822, "19 Feb 21 15:00 UTC") + return t + }, + Limit: 30, + Detector: &fd.DisabledDetectorMock{}, + } + + err := listRun(&opts) + assert.EqualError(t, err, `the owner handle "nonexist" was not recognized as either a GitHub user or an organization`) + assert.Equal(t, "", stderr.String()) + assert.Equal(t, "", stdout.String()) +} From 852bc0035474026cc95e842d0708ba251403429a Mon Sep 17 00:00:00 2001 From: Alex Petrov Date: Wed, 29 Mar 2023 18:39:11 -0400 Subject: [PATCH 20/35] feat: allow filtering workflow runs by statuses --- pkg/cmd/run/list/list.go | 3 +++ pkg/cmd/run/list/list_test.go | 25 +++++++++++++++++++++++++ pkg/cmd/run/shared/shared.go | 21 +++++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/pkg/cmd/run/list/list.go b/pkg/cmd/run/list/list.go index 2b2b78109..4b085a1a1 100644 --- a/pkg/cmd/run/list/list.go +++ b/pkg/cmd/run/list/list.go @@ -31,6 +31,7 @@ type ListOptions struct { WorkflowSelector string Branch string Actor string + Status string now time.Time } @@ -67,6 +68,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.WorkflowSelector, "workflow", "w", "", "Filter runs by workflow") cmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "Filter runs by branch") cmd.Flags().StringVarP(&opts.Actor, "user", "u", "", "Filter runs by user who triggered the run") + cmdutil.StringEnumFlag(cmd, &opts.Status, "status", "s", "", shared.AllStatuses, "Filter runs by status") cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.RunFields) _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "branch") @@ -89,6 +91,7 @@ func listRun(opts *ListOptions) error { filters := &shared.FilterOptions{ Branch: opts.Branch, Actor: opts.Actor, + Status: opts.Status, } opts.IO.StartProgressIndicator() diff --git a/pkg/cmd/run/list/list_test.go b/pkg/cmd/run/list/list_test.go index aa7fe993c..96a9ab912 100644 --- a/pkg/cmd/run/list/list_test.go +++ b/pkg/cmd/run/list/list_test.go @@ -69,6 +69,14 @@ func TestNewCmdList(t *testing.T) { Actor: "bak1an", }, }, + { + name: "status", + cli: "--status completed", + wants: ListOptions{ + Limit: defaultLimit, + Status: "completed", + }, + }, } for _, tt := range tests { @@ -394,6 +402,23 @@ func TestListRun(t *testing.T) { }, wantErr: true, }, + { + name: "status filter applied", + opts: &ListOptions{ + Limit: defaultLimit, + Status: "queued", + }, + isTTY: true, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.QueryMatcher("GET", "repos/OWNER/REPO/actions/runs", url.Values{ + "status": []string{"queued"}, + }), + httpmock.JSONResponse(shared.RunsPayload{}), + ) + }, + wantErr: true, + }, } for _, tt := range tests { diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index db347eee5..a165f94a3 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -44,6 +44,23 @@ type Status string type Conclusion string type Level string +var AllStatuses = []string{ + "queued", + "completed", + "in_progress", + "requested", + "waiting", + "action_required", + "cancelled", + "failure", + "neutral", + "skipped", + "stale", + "startup_failure", + "success", + "timed_out", +} + var RunFields = []string{ "name", "displayTitle", @@ -284,6 +301,7 @@ type FilterOptions struct { WorkflowID int64 // avoid loading workflow name separately and use the provided one WorkflowName string + Status string } // GetRunsWithFilter fetches 50 runs from the API and filters them in-memory @@ -326,6 +344,9 @@ func GetRuns(client *api.Client, repo ghrepo.Interface, opts *FilterOptions, lim if opts.Actor != "" { path += fmt.Sprintf("&actor=%s", url.QueryEscape(opts.Actor)) } + if opts.Status != "" { + path += fmt.Sprintf("&status=%s", url.QueryEscape(opts.Status)) + } } var result *RunsPayload From 1fb12a98ea8afcf4dc7a1bf1b4b3a2da8b2d3b4a Mon Sep 17 00:00:00 2001 From: nate smith Date: Wed, 29 Mar 2023 06:25:35 -0700 Subject: [PATCH 21/35] improve test resiliency --- pkg/cmd/run/list/list_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/run/list/list_test.go b/pkg/cmd/run/list/list_test.go index 96a9ab912..d33af7167 100644 --- a/pkg/cmd/run/list/list_test.go +++ b/pkg/cmd/run/list/list_test.go @@ -123,6 +123,7 @@ func TestListRun(t *testing.T) { wantErr bool wantOut string wantErrOut string + wantErrMsg string stubs func(*httpmock.Registry) isTTY bool }{ @@ -343,7 +344,8 @@ func TestListRun(t *testing.T) { httpmock.JSONResponse(shared.RunsPayload{}), ) }, - wantErr: true, + wantErr: true, + wantErrMsg: "no runs found", }, { name: "workflow selector", @@ -384,7 +386,8 @@ func TestListRun(t *testing.T) { httpmock.JSONResponse(shared.RunsPayload{}), ) }, - wantErr: true, + wantErr: true, + wantErrMsg: "no runs found", }, { name: "actor filter applied", @@ -400,7 +403,8 @@ func TestListRun(t *testing.T) { httpmock.JSONResponse(shared.RunsPayload{}), ) }, - wantErr: true, + wantErr: true, + wantErrMsg: "no runs found", }, { name: "status filter applied", @@ -417,7 +421,8 @@ func TestListRun(t *testing.T) { httpmock.JSONResponse(shared.RunsPayload{}), ) }, - wantErr: true, + wantErr: true, + wantErrMsg: "no runs found", }, } @@ -441,6 +446,7 @@ func TestListRun(t *testing.T) { err := listRun(tt.opts) if tt.wantErr { assert.Error(t, err) + assert.Equal(t, tt.wantErrMsg, err.Error()) } else { assert.NoError(t, err) } From 36436cb8b8afd1adbe77dc2ae70fd2560d75715c Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Mon, 13 Mar 2023 15:15:38 -0700 Subject: [PATCH 22/35] Retry fetching repo from template Fixes #7055 --- pkg/cmd/repo/create/create.go | 53 +++++++++------- pkg/cmd/repo/create/create_test.go | 98 ++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 22 deletions(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index 6d6eeef30..105c7f130 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -9,8 +9,10 @@ import ( "path/filepath" "sort" "strings" + "time" "github.com/MakeNowJust/heredoc" + "github.com/cenkalti/backoff/v4" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" @@ -21,6 +23,10 @@ import ( "github.com/spf13/cobra" ) +type errWithExitCode interface { + ExitCode() int +} + type iprompter interface { Input(string, string) (string, error) Select(string, string, []string) (int, error) @@ -33,6 +39,7 @@ type CreateOptions struct { Config func() (config.Config, error) IO *iostreams.IOStreams Prompter iprompter + BackOff backoff.BackOff Name string Description string @@ -328,7 +335,6 @@ func createFromScratch(opts *CreateOptions) error { InitReadme: opts.AddReadme, } - var templateRepoMainBranch string if opts.Template != "" { var templateRepo ghrepo.Interface apiClient := api.NewClientFromHTTP(httpClient) @@ -352,7 +358,6 @@ func createFromScratch(opts *CreateOptions) error { } input.TemplateRepositoryID = repo.ID - templateRepoMainBranch = repo.DefaultBranchRef.Name } repo, err := repoCreate(httpClient, repoToCreate.RepoHost(), input) @@ -387,17 +392,12 @@ func createFromScratch(opts *CreateOptions) error { remoteURL := ghrepo.FormatRemoteURL(repo, protocol) - if opts.LicenseTemplate == "" && opts.GitIgnoreTemplate == "" { + if opts.LicenseTemplate == "" && opts.GitIgnoreTemplate == "" && opts.Template == "" { // cloning empty repository or template - checkoutBranch := "" - if opts.Template != "" { - // use the template's default branch - checkoutBranch = templateRepoMainBranch - } - if err := localInit(opts.GitClient, remoteURL, repo.RepoName(), checkoutBranch); err != nil { + if err := localInit(opts.GitClient, remoteURL, repo.RepoName()); err != nil { return err } - } else if _, err := opts.GitClient.Clone(context.Background(), remoteURL, []string{}); err != nil { + } else if err := cloneWithRetry(opts, remoteURL); err != nil { return err } } @@ -563,6 +563,25 @@ func createFromLocal(opts *CreateOptions) error { return nil } +func cloneWithRetry(opts *CreateOptions, remoteURL string) error { + // Allow injecting alternative BackOff in tests. + if opts.BackOff == nil { + opts.BackOff = backoff.NewConstantBackOff(3 * time.Second) + } + + ctx := context.Background() + return backoff.Retry(func() error { + _, err := opts.GitClient.Clone(ctx, remoteURL, []string{}) + + var execError errWithExitCode + if errors.As(err, &execError) && execError.ExitCode() == 128 { + return err + } + + return backoff.Permanent(err) + }, backoff.WithContext(backoff.WithMaxRetries(opts.BackOff, 3), ctx)) +} + func sourceInit(gitClient *git.Client, io *iostreams.IOStreams, remoteURL, baseRemote string) error { cs := io.ColorScheme() remoteAdd, err := gitClient.Command(context.Background(), "remote", "add", baseRemote, remoteURL) @@ -620,7 +639,7 @@ func isLocalRepo(gitClient *git.Client) (bool, error) { } // clone the checkout branch to specified path -func localInit(gitClient *git.Client, remoteURL, path, checkoutBranch string) error { +func localInit(gitClient *git.Client, remoteURL, path string) error { ctx := context.Background() gitInit, err := gitClient.Command(ctx, "init", path) if err != nil { @@ -644,17 +663,7 @@ func localInit(gitClient *git.Client, remoteURL, path, checkoutBranch string) er return err } - if checkoutBranch == "" { - return nil - } - - refspec := fmt.Sprintf("+refs/heads/%[1]s:refs/remotes/origin/%[1]s", checkoutBranch) - err = gc.Fetch(ctx, "origin", refspec) - if err != nil { - return err - } - - return gc.CheckoutBranch(ctx, checkoutBranch) + return nil } func interactiveGitIgnore(client *http.Client, hostname string, prompter iprompter) (string, error) { diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index 4ba48990b..d773b06f5 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -119,6 +119,18 @@ func TestNewCmdCreate(t *testing.T) { IncludeAllBranches: true, }, }, + { + name: "template with .gitignore", + cli: "template-repo --template mytemplate --gitignore ../.gitignore --public", + wantsErr: true, + errMsg: ".gitignore and license templates are not added when template is provided", + }, + { + name: "template with license", + cli: "template-repo --template mytemplate --license ../.license --public", + wantsErr: true, + errMsg: ".gitignore and license templates are not added when template is provided", + }, } for _, tt := range tests { @@ -557,6 +569,92 @@ func Test_createRun(t *testing.T) { }, wantStdout: "https://github.com/OWNER/REPO\n", }, + { + name: "noninteractive clone from scratch", + opts: &CreateOptions{ + Interactive: false, + Name: "REPO", + Visibility: "PRIVATE", + Clone: true, + }, + tty: false, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`mutation RepositoryCreate\b`), + httpmock.StringResponse(` + { + "data": { + "createRepository": { + "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login":"OWNER"}, + "url": "https://github.com/OWNER/REPO" + } + } + } + }`)) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git init REPO`, 0, "") + cs.Register(`git -C REPO remote add origin https://github.com/OWNER/REPO`, 0, "") + }, + wantStdout: "https://github.com/OWNER/REPO\n", + }, + { + name: "noninteractive create from template", + opts: &CreateOptions{ + Interactive: false, + Name: "REPO", + Visibility: "PRIVATE", + Clone: true, + Template: "mytemplate", + }, + tty: false, + httpStubs: func(reg *httpmock.Registry) { + // Test resolving repo owner from repo name only. + reg.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"OWNER"}}}`)) + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQLQuery(`{ + "data": { + "repository": { + "id": "REPOID" + } + } + }`, func(s string, m map[string]interface{}) { + assert.Equal(t, "OWNER", m["owner"]) + assert.Equal(t, "mytemplate", m["name"]) + }), + ) + reg.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"id":"OWNERID"}}}`)) + reg.Register( + httpmock.GraphQL(`mutation CloneTemplateRepository\b`), + httpmock.GraphQLMutation(` + { + "data": { + "cloneTemplateRepository": { + "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login":"OWNER"}, + "url": "https://github.com/OWNER/REPO" + } + } + } + }`, func(m map[string]interface{}) { + assert.Equal(t, "REPOID", m["repositoryId"]) + })) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git clone https://github.com/OWNER/REPO`, 0, "") + }, + wantStdout: "https://github.com/OWNER/REPO\n", + }, } for _, tt := range tests { prompterMock := &prompter.PrompterMock{} From 3085c39a730782dcc4292e76ba24b0449167152b Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Wed, 29 Mar 2023 07:43:21 -0700 Subject: [PATCH 23/35] Specify branch to force exit code --- pkg/cmd/repo/create/create.go | 18 +++++++++++++++--- pkg/cmd/repo/create/create_test.go | 13 ++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index 105c7f130..0aae16aac 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -1,9 +1,11 @@ package create import ( + "bytes" "context" "errors" "fmt" + "io" "net/http" "os/exec" "path/filepath" @@ -335,6 +337,7 @@ func createFromScratch(opts *CreateOptions) error { InitReadme: opts.AddReadme, } + var templateRepoMainBranch string if opts.Template != "" { var templateRepo ghrepo.Interface apiClient := api.NewClientFromHTTP(httpClient) @@ -358,6 +361,7 @@ func createFromScratch(opts *CreateOptions) error { } input.TemplateRepositoryID = repo.ID + templateRepoMainBranch = repo.DefaultBranchRef.Name } repo, err := repoCreate(httpClient, repoToCreate.RepoHost(), input) @@ -397,7 +401,7 @@ func createFromScratch(opts *CreateOptions) error { if err := localInit(opts.GitClient, remoteURL, repo.RepoName()); err != nil { return err } - } else if err := cloneWithRetry(opts, remoteURL); err != nil { + } else if err := cloneWithRetry(opts, remoteURL, templateRepoMainBranch); err != nil { return err } } @@ -563,19 +567,27 @@ func createFromLocal(opts *CreateOptions) error { return nil } -func cloneWithRetry(opts *CreateOptions, remoteURL string) error { +func cloneWithRetry(opts *CreateOptions, remoteURL, branch string) error { // Allow injecting alternative BackOff in tests. if opts.BackOff == nil { opts.BackOff = backoff.NewConstantBackOff(3 * time.Second) } + var args []string + if branch != "" { + args = append(args, "--branch", branch) + } + ctx := context.Background() return backoff.Retry(func() error { - _, err := opts.GitClient.Clone(ctx, remoteURL, []string{}) + stderr := &bytes.Buffer{} + _, err := opts.GitClient.Clone(ctx, remoteURL, args, git.WithStderr(stderr)) var execError errWithExitCode if errors.As(err, &execError) && execError.ExitCode() == 128 { return err + } else { + io.Copy(opts.IO.ErrOut, stderr) } return backoff.Permanent(err) diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index d773b06f5..5b5fd72bb 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -6,6 +6,7 @@ import ( "net/http" "testing" + "github.com/cenkalti/backoff/v4" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/prompter" @@ -602,13 +603,14 @@ func Test_createRun(t *testing.T) { wantStdout: "https://github.com/OWNER/REPO\n", }, { - name: "noninteractive create from template", + name: "noninteractive create from template with retry", opts: &CreateOptions{ Interactive: false, Name: "REPO", Visibility: "PRIVATE", Clone: true, Template: "mytemplate", + BackOff: &backoff.ZeroBackOff{}, }, tty: false, httpStubs: func(reg *httpmock.Registry) { @@ -621,7 +623,10 @@ func Test_createRun(t *testing.T) { httpmock.GraphQLQuery(`{ "data": { "repository": { - "id": "REPOID" + "id": "REPOID", + "defaultBranchRef": { + "name": "main" + } } } }`, func(s string, m map[string]interface{}) { @@ -651,7 +656,9 @@ func Test_createRun(t *testing.T) { })) }, execStubs: func(cs *run.CommandStubber) { - cs.Register(`git clone https://github.com/OWNER/REPO`, 0, "") + // fatal: Remote branch main not found in upstream origin + cs.Register(`git clone --branch main https://github.com/OWNER/REPO`, 128, "") + cs.Register(`git clone --branch main https://github.com/OWNER/REPO`, 0, "") }, wantStdout: "https://github.com/OWNER/REPO\n", }, From d104926ce218787d0e8547b326a0557b32e85a83 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Wed, 29 Mar 2023 08:18:05 -0700 Subject: [PATCH 24/35] Resolve lint error --- pkg/cmd/repo/create/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index 0aae16aac..640fee632 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -587,7 +587,7 @@ func cloneWithRetry(opts *CreateOptions, remoteURL, branch string) error { if errors.As(err, &execError) && execError.ExitCode() == 128 { return err } else { - io.Copy(opts.IO.ErrOut, stderr) + _, _ = io.Copy(opts.IO.ErrOut, stderr) } return backoff.Permanent(err) From 82662685e3cf3e93fd0c5c81b6680c6a06491f1d Mon Sep 17 00:00:00 2001 From: Benjamin Chadwick <73488828+bchadwic@users.noreply.github.com> Date: Wed, 29 Mar 2023 09:09:44 -0700 Subject: [PATCH 25/35] Autocomplete branch flags (#6031) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić --- git/client.go | 29 +++++++++++++++++++++++++++++ pkg/cmd/browse/browse.go | 2 ++ pkg/cmd/pr/create/create.go | 2 ++ pkg/cmd/pr/edit/edit.go | 2 ++ pkg/cmd/pr/list/list.go | 3 ++- pkg/cmd/release/create/create.go | 2 ++ pkg/cmd/release/edit/edit.go | 2 ++ pkg/cmd/repo/view/view.go | 2 ++ pkg/cmd/run/list/list.go | 2 ++ pkg/cmd/search/prs/prs.go | 2 ++ pkg/cmd/workflow/run/run.go | 2 ++ pkg/cmd/workflow/view/view.go | 2 ++ pkg/cmdutil/flags.go | 21 +++++++++++++++++++++ 13 files changed, 72 insertions(+), 1 deletion(-) diff --git a/git/client.go b/git/client.go index 43c94fef1..a7d59899e 100644 --- a/git/client.go +++ b/git/client.go @@ -354,6 +354,22 @@ func (c *Client) HasLocalBranch(ctx context.Context, branch string) bool { return err == nil } +func (c *Client) TrackingBranchNames(ctx context.Context, prefix string) []string { + args := []string{"branch", "-r", "--format", "%(refname:strip=3)"} + if prefix != "" { + args = append(args, "--list", fmt.Sprintf("*/%s*", escapeGlob(prefix))) + } + cmd, err := c.Command(ctx, args...) + if err != nil { + return nil + } + output, err := cmd.Output() + if err != nil { + return nil + } + return strings.Split(string(output), "\n") +} + // ToplevelDir returns the top-level directory path of the current repository. func (c *Client) ToplevelDir(ctx context.Context) (string, error) { out, err := c.revParse(ctx, "--show-toplevel") @@ -623,3 +639,16 @@ func populateResolvedRemotes(remotes RemoteSet, resolved []string) { } } } + +var globReplacer = strings.NewReplacer( + "*", `\*`, + "?", `\?`, + "[", `\[`, + "]", `\]`, + "{", `\{`, + "}", `\}`, +) + +func escapeGlob(p string) string { + return globReplacer.Replace(p) +} diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index 0d8239754..160d72aa2 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -144,6 +144,8 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co cmd.Flags().StringVarP(&opts.Commit, "commit", "c", "", "Select another commit by passing in the commit SHA, default is the last commit") cmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "Select another branch by passing in the branch name") + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "branch") + // Preserve backwards compatibility for when commit flag used to be a boolean flag. cmd.Flags().Lookup("commit").NoOptDefVal = emptyCommitFlag diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index c427f4d68..4fc61f362 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -189,6 +189,8 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl.StringVar(&opts.RecoverFile, "recover", "", "Recover input from a failed run of create") fl.StringVarP(&opts.Template, "template", "T", "", "Template `file` to use as starting body text") + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "base", "head") + _ = cmd.RegisterFlagCompletionFunc("reviewer", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { results, err := requestableReviewersForCompletion(opts) if err != nil { diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 216d0d6d2..915eba657 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -148,6 +148,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd.Flags().StringSliceVar(&opts.Editable.Projects.Remove, "remove-project", nil, "Remove the pull request from projects by `name`") cmd.Flags().StringVarP(&opts.Editable.Milestone.Value, "milestone", "m", "", "Edit the milestone the pull request belongs to by `name`") + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "base") + for _, flagName := range []string{"add-reviewer", "remove-reviewer"} { _ = cmd.RegisterFlagCompletionFunc(flagName, func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { baseRepo, err := f.BaseRepo() diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index a56ac0e5d..cea7c2b19 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -109,9 +109,10 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.Assignee, "assignee", "a", "", "Filter by assignee") cmd.Flags().StringVarP(&opts.Search, "search", "S", "", "Search pull requests with `query`") cmdutil.NilBoolFlag(cmd, &opts.Draft, "draft", "d", "Filter by draft state") - cmdutil.AddJSONFlags(cmd, &opts.Exporter, api.PullRequestFields) + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "base", "head") + return cmd } diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index e667f65ef..dea14307f 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -177,6 +177,8 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmdutil.NilBoolFlag(cmd, &opts.IsLatest, "latest", "", "Mark this release as \"Latest\" (default: automatic based on date and version)") cmd.Flags().BoolVarP(&opts.VerifyTag, "verify-tag", "", false, "Abort in case the git tag doesn't already exist in the remote repository") + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "target") + return cmd } diff --git a/pkg/cmd/release/edit/edit.go b/pkg/cmd/release/edit/edit.go index 89d365a9a..cac9c0e48 100644 --- a/pkg/cmd/release/edit/edit.go +++ b/pkg/cmd/release/edit/edit.go @@ -82,6 +82,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd.Flags().StringVar(&opts.TagName, "tag", "", "The name of the tag") cmd.Flags().StringVarP(¬esFile, "notes-file", "F", "", "Read release notes from `file` (use \"-\" to read from standard input)") + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "target") + return cmd } diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index 722301397..3351d9cd9 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -68,6 +68,8 @@ With '--branch', view a specific branch of the repository.`, cmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "View a specific branch of the repository") cmdutil.AddJSONFlags(cmd, &opts.Exporter, api.RepositoryFields) + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "branch") + return cmd } diff --git a/pkg/cmd/run/list/list.go b/pkg/cmd/run/list/list.go index 0fe685110..2b2b78109 100644 --- a/pkg/cmd/run/list/list.go +++ b/pkg/cmd/run/list/list.go @@ -69,6 +69,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.Actor, "user", "u", "", "Filter runs by user who triggered the run") cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.RunFields) + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "branch") + return cmd } diff --git a/pkg/cmd/search/prs/prs.go b/pkg/cmd/search/prs/prs.go index 2fd4961ff..2ef37b2ef 100644 --- a/pkg/cmd/search/prs/prs.go +++ b/pkg/cmd/search/prs/prs.go @@ -184,5 +184,7 @@ func NewCmdPrs(f *cmdutil.Factory, runF func(*shared.IssuesOptions) error) *cobr cmd.Flags().StringVar(&opts.Query.Qualifiers.ReviewedBy, "reviewed-by", "", "Filter on `user` who reviewed") cmdutil.StringEnumFlag(cmd, &opts.Query.Qualifiers.Status, "checks", "", "", []string{"pending", "success", "failure"}, "Filter based on status of the checks") + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "base", "head") + return cmd } diff --git a/pkg/cmd/workflow/run/run.go b/pkg/cmd/workflow/run/run.go index ca703027a..564b41985 100644 --- a/pkg/cmd/workflow/run/run.go +++ b/pkg/cmd/workflow/run/run.go @@ -128,6 +128,8 @@ func NewCmdRun(f *cmdutil.Factory, runF func(*RunOptions) error) *cobra.Command cmd.Flags().StringArrayVarP(&opts.RawFields, "raw-field", "f", nil, "Add a string parameter in `key=value` format") cmd.Flags().BoolVar(&opts.JSON, "json", false, "Read workflow inputs as JSON via STDIN") + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "ref") + return cmd } diff --git a/pkg/cmd/workflow/view/view.go b/pkg/cmd/workflow/view/view.go index 52a364be8..f45024828 100644 --- a/pkg/cmd/workflow/view/view.go +++ b/pkg/cmd/workflow/view/view.go @@ -85,6 +85,8 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd.Flags().BoolVarP(&opts.YAML, "yaml", "y", false, "View the workflow yaml file") cmd.Flags().StringVarP(&opts.Ref, "ref", "r", "", "The branch or tag name which contains the version of the workflow file you'd like to view") + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "ref") + return cmd } diff --git a/pkg/cmdutil/flags.go b/pkg/cmdutil/flags.go index 2a73a79de..c0064099c 100644 --- a/pkg/cmdutil/flags.go +++ b/pkg/cmdutil/flags.go @@ -1,6 +1,7 @@ package cmdutil import ( + "context" "fmt" "strconv" "strings" @@ -44,6 +45,26 @@ func StringSliceEnumFlag(cmd *cobra.Command, p *[]string, name, shorthand string return f } +type gitClient interface { + TrackingBranchNames(context.Context, string) []string +} + +// RegisterBranchCompletionFlags suggests and autocompletes known remote git branches for flags passed +func RegisterBranchCompletionFlags(gitc gitClient, cmd *cobra.Command, flags ...string) error { + for _, flag := range flags { + err := cmd.RegisterFlagCompletionFunc(flag, func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + if repoFlag := cmd.Flag("repo"); repoFlag != nil && repoFlag.Changed { + return nil, cobra.ShellCompDirectiveNoFileComp + } + return gitc.TrackingBranchNames(context.TODO(), toComplete), cobra.ShellCompDirectiveNoFileComp + }) + if err != nil { + return err + } + } + return nil +} + func formatValuesForUsageDocs(values []string) string { return fmt.Sprintf("{%s}", strings.Join(values, "|")) } From 66706f0880813d4bbf1411840c0bdd9c64d15002 Mon Sep 17 00:00:00 2001 From: SonicGDX <114670430+SonicGDX@users.noreply.github.com> Date: Wed, 29 Mar 2023 17:36:40 +0100 Subject: [PATCH 26/35] Fix typo in README.md Signed-off-by: SonicGDX<114670430+SonicGDX@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c95722466..38c44c788 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ For more information, see [Linux & BSD installation](./docs/install_linux.md). | `winget install --id GitHub.cli` | `winget upgrade --id GitHub.cli` | > **Note** -> The Windows installer modifes your PATH. When using Windows Terminal, you will need to **open a new window** for the changes to take affect. (Simply opening a new tab will _not_ be sufficient.) +> The Windows installer modifies your PATH. When using Windows Terminal, you will need to **open a new window** for the changes to take affect. (Simply opening a new tab will _not_ be sufficient.) #### scoop From f21f1ca4ef62ee72e13353be557daec813985f44 Mon Sep 17 00:00:00 2001 From: vaindil Date: Wed, 29 Mar 2023 17:23:22 -0400 Subject: [PATCH 27/35] check SSH key existence before uploading --- pkg/cmd/auth/shared/login_flow.go | 6 +- pkg/cmd/auth/shared/login_flow_test.go | 5 +- pkg/cmd/ssh-key/add/add.go | 8 +- pkg/cmd/ssh-key/add/add_test.go | 107 ++++++++++++++---- pkg/cmd/ssh-key/add/http.go | 34 +++++- pkg/cmd/ssh-key/list/list.go | 3 +- .../{list/http.go => shared/user_keys.go} | 4 +- 7 files changed, 130 insertions(+), 37 deletions(-) rename pkg/cmd/ssh-key/{list/http.go => shared/user_keys.go} (91%) diff --git a/pkg/cmd/auth/shared/login_flow.go b/pkg/cmd/auth/shared/login_flow.go index 837ff0d64..4f2cb9e0c 100644 --- a/pkg/cmd/auth/shared/login_flow.go +++ b/pkg/cmd/auth/shared/login_flow.go @@ -200,7 +200,7 @@ func Login(opts *LoginOptions) error { } if keyToUpload != "" { - err := sshKeyUpload(httpClient, hostname, keyToUpload, keyTitle) + err := sshKeyUpload(httpClient, hostname, keyToUpload, keyTitle, opts.IO) if err != nil { return err } @@ -223,14 +223,14 @@ func scopesSentence(scopes []string, isEnterprise bool) string { return strings.Join(quoted, ", ") } -func sshKeyUpload(httpClient *http.Client, hostname, keyFile string, title string) error { +func sshKeyUpload(httpClient *http.Client, hostname, keyFile string, title string, ios *iostreams.IOStreams) error { f, err := os.Open(keyFile) if err != nil { return err } defer f.Close() - return add.SSHKeyUpload(httpClient, hostname, f, title) + return add.SSHKeyUpload(httpClient, hostname, f, title, false, ios) } func getCurrentLogin(httpClient httpClient, hostname, authToken string) (string, error) { diff --git a/pkg/cmd/auth/shared/login_flow_test.go b/pkg/cmd/auth/shared/login_flow_test.go index f01ba46ff..92ae75915 100644 --- a/pkg/cmd/auth/shared/login_flow_test.go +++ b/pkg/cmd/auth/shared/login_flow_test.go @@ -38,6 +38,9 @@ func TestLogin_ssh(t *testing.T) { tr.Register( httpmock.GraphQL(`query UserCurrent\b`), httpmock.StringResponse(`{"data":{"viewer":{ "login": "monalisa" }}}`)) + tr.Register( + httpmock.REST("GET", "api/v3/user/keys"), + httpmock.StringResponse(`[]`)) tr.Register( httpmock.REST("POST", "api/v3/user/keys"), httpmock.StringResponse(`{}`)) @@ -78,7 +81,7 @@ func TestLogin_ssh(t *testing.T) { } assert.Equal(t, expected, args) // simulate that the public key file has been generated - _ = os.WriteFile(keyFile+".pub", []byte("PUBKEY"), 0600) + _ = os.WriteFile(keyFile+".pub", []byte("PUBKEY asdf"), 0600) }) cfg := tinyConfig{} diff --git a/pkg/cmd/ssh-key/add/add.go b/pkg/cmd/ssh-key/add/add.go index e3f5088d2..df977b291 100644 --- a/pkg/cmd/ssh-key/add/add.go +++ b/pkg/cmd/ssh-key/add/add.go @@ -1,7 +1,6 @@ package add import ( - "fmt" "io" "net/http" "os" @@ -79,14 +78,11 @@ func runAdd(opts *AddOptions) error { hostname, _ := cfg.Authentication().DefaultHost() - err = SSHKeyUpload(httpClient, hostname, keyReader, opts.Title) + err = SSHKeyUpload(httpClient, hostname, keyReader, opts.Title, true, opts.IO) if err != nil { return err } - if opts.IO.IsStdoutTTY() { - cs := opts.IO.ColorScheme() - fmt.Fprintf(opts.IO.ErrOut, "%s Public key added to your account\n", cs.SuccessIcon()) - } + // SSHKeyUpload prints the success message. return nil } diff --git a/pkg/cmd/ssh-key/add/add_test.go b/pkg/cmd/ssh-key/add/add_test.go index 565528ddd..8497dc991 100644 --- a/pkg/cmd/ssh-key/add/add_test.go +++ b/pkg/cmd/ssh-key/add/add_test.go @@ -11,33 +11,94 @@ import ( ) func Test_runAdd(t *testing.T) { - ios, stdin, stdout, stderr := iostreams.Test() - ios.SetStdinTTY(false) - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) + tests := []struct { + name string + stdin string + opts AddOptions + httpStubs func(*httpmock.Registry) + wantStdout string + wantStderr string + wantErrMsg string + }{ + { + name: "valid key format, not already in use", + stdin: "ssh-ed25519 asdf", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "user/keys"), + httpmock.StringResponse("[]")) + reg.Register( + httpmock.REST("POST", "user/keys"), + httpmock.RESTPayload(200, ``, func(payload map[string]interface{}) { + assert.Contains(t, payload, "key") + assert.Empty(t, payload["title"]) + })) + }, + wantStdout: "", + wantStderr: "✓ Public key added to your account\n", + wantErrMsg: "", + opts: AddOptions{KeyFile: "-"}, + }, + { + name: "valid key format, already in use", + stdin: "ssh-ed25519 asdf title", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "user/keys"), + httpmock.StringResponse(`[ + { + "id": 1, + "key": "ssh-ed25519 asdf", + "title": "anything" + } + ]`)) + }, + wantStdout: "", + wantStderr: "✓ Public key already exists on your account\n", + wantErrMsg: "", + opts: AddOptions{KeyFile: "-"}, + }, + { + name: "invalid key format", + stdin: "ssh-ed25519", + wantStdout: "", + wantStderr: "X Error: provided key is not in a valid format\n", + wantErrMsg: "SilentError", + opts: AddOptions{KeyFile: "-"}, + }, + } - stdin.WriteString("PUBKEY") + for _, tt := range tests { + ios, stdin, stdout, stderr := iostreams.Test() + ios.SetStdinTTY(false) + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) - tr := httpmock.Registry{} - defer tr.Verify(t) + stdin.WriteString(tt.stdin) - tr.Register( - httpmock.REST("POST", "user/keys"), - httpmock.StringResponse(`{}`)) + reg := &httpmock.Registry{} - err := runAdd(&AddOptions{ - IO: ios, - Config: func() (config.Config, error) { + tt.opts.IO = ios + tt.opts.HTTPClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + if tt.httpStubs != nil { + tt.httpStubs(reg) + } + tt.opts.Config = func() (config.Config, error) { return config.NewBlankConfig(), nil - }, - HTTPClient: func() (*http.Client, error) { - return &http.Client{Transport: &tr}, nil - }, - KeyFile: "-", - Title: "my sacred key", - }) - assert.NoError(t, err) + } - assert.Equal(t, "", stdout.String()) - assert.Equal(t, "✓ Public key added to your account\n", stderr.String()) + t.Run(tt.name, func(t *testing.T) { + defer reg.Verify(t) + err := runAdd(&tt.opts) + if tt.wantErrMsg != "" { + assert.Equal(t, tt.wantErrMsg, err.Error()) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + }) + } } diff --git a/pkg/cmd/ssh-key/add/http.go b/pkg/cmd/ssh-key/add/http.go index 1b8b7b245..9a603393d 100644 --- a/pkg/cmd/ssh-key/add/http.go +++ b/pkg/cmd/ssh-key/add/http.go @@ -3,21 +3,50 @@ package add import ( "bytes" "encoding/json" + "fmt" "io" "net/http" + "strings" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghinstance" + "github.com/cli/cli/v2/pkg/cmd/ssh-key/shared" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" ) -func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, title string) error { +func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, title string, printSuccessMsgs bool, ios *iostreams.IOStreams) error { url := ghinstance.RESTPrefix(hostname) + "user/keys" + cs := ios.ColorScheme() keyBytes, err := io.ReadAll(keyFile) if err != nil { return err } + userKey := string(keyBytes) + splitKey := strings.Fields(userKey) + if len(splitKey) < 2 { + fmt.Fprintf(ios.ErrOut, "%s Error: provided key is not in a valid format\n", cs.FailureIcon()) + return cmdutil.SilentError + } + + userKey = splitKey[0] + " " + splitKey[1] + + keys, err := shared.UserKeys(httpClient, hostname, "") + if err != nil { + return err + } + + for _, k := range keys { + if k.Key == userKey { + if printSuccessMsgs && ios.IsStdoutTTY() { + fmt.Fprintf(ios.ErrOut, "%s Public key already exists on your account\n", cs.SuccessIcon()) + } + return nil + } + } + payload := map[string]string{ "title": title, "key": string(keyBytes), @@ -48,5 +77,8 @@ func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, t return err } + if printSuccessMsgs { + fmt.Fprintf(ios.ErrOut, "%s Public key added to your account\n", cs.SuccessIcon()) + } return nil } diff --git a/pkg/cmd/ssh-key/list/list.go b/pkg/cmd/ssh-key/list/list.go index d057b198c..0c0cbcd0b 100644 --- a/pkg/cmd/ssh-key/list/list.go +++ b/pkg/cmd/ssh-key/list/list.go @@ -7,6 +7,7 @@ import ( "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/text" + "github.com/cli/cli/v2/pkg/cmd/ssh-key/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/utils" @@ -55,7 +56,7 @@ func listRun(opts *ListOptions) error { host, _ := cfg.Authentication().DefaultHost() - sshKeys, err := userKeys(apiClient, host, "") + sshKeys, err := shared.UserKeys(apiClient, host, "") if err != nil { return err } diff --git a/pkg/cmd/ssh-key/list/http.go b/pkg/cmd/ssh-key/shared/user_keys.go similarity index 91% rename from pkg/cmd/ssh-key/list/http.go rename to pkg/cmd/ssh-key/shared/user_keys.go index 7275615e0..57f309d19 100644 --- a/pkg/cmd/ssh-key/list/http.go +++ b/pkg/cmd/ssh-key/shared/user_keys.go @@ -1,4 +1,4 @@ -package list +package shared import ( "encoding/json" @@ -18,7 +18,7 @@ type sshKey struct { CreatedAt time.Time `json:"created_at"` } -func userKeys(httpClient *http.Client, host, userHandle string) ([]sshKey, error) { +func UserKeys(httpClient *http.Client, host, userHandle string) ([]sshKey, error) { resource := "user/keys" if userHandle != "" { resource = fmt.Sprintf("users/%s/keys", userHandle) From a4b9d41e79b0e188bda76c213c168737a51e8b17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 30 Mar 2023 11:30:12 +0200 Subject: [PATCH 28/35] Improve Amazon Linux install instructions --- docs/install_linux.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/install_linux.md b/docs/install_linux.md index 26ffeda90..0825c3305 100644 --- a/docs/install_linux.md +++ b/docs/install_linux.md @@ -59,6 +59,7 @@ sudo dnf update gh Install using our package repository for immediate access to latest releases: ```bash +type -p yum-config-manager >/dev/null || sudo yum install yum-utils sudo yum-config-manager --add-repo https://cli.github.com/packages/rpm/gh-cli.repo sudo yum install gh ``` From 0e5e7de9af4616a672a4b19acdee4aa0d13b5c67 Mon Sep 17 00:00:00 2001 From: vaindil Date: Thu, 30 Mar 2023 16:36:56 -0400 Subject: [PATCH 29/35] don't parse key bytes twice --- pkg/cmd/ssh-key/add/http.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/ssh-key/add/http.go b/pkg/cmd/ssh-key/add/http.go index 9a603393d..85e4a4dc0 100644 --- a/pkg/cmd/ssh-key/add/http.go +++ b/pkg/cmd/ssh-key/add/http.go @@ -24,14 +24,14 @@ func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, t return err } - userKey := string(keyBytes) - splitKey := strings.Fields(userKey) + fullUserKey := string(keyBytes) + splitKey := strings.Fields(fullUserKey) if len(splitKey) < 2 { fmt.Fprintf(ios.ErrOut, "%s Error: provided key is not in a valid format\n", cs.FailureIcon()) return cmdutil.SilentError } - userKey = splitKey[0] + " " + splitKey[1] + keyToCompare := splitKey[0] + " " + splitKey[1] keys, err := shared.UserKeys(httpClient, hostname, "") if err != nil { @@ -39,8 +39,8 @@ func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, t } for _, k := range keys { - if k.Key == userKey { - if printSuccessMsgs && ios.IsStdoutTTY() { + if k.Key == keyToCompare { + if printSuccessMsgs && ios.IsStderrTTY() { fmt.Fprintf(ios.ErrOut, "%s Public key already exists on your account\n", cs.SuccessIcon()) } return nil @@ -49,7 +49,7 @@ func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, t payload := map[string]string{ "title": title, - "key": string(keyBytes), + "key": fullUserKey, } payloadBytes, err := json.Marshal(payload) @@ -77,7 +77,7 @@ func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, t return err } - if printSuccessMsgs { + if printSuccessMsgs && ios.IsStderrTTY() { fmt.Fprintf(ios.ErrOut, "%s Public key added to your account\n", cs.SuccessIcon()) } return nil From 7006d3f0a59abe23546757d3dd571be181ac045e Mon Sep 17 00:00:00 2001 From: vaindil Date: Fri, 31 Mar 2023 13:47:25 -0400 Subject: [PATCH 30/35] move SSH key errors out of upload function --- pkg/cmd/auth/shared/login_flow.go | 15 ++++++++----- pkg/cmd/ssh-key/add/add.go | 12 +++++++++-- pkg/cmd/ssh-key/add/add_test.go | 4 ++-- pkg/cmd/ssh-key/add/http.go | 35 ++++++++++++------------------- 4 files changed, 35 insertions(+), 31 deletions(-) diff --git a/pkg/cmd/auth/shared/login_flow.go b/pkg/cmd/auth/shared/login_flow.go index 4f2cb9e0c..7c2ff1639 100644 --- a/pkg/cmd/auth/shared/login_flow.go +++ b/pkg/cmd/auth/shared/login_flow.go @@ -200,11 +200,16 @@ func Login(opts *LoginOptions) error { } if keyToUpload != "" { - err := sshKeyUpload(httpClient, hostname, keyToUpload, keyTitle, opts.IO) + uploaded, err := sshKeyUpload(httpClient, hostname, keyToUpload, keyTitle) if err != nil { return err } - fmt.Fprintf(opts.IO.ErrOut, "%s Uploaded the SSH key to your GitHub account: %s\n", cs.SuccessIcon(), cs.Bold(keyToUpload)) + + if uploaded { + fmt.Fprintf(opts.IO.ErrOut, "%s Uploaded the SSH key to your GitHub account: %s\n", cs.SuccessIcon(), cs.Bold(keyToUpload)) + } else { + fmt.Fprintf(opts.IO.ErrOut, "%s SSH key already existed on your GitHub account: %s\n", cs.SuccessIcon(), cs.Bold(keyToUpload)) + } } fmt.Fprintf(opts.IO.ErrOut, "%s Logged in as %s\n", cs.SuccessIcon(), cs.Bold(username)) @@ -223,14 +228,14 @@ func scopesSentence(scopes []string, isEnterprise bool) string { return strings.Join(quoted, ", ") } -func sshKeyUpload(httpClient *http.Client, hostname, keyFile string, title string, ios *iostreams.IOStreams) error { +func sshKeyUpload(httpClient *http.Client, hostname, keyFile string, title string) (bool, error) { f, err := os.Open(keyFile) if err != nil { - return err + return false, err } defer f.Close() - return add.SSHKeyUpload(httpClient, hostname, f, title, false, ios) + return add.SSHKeyUpload(httpClient, hostname, f, title) } func getCurrentLogin(httpClient httpClient, hostname, authToken string) (string, error) { diff --git a/pkg/cmd/ssh-key/add/add.go b/pkg/cmd/ssh-key/add/add.go index df977b291..84d39d1e7 100644 --- a/pkg/cmd/ssh-key/add/add.go +++ b/pkg/cmd/ssh-key/add/add.go @@ -1,6 +1,7 @@ package add import ( + "fmt" "io" "net/http" "os" @@ -78,11 +79,18 @@ func runAdd(opts *AddOptions) error { hostname, _ := cfg.Authentication().DefaultHost() - err = SSHKeyUpload(httpClient, hostname, keyReader, opts.Title, true, opts.IO) + uploaded, err := SSHKeyUpload(httpClient, hostname, keyReader, opts.Title) if err != nil { return err } - // SSHKeyUpload prints the success message. + cs := opts.IO.ColorScheme() + + if uploaded { + fmt.Fprintf(opts.IO.ErrOut, "%s Public key added to your account\n", cs.SuccessIcon()) + } else { + fmt.Fprintf(opts.IO.ErrOut, "%s Public key already exists on your account\n", cs.SuccessIcon()) + } + return nil } diff --git a/pkg/cmd/ssh-key/add/add_test.go b/pkg/cmd/ssh-key/add/add_test.go index 8497dc991..d49aaea90 100644 --- a/pkg/cmd/ssh-key/add/add_test.go +++ b/pkg/cmd/ssh-key/add/add_test.go @@ -62,8 +62,8 @@ func Test_runAdd(t *testing.T) { name: "invalid key format", stdin: "ssh-ed25519", wantStdout: "", - wantStderr: "X Error: provided key is not in a valid format\n", - wantErrMsg: "SilentError", + wantStderr: "", + wantErrMsg: "provided key is not in a valid format", opts: AddOptions{KeyFile: "-"}, }, } diff --git a/pkg/cmd/ssh-key/add/http.go b/pkg/cmd/ssh-key/add/http.go index 85e4a4dc0..e716425e0 100644 --- a/pkg/cmd/ssh-key/add/http.go +++ b/pkg/cmd/ssh-key/add/http.go @@ -3,7 +3,7 @@ package add import ( "bytes" "encoding/json" - "fmt" + "errors" "io" "net/http" "strings" @@ -11,39 +11,33 @@ import ( "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/pkg/cmd/ssh-key/shared" - "github.com/cli/cli/v2/pkg/cmdutil" - "github.com/cli/cli/v2/pkg/iostreams" ) -func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, title string, printSuccessMsgs bool, ios *iostreams.IOStreams) error { +// Uploads the provided SSH key. Returns true if the key was uploaded, false if it was not. +func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, title string) (bool, error) { url := ghinstance.RESTPrefix(hostname) + "user/keys" - cs := ios.ColorScheme() keyBytes, err := io.ReadAll(keyFile) if err != nil { - return err + return false, err } fullUserKey := string(keyBytes) splitKey := strings.Fields(fullUserKey) if len(splitKey) < 2 { - fmt.Fprintf(ios.ErrOut, "%s Error: provided key is not in a valid format\n", cs.FailureIcon()) - return cmdutil.SilentError + return false, errors.New("provided key is not in a valid format") } keyToCompare := splitKey[0] + " " + splitKey[1] keys, err := shared.UserKeys(httpClient, hostname, "") if err != nil { - return err + return false, err } for _, k := range keys { if k.Key == keyToCompare { - if printSuccessMsgs && ios.IsStderrTTY() { - fmt.Fprintf(ios.ErrOut, "%s Public key already exists on your account\n", cs.SuccessIcon()) - } - return nil + return false, nil } } @@ -54,31 +48,28 @@ func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, t payloadBytes, err := json.Marshal(payload) if err != nil { - return err + return false, err } req, err := http.NewRequest("POST", url, bytes.NewBuffer(payloadBytes)) if err != nil { - return err + return false, err } resp, err := httpClient.Do(req) if err != nil { - return err + return false, err } defer resp.Body.Close() if resp.StatusCode > 299 { - return api.HandleHTTPError(resp) + return false, api.HandleHTTPError(resp) } _, err = io.Copy(io.Discard, resp.Body) if err != nil { - return err + return false, err } - if printSuccessMsgs && ios.IsStderrTTY() { - fmt.Fprintf(ios.ErrOut, "%s Public key added to your account\n", cs.SuccessIcon()) - } - return nil + return true, nil } From 18f2484080dc81a92166c9127d6d70b3beeaf52a Mon Sep 17 00:00:00 2001 From: Kenichi Kocha <64531971+kkocha@users.noreply.github.com> Date: Mon, 3 Apr 2023 23:40:37 +0900 Subject: [PATCH 31/35] docs: add commit SHA arg to gh browse help (#7267) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić --- pkg/cmd/browse/browse.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index 160d72aa2..6a9456eb7 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -59,7 +59,7 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co cmd := &cobra.Command{ Long: "Open the GitHub repository in the web browser.", Short: "Open the repository in the browser", - Use: "browse [ | ]", + Use: "browse [ | | ]", Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` $ gh browse @@ -87,7 +87,8 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co "help:arguments": heredoc.Doc(` A browser location can be specified using arguments in the following format: - by number for issue or pull request, e.g. "123"; or - - by path for opening folders and files, e.g. "cmd/gh/main.go" + - by path for opening folders and files, e.g. "cmd/gh/main.go"; or + - by commit SHA `), "help:environment": heredoc.Doc(` To configure a web browser other than the default, use the BROWSER environment variable. From 6261bc697188321d43d18d915832255830175b45 Mon Sep 17 00:00:00 2001 From: Kenichi Kocha <64531971+kkocha@users.noreply.github.com> Date: Tue, 4 Apr 2023 10:36:09 +0900 Subject: [PATCH 32/35] Additional help doc and example for auth setup-git (#7243) --- pkg/cmd/auth/setupgit/setupgit.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/auth/setupgit/setupgit.go b/pkg/cmd/auth/setupgit/setupgit.go index a041ebfc1..a3c991f4f 100644 --- a/pkg/cmd/auth/setupgit/setupgit.go +++ b/pkg/cmd/auth/setupgit/setupgit.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/pkg/cmd/auth/shared" "github.com/cli/cli/v2/pkg/cmdutil" @@ -29,8 +30,26 @@ func NewCmdSetupGit(f *cmdutil.Factory, runF func(*SetupGitOptions) error) *cobr } cmd := &cobra.Command{ - Short: "Configure git to use GitHub CLI as a credential helper", Use: "setup-git", + Short: "Setup git with GitHub CLI", + Long: heredoc.Docf(` + This command configures git to use GitHub CLI as a credential helper. + For more information on git credential helpers please reference: + https://git-scm.com/docs/gitcredentials. + + By default, GitHub CLI will be set as the credential helper for all authenticated hosts. + If there is no authenticated hosts the command fails with an error. + + Alternatively, use the %[1]s--hostname%[1]s flag to specify a single host to be configured. + If the host is not authenticated with, the command fails with an error. + `, "`"), + Example: heredoc.Doc(` + # Configure git to use GitHub CLI as the credential helper for all authenticated hosts + $ gh auth setup-git + + # Configure git to use GitHub CLI as the credential helper for enterprise.internal host + $ gh auth setup-git --hostname enterprise.internal + `), RunE: func(cmd *cobra.Command, args []string) error { opts.gitConfigure = &shared.GitCredentialFlow{ Executable: f.Executable(), From 126341205500b7f69ecef223f661a30b3fa7df03 Mon Sep 17 00:00:00 2001 From: Kenichi Kocha <64531971+kkocha@users.noreply.github.com> Date: Tue, 4 Apr 2023 11:54:40 +0900 Subject: [PATCH 33/35] fix: make number arg, commit arg, and flags mutually exclusive (#7268) * make number, commit, and flags mutually exclusive * break condition into arg own check --- pkg/cmd/browse/browse.go | 4 ++++ pkg/cmd/browse/browse_test.go | 22 +++++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index 6a9456eb7..42664d77c 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -125,6 +125,10 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co return err } + if (isNumber(opts.SelectorArg) || isCommit(opts.SelectorArg)) && (opts.Branch != "" || opts.Commit != "") { + return cmdutil.FlagErrorf("%q is an invalid argument when using `--branch` or `--commit`", opts.SelectorArg) + } + if cmd.Flags().Changed("repo") { opts.GitClient = &remoteGitClient{opts.BaseRepo, opts.HttpClient} } diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index 440970f29..4995e5cdf 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -162,7 +162,27 @@ func TestNewCmdBrowse(t *testing.T) { }, { name: "passed both branch and commit flags", - cli: "main.go --branch main --comit=12a4", + cli: "main.go --branch main --commit=12a4", + wantsErr: true, + }, + { + name: "passed both number arg and branch flag", + cli: "1 --branch trunk", + wantsErr: true, + }, + { + name: "passed both number arg and commit flag", + cli: "1 --commit=12a4", + wantsErr: true, + }, + { + name: "passed both commit SHA arg and branch flag", + cli: "de07febc26e19000f8c9e821207f3bc34a3c8038 --branch trunk", + wantsErr: true, + }, + { + name: "passed both commit SHA arg and commit flag", + cli: "de07febc26e19000f8c9e821207f3bc34a3c8038 --commit=12a4", wantsErr: true, }, } From 5d56dfbf423ce95f73d65b741d2a6578cddca087 Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 4 Apr 2023 11:13:43 -0700 Subject: [PATCH 34/35] add --insecure-storage and make secure storage the default --- pkg/cmd/auth/login/login.go | 29 ++++++------ pkg/cmd/auth/login/login_test.go | 76 ++++++++++++++++++++------------ 2 files changed, 63 insertions(+), 42 deletions(-) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 3eda42ae0..258853164 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -30,12 +30,12 @@ type LoginOptions struct { Interactive bool - Hostname string - Scopes []string - Token string - Web bool - GitProtocol string - SecureStorage bool + Hostname string + Scopes []string + Token string + Web bool + GitProtocol string + InsecureStorage bool } func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Command { @@ -124,7 +124,13 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm cmd.Flags().BoolVar(&tokenStdin, "with-token", false, "Read token from standard input") cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open a browser to authenticate") cmdutil.StringEnumFlag(cmd, &opts.GitProtocol, "git-protocol", "p", "", []string{"ssh", "https"}, "The protocol to use for git operations") - cmd.Flags().BoolVarP(&opts.SecureStorage, "secure-storage", "", false, "Save authentication credentials in secure credential store") + + // secure storage became the default on 2023/4/04; this flag is left as a no-op for backwards compatibility + var secureStorage bool + cmd.Flags().BoolVar(&secureStorage, "secure-storage", false, "Save authentication credentials in secure credential store") + _ = cmd.Flags().MarkHidden("secure-storage") + + cmd.Flags().BoolVar(&opts.InsecureStorage, "insecure-storage", false, "Save authentication credentials in plain text instead of credential store") return cmd } @@ -136,11 +142,6 @@ func loginRun(opts *LoginOptions) error { } authCfg := cfg.Authentication() - if opts.SecureStorage { - cs := opts.IO.ColorScheme() - fmt.Fprintf(opts.IO.ErrOut, "%s Using secure storage could break installed extensions\n", cs.WarningIcon()) - } - hostname := opts.Hostname if opts.Interactive && hostname == "" { var err error @@ -166,7 +167,7 @@ func loginRun(opts *LoginOptions) error { return fmt.Errorf("error validating token: %w", err) } // Adding a user key ensures that a nonempty host section gets written to the config file. - return authCfg.Login(hostname, "x-access-token", opts.Token, opts.GitProtocol, opts.SecureStorage) + return authCfg.Login(hostname, "x-access-token", opts.Token, opts.GitProtocol, !opts.InsecureStorage) } existingToken, _ := authCfg.Token(hostname) @@ -195,7 +196,7 @@ func loginRun(opts *LoginOptions) error { Prompter: opts.Prompter, GitClient: opts.GitClient, Browser: opts.Browser, - SecureStorage: opts.SecureStorage, + SecureStorage: !opts.InsecureStorage, }) } diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index 9f95cc9ee..72d796a39 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -178,16 +178,31 @@ func Test_NewCmdLogin(t *testing.T) { stdinTTY: true, cli: "--secure-storage", wants: LoginOptions{ - Interactive: true, - SecureStorage: true, + Interactive: true, }, }, { name: "nontty secure-storage", cli: "--secure-storage", wants: LoginOptions{ - Hostname: "github.com", - SecureStorage: true, + Hostname: "github.com", + }, + }, + { + name: "tty insecure-storage", + stdinTTY: true, + cli: "--insecure-storage", + wants: LoginOptions{ + Interactive: true, + InsecureStorage: true, + }, + }, + { + name: "nontty insecure-storage", + cli: "--insecure-storage", + wants: LoginOptions{ + Hostname: "github.com", + InsecureStorage: true, }, }, } @@ -251,10 +266,11 @@ func Test_loginRun_nontty(t *testing.T) { wantSecureToken string }{ { - name: "with token", + name: "insecure with token", opts: &LoginOptions{ - Hostname: "github.com", - Token: "abc123", + Hostname: "github.com", + Token: "abc123", + InsecureStorage: true, }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org")) @@ -262,11 +278,12 @@ func Test_loginRun_nontty(t *testing.T) { wantHosts: "github.com:\n oauth_token: abc123\n user: x-access-token\n", }, { - name: "with token and https git-protocol", + name: "insecure with token and https git-protocol", opts: &LoginOptions{ - Hostname: "github.com", - Token: "abc123", - GitProtocol: "https", + Hostname: "github.com", + Token: "abc123", + GitProtocol: "https", + InsecureStorage: true, }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org")) @@ -276,8 +293,9 @@ func Test_loginRun_nontty(t *testing.T) { { name: "with token and non-default host", opts: &LoginOptions{ - Hostname: "albert.wesker", - Token: "abc123", + Hostname: "albert.wesker", + Token: "abc123", + InsecureStorage: true, }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.ScopesResponder("repo,read:org")) @@ -309,8 +327,9 @@ func Test_loginRun_nontty(t *testing.T) { { name: "has admin scope", opts: &LoginOptions{ - Hostname: "github.com", - Token: "abc456", + Hostname: "github.com", + Token: "abc456", + InsecureStorage: true, }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,admin:org")) @@ -358,16 +377,14 @@ func Test_loginRun_nontty(t *testing.T) { { name: "with token and secure storage", opts: &LoginOptions{ - Hostname: "github.com", - Token: "abc123", - SecureStorage: true, + Hostname: "github.com", + Token: "abc123", }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org")) }, wantHosts: "github.com:\n user: x-access-token\n", wantSecureToken: "abc123", - wantStderr: "! Using secure storage could break installed extensions\n", }, } @@ -463,8 +480,9 @@ func Test_loginRun_Survey(t *testing.T) { { name: "hostname set", opts: &LoginOptions{ - Hostname: "rebecca.chambers", - Interactive: true, + Hostname: "rebecca.chambers", + Interactive: true, + InsecureStorage: true, }, wantHosts: heredoc.Doc(` rebecca.chambers: @@ -504,7 +522,8 @@ func Test_loginRun_Survey(t *testing.T) { git_protocol: https `), opts: &LoginOptions{ - Interactive: true, + Interactive: true, + InsecureStorage: true, }, prompterStubs: func(pm *prompter.PrompterMock) { pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) { @@ -543,7 +562,8 @@ func Test_loginRun_Survey(t *testing.T) { git_protocol: https `), opts: &LoginOptions{ - Interactive: true, + Interactive: true, + InsecureStorage: true, }, prompterStubs: func(pm *prompter.PrompterMock) { pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) { @@ -573,7 +593,8 @@ func Test_loginRun_Survey(t *testing.T) { git_protocol: ssh `), opts: &LoginOptions{ - Interactive: true, + Interactive: true, + InsecureStorage: true, }, prompterStubs: func(pm *prompter.PrompterMock) { pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) { @@ -593,9 +614,8 @@ func Test_loginRun_Survey(t *testing.T) { { name: "secure storage", opts: &LoginOptions{ - Hostname: "github.com", - Interactive: true, - SecureStorage: true, + Hostname: "github.com", + Interactive: true, }, prompterStubs: func(pm *prompter.PrompterMock) { pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) { @@ -617,7 +637,7 @@ func Test_loginRun_Survey(t *testing.T) { user: jillv git_protocol: https `), - wantErrOut: regexp.MustCompile("! Using secure storage could break installed extensions"), + wantErrOut: regexp.MustCompile("Logged in as jillv"), wantSecureToken: "def456", }, } From d9db81db4ff35d38fb8abf3fd27c46de532c198f Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 4 Apr 2023 11:13:53 -0700 Subject: [PATCH 35/35] fix apparent typo --- pkg/cmd/auth/token/token.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/auth/token/token.go b/pkg/cmd/auth/token/token.go index 5ae555bb6..fee8dc638 100644 --- a/pkg/cmd/auth/token/token.go +++ b/pkg/cmd/auth/token/token.go @@ -38,7 +38,7 @@ func NewCmdToken(f *cmdutil.Factory, runF func(*TokenOptions) error) *cobra.Comm cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance authenticated with") cmd.Flags().BoolVarP(&opts.SecureStorage, "secure-storage", "", false, "Search only secure credential store for authentication token") - _ = cmd.Flags().MarkHidden("secure-storeage") + _ = cmd.Flags().MarkHidden("secure-storage") return cmd }