From e0d2fc8eaabd62f06058df10ffbccd02a67d4c5c Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Mon, 7 Aug 2023 07:35:47 -0700 Subject: [PATCH 01/23] Use filepath.Base to sanitize path for archive downloads (#7805) --- pkg/cmd/release/download/download.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/release/download/download.go b/pkg/cmd/release/download/download.go index 16e95db29..bab6bd19b 100644 --- a/pkg/cmd/release/download/download.go +++ b/pkg/cmd/release/download/download.go @@ -290,7 +290,7 @@ func downloadAsset(dest *destinationWriter, httpClient *http.Client, assetURL, f return fmt.Errorf("unable to parse file name of archive: %w", err) } if serverFileName, ok := params["filename"]; ok { - fileName = filepath.Clean(serverFileName) + fileName = filepath.Base(serverFileName) } else { return errors.New("unable to determine file name of archive") } From cd65409328e578e2249ce7f365ec121389c55b94 Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Mon, 7 Aug 2023 17:12:46 -0700 Subject: [PATCH 02/23] switch to Prompter.MultiSelect --- pkg/cmd/repo/edit/edit.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index bbbd94dd5..64df48aa7 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -441,15 +441,17 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { if r.RebaseMergeAllowed { defaultMergeOptions = append(defaultMergeOptions, allowRebaseMerge) } - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.MultiSelect{ - Message: "Allowed merge strategies", - Default: defaultMergeOptions, - Options: []string{allowMergeCommits, allowSquashMerge, allowRebaseMerge}, - }, &selectedMergeOptions) + mergeOpts := []string{allowMergeCommits, allowSquashMerge, allowRebaseMerge} + selected, err := opts.Prompter.MultiSelect( + "Allowed merge strategies", + defaultMergeOptions, + mergeOpts) if err != nil { return err } + for _, i := range selected { + selectedMergeOptions = append(selectedMergeOptions, mergeOpts[i]) + } enableMergeCommit := isIncluded(allowMergeCommits, selectedMergeOptions) opts.Edits.EnableMergeCommit = &enableMergeCommit enableSquashMerge := isIncluded(allowSquashMerge, selectedMergeOptions) From 7d470c4df4321c4da37962ad511a165b61b9ed7e Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Mon, 7 Aug 2023 17:59:12 -0700 Subject: [PATCH 03/23] name MultiSelect parameters in interface I wanted the parameters to show up in my autocomplete --- internal/prompter/prompter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/prompter/prompter.go b/internal/prompter/prompter.go index cd90fcd2d..df6819e1f 100644 --- a/internal/prompter/prompter.go +++ b/internal/prompter/prompter.go @@ -14,7 +14,7 @@ import ( //go:generate moq -rm -out prompter_mock.go . Prompter type Prompter interface { Select(string, string, []string) (int, error) - MultiSelect(string, []string, []string) ([]int, error) + MultiSelect(prompt string, defaults []string, options []string) ([]int, error) Input(string, string) (string, error) InputHostname() (string, error) Password(string) (string, error) From aed3a6774964eb23c3a90141104a8bc08b7da654 Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Mon, 7 Aug 2023 17:59:56 -0700 Subject: [PATCH 04/23] WIP porting repo edit to opts.Prompter --- pkg/cmd/repo/edit/edit.go | 39 ++++++++++------------- pkg/cmd/repo/edit/edit_test.go | 58 +++++++++++++++++++++++++++++----- 2 files changed, 67 insertions(+), 30 deletions(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index 64df48aa7..4841362e7 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -279,7 +279,7 @@ func editRun(ctx context.Context, opts *EditOptions) error { return nil } -func interactiveChoice(r *api.Repository) ([]string, error) { +func interactiveChoice(opts EditOptions, r *api.Repository) ([]string, error) { options := []string{ optionDefaultBranchName, optionDescription, @@ -298,11 +298,14 @@ func interactiveChoice(r *api.Repository) ([]string, error) { options = append(options, optionAllowForking) } var answers []string - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne(&survey.MultiSelect{ - Message: "What do you want to edit?", - Options: options, - }, &answers, survey.WithPageSize(11)) + selected, err := opts.Prompter.MultiSelect("What do you want to edit?", nil, options) + if err != nil { + return nil, err + } + for _, i := range selected { + answers = append(answers, options[i]) + } + return answers, err } @@ -310,7 +313,7 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { for _, v := range r.RepositoryTopics.Nodes { opts.topicsCache = append(opts.topicsCache, v.Topic.Name) } - choices, err := interactiveChoice(r) + choices, err := interactiveChoice(*opts, r) if err != nil { return err } @@ -318,14 +321,11 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { switch c { case optionDescription: opts.Edits.Description = &r.Description - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Input{ - Message: "Description of the repository", - Default: r.Description, - }, opts.Edits.Description) + answer, err := opts.Prompter.Input("Description of the repository", r.Description) if err != nil { return err } + opts.Edits.Description = &answer case optionHomePageURL: opts.Edits.Homepage = &r.HomepageURL //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter @@ -337,11 +337,7 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { return err } case optionTopics: - var addTopics string - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Input{ - Message: "Add topics?(csv format)", - }, &addTopics) + addTopics, err := opts.Prompter.Input("Add topics?(csv format)", "") if err != nil { return err } @@ -350,14 +346,13 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { } if len(opts.topicsCache) > 0 { - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.MultiSelect{ - Message: "Remove Topics", - Options: opts.topicsCache, - }, &opts.RemoveTopics) + selected, err := opts.Prompter.MultiSelect("Remove Topics", nil, opts.topicsCache) if err != nil { return err } + for _, i := range selected { + opts.RemoveTopics = append(opts.RemoveTopics, opts.topicsCache[i]) + } } case optionDefaultBranchName: opts.Edits.DefaultBranch = &r.DefaultBranchRef.Name diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go index 7302b5169..98f1cb8cc 100644 --- a/pkg/cmd/repo/edit/edit_test.go +++ b/pkg/cmd/repo/edit/edit_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/pkg/prompt" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" @@ -174,11 +175,26 @@ func Test_editRun(t *testing.T) { } } +// TODO consider unit test for interactiveRepoEdit that exercises every prompt type + func Test_editRun_interactive(t *testing.T) { + editList := []string{ + "Default Branch Name", + "Description", + "Home Page URL", + "Issues", + "Merge Options", + "Projects", + "Template Repository", + "Topics", + "Visibility", + "Wikis"} + tests := []struct { name string opts EditOptions askStubs func(*prompt.AskStubber) + promptStubs func(*prompter.MockPrompter) httpStubs func(*testing.T, *httpmock.Registry) wantsStderr string wantsErr string @@ -189,9 +205,16 @@ func Test_editRun_interactive(t *testing.T) { Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), InteractiveMode: true, }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("What do you want to edit?").AnswerWith([]string{"Description"}) - as.StubPrompt("Description of the repository").AnswerWith("awesome repo description") + askStubs: func(as *prompt.AskStubber) {}, + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterMultiSelect("What do you want to edit?", nil, editList, + func(_ string, _, opts []string) ([]int, error) { + return []int{1}, nil + }) + pm.RegisterInput("Description of the repository", + func(_, _ string) (string, error) { + return "awesome repo description", nil + }) }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( @@ -229,11 +252,24 @@ func Test_editRun_interactive(t *testing.T) { Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), InteractiveMode: true, }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("What do you want to edit?").AnswerWith([]string{"Description", "Topics"}) - as.StubPrompt("Description of the repository").AnswerWith("awesome repo description") - as.StubPrompt("Add topics?(csv format)").AnswerWith("a, b,c,d ") - as.StubPrompt("Remove Topics").AnswerWith([]string{"x"}) + askStubs: func(as *prompt.AskStubber) {}, + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterMultiSelect("What do you want to edit?", nil, editList, + func(_ string, _, opts []string) ([]int, error) { + return []int{1, 7}, nil + }) + pm.RegisterInput("Description of the repository", + func(_, _ string) (string, error) { + return "awesome repo description", nil + }) + pm.RegisterInput("Add topics?(csv format)", + func(_, _ string) (string, error) { + return "a, b,c,d ", nil + }) + pm.RegisterMultiSelect("Remove Topics", nil, []string{"x"}, + func(_ string, _, opts []string) ([]int, error) { + return []int{0}, nil + }) }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( @@ -333,6 +369,12 @@ func Test_editRun_interactive(t *testing.T) { tt.httpStubs(t, httpReg) } + pm := prompter.NewMockPrompter(t) + tt.opts.Prompter = pm + if tt.promptStubs != nil { + tt.promptStubs(pm) + } + opts := &tt.opts opts.HTTPClient = &http.Client{Transport: httpReg} opts.IO = ios From 444ca57e173f8559ad6c9697aa55d76f08c62888 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Tue, 8 Aug 2023 09:50:45 -0400 Subject: [PATCH 05/23] Update CONTRIBUTING.md Bringing the contribution documentation in line with the release documentation as Go 1.19+ is required. --- .github/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 3b2584063..eed468389 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -23,7 +23,7 @@ Please avoid: ## Building the project Prerequisites: -- Go 1.16+ +- Go 1.19+ Build with: * Unix-like systems: `make` From 93e1511bae2923f617d99526281c6b8918a7c3ab Mon Sep 17 00:00:00 2001 From: John Keech Date: Tue, 8 Aug 2023 09:32:06 -0700 Subject: [PATCH 06/23] Codespaces: Use the host name from the logged in server for commands (#7795) * Use the host name from the logged in server for codespace commands * Fix existing tests * Add tests for server url configuration * Rename defaultApiUrl to defaultAPIURL and comment cleanup * Switch to t.Setenv in codespaces api tests * Switch to t.Setenv in other tests * Support custom server in web flows for list and create * Rename GetServerURL() to ServerURL() --- internal/codespaces/api/api.go | 64 ++++-- internal/codespaces/api/api_test.go | 190 +++++++++++++++++- pkg/cmd/codespace/common.go | 1 + pkg/cmd/codespace/create.go | 4 +- pkg/cmd/codespace/create_test.go | 30 +++ pkg/cmd/codespace/list.go | 4 +- pkg/cmd/codespace/list_test.go | 24 +++ pkg/cmd/codespace/mock_api.go | 151 ++++++++------ pkg/cmd/codespace/root.go | 19 +- pkg/cmd/project/close/close_test.go | 4 +- pkg/cmd/project/copy/copy_test.go | 4 +- pkg/cmd/project/create/create_test.go | 4 +- pkg/cmd/project/delete/delete_test.go | 4 +- pkg/cmd/project/edit/edit_test.go | 4 +- .../project/field-create/field_create_test.go | 4 +- .../project/field-delete/field_delete_test.go | 4 +- pkg/cmd/project/field-list/field_list_test.go | 4 +- pkg/cmd/project/item-add/item_add_test.go | 4 +- .../project/item-archive/item_archive_test.go | 4 +- .../project/item-create/item_create_test.go | 4 +- .../project/item-delete/item_delete_test.go | 4 +- pkg/cmd/project/item-edit/item_edit_test.go | 4 +- pkg/cmd/project/item-list/item_list_test.go | 4 +- pkg/cmd/project/list/list_test.go | 4 +- pkg/cmd/project/view/view_test.go | 4 +- pkg/cmd/root/root.go | 52 +---- 26 files changed, 409 insertions(+), 194 deletions(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index d66aff065..c55b92400 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -1,13 +1,12 @@ package api // For descriptions of service interfaces, see: -// - https://online.visualstudio.com/api/swagger (for visualstudio.com) // - https://docs.github.com/en/rest/reference/repos (for api.github.com) // - https://github.com/github/github/blob/master/app/api/codespaces.rb (for vscs_internal) // TODO(adonovan): replace the last link with a public doc URL when available. // TODO(adonovan): a possible reorganization would be to split this -// file into three internal packages, one per backend service, and to +// file into two internal packages, one per backend service, and to // rename api.API to github.Client: // // - github.GetUser(github.Client) @@ -20,7 +19,6 @@ package api // - codespaces.GetToken(Client, login, name) // - codespaces.List(Client, user) // - codespaces.Start(Client, token, codespace) -// - visualstudio.GetRegionLocation(http.Client) // no dependency on github // // This would make the meaning of each operation clearer. @@ -34,6 +32,7 @@ import ( "io" "net/http" "net/url" + "os" "reflect" "regexp" "strconv" @@ -42,13 +41,14 @@ import ( "github.com/cenkalti/backoff/v4" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghinstance" + "github.com/cli/cli/v2/pkg/cmdutil" "github.com/opentracing/opentracing-go" ) const ( - githubServer = "https://github.com" - githubAPI = "https://api.github.com" - vscsAPI = "https://online.visualstudio.com" + defaultAPIURL = "https://api.github.com" + defaultServerURL = "https://github.com" ) const ( @@ -60,31 +60,40 @@ const ( // API is the interface to the codespace service. type API struct { - client httpClient - vscsAPI string + client func() (*http.Client, error) githubAPI string githubServer string retryBackoff time.Duration } -type httpClient interface { - Do(req *http.Request) (*http.Response, error) -} - // New creates a new API client connecting to the configured endpoints with the HTTP client. -func New(serverURL, apiURL, vscsURL string, httpClient httpClient) *API { - if serverURL == "" { - serverURL = githubServer - } +func New(f *cmdutil.Factory) *API { + apiURL := os.Getenv("GITHUB_API_URL") if apiURL == "" { - apiURL = githubAPI + cfg, err := f.Config() + if err != nil { + // fallback to the default api endpoint + apiURL = defaultAPIURL + } else { + host, _ := cfg.Authentication().DefaultHost() + apiURL = ghinstance.RESTPrefix(host) + } } - if vscsURL == "" { - vscsURL = vscsAPI + + serverURL := os.Getenv("GITHUB_SERVER_URL") + if serverURL == "" { + cfg, err := f.Config() + if err != nil { + // fallback to the default server endpoint + serverURL = defaultServerURL + } else { + host, _ := cfg.Authentication().DefaultHost() + serverURL = ghinstance.HostPrefix(host) + } } + return &API{ - client: httpClient, - vscsAPI: strings.TrimSuffix(vscsURL, "/"), + client: f.HttpClient, githubAPI: strings.TrimSuffix(apiURL, "/"), githubServer: strings.TrimSuffix(serverURL, "/"), retryBackoff: 100 * time.Millisecond, @@ -97,6 +106,11 @@ type User struct { Type string `json:"type"` } +// ServerURL returns the server url (not the API url), such as https://github.com +func (a *API) ServerURL() string { + return a.githubServer +} + // GetUser returns the user associated with the given token. func (a *API) GetUser(ctx context.Context) (*User, error) { req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/user", nil) @@ -1119,7 +1133,13 @@ func (a *API) do(ctx context.Context, req *http.Request, spanName string) (*http span, ctx := opentracing.StartSpanFromContext(ctx, spanName) defer span.Finish() req = req.WithContext(ctx) - return a.client.Do(req) + + httpClient, err := a.client() + if err != nil { + return nil, err + } + + return httpClient.Do(req) } // setHeaders sets the required headers for the API. diff --git a/internal/codespaces/api/api_test.go b/internal/codespaces/api/api_test.go index 5e9316fc1..75f82c39e 100644 --- a/internal/codespaces/api/api_test.go +++ b/internal/codespaces/api/api_test.go @@ -3,12 +3,16 @@ package api import ( "context" "encoding/json" + "errors" "fmt" "net/http" "net/http/httptest" "reflect" "strconv" "testing" + + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/pkg/cmdutil" ) func generateCodespaceList(start int, end int) []*Codespace { @@ -126,13 +130,181 @@ func createFakeCreateEndpointServer(t *testing.T, wantStatus int) *httptest.Serv })) } +func createHttpClient() (*http.Client, error) { + return &http.Client{}, nil +} + +func TestNew_APIURL_dotcomConfig(t *testing.T) { + t.Setenv("GITHUB_API_URL", "") + cfg := &config.ConfigMock{ + AuthenticationFunc: func() *config.AuthConfig { + return &config.AuthConfig{} + }, + } + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return cfg, nil + }, + } + api := New(f) + + if api.githubAPI != "https://api.github.com" { + t.Fatalf("expected https://api.github.com, got %s", api.githubAPI) + } + if len(cfg.AuthenticationCalls()) != 1 { + t.Fatalf("API url was not pulled from the config") + } +} + +func TestNew_APIURL_customConfig(t *testing.T) { + t.Setenv("GITHUB_API_URL", "") + cfg := &config.ConfigMock{ + AuthenticationFunc: func() *config.AuthConfig { + authCfg := &config.AuthConfig{} + authCfg.SetDefaultHost("github.mycompany.com", "GH_HOST") + return authCfg + }, + } + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return cfg, nil + }, + } + api := New(f) + + if api.githubAPI != "https://github.mycompany.com/api/v3" { + t.Fatalf("expected https://github.mycompany.com/api/v3, got %s", api.githubAPI) + } + if len(cfg.AuthenticationCalls()) != 1 { + t.Fatalf("API url was not pulled from the config") + } +} + +func TestNew_APIURL_env(t *testing.T) { + t.Setenv("GITHUB_API_URL", "https://api.mycompany.com") + cfg := &config.ConfigMock{ + AuthenticationFunc: func() *config.AuthConfig { + return &config.AuthConfig{} + }, + } + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return cfg, nil + }, + } + api := New(f) + + if api.githubAPI != "https://api.mycompany.com" { + t.Fatalf("expected https://api.mycompany.com, got %s", api.githubAPI) + } + if len(cfg.AuthenticationCalls()) != 0 { + t.Fatalf("Configuration was checked instead of using the GITHUB_API_URL environment variable") + } +} + +func TestNew_APIURL_dotcomFallback(t *testing.T) { + t.Setenv("GITHUB_API_URL", "") + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return nil, errors.New("Failed to load") + }, + } + api := New(f) + + if api.githubAPI != "https://api.github.com" { + t.Fatalf("expected https://api.github.com, got %s", api.githubAPI) + } +} + +func TestNew_ServerURL_dotcomConfig(t *testing.T) { + t.Setenv("GITHUB_SERVER_URL", "") + cfg := &config.ConfigMock{ + AuthenticationFunc: func() *config.AuthConfig { + return &config.AuthConfig{} + }, + } + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return cfg, nil + }, + } + api := New(f) + + if api.githubServer != "https://github.com" { + t.Fatalf("expected https://github.com, got %s", api.githubServer) + } + if len(cfg.AuthenticationCalls()) != 1 { + t.Fatalf("Server url was not pulled from the config") + } +} + +func TestNew_ServerURL_customConfig(t *testing.T) { + t.Setenv("GITHUB_SERVER_URL", "") + cfg := &config.ConfigMock{ + AuthenticationFunc: func() *config.AuthConfig { + authCfg := &config.AuthConfig{} + authCfg.SetDefaultHost("github.mycompany.com", "GH_HOST") + return authCfg + }, + } + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return cfg, nil + }, + } + api := New(f) + + if api.githubServer != "https://github.mycompany.com" { + t.Fatalf("expected https://github.mycompany.com, got %s", api.githubServer) + } + if len(cfg.AuthenticationCalls()) != 1 { + t.Fatalf("Server url was not pulled from the config") + } +} + +func TestNew_ServerURL_env(t *testing.T) { + t.Setenv("GITHUB_SERVER_URL", "https://mycompany.com") + cfg := &config.ConfigMock{ + AuthenticationFunc: func() *config.AuthConfig { + return &config.AuthConfig{} + }, + } + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return cfg, nil + }, + } + api := New(f) + + if api.githubServer != "https://mycompany.com" { + t.Fatalf("expected https://mycompany.com, got %s", api.githubServer) + } + if len(cfg.AuthenticationCalls()) != 0 { + t.Fatalf("Configuration was checked instead of using the GITHUB_SERVER_URL environment variable") + } +} + +func TestNew_ServerURL_dotcomFallback(t *testing.T) { + t.Setenv("GITHUB_SERVER_URL", "") + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return nil, errors.New("Failed to load") + }, + } + api := New(f) + + if api.githubServer != "https://github.com" { + t.Fatalf("expected https://github.com, got %s", api.githubServer) + } +} + func TestCreateCodespaces(t *testing.T) { svr := createFakeCreateEndpointServer(t, http.StatusCreated) defer svr.Close() api := API{ githubAPI: svr.URL, - client: &http.Client{}, + client: createHttpClient, } ctx := context.TODO() @@ -161,7 +333,7 @@ func TestCreateCodespaces_displayName(t *testing.T) { api := API{ githubAPI: svr.URL, - client: &http.Client{}, + client: createHttpClient, } retentionPeriod := 0 @@ -186,7 +358,7 @@ func TestCreateCodespaces_Pending(t *testing.T) { api := API{ githubAPI: svr.URL, - client: &http.Client{}, + client: createHttpClient, retryBackoff: 0, } @@ -213,7 +385,7 @@ func TestListCodespaces_limited(t *testing.T) { api := API{ githubAPI: svr.URL, - client: &http.Client{}, + client: createHttpClient, } ctx := context.TODO() codespaces, err := api.ListCodespaces(ctx, ListCodespacesOptions{Limit: 200}) @@ -238,7 +410,7 @@ func TestListCodespaces_unlimited(t *testing.T) { api := API{ githubAPI: svr.URL, - client: &http.Client{}, + client: createHttpClient, } ctx := context.TODO() codespaces, err := api.ListCodespaces(ctx, ListCodespacesOptions{}) @@ -329,7 +501,7 @@ func runRepoSearchTest(t *testing.T, searchText, wantQueryText, wantSort, wantMa api := API{ githubAPI: svr.URL, - client: &http.Client{}, + client: createHttpClient, } ctx := context.Background() @@ -374,7 +546,7 @@ func TestRetries(t *testing.T) { t.Cleanup(srv.Close) a := &API{ githubAPI: srv.URL, - client: &http.Client{}, + client: createHttpClient, } cs, err := a.GetCodespace(context.Background(), "test", false) if err != nil { @@ -562,7 +734,7 @@ func TestAPI_EditCodespace(t *testing.T) { defer svr.Close() a := &API{ - client: &http.Client{}, + client: createHttpClient, githubAPI: svr.URL, } got, err := a.EditCodespace(tt.args.ctx, tt.args.codespaceName, tt.args.params) @@ -602,7 +774,7 @@ func TestAPI_EditCodespacePendingOperation(t *testing.T) { defer svr.Close() a := &API{ - client: &http.Client{}, + client: createHttpClient, githubAPI: svr.URL, } diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index f6f6fb937..006669151 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -88,6 +88,7 @@ func startLiveShareSession(ctx context.Context, codespace *api.Codespace, a *App //go:generate moq -fmt goimports -rm -skip-ensure -out mock_api.go . apiClient type apiClient interface { + ServerURL() string GetUser(ctx context.Context) (*api.User, error) GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) GetOrgMemberCodespace(ctx context.Context, orgName string, userName string, codespaceName string) (*api.Codespace, error) diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 6f973d3cb..889f0b33b 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -129,7 +129,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { } if opts.useWeb && userInputs.Repository == "" { - return a.browser.Browse("https://github.com/codespaces/new") + return a.browser.Browse(fmt.Sprintf("%s/codespaces/new", a.apiClient.ServerURL())) } promptForRepoAndBranch := userInputs.Repository == "" && !opts.useWeb @@ -293,7 +293,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { } if opts.useWeb { - return a.browser.Browse(fmt.Sprintf("https://github.com/codespaces/new?repo=%d&ref=%s&machine=%s&location=%s", createParams.RepositoryID, createParams.Branch, createParams.Machine, createParams.Location)) + return a.browser.Browse(fmt.Sprintf("%s/codespaces/new?repo=%d&ref=%s&machine=%s&location=%s", a.apiClient.ServerURL(), createParams.RepositoryID, createParams.Branch, createParams.Machine, createParams.Location)) } var codespace *api.Codespace diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index 7b74c206d..caf6df271 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -453,11 +453,32 @@ Alternatively, you can run "create" with the "--default-permissions" option to c }, { name: "return default url when using web flag without other flags", + fields: fields{ + apiClient: apiCreateDefaults(&apiClientMock{ + ServerURLFunc: func() string { + return "https://github.com" + }, + }), + }, opts: createOptions{ useWeb: true, }, wantURL: "https://github.com/codespaces/new", }, + { + name: "return custom server url when using web flag", + fields: fields{ + apiClient: apiCreateDefaults(&apiClientMock{ + ServerURLFunc: func() string { + return "https://github.mycompany.com" + }, + }), + }, + opts: createOptions{ + useWeb: true, + }, + wantURL: "https://github.mycompany.com/codespaces/new", + }, { name: "skip machine check when using web flag and no machine provided", fields: fields{ @@ -473,6 +494,9 @@ Alternatively, you can run "create" with the "--default-permissions" option to c Name: "monalisa-dotfiles-abcd1234", }, nil }, + ServerURLFunc: func() string { + return "https://github.com" + }, }), }, opts: createOptions{ @@ -499,6 +523,9 @@ Alternatively, you can run "create" with the "--default-permissions" option to c Name: "monalisa-dotfiles-abcd1234", }, nil }, + ServerURLFunc: func() string { + return "https://github.com" + }, }), }, opts: createOptions{ @@ -524,6 +551,9 @@ Alternatively, you can run "create" with the "--default-permissions" option to c Machine: api.CodespaceMachine{Name: "GIGA"}, }, nil }, + ServerURLFunc: func() string { + return "https://github.com" + }, }), }, opts: createOptions{ diff --git a/pkg/cmd/codespace/list.go b/pkg/cmd/codespace/list.go index bba34ff45..4775e3db5 100644 --- a/pkg/cmd/codespace/list.go +++ b/pkg/cmd/codespace/list.go @@ -79,7 +79,7 @@ func newListCmd(app *App) *cobra.Command { func (a *App) List(ctx context.Context, opts *listOptions, exporter cmdutil.Exporter) error { if opts.useWeb && opts.repo == "" { - return a.browser.Browse("https://github.com/codespaces") + return a.browser.Browse(fmt.Sprintf("%s/codespaces", a.apiClient.ServerURL())) } var codespaces []*api.Codespace @@ -113,7 +113,7 @@ func (a *App) List(ctx context.Context, opts *listOptions, exporter cmdutil.Expo } if opts.useWeb && codespaces[0].Repository.ID > 0 { - return a.browser.Browse(fmt.Sprintf("https://github.com/codespaces?repository_id=%d", codespaces[0].Repository.ID)) + return a.browser.Browse(fmt.Sprintf("%s/codespaces?repository_id=%d", a.apiClient.ServerURL(), codespaces[0].Repository.ID)) } //nolint:staticcheck // SA1019: utils.NewTablePrinter is deprecated: use internal/tableprinter diff --git a/pkg/cmd/codespace/list_test.go b/pkg/cmd/codespace/list_test.go index d99b8b97e..35f30581b 100644 --- a/pkg/cmd/codespace/list_test.go +++ b/pkg/cmd/codespace/list_test.go @@ -170,11 +170,32 @@ func TestApp_List(t *testing.T) { }, { name: "list codespaces,--web", + fields: fields{ + apiClient: &apiClientMock{ + ServerURLFunc: func() string { + return "https://github.com" + }, + }, + }, opts: &listOptions{ useWeb: true, }, wantURL: "https://github.com/codespaces", }, + { + name: "list codespaces,--web with custom server url", + fields: fields{ + apiClient: &apiClientMock{ + ServerURLFunc: func() string { + return "https://github.mycompany.com" + }, + }, + }, + opts: &listOptions{ + useWeb: true, + }, + wantURL: "https://github.mycompany.com/codespaces", + }, { name: "list codespaces,--web, --repo flag", fields: fields{ @@ -199,6 +220,9 @@ func TestApp_List(t *testing.T) { }, }, nil }, + ServerURLFunc: func() string { + return "https://github.com" + }, }, }, opts: &listOptions{ diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index 0796679fc..7e211fd7c 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -7,7 +7,7 @@ import ( "context" "sync" - "github.com/cli/cli/v2/internal/codespaces/api" + codespacesAPI "github.com/cli/cli/v2/internal/codespaces/api" ) // apiClientMock is a mock implementation of apiClient. @@ -16,43 +16,46 @@ import ( // // // make and configure a mocked apiClient // mockedapiClient := &apiClientMock{ -// CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { +// CreateCodespaceFunc: func(ctx context.Context, params *codespacesAPI.CreateCodespaceParams) (*codespacesAPI.Codespace, error) { // panic("mock out the CreateCodespace method") // }, // DeleteCodespaceFunc: func(ctx context.Context, name string, orgName string, userName string) error { // panic("mock out the DeleteCodespace method") // }, -// EditCodespaceFunc: func(ctx context.Context, codespaceName string, params *api.EditCodespaceParams) (*api.Codespace, error) { +// EditCodespaceFunc: func(ctx context.Context, codespaceName string, params *codespacesAPI.EditCodespaceParams) (*codespacesAPI.Codespace, error) { // panic("mock out the EditCodespace method") // }, -// GetCodespaceFunc: func(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) { +// GetCodespaceFunc: func(ctx context.Context, name string, includeConnection bool) (*codespacesAPI.Codespace, error) { // panic("mock out the GetCodespace method") // }, -// GetCodespaceBillableOwnerFunc: func(ctx context.Context, nwo string) (*api.User, error) { +// GetCodespaceBillableOwnerFunc: func(ctx context.Context, nwo string) (*codespacesAPI.User, error) { // panic("mock out the GetCodespaceBillableOwner method") // }, -// GetCodespaceRepoSuggestionsFunc: func(ctx context.Context, partialSearch string, params api.RepoSearchParameters) ([]string, error) { +// GetCodespaceRepoSuggestionsFunc: func(ctx context.Context, partialSearch string, params codespacesAPI.RepoSearchParameters) ([]string, error) { // panic("mock out the GetCodespaceRepoSuggestions method") // }, -// GetCodespaceRepositoryContentsFunc: func(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) { +// GetCodespaceRepositoryContentsFunc: func(ctx context.Context, codespace *codespacesAPI.Codespace, path string) ([]byte, error) { // panic("mock out the GetCodespaceRepositoryContents method") // }, -// GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*api.Machine, error) { +// GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) { // panic("mock out the GetCodespacesMachines method") // }, -// GetOrgMemberCodespaceFunc: func(ctx context.Context, orgName string, userName string, codespaceName string) (*api.Codespace, error) { +// GetOrgMemberCodespaceFunc: func(ctx context.Context, orgName string, userName string, codespaceName string) (*codespacesAPI.Codespace, error) { // panic("mock out the GetOrgMemberCodespace method") // }, -// GetRepositoryFunc: func(ctx context.Context, nwo string) (*api.Repository, error) { +// GetRepositoryFunc: func(ctx context.Context, nwo string) (*codespacesAPI.Repository, error) { // panic("mock out the GetRepository method") // }, -// GetUserFunc: func(ctx context.Context) (*api.User, error) { +// ServerURLFunc: func() string { +// panic("mock out the ServerURL method") +// }, +// GetUserFunc: func(ctx context.Context) (*codespacesAPI.User, error) { // panic("mock out the GetUser method") // }, -// ListCodespacesFunc: func(ctx context.Context, opts api.ListCodespacesOptions) ([]*api.Codespace, error) { +// ListCodespacesFunc: func(ctx context.Context, opts codespacesAPI.ListCodespacesOptions) ([]*codespacesAPI.Codespace, error) { // panic("mock out the ListCodespaces method") // }, -// ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { +// ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) { // panic("mock out the ListDevContainers method") // }, // StartCodespaceFunc: func(ctx context.Context, name string) error { @@ -69,43 +72,46 @@ import ( // } type apiClientMock struct { // CreateCodespaceFunc mocks the CreateCodespace method. - CreateCodespaceFunc func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) + CreateCodespaceFunc func(ctx context.Context, params *codespacesAPI.CreateCodespaceParams) (*codespacesAPI.Codespace, error) // DeleteCodespaceFunc mocks the DeleteCodespace method. DeleteCodespaceFunc func(ctx context.Context, name string, orgName string, userName string) error // EditCodespaceFunc mocks the EditCodespace method. - EditCodespaceFunc func(ctx context.Context, codespaceName string, params *api.EditCodespaceParams) (*api.Codespace, error) + EditCodespaceFunc func(ctx context.Context, codespaceName string, params *codespacesAPI.EditCodespaceParams) (*codespacesAPI.Codespace, error) // GetCodespaceFunc mocks the GetCodespace method. - GetCodespaceFunc func(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) + GetCodespaceFunc func(ctx context.Context, name string, includeConnection bool) (*codespacesAPI.Codespace, error) // GetCodespaceBillableOwnerFunc mocks the GetCodespaceBillableOwner method. - GetCodespaceBillableOwnerFunc func(ctx context.Context, nwo string) (*api.User, error) + GetCodespaceBillableOwnerFunc func(ctx context.Context, nwo string) (*codespacesAPI.User, error) // GetCodespaceRepoSuggestionsFunc mocks the GetCodespaceRepoSuggestions method. - GetCodespaceRepoSuggestionsFunc func(ctx context.Context, partialSearch string, params api.RepoSearchParameters) ([]string, error) + GetCodespaceRepoSuggestionsFunc func(ctx context.Context, partialSearch string, params codespacesAPI.RepoSearchParameters) ([]string, error) // GetCodespaceRepositoryContentsFunc mocks the GetCodespaceRepositoryContents method. - GetCodespaceRepositoryContentsFunc func(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) + GetCodespaceRepositoryContentsFunc func(ctx context.Context, codespace *codespacesAPI.Codespace, path string) ([]byte, error) // GetCodespacesMachinesFunc mocks the GetCodespacesMachines method. - GetCodespacesMachinesFunc func(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*api.Machine, error) + GetCodespacesMachinesFunc func(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) // GetOrgMemberCodespaceFunc mocks the GetOrgMemberCodespace method. - GetOrgMemberCodespaceFunc func(ctx context.Context, orgName string, userName string, codespaceName string) (*api.Codespace, error) + GetOrgMemberCodespaceFunc func(ctx context.Context, orgName string, userName string, codespaceName string) (*codespacesAPI.Codespace, error) // GetRepositoryFunc mocks the GetRepository method. - GetRepositoryFunc func(ctx context.Context, nwo string) (*api.Repository, error) + GetRepositoryFunc func(ctx context.Context, nwo string) (*codespacesAPI.Repository, error) + + // ServerURLFunc mocks the ServerURL method. + ServerURLFunc func() string // GetUserFunc mocks the GetUser method. - GetUserFunc func(ctx context.Context) (*api.User, error) + GetUserFunc func(ctx context.Context) (*codespacesAPI.User, error) // ListCodespacesFunc mocks the ListCodespaces method. - ListCodespacesFunc func(ctx context.Context, opts api.ListCodespacesOptions) ([]*api.Codespace, error) + ListCodespacesFunc func(ctx context.Context, opts codespacesAPI.ListCodespacesOptions) ([]*codespacesAPI.Codespace, error) // ListDevContainersFunc mocks the ListDevContainers method. - ListDevContainersFunc func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) + ListDevContainersFunc func(ctx context.Context, repoID int, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) // StartCodespaceFunc mocks the StartCodespace method. StartCodespaceFunc func(ctx context.Context, name string) error @@ -120,7 +126,7 @@ type apiClientMock struct { // Ctx is the ctx argument value. Ctx context.Context // Params is the params argument value. - Params *api.CreateCodespaceParams + Params *codespacesAPI.CreateCodespaceParams } // DeleteCodespace holds details about calls to the DeleteCodespace method. DeleteCodespace []struct { @@ -140,7 +146,7 @@ type apiClientMock struct { // CodespaceName is the codespaceName argument value. CodespaceName string // Params is the params argument value. - Params *api.EditCodespaceParams + Params *codespacesAPI.EditCodespaceParams } // GetCodespace holds details about calls to the GetCodespace method. GetCodespace []struct { @@ -165,14 +171,14 @@ type apiClientMock struct { // PartialSearch is the partialSearch argument value. PartialSearch string // Params is the params argument value. - Params api.RepoSearchParameters + Params codespacesAPI.RepoSearchParameters } // GetCodespaceRepositoryContents holds details about calls to the GetCodespaceRepositoryContents method. GetCodespaceRepositoryContents []struct { // Ctx is the ctx argument value. Ctx context.Context // Codespace is the codespace argument value. - Codespace *api.Codespace + Codespace *codespacesAPI.Codespace // Path is the path argument value. Path string } @@ -207,6 +213,9 @@ type apiClientMock struct { // Nwo is the nwo argument value. Nwo string } + // ServerURL holds details about calls to the ServerURL method. + ServerURL []struct { + } // GetUser holds details about calls to the GetUser method. GetUser []struct { // Ctx is the ctx argument value. @@ -217,7 +226,7 @@ type apiClientMock struct { // Ctx is the ctx argument value. Ctx context.Context // Opts is the opts argument value. - Opts api.ListCodespacesOptions + Opts codespacesAPI.ListCodespacesOptions } // ListDevContainers holds details about calls to the ListDevContainers method. ListDevContainers []struct { @@ -259,6 +268,7 @@ type apiClientMock struct { lockGetCodespacesMachines sync.RWMutex lockGetOrgMemberCodespace sync.RWMutex lockGetRepository sync.RWMutex + lockServerURL sync.RWMutex lockGetUser sync.RWMutex lockListCodespaces sync.RWMutex lockListDevContainers sync.RWMutex @@ -267,13 +277,13 @@ type apiClientMock struct { } // CreateCodespace calls CreateCodespaceFunc. -func (mock *apiClientMock) CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { +func (mock *apiClientMock) CreateCodespace(ctx context.Context, params *codespacesAPI.CreateCodespaceParams) (*codespacesAPI.Codespace, error) { if mock.CreateCodespaceFunc == nil { panic("apiClientMock.CreateCodespaceFunc: method is nil but apiClient.CreateCodespace was just called") } callInfo := struct { Ctx context.Context - Params *api.CreateCodespaceParams + Params *codespacesAPI.CreateCodespaceParams }{ Ctx: ctx, Params: params, @@ -290,11 +300,11 @@ func (mock *apiClientMock) CreateCodespace(ctx context.Context, params *api.Crea // len(mockedapiClient.CreateCodespaceCalls()) func (mock *apiClientMock) CreateCodespaceCalls() []struct { Ctx context.Context - Params *api.CreateCodespaceParams + Params *codespacesAPI.CreateCodespaceParams } { var calls []struct { Ctx context.Context - Params *api.CreateCodespaceParams + Params *codespacesAPI.CreateCodespaceParams } mock.lockCreateCodespace.RLock() calls = mock.calls.CreateCodespace @@ -347,14 +357,14 @@ func (mock *apiClientMock) DeleteCodespaceCalls() []struct { } // EditCodespace calls EditCodespaceFunc. -func (mock *apiClientMock) EditCodespace(ctx context.Context, codespaceName string, params *api.EditCodespaceParams) (*api.Codespace, error) { +func (mock *apiClientMock) EditCodespace(ctx context.Context, codespaceName string, params *codespacesAPI.EditCodespaceParams) (*codespacesAPI.Codespace, error) { if mock.EditCodespaceFunc == nil { panic("apiClientMock.EditCodespaceFunc: method is nil but apiClient.EditCodespace was just called") } callInfo := struct { Ctx context.Context CodespaceName string - Params *api.EditCodespaceParams + Params *codespacesAPI.EditCodespaceParams }{ Ctx: ctx, CodespaceName: codespaceName, @@ -373,12 +383,12 @@ func (mock *apiClientMock) EditCodespace(ctx context.Context, codespaceName stri func (mock *apiClientMock) EditCodespaceCalls() []struct { Ctx context.Context CodespaceName string - Params *api.EditCodespaceParams + Params *codespacesAPI.EditCodespaceParams } { var calls []struct { Ctx context.Context CodespaceName string - Params *api.EditCodespaceParams + Params *codespacesAPI.EditCodespaceParams } mock.lockEditCodespace.RLock() calls = mock.calls.EditCodespace @@ -387,7 +397,7 @@ func (mock *apiClientMock) EditCodespaceCalls() []struct { } // GetCodespace calls GetCodespaceFunc. -func (mock *apiClientMock) GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) { +func (mock *apiClientMock) GetCodespace(ctx context.Context, name string, includeConnection bool) (*codespacesAPI.Codespace, error) { if mock.GetCodespaceFunc == nil { panic("apiClientMock.GetCodespaceFunc: method is nil but apiClient.GetCodespace was just called") } @@ -427,7 +437,7 @@ func (mock *apiClientMock) GetCodespaceCalls() []struct { } // GetCodespaceBillableOwner calls GetCodespaceBillableOwnerFunc. -func (mock *apiClientMock) GetCodespaceBillableOwner(ctx context.Context, nwo string) (*api.User, error) { +func (mock *apiClientMock) GetCodespaceBillableOwner(ctx context.Context, nwo string) (*codespacesAPI.User, error) { if mock.GetCodespaceBillableOwnerFunc == nil { panic("apiClientMock.GetCodespaceBillableOwnerFunc: method is nil but apiClient.GetCodespaceBillableOwner was just called") } @@ -463,14 +473,14 @@ func (mock *apiClientMock) GetCodespaceBillableOwnerCalls() []struct { } // GetCodespaceRepoSuggestions calls GetCodespaceRepoSuggestionsFunc. -func (mock *apiClientMock) GetCodespaceRepoSuggestions(ctx context.Context, partialSearch string, params api.RepoSearchParameters) ([]string, error) { +func (mock *apiClientMock) GetCodespaceRepoSuggestions(ctx context.Context, partialSearch string, params codespacesAPI.RepoSearchParameters) ([]string, error) { if mock.GetCodespaceRepoSuggestionsFunc == nil { panic("apiClientMock.GetCodespaceRepoSuggestionsFunc: method is nil but apiClient.GetCodespaceRepoSuggestions was just called") } callInfo := struct { Ctx context.Context PartialSearch string - Params api.RepoSearchParameters + Params codespacesAPI.RepoSearchParameters }{ Ctx: ctx, PartialSearch: partialSearch, @@ -489,12 +499,12 @@ func (mock *apiClientMock) GetCodespaceRepoSuggestions(ctx context.Context, part func (mock *apiClientMock) GetCodespaceRepoSuggestionsCalls() []struct { Ctx context.Context PartialSearch string - Params api.RepoSearchParameters + Params codespacesAPI.RepoSearchParameters } { var calls []struct { Ctx context.Context PartialSearch string - Params api.RepoSearchParameters + Params codespacesAPI.RepoSearchParameters } mock.lockGetCodespaceRepoSuggestions.RLock() calls = mock.calls.GetCodespaceRepoSuggestions @@ -503,13 +513,13 @@ func (mock *apiClientMock) GetCodespaceRepoSuggestionsCalls() []struct { } // GetCodespaceRepositoryContents calls GetCodespaceRepositoryContentsFunc. -func (mock *apiClientMock) GetCodespaceRepositoryContents(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) { +func (mock *apiClientMock) GetCodespaceRepositoryContents(ctx context.Context, codespace *codespacesAPI.Codespace, path string) ([]byte, error) { if mock.GetCodespaceRepositoryContentsFunc == nil { panic("apiClientMock.GetCodespaceRepositoryContentsFunc: method is nil but apiClient.GetCodespaceRepositoryContents was just called") } callInfo := struct { Ctx context.Context - Codespace *api.Codespace + Codespace *codespacesAPI.Codespace Path string }{ Ctx: ctx, @@ -528,12 +538,12 @@ func (mock *apiClientMock) GetCodespaceRepositoryContents(ctx context.Context, c // len(mockedapiClient.GetCodespaceRepositoryContentsCalls()) func (mock *apiClientMock) GetCodespaceRepositoryContentsCalls() []struct { Ctx context.Context - Codespace *api.Codespace + Codespace *codespacesAPI.Codespace Path string } { var calls []struct { Ctx context.Context - Codespace *api.Codespace + Codespace *codespacesAPI.Codespace Path string } mock.lockGetCodespaceRepositoryContents.RLock() @@ -543,7 +553,7 @@ func (mock *apiClientMock) GetCodespaceRepositoryContentsCalls() []struct { } // GetCodespacesMachines calls GetCodespacesMachinesFunc. -func (mock *apiClientMock) GetCodespacesMachines(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*api.Machine, error) { +func (mock *apiClientMock) GetCodespacesMachines(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) { if mock.GetCodespacesMachinesFunc == nil { panic("apiClientMock.GetCodespacesMachinesFunc: method is nil but apiClient.GetCodespacesMachines was just called") } @@ -591,7 +601,7 @@ func (mock *apiClientMock) GetCodespacesMachinesCalls() []struct { } // GetOrgMemberCodespace calls GetOrgMemberCodespaceFunc. -func (mock *apiClientMock) GetOrgMemberCodespace(ctx context.Context, orgName string, userName string, codespaceName string) (*api.Codespace, error) { +func (mock *apiClientMock) GetOrgMemberCodespace(ctx context.Context, orgName string, userName string, codespaceName string) (*codespacesAPI.Codespace, error) { if mock.GetOrgMemberCodespaceFunc == nil { panic("apiClientMock.GetOrgMemberCodespaceFunc: method is nil but apiClient.GetOrgMemberCodespace was just called") } @@ -635,7 +645,7 @@ func (mock *apiClientMock) GetOrgMemberCodespaceCalls() []struct { } // GetRepository calls GetRepositoryFunc. -func (mock *apiClientMock) GetRepository(ctx context.Context, nwo string) (*api.Repository, error) { +func (mock *apiClientMock) GetRepository(ctx context.Context, nwo string) (*codespacesAPI.Repository, error) { if mock.GetRepositoryFunc == nil { panic("apiClientMock.GetRepositoryFunc: method is nil but apiClient.GetRepository was just called") } @@ -670,8 +680,35 @@ func (mock *apiClientMock) GetRepositoryCalls() []struct { return calls } +// ServerURL calls ServerURLFunc. +func (mock *apiClientMock) ServerURL() string { + if mock.ServerURLFunc == nil { + panic("apiClientMock.ServerURLFunc: method is nil but apiClient.ServerURL was just called") + } + callInfo := struct { + }{} + mock.lockServerURL.Lock() + mock.calls.ServerURL = append(mock.calls.ServerURL, callInfo) + mock.lockServerURL.Unlock() + return mock.ServerURLFunc() +} + +// ServerURLCalls gets all the calls that were made to ServerURL. +// Check the length with: +// +// len(mockedapiClient.ServerURLCalls()) +func (mock *apiClientMock) ServerURLCalls() []struct { +} { + var calls []struct { + } + mock.lockServerURL.RLock() + calls = mock.calls.ServerURL + mock.lockServerURL.RUnlock() + return calls +} + // GetUser calls GetUserFunc. -func (mock *apiClientMock) GetUser(ctx context.Context) (*api.User, error) { +func (mock *apiClientMock) GetUser(ctx context.Context) (*codespacesAPI.User, error) { if mock.GetUserFunc == nil { panic("apiClientMock.GetUserFunc: method is nil but apiClient.GetUser was just called") } @@ -703,13 +740,13 @@ func (mock *apiClientMock) GetUserCalls() []struct { } // ListCodespaces calls ListCodespacesFunc. -func (mock *apiClientMock) ListCodespaces(ctx context.Context, opts api.ListCodespacesOptions) ([]*api.Codespace, error) { +func (mock *apiClientMock) ListCodespaces(ctx context.Context, opts codespacesAPI.ListCodespacesOptions) ([]*codespacesAPI.Codespace, error) { if mock.ListCodespacesFunc == nil { panic("apiClientMock.ListCodespacesFunc: method is nil but apiClient.ListCodespaces was just called") } callInfo := struct { Ctx context.Context - Opts api.ListCodespacesOptions + Opts codespacesAPI.ListCodespacesOptions }{ Ctx: ctx, Opts: opts, @@ -726,11 +763,11 @@ func (mock *apiClientMock) ListCodespaces(ctx context.Context, opts api.ListCode // len(mockedapiClient.ListCodespacesCalls()) func (mock *apiClientMock) ListCodespacesCalls() []struct { Ctx context.Context - Opts api.ListCodespacesOptions + Opts codespacesAPI.ListCodespacesOptions } { var calls []struct { Ctx context.Context - Opts api.ListCodespacesOptions + Opts codespacesAPI.ListCodespacesOptions } mock.lockListCodespaces.RLock() calls = mock.calls.ListCodespaces @@ -739,7 +776,7 @@ func (mock *apiClientMock) ListCodespacesCalls() []struct { } // ListDevContainers calls ListDevContainersFunc. -func (mock *apiClientMock) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { +func (mock *apiClientMock) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) { if mock.ListDevContainersFunc == nil { panic("apiClientMock.ListDevContainersFunc: method is nil but apiClient.ListDevContainers was just called") } diff --git a/pkg/cmd/codespace/root.go b/pkg/cmd/codespace/root.go index 28a80edae..d1675a8f7 100644 --- a/pkg/cmd/codespace/root.go +++ b/pkg/cmd/codespace/root.go @@ -1,15 +1,28 @@ package codespace import ( + codespacesAPI "github.com/cli/cli/v2/internal/codespaces/api" + + "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" ) -func NewRootCmd(app *App) *cobra.Command { +func NewCmdCodespace(f *cmdutil.Factory) *cobra.Command { root := &cobra.Command{ - Use: "codespace", - Short: "Connect to and manage codespaces", + Use: "codespace", + Short: "Connect to and manage codespaces", + Aliases: []string{"cs"}, + GroupID: "core", } + app := NewApp( + f.IOStreams, + f, + codespacesAPI.New(f), + f.Browser, + f.Remotes, + ) + root.AddCommand(newCodeCmd(app)) root.AddCommand(newCreateCmd(app)) root.AddCommand(newEditCmd(app)) diff --git a/pkg/cmd/project/close/close_test.go b/pkg/cmd/project/close/close_test.go index a19dcdabd..96ec6ac50 100644 --- a/pkg/cmd/project/close/close_test.go +++ b/pkg/cmd/project/close/close_test.go @@ -1,7 +1,6 @@ package close import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -56,8 +55,7 @@ func TestNewCmdClose(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/copy/copy_test.go b/pkg/cmd/project/copy/copy_test.go index d8abf5ec9..3caec592d 100644 --- a/pkg/cmd/project/copy/copy_test.go +++ b/pkg/cmd/project/copy/copy_test.go @@ -1,7 +1,6 @@ package copy import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -75,8 +74,7 @@ func TestNewCmdCopy(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/create/create_test.go b/pkg/cmd/project/create/create_test.go index 3ef773cb9..74d1747b7 100644 --- a/pkg/cmd/project/create/create_test.go +++ b/pkg/cmd/project/create/create_test.go @@ -1,7 +1,6 @@ package create import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -45,8 +44,7 @@ func TestNewCmdCreate(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/delete/delete_test.go b/pkg/cmd/project/delete/delete_test.go index a27180266..589ef72bc 100644 --- a/pkg/cmd/project/delete/delete_test.go +++ b/pkg/cmd/project/delete/delete_test.go @@ -1,7 +1,6 @@ package delete import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -49,8 +48,7 @@ func TestNewCmdDelete(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/edit/edit_test.go b/pkg/cmd/project/edit/edit_test.go index 605c5f5cc..da30b0935 100644 --- a/pkg/cmd/project/edit/edit_test.go +++ b/pkg/cmd/project/edit/edit_test.go @@ -1,7 +1,6 @@ package edit import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -92,8 +91,7 @@ func TestNewCmdEdit(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/field-create/field_create_test.go b/pkg/cmd/project/field-create/field_create_test.go index 31328a04f..01904d9e9 100644 --- a/pkg/cmd/project/field-create/field_create_test.go +++ b/pkg/cmd/project/field-create/field_create_test.go @@ -1,7 +1,6 @@ package fieldcreate import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -79,8 +78,7 @@ func TestNewCmdCreateField(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/field-delete/field_delete_test.go b/pkg/cmd/project/field-delete/field_delete_test.go index 61783d44d..6b1e6c763 100644 --- a/pkg/cmd/project/field-delete/field_delete_test.go +++ b/pkg/cmd/project/field-delete/field_delete_test.go @@ -1,7 +1,6 @@ package fielddelete import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -43,8 +42,7 @@ func TestNewCmdDeleteField(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/field-list/field_list_test.go b/pkg/cmd/project/field-list/field_list_test.go index 60e61188d..c6c2f97c6 100644 --- a/pkg/cmd/project/field-list/field_list_test.go +++ b/pkg/cmd/project/field-list/field_list_test.go @@ -1,7 +1,6 @@ package fieldlist import ( - "os" "testing" "github.com/cli/cli/v2/internal/tableprinter" @@ -53,8 +52,7 @@ func TestNewCmdList(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/item-add/item_add_test.go b/pkg/cmd/project/item-add/item_add_test.go index 4f1ebd4f6..0beb1fed6 100644 --- a/pkg/cmd/project/item-add/item_add_test.go +++ b/pkg/cmd/project/item-add/item_add_test.go @@ -1,7 +1,6 @@ package itemadd import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -65,8 +64,7 @@ func TestNewCmdaddItem(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/item-archive/item_archive_test.go b/pkg/cmd/project/item-archive/item_archive_test.go index 85a34af22..6eb8336f2 100644 --- a/pkg/cmd/project/item-archive/item_archive_test.go +++ b/pkg/cmd/project/item-archive/item_archive_test.go @@ -1,7 +1,6 @@ package itemarchive import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -73,8 +72,7 @@ func TestNewCmdarchiveItem(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/item-create/item_create_test.go b/pkg/cmd/project/item-create/item_create_test.go index d49144f3c..1482fe268 100644 --- a/pkg/cmd/project/item-create/item_create_test.go +++ b/pkg/cmd/project/item-create/item_create_test.go @@ -1,7 +1,6 @@ package itemcreate import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -73,8 +72,7 @@ func TestNewCmdCreateItem(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/item-delete/item_delete_test.go b/pkg/cmd/project/item-delete/item_delete_test.go index c5a3f01fd..4d3e97059 100644 --- a/pkg/cmd/project/item-delete/item_delete_test.go +++ b/pkg/cmd/project/item-delete/item_delete_test.go @@ -1,7 +1,6 @@ package itemdelete import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -65,8 +64,7 @@ func TestNewCmdDeleteItem(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/item-edit/item_edit_test.go b/pkg/cmd/project/item-edit/item_edit_test.go index 156871f32..5cdc69b87 100644 --- a/pkg/cmd/project/item-edit/item_edit_test.go +++ b/pkg/cmd/project/item-edit/item_edit_test.go @@ -1,7 +1,6 @@ package itemedit import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -105,8 +104,7 @@ func TestNewCmdeditItem(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/item-list/item_list_test.go b/pkg/cmd/project/item-list/item_list_test.go index d618451b1..490473691 100644 --- a/pkg/cmd/project/item-list/item_list_test.go +++ b/pkg/cmd/project/item-list/item_list_test.go @@ -1,7 +1,6 @@ package itemlist import ( - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -54,8 +53,7 @@ func TestNewCmdList(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/list/list_test.go b/pkg/cmd/project/list/list_test.go index 48fb689ed..492d224b0 100644 --- a/pkg/cmd/project/list/list_test.go +++ b/pkg/cmd/project/list/list_test.go @@ -2,7 +2,6 @@ package list import ( "bytes" - "os" "testing" "github.com/cli/cli/v2/internal/tableprinter" @@ -56,8 +55,7 @@ func TestNewCmdlist(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/project/view/view_test.go b/pkg/cmd/project/view/view_test.go index 5c76c1413..35f6dfc9b 100644 --- a/pkg/cmd/project/view/view_test.go +++ b/pkg/cmd/project/view/view_test.go @@ -2,7 +2,6 @@ package view import ( "bytes" - "os" "testing" "github.com/cli/cli/v2/pkg/cmd/project/shared/queries" @@ -57,8 +56,7 @@ func TestNewCmdview(t *testing.T) { }, } - os.Setenv("GH_TOKEN", "auth-token") - defer os.Unsetenv("GH_TOKEN") + t.Setenv("GH_TOKEN", "auth-token") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index fd6378216..8c8420bee 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -5,11 +5,9 @@ import ( "net/http" "os" "strings" - "sync" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - codespacesAPI "github.com/cli/cli/v2/internal/codespaces/api" actionsCmd "github.com/cli/cli/v2/pkg/cmd/actions" aliasCmd "github.com/cli/cli/v2/pkg/cmd/alias" "github.com/cli/cli/v2/pkg/cmd/alias/shared" @@ -134,7 +132,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) (*cobra.Command, cmd.AddCommand(variableCmd.NewCmdVariable(f)) cmd.AddCommand(sshKeyCmd.NewCmdSSHKey(f)) cmd.AddCommand(statusCmd.NewCmdStatus(f, nil)) - cmd.AddCommand(newCodespaceCmd(f)) + cmd.AddCommand(codespaceCmd.NewCmdCodespace(f)) cmd.AddCommand(projectCmd.NewCmdProject(f)) // below here at the commands that require the "intelligent" BaseRepo resolver @@ -241,51 +239,3 @@ func bareHTTPClient(f *cmdutil.Factory, version string) func() (*http.Client, er return api.NewHTTPClient(opts) } } - -func newCodespaceCmd(f *cmdutil.Factory) *cobra.Command { - serverURL := os.Getenv("GITHUB_SERVER_URL") - apiURL := os.Getenv("GITHUB_API_URL") - vscsURL := os.Getenv("INTERNAL_VSCS_TARGET_URL") - app := codespaceCmd.NewApp( - f.IOStreams, - f, - codespacesAPI.New( - serverURL, - apiURL, - vscsURL, - &lazyLoadedHTTPClient{factory: f}, - ), - f.Browser, - f.Remotes, - ) - cmd := codespaceCmd.NewRootCmd(app) - cmd.Use = "codespace" - cmd.Aliases = []string{"cs"} - cmd.GroupID = "core" - return cmd -} - -type lazyLoadedHTTPClient struct { - factory *cmdutil.Factory - - httpClientMu sync.RWMutex // guards httpClient - httpClient *http.Client -} - -func (l *lazyLoadedHTTPClient) Do(req *http.Request) (*http.Response, error) { - l.httpClientMu.RLock() - httpClient := l.httpClient - l.httpClientMu.RUnlock() - - if httpClient == nil { - var err error - l.httpClientMu.Lock() - l.httpClient, err = l.factory.HttpClient() - l.httpClientMu.Unlock() - if err != nil { - return nil, err - } - } - - return l.httpClient.Do(req) -} From bd12522cb2d932b5b869bf815b8c57a975dd8541 Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Tue, 8 Aug 2023 15:37:58 -0700 Subject: [PATCH 07/23] finish porting existing tests --- pkg/cmd/repo/edit/edit.go | 15 +++++---------- pkg/cmd/repo/edit/edit_test.go | 31 ++++++++++++++++--------------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index 4841362e7..58e32a262 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -458,24 +458,19 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { } opts.Edits.EnableAutoMerge = &r.AutoMergeAllowed - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: "Enable Auto Merge?", - Default: r.AutoMergeAllowed, - }, opts.Edits.EnableAutoMerge) + c, err := opts.Prompter.Confirm("Enable Auto Merge?", r.AutoMergeAllowed) if err != nil { return err } + opts.Edits.EnableAutoMerge = &c opts.Edits.DeleteBranchOnMerge = &r.DeleteBranchOnMerge - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: "Automatically delete head branches after merging?", - Default: r.DeleteBranchOnMerge, - }, opts.Edits.DeleteBranchOnMerge) + c, err = opts.Prompter.Confirm( + "Automatically delete head branches after merging?", r.DeleteBranchOnMerge) if err != nil { return err } + opts.Edits.DeleteBranchOnMerge = &c case optionTemplateRepo: opts.Edits.IsTemplate = &r.IsTemplate //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go index 98f1cb8cc..198b2fabd 100644 --- a/pkg/cmd/repo/edit/edit_test.go +++ b/pkg/cmd/repo/edit/edit_test.go @@ -7,8 +7,6 @@ import ( "net/http" "testing" - "github.com/cli/cli/v2/pkg/prompt" - "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmdutil" @@ -193,7 +191,6 @@ func Test_editRun_interactive(t *testing.T) { tests := []struct { name string opts EditOptions - askStubs func(*prompt.AskStubber) promptStubs func(*prompter.MockPrompter) httpStubs func(*testing.T, *httpmock.Registry) wantsStderr string @@ -205,7 +202,6 @@ func Test_editRun_interactive(t *testing.T) { Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), InteractiveMode: true, }, - askStubs: func(as *prompt.AskStubber) {}, promptStubs: func(pm *prompter.MockPrompter) { pm.RegisterMultiSelect("What do you want to edit?", nil, editList, func(_ string, _, opts []string) ([]int, error) { @@ -252,7 +248,6 @@ func Test_editRun_interactive(t *testing.T) { Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), InteractiveMode: true, }, - askStubs: func(as *prompt.AskStubber) {}, promptStubs: func(pm *prompter.MockPrompter) { pm.RegisterMultiSelect("What do you want to edit?", nil, editList, func(_ string, _, opts []string) ([]int, error) { @@ -312,11 +307,22 @@ func Test_editRun_interactive(t *testing.T) { Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), InteractiveMode: true, }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("What do you want to edit?").AnswerWith([]string{"Merge Options"}) - as.StubPrompt("Allowed merge strategies").AnswerWith([]string{allowMergeCommits, allowRebaseMerge}) - as.StubPrompt("Enable Auto Merge?").AnswerWith(false) - as.StubPrompt("Automatically delete head branches after merging?").AnswerWith(false) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterMultiSelect("What do you want to edit?", nil, editList, + func(_ string, _, opts []string) ([]int, error) { + return []int{4}, nil + }) + pm.RegisterMultiSelect("Allowed merge strategies", nil, + []string{allowMergeCommits, allowSquashMerge, allowRebaseMerge}, + func(_ string, _, opts []string) ([]int, error) { + return []int{0, 2}, nil + }) + pm.RegisterConfirm("Enable Auto Merge?", func(_ string, _ bool) (bool, error) { + return false, nil + }) + pm.RegisterConfirm("Automatically delete head branches after merging?", func(_ string, _ bool) (bool, error) { + return false, nil + }) }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( @@ -378,11 +384,6 @@ func Test_editRun_interactive(t *testing.T) { opts := &tt.opts opts.HTTPClient = &http.Client{Transport: httpReg} opts.IO = ios - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - if tt.askStubs != nil { - tt.askStubs(as) - } err := editRun(context.Background(), opts) if tt.wantsErr == "" { From 8927040028c1aba8daa3c294bf39f58d008bcd54 Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Tue, 8 Aug 2023 15:58:16 -0700 Subject: [PATCH 08/23] remove dead code --- pkg/cmd/repo/edit/edit.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index 58e32a262..5731302c8 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -394,16 +394,6 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { if err != nil { return err } - case optionDiscussions: - opts.Edits.EnableDiscussions = &r.HasDiscussionsEnabled - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: "Enable Discussions?", - Default: r.HasDiscussionsEnabled, - }, opts.Edits.EnableDiscussions) - if err != nil { - return err - } case optionVisibility: opts.Edits.Visibility = &r.Visibility visibilityOptions := []string{"public", "private", "internal"} From c1296194ee5c921da7d2d462e0b2faefe12cff0c Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Tue, 8 Aug 2023 17:28:06 -0700 Subject: [PATCH 09/23] port more prompts and cover with new tests --- pkg/cmd/repo/edit/edit.go | 43 ++++++------------- pkg/cmd/repo/edit/edit_test.go | 78 +++++++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 32 deletions(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index 5731302c8..26d1ce380 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -319,6 +319,7 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { } for _, c := range choices { switch c { + // TODO these initial assignments are no-ops; delete them case optionDescription: opts.Edits.Description = &r.Description answer, err := opts.Prompter.Input("Description of the repository", r.Description) @@ -328,14 +329,11 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { opts.Edits.Description = &answer case optionHomePageURL: opts.Edits.Homepage = &r.HomepageURL - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Input{ - Message: "Repository home page URL", - Default: r.HomepageURL, - }, opts.Edits.Homepage) + a, err := opts.Prompter.Input("Repository home page URL", r.HomepageURL) if err != nil { return err } + opts.Edits.Homepage = &a case optionTopics: addTopics, err := opts.Prompter.Input("Add topics?(csv format)", "") if err != nil { @@ -356,44 +354,32 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { } case optionDefaultBranchName: opts.Edits.DefaultBranch = &r.DefaultBranchRef.Name - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Input{ - Message: "Default branch name", - Default: r.DefaultBranchRef.Name, - }, opts.Edits.DefaultBranch) + name, err := opts.Prompter.Input("Default branch name", r.DefaultBranchRef.Name) if err != nil { return err } + opts.Edits.DefaultBranch = &name case optionWikis: opts.Edits.EnableWiki = &r.HasWikiEnabled - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: "Enable Wikis?", - Default: r.HasWikiEnabled, - }, opts.Edits.EnableWiki) + c, err := opts.Prompter.Confirm("Enable Wikis?", r.HasWikiEnabled) if err != nil { return err } + opts.Edits.EnableWiki = &c case optionIssues: opts.Edits.EnableIssues = &r.HasIssuesEnabled - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: "Enable Issues?", - Default: r.HasIssuesEnabled, - }, opts.Edits.EnableIssues) + a, err := opts.Prompter.Confirm("Enable Issues?", r.HasIssuesEnabled) if err != nil { return err } + opts.Edits.EnableIssues = &a case optionProjects: opts.Edits.EnableProjects = &r.HasProjectsEnabled - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: "Enable Projects?", - Default: r.HasProjectsEnabled, - }, opts.Edits.EnableProjects) + a, err := opts.Prompter.Confirm("Enable Projects?", r.HasProjectsEnabled) if err != nil { return err } + opts.Edits.EnableProjects = &a case optionVisibility: opts.Edits.Visibility = &r.Visibility visibilityOptions := []string{"public", "private", "internal"} @@ -463,14 +449,11 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { opts.Edits.DeleteBranchOnMerge = &c case optionTemplateRepo: opts.Edits.IsTemplate = &r.IsTemplate - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: "Convert into a template repository?", - Default: r.IsTemplate, - }, opts.Edits.IsTemplate) + c, err := opts.Prompter.Confirm("Convert into a template repository?", r.IsTemplate) if err != nil { return err } + opts.Edits.IsTemplate = &c case optionAllowForking: opts.Edits.AllowForking = &r.ForkingAllowed //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go index 198b2fabd..0c6a6d2f6 100644 --- a/pkg/cmd/repo/edit/edit_test.go +++ b/pkg/cmd/repo/edit/edit_test.go @@ -173,8 +173,6 @@ func Test_editRun(t *testing.T) { } } -// TODO consider unit test for interactiveRepoEdit that exercises every prompt type - func Test_editRun_interactive(t *testing.T) { editList := []string{ "Default Branch Name", @@ -196,6 +194,82 @@ func Test_editRun_interactive(t *testing.T) { wantsStderr string wantsErr string }{ + // TODO forking of an org repo + { + name: "the rest", + opts: EditOptions{ + Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), + InteractiveMode: true, + }, + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterMultiSelect("What do you want to edit?", nil, editList, + func(_ string, _, opts []string) ([]int, error) { + return []int{0, 2, 3, 5, 6, 8, 9}, nil + }) + pm.RegisterInput("Default branch name", func(_, _ string) (string, error) { + return "trunk", nil + }) + pm.RegisterInput("Repository home page URL", func(_, _ string) (string, error) { + return "https://zombo.com", nil + }) + pm.RegisterConfirm("Enable Issues?", func(_ string, _ bool) (bool, error) { + return true, nil + }) + pm.RegisterConfirm("Enable Projects?", func(_ string, _ bool) (bool, error) { + return true, nil + }) + pm.RegisterConfirm("Convert into a template repository?", func(_ string, _ bool) (bool, error) { + return true, nil + }) + pm.RegisterSelect("Visibility", []string{"public", "private", "internal"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "private") + }) + pm.RegisterConfirm("Do you want to change visibility to private?", func(_ string, _ bool) (bool, error) { + return true, nil + }) + pm.RegisterConfirm("Enable Wikis?", func(_ string, _ bool) (bool, error) { + return true, nil + }) + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { + "data": { + "repository": { + "visibility": "public", + "description": "description", + "homePageUrl": "https://url.com", + "defaultBranchRef": { + "name": "main" + }, + "stargazerCount": 10, + "isInOrganization": false, + "repositoryTopics": { + "nodes": [{ + "topic": { + "name": "x" + } + }] + } + } + } + }`)) + reg.Register( + httpmock.REST("PATCH", "repos/OWNER/REPO"), + httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) { + assert.Equal(t, "trunk", payload["default_branch"]) + assert.Equal(t, "https://zombo.com", payload["homepage"]) + assert.Equal(t, true, payload["has_issues"]) + assert.Equal(t, true, payload["has_projects"]) + assert.Equal(t, "private", payload["visibility"]) + assert.Equal(t, true, payload["is_template"]) + assert.Equal(t, true, payload["has_wiki"]) + })) + }, + }, { name: "updates repo description", opts: EditOptions{ From 3e2f09d97046ea808d7fc05df1a164973db4996f Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Tue, 8 Aug 2023 17:35:35 -0700 Subject: [PATCH 10/23] strip more dead code --- pkg/cmd/repo/edit/edit.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index 26d1ce380..fb3d87bea 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -319,16 +319,13 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { } for _, c := range choices { switch c { - // TODO these initial assignments are no-ops; delete them case optionDescription: - opts.Edits.Description = &r.Description answer, err := opts.Prompter.Input("Description of the repository", r.Description) if err != nil { return err } opts.Edits.Description = &answer case optionHomePageURL: - opts.Edits.Homepage = &r.HomepageURL a, err := opts.Prompter.Input("Repository home page URL", r.HomepageURL) if err != nil { return err @@ -353,35 +350,30 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { } } case optionDefaultBranchName: - opts.Edits.DefaultBranch = &r.DefaultBranchRef.Name name, err := opts.Prompter.Input("Default branch name", r.DefaultBranchRef.Name) if err != nil { return err } opts.Edits.DefaultBranch = &name case optionWikis: - opts.Edits.EnableWiki = &r.HasWikiEnabled c, err := opts.Prompter.Confirm("Enable Wikis?", r.HasWikiEnabled) if err != nil { return err } opts.Edits.EnableWiki = &c case optionIssues: - opts.Edits.EnableIssues = &r.HasIssuesEnabled a, err := opts.Prompter.Confirm("Enable Issues?", r.HasIssuesEnabled) if err != nil { return err } opts.Edits.EnableIssues = &a case optionProjects: - opts.Edits.EnableProjects = &r.HasProjectsEnabled a, err := opts.Prompter.Confirm("Enable Projects?", r.HasProjectsEnabled) if err != nil { return err } opts.Edits.EnableProjects = &a case optionVisibility: - opts.Edits.Visibility = &r.Visibility visibilityOptions := []string{"public", "private", "internal"} selected, err := opts.Prompter.Select("Visibility", strings.ToLower(r.Visibility), visibilityOptions) if err != nil { @@ -448,7 +440,6 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { } opts.Edits.DeleteBranchOnMerge = &c case optionTemplateRepo: - opts.Edits.IsTemplate = &r.IsTemplate c, err := opts.Prompter.Confirm("Convert into a template repository?", r.IsTemplate) if err != nil { return err From 480505511ce7d1775f7ce23ea23db28514461561 Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Tue, 8 Aug 2023 17:53:21 -0700 Subject: [PATCH 11/23] test for forking in org --- pkg/cmd/repo/edit/edit.go | 12 +++------ pkg/cmd/repo/edit/edit_test.go | 48 +++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index fb3d87bea..cb11e2622 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -10,7 +10,6 @@ import ( "strings" "time" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" fd "github.com/cli/cli/v2/internal/featuredetection" @@ -19,7 +18,6 @@ import ( "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/cli/cli/v2/pkg/set" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" @@ -446,15 +444,13 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { } opts.Edits.IsTemplate = &c case optionAllowForking: - opts.Edits.AllowForking = &r.ForkingAllowed - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: "Allow forking (of an organization repository)?", - Default: r.ForkingAllowed, - }, opts.Edits.AllowForking) + c, err := opts.Prompter.Confirm( + "Allow forking (of an organization repository)?", + r.ForkingAllowed) if err != nil { return err } + opts.Edits.AllowForking = &c } } return nil diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go index 0c6a6d2f6..07f7d4d55 100644 --- a/pkg/cmd/repo/edit/edit_test.go +++ b/pkg/cmd/repo/edit/edit_test.go @@ -194,7 +194,53 @@ func Test_editRun_interactive(t *testing.T) { wantsStderr string wantsErr string }{ - // TODO forking of an org repo + { + name: "forking of org repo", + opts: EditOptions{ + Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), + InteractiveMode: true, + }, + promptStubs: func(pm *prompter.MockPrompter) { + el := append(editList, optionAllowForking) + pm.RegisterMultiSelect("What do you want to edit?", nil, el, + func(_ string, _, opts []string) ([]int, error) { + return []int{10}, nil + }) + pm.RegisterConfirm("Allow forking (of an organization repository)?", func(_ string, _ bool) (bool, error) { + return true, nil + }) + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { + "data": { + "repository": { + "visibility": "public", + "description": "description", + "homePageUrl": "https://url.com", + "defaultBranchRef": { + "name": "main" + }, + "isInOrganization": true, + "repositoryTopics": { + "nodes": [{ + "topic": { + "name": "x" + } + }] + } + } + } + }`)) + reg.Register( + httpmock.REST("PATCH", "repos/OWNER/REPO"), + httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) { + assert.Equal(t, true, payload["allow_forking"]) + })) + }, + }, { name: "the rest", opts: EditOptions{ From 8ed632afb9ea1164b75d42c3b5754f7ea0a7e65e Mon Sep 17 00:00:00 2001 From: vaindil Date: Thu, 10 Aug 2023 11:51:48 -0400 Subject: [PATCH 12/23] Allow --org parameter in lieu of a repo context for rulesets, add current_user_can_bypass to rs view (#7747) --- pkg/cmd/ruleset/list/list.go | 12 +++- pkg/cmd/ruleset/list/list_test.go | 55 +++++++++++++++++-- pkg/cmd/ruleset/shared/shared.go | 11 ++-- .../view/fixtures/rulesetViewRepo.json | 1 + pkg/cmd/ruleset/view/view.go | 16 +++++- pkg/cmd/ruleset/view/view_test.go | 55 ++++++++++++++++++- 6 files changed, 132 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 60afc69ab..6d13441ea 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -97,9 +97,15 @@ func listRun(opts *ListOptions) error { return err } - repoI, err := opts.BaseRepo() - if err != nil { - return err + var repoI ghrepo.Interface + + // only one of the repo or org context is necessary + if opts.Organization == "" { + var repoErr error + repoI, repoErr = opts.BaseRepo() + if repoErr != nil { + return repoErr + } } hostname, _ := ghAuth.DefaultHost() diff --git a/pkg/cmd/ruleset/list/list_test.go b/pkg/cmd/ruleset/list/list_test.go index d075e46d6..1c370fcfb 100644 --- a/pkg/cmd/ruleset/list/list_test.go +++ b/pkg/cmd/ruleset/list/list_test.go @@ -9,6 +9,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" @@ -171,11 +172,12 @@ func Test_listRun(t *testing.T) { name: "list org rulesets", isTTY: true, opts: ListOptions{ - Organization: "my-org", + IncludeParents: true, + Organization: "my-org", }, wantStdout: heredoc.Doc(` - Showing 3 of 3 rulesets in my-org + Showing 3 of 3 rulesets in my-org and its parents ID NAME SOURCE STATUS RULES 4 test OWNER/REPO (repo) evaluate 1 @@ -191,6 +193,45 @@ func Test_listRun(t *testing.T) { wantStderr: "", wantBrowse: "", }, + { + name: "list repo rulesets, no rulesets", + isTTY: true, + opts: ListOptions{ + IncludeParents: true, + }, + wantStdout: "", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepoRulesetList\b`), + httpmock.JSONResponse(shared.RulesetList{ + TotalCount: 0, + Rulesets: []shared.RulesetGraphQL{}, + }), + ) + }, + wantErr: "no rulesets found in OWNER/REPO or its parents", + wantStderr: "", + wantBrowse: "", + }, + { + name: "list org rulesets, no rulesets", + isTTY: true, + opts: ListOptions{ + Organization: "my-org", + }, + wantStdout: "", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query OrgRulesetList\b`), + httpmock.JSONResponse(shared.RulesetList{ + TotalCount: 0, + Rulesets: []shared.RulesetGraphQL{}, + }), + ) + }, + wantErr: "no rulesets found in my-org", + wantBrowse: "", + }, { name: "machine-readable", isTTY: false, @@ -269,9 +310,15 @@ func Test_listRun(t *testing.T) { tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } - tt.opts.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.FromFullName("OWNER/REPO") + + // only set this if org is not set, because the repo isn't needed if --org is provided and + // leaving it undefined will catch potential errors + if tt.opts.Organization == "" { + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("OWNER/REPO") + } } + browser := &browser.Stub{} tt.opts.Browser = browser diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index a71b20522..3ec1c83ee 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -24,11 +24,12 @@ type RulesetGraphQL struct { } type RulesetREST struct { - Id int - Name string - Target string - Enforcement string - BypassActors []struct { + Id int + Name string + Target string + Enforcement string + CurrentUserCanBypass string `json:"current_user_can_bypass"` + BypassActors []struct { ActorId int `json:"actor_id"` ActorType string `json:"actor_type"` BypassMode string `json:"bypass_mode"` diff --git a/pkg/cmd/ruleset/view/fixtures/rulesetViewRepo.json b/pkg/cmd/ruleset/view/fixtures/rulesetViewRepo.json index 5824b341d..53e6489a5 100644 --- a/pkg/cmd/ruleset/view/fixtures/rulesetViewRepo.json +++ b/pkg/cmd/ruleset/view/fixtures/rulesetViewRepo.json @@ -5,6 +5,7 @@ "source_type": "Repository", "source": "my-owner/repo-name", "enforcement": "active", + "current_user_can_bypass": "pull_requests_only", "conditions": { "ref_name": { "exclude": [], diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index 813f72c51..0b79302e5 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -113,9 +113,15 @@ func viewRun(opts *ViewOptions) error { return err } - repoI, err := opts.BaseRepo() - if err != nil { - return err + var repoI ghrepo.Interface + + // only one of the repo or org context is necessary + if opts.Organization == "" { + var repoErr error + repoI, repoErr = opts.BaseRepo() + if repoErr != nil { + return repoErr + } } hostname, _ := ghAuth.DefaultHost() @@ -194,6 +200,10 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintf(w, "%s\n", rs.Enforcement) } + if rs.CurrentUserCanBypass != "" { + fmt.Fprintf(w, "You can bypass: %s\n", strings.ReplaceAll(rs.CurrentUserCanBypass, "_", " ")) + } + fmt.Fprintf(w, "\n%s\n", cs.Bold("Bypass List")) if len(rs.BypassActors) == 0 { fmt.Fprintf(w, "This ruleset cannot be bypassed\n") diff --git a/pkg/cmd/ruleset/view/view_test.go b/pkg/cmd/ruleset/view/view_test.go index ce9c21db4..5e2846441 100644 --- a/pkg/cmd/ruleset/view/view_test.go +++ b/pkg/cmd/ruleset/view/view_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" @@ -156,6 +157,7 @@ func Test_viewRun(t *testing.T) { ID: 42 Source: my-owner/repo-name (Repository) Enforcement: Active + You can bypass: pull requests only Bypass List - OrganizationAdmin (ID: 1), mode: always @@ -233,7 +235,48 @@ func Test_viewRun(t *testing.T) { wantBrowse: "", }, { - name: "prompter", + name: "interactive mode, repo, no rulesets found", + isTTY: true, + opts: ViewOptions{ + InteractiveMode: true, + }, + wantStdout: "", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepoRulesetList\b`), + httpmock.JSONResponse(shared.RulesetList{ + TotalCount: 0, + Rulesets: []shared.RulesetGraphQL{}, + }), + ) + }, + wantErr: "no rulesets found in my-owner/repo-name", + wantStderr: "", + wantBrowse: "", + }, + { + name: "interactive mode, org, no rulesets found", + isTTY: true, + opts: ViewOptions{ + InteractiveMode: true, + Organization: "my-owner", + }, + wantStdout: "", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query OrgRulesetList\b`), + httpmock.JSONResponse(shared.RulesetList{ + TotalCount: 0, + Rulesets: []shared.RulesetGraphQL{}, + }), + ) + }, + wantErr: "no rulesets found in my-owner", + wantStderr: "", + wantBrowse: "", + }, + { + name: "interactive mode, prompter", isTTY: true, opts: ViewOptions{ InteractiveMode: true, @@ -338,9 +381,15 @@ func Test_viewRun(t *testing.T) { tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } - tt.opts.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.FromFullName("my-owner/repo-name") + + // only set this if org is not set, because the repo isn't needed if --org is provided and + // leaving it undefined will catch potential errors + if tt.opts.Organization == "" { + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("my-owner/repo-name") + } } + browser := &browser.Stub{} tt.opts.Browser = browser From aa231319ca7b06b0c4cabe3015e5e75af21b658a Mon Sep 17 00:00:00 2001 From: cawfeecake <48775802+cawfeecake@users.noreply.github.com> Date: Thu, 10 Aug 2023 09:08:15 -0700 Subject: [PATCH 13/23] Add missing "ls" aliases to list commands (#7818) --- pkg/cmd/cache/list/list.go | 3 ++- pkg/cmd/org/list/list.go | 1 + pkg/cmd/project/list/list.go | 3 ++- pkg/cmd/ruleset/list/list.go | 3 ++- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/cache/list/list.go b/pkg/cmd/cache/list/list.go index 9d16a9323..d2f4aa10d 100644 --- a/pkg/cmd/cache/list/list.go +++ b/pkg/cmd/cache/list/list.go @@ -47,7 +47,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman # List caches sorted by least recently accessed $ gh cache list --sort last_accessed_at --order asc `), - Args: cobra.NoArgs, + Aliases: []string{"ls"}, + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo diff --git a/pkg/cmd/org/list/list.go b/pkg/cmd/org/list/list.go index c30dc8b1a..54e5daf5a 100644 --- a/pkg/cmd/org/list/list.go +++ b/pkg/cmd/org/list/list.go @@ -39,6 +39,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman # List more organizations $ gh org list --limit 100 `), + Aliases: []string{"ls"}, RunE: func(cmd *cobra.Command, args []string) error { if opts.Limit < 1 { return cmdutil.FlagErrorf("invalid limit: %v", opts.Limit) diff --git a/pkg/cmd/project/list/list.go b/pkg/cmd/project/list/list.go index 661cbcd10..c92434522 100644 --- a/pkg/cmd/project/list/list.go +++ b/pkg/cmd/project/list/list.go @@ -33,8 +33,8 @@ type listConfig struct { func NewCmdList(f *cmdutil.Factory, runF func(config listConfig) error) *cobra.Command { opts := listOpts{} listCmd := &cobra.Command{ - Short: "List the projects for an owner", Use: "list", + Short: "List the projects for an owner", Example: heredoc.Doc(` # list the current user's projects gh project list @@ -42,6 +42,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(config listConfig) error) *cobra.C # list the projects for org github including closed projects gh project list --owner github --closed `), + Aliases: []string{"ls"}, RunE: func(cmd *cobra.Command, args []string) error { client, err := client.New(f) if err != nil { diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 6d13441ea..c88f2d1e0 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -62,7 +62,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman # List rulesets in an organization $ gh ruleset list --org org-name `), - Args: cobra.ExactArgs(0), + Aliases: []string{"ls"}, + Args: cobra.ExactArgs(0), RunE: func(cmd *cobra.Command, args []string) error { if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" && opts.Organization != "" { return cmdutil.FlagErrorf("only one of --repo and --org may be specified") From f5477d931b746d60709e2097d5e96c5308a61168 Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Thu, 10 Aug 2023 17:34:17 -0700 Subject: [PATCH 14/23] pass prompter around instead of opts --- pkg/cmd/repo/edit/edit.go | 47 ++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index cb11e2622..112aa5131 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -15,7 +15,6 @@ import ( fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" - "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/pkg/set" @@ -23,6 +22,13 @@ import ( "golang.org/x/sync/errgroup" ) +type iprompter interface { + MultiSelect(prompt string, defaults []string, options []string) ([]int, error) + Input(string, string) (string, error) + Confirm(string, bool) (bool, error) + Select(string, string, []string) (int, error) +} + const ( allowMergeCommits = "Allow Merge Commits" allowSquashMerge = "Allow Squash Merging" @@ -51,7 +57,7 @@ type EditOptions struct { RemoveTopics []string InteractiveMode bool Detector fd.Detector - Prompter prompter.Prompter + Prompter iprompter // Cache of current repo topics to avoid retrieving them // in multiple flows. topicsCache []string @@ -277,7 +283,7 @@ func editRun(ctx context.Context, opts *EditOptions) error { return nil } -func interactiveChoice(opts EditOptions, r *api.Repository) ([]string, error) { +func interactiveChoice(p iprompter, r *api.Repository) ([]string, error) { options := []string{ optionDefaultBranchName, optionDescription, @@ -296,7 +302,7 @@ func interactiveChoice(opts EditOptions, r *api.Repository) ([]string, error) { options = append(options, optionAllowForking) } var answers []string - selected, err := opts.Prompter.MultiSelect("What do you want to edit?", nil, options) + selected, err := p.MultiSelect("What do you want to edit?", nil, options) if err != nil { return nil, err } @@ -311,26 +317,27 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { for _, v := range r.RepositoryTopics.Nodes { opts.topicsCache = append(opts.topicsCache, v.Topic.Name) } - choices, err := interactiveChoice(*opts, r) + p := opts.Prompter + choices, err := interactiveChoice(p, r) if err != nil { return err } for _, c := range choices { switch c { case optionDescription: - answer, err := opts.Prompter.Input("Description of the repository", r.Description) + answer, err := p.Input("Description of the repository", r.Description) if err != nil { return err } opts.Edits.Description = &answer case optionHomePageURL: - a, err := opts.Prompter.Input("Repository home page URL", r.HomepageURL) + a, err := p.Input("Repository home page URL", r.HomepageURL) if err != nil { return err } opts.Edits.Homepage = &a case optionTopics: - addTopics, err := opts.Prompter.Input("Add topics?(csv format)", "") + addTopics, err := p.Input("Add topics?(csv format)", "") if err != nil { return err } @@ -339,7 +346,7 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { } if len(opts.topicsCache) > 0 { - selected, err := opts.Prompter.MultiSelect("Remove Topics", nil, opts.topicsCache) + selected, err := p.MultiSelect("Remove Topics", nil, opts.topicsCache) if err != nil { return err } @@ -348,32 +355,32 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { } } case optionDefaultBranchName: - name, err := opts.Prompter.Input("Default branch name", r.DefaultBranchRef.Name) + name, err := p.Input("Default branch name", r.DefaultBranchRef.Name) if err != nil { return err } opts.Edits.DefaultBranch = &name case optionWikis: - c, err := opts.Prompter.Confirm("Enable Wikis?", r.HasWikiEnabled) + c, err := p.Confirm("Enable Wikis?", r.HasWikiEnabled) if err != nil { return err } opts.Edits.EnableWiki = &c case optionIssues: - a, err := opts.Prompter.Confirm("Enable Issues?", r.HasIssuesEnabled) + a, err := p.Confirm("Enable Issues?", r.HasIssuesEnabled) if err != nil { return err } opts.Edits.EnableIssues = &a case optionProjects: - a, err := opts.Prompter.Confirm("Enable Projects?", r.HasProjectsEnabled) + a, err := p.Confirm("Enable Projects?", r.HasProjectsEnabled) if err != nil { return err } opts.Edits.EnableProjects = &a case optionVisibility: visibilityOptions := []string{"public", "private", "internal"} - selected, err := opts.Prompter.Select("Visibility", strings.ToLower(r.Visibility), visibilityOptions) + selected, err := p.Select("Visibility", strings.ToLower(r.Visibility), visibilityOptions) if err != nil { return err } @@ -382,7 +389,7 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { (r.StargazerCount > 0 || r.Watchers.TotalCount > 0) { cs := opts.IO.ColorScheme() fmt.Fprintf(opts.IO.ErrOut, "%s Changing the repository visibility to private will cause permanent loss of stars and watchers.\n", cs.WarningIcon()) - confirmed, err = opts.Prompter.Confirm("Do you want to change visibility to private?", false) + confirmed, err = p.Confirm("Do you want to change visibility to private?", false) if err != nil { return err } @@ -403,7 +410,7 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { defaultMergeOptions = append(defaultMergeOptions, allowRebaseMerge) } mergeOpts := []string{allowMergeCommits, allowSquashMerge, allowRebaseMerge} - selected, err := opts.Prompter.MultiSelect( + selected, err := p.MultiSelect( "Allowed merge strategies", defaultMergeOptions, mergeOpts) @@ -424,27 +431,27 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { } opts.Edits.EnableAutoMerge = &r.AutoMergeAllowed - c, err := opts.Prompter.Confirm("Enable Auto Merge?", r.AutoMergeAllowed) + c, err := p.Confirm("Enable Auto Merge?", r.AutoMergeAllowed) if err != nil { return err } opts.Edits.EnableAutoMerge = &c opts.Edits.DeleteBranchOnMerge = &r.DeleteBranchOnMerge - c, err = opts.Prompter.Confirm( + c, err = p.Confirm( "Automatically delete head branches after merging?", r.DeleteBranchOnMerge) if err != nil { return err } opts.Edits.DeleteBranchOnMerge = &c case optionTemplateRepo: - c, err := opts.Prompter.Confirm("Convert into a template repository?", r.IsTemplate) + c, err := p.Confirm("Convert into a template repository?", r.IsTemplate) if err != nil { return err } opts.Edits.IsTemplate = &c case optionAllowForking: - c, err := opts.Prompter.Confirm( + c, err := p.Confirm( "Allow forking (of an organization repository)?", r.ForkingAllowed) if err != nil { From c5eb188698a9a7234dca03ba91bb98b5674d46ad Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Tue, 15 Aug 2023 23:58:45 +0900 Subject: [PATCH 15/23] Add clobber flag to `alias set` (#7787) --- pkg/cmd/alias/set/set.go | 57 ++-- pkg/cmd/alias/set/set_test.go | 561 +++++++++++++++++----------------- 2 files changed, 317 insertions(+), 301 deletions(-) diff --git a/pkg/cmd/alias/set/set.go b/pkg/cmd/alias/set/set.go index 20c86c473..77de9ca5d 100644 --- a/pkg/cmd/alias/set/set.go +++ b/pkg/cmd/alias/set/set.go @@ -17,9 +17,10 @@ type SetOptions struct { Config func() (config.Config, error) IO *iostreams.IOStreams - Name string - Expansion string - IsShell bool + Name string + Expansion string + IsShell bool + OverwriteExisting bool validAliasName func(string) bool validAliasExpansion func(string) bool @@ -86,6 +87,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command } cmd.Flags().BoolVarP(&opts.IsShell, "shell", "s", false, "Declare an alias to be passed through a shell interpreter") + cmd.Flags().BoolVar(&opts.OverwriteExisting, "clobber", false, "Overwrite existing aliases of the same name") return cmd } @@ -104,31 +106,39 @@ func setRun(opts *SetOptions) error { return fmt.Errorf("did not understand expansion: %w", err) } - isTerminal := opts.IO.IsStdoutTTY() - if isTerminal { - fmt.Fprintf(opts.IO.ErrOut, "- Adding alias for %s: %s\n", cs.Bold(opts.Name), cs.Bold(expansion)) - } - if opts.IsShell && !strings.HasPrefix(expansion, "!") { expansion = "!" + expansion } + isTerminal := opts.IO.IsStdoutTTY() + if isTerminal { + fmt.Fprintf(opts.IO.ErrOut, "- Creating alias for %s: %s\n", cs.Bold(opts.Name), cs.Bold(expansion)) + } + + var existingAlias bool + if _, err := aliasCfg.Get(opts.Name); err == nil { + existingAlias = true + } + if !opts.validAliasName(opts.Name) { - return fmt.Errorf("could not create alias: %q is already a gh command, extension, or alias", opts.Name) + if !existingAlias { + return fmt.Errorf("%s Could not create alias %s: already a gh command or extension", + cs.FailureIcon(), + cs.Bold(opts.Name)) + } + + if existingAlias && !opts.OverwriteExisting { + return fmt.Errorf("%s Could not create alias %s: name already taken, use the --clobber flag to overwrite it", + cs.FailureIcon(), + cs.Bold(opts.Name), + ) + } } if !opts.validAliasExpansion(expansion) { - return fmt.Errorf("could not create alias: %s does not correspond to a gh command, extension, or alias", expansion) - } - - successMsg := fmt.Sprintf("%s Added alias.", cs.SuccessIcon()) - if oldExpansion, err := aliasCfg.Get(opts.Name); err == nil { - successMsg = fmt.Sprintf("%s Changed alias %s from %s to %s", - cs.SuccessIcon(), - cs.Bold(opts.Name), - cs.Bold(oldExpansion), - cs.Bold(expansion), - ) + return fmt.Errorf("%s Could not create alias %s: expansion does not correspond to a gh command, extension, or alias", + cs.FailureIcon(), + cs.Bold(opts.Name)) } aliasCfg.Add(opts.Name, expansion) @@ -138,6 +148,13 @@ func setRun(opts *SetOptions) error { return err } + successMsg := fmt.Sprintf("%s Added alias %s", cs.SuccessIcon(), cs.Bold(opts.Name)) + if existingAlias && opts.OverwriteExisting { + successMsg = fmt.Sprintf("%s Changed alias %s", + cs.WarningIcon(), + cs.Bold(opts.Name)) + } + if isTerminal { fmt.Fprintln(opts.IO.ErrOut, successMsg) } diff --git a/pkg/cmd/alias/set/set_test.go b/pkg/cmd/alias/set/set_test.go index 54fcabf5b..4acb8b8b0 100644 --- a/pkg/cmd/alias/set/set_test.go +++ b/pkg/cmd/alias/set/set_test.go @@ -2,314 +2,313 @@ package set import ( "bytes" - "io" + "fmt" "testing" - "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/pkg/cmd/alias/shared" "github.com/cli/cli/v2/pkg/cmdutil" - "github.com/cli/cli/v2/pkg/extensions" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/test" "github.com/google/shlex" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func runCommand(cfg config.Config, isTTY bool, cli string, in string) (*test.CmdOut, error) { - ios, stdin, stdout, stderr := iostreams.Test() - ios.SetStdoutTTY(isTTY) - ios.SetStdinTTY(isTTY) - ios.SetStderrTTY(isTTY) - stdin.WriteString(in) - - factory := &cmdutil.Factory{ - IOStreams: ios, - Config: func() (config.Config, error) { - return cfg, nil +func TestNewCmdSet(t *testing.T) { + tests := []struct { + name string + input string + output SetOptions + wantErr bool + errMsg string + }{ + { + name: "no arguments", + input: "", + wantErr: true, + errMsg: "accepts 2 arg(s), received 0", }, - ExtensionManager: &extensions.ExtensionManagerMock{ - ListFunc: func() []extensions.Extension { - return []extensions.Extension{} + { + name: "only one argument", + input: "name", + wantErr: true, + errMsg: "accepts 2 arg(s), received 1", + }, + { + name: "name and expansion", + input: "alias-name alias-expansion", + output: SetOptions{ + Name: "alias-name", + Expansion: "alias-expansion", + }, + }, + { + name: "shell flag", + input: "alias-name alias-expansion --shell", + output: SetOptions{ + Name: "alias-name", + Expansion: "alias-expansion", + IsShell: true, + }, + }, + { + name: "clobber flag", + input: "alias-name alias-expansion --clobber", + output: SetOptions{ + Name: "alias-name", + Expansion: "alias-expansion", + OverwriteExisting: true, }, }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + f := &cmdutil.Factory{ + IOStreams: ios, + } + argv, err := shlex.Split(tt.input) + assert.NoError(t, err) + var gotOpts *SetOptions + cmd := NewCmdSet(f, func(opts *SetOptions) error { + gotOpts = opts + return nil + }) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) - cmd := NewCmdSet(factory, nil) - - // Create fake command structure for testing. - rootCmd := &cobra.Command{} - rootCmd.AddCommand(cmd) - prCmd := &cobra.Command{Use: "pr"} - prCmd.AddCommand(&cobra.Command{Use: "checkout"}) - prCmd.AddCommand(&cobra.Command{Use: "status"}) - rootCmd.AddCommand(prCmd) - issueCmd := &cobra.Command{Use: "issue"} - issueCmd.AddCommand(&cobra.Command{Use: "list"}) - rootCmd.AddCommand(issueCmd) - apiCmd := &cobra.Command{Use: "api"} - apiCmd.AddCommand(&cobra.Command{Use: "graphql"}) - rootCmd.AddCommand(apiCmd) - - argv, err := shlex.Split("set " + cli) - if err != nil { - return nil, err - } - rootCmd.SetArgs(argv) - - rootCmd.SetIn(stdin) - rootCmd.SetOut(io.Discard) - rootCmd.SetErr(io.Discard) - - _, err = rootCmd.ExecuteC() - return &test.CmdOut{ - OutBuf: stdout, - ErrBuf: stderr, - }, err -} - -func TestAliasSet_gh_command(t *testing.T) { - cfg := config.NewFromString(``) - - _, err := runCommand(cfg, true, "pr 'pr status'", "") - assert.EqualError(t, err, `could not create alias: "pr" is already a gh command, extension, or alias`) -} - -func TestAliasSet_empty_aliases(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(heredoc.Doc(` - aliases: - editor: vim - `)) - - output, err := runCommand(cfg, true, "co 'pr checkout'", "") - - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Added alias") - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.String(), "") - - expected := `aliases: - co: pr checkout -editor: vim -` - assert.Equal(t, expected, mainBuf.String()) -} - -func TestAliasSet_existing_alias(t *testing.T) { - _ = config.StubWriteConfig(t) - - cfg := config.NewFromString(heredoc.Doc(` - aliases: - co: pr checkout - `)) - - output, err := runCommand(cfg, true, "co 'pr checkout -Rcool/repo'", "") - require.NoError(t, err) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Changed alias.*co.*from.*pr checkout.*to.*pr checkout -Rcool/repo") -} - -func TestAliasSet_space_args(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(``) - - output, err := runCommand(cfg, true, `il 'issue list -l "cool story"'`, "") - require.NoError(t, err) - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), `Adding alias for.*il.*issue list -l "cool story"`) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, mainBuf.String(), `il: issue list -l "cool story"`) -} - -func TestAliasSet_arg_processing(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cases := []struct { - Cmd string - ExpectedOutputLine string - ExpectedConfigLine string - }{ - {`il "issue list"`, "- Adding alias for.*il.*issue list", "il: issue list"}, - - {`iz 'issue list'`, "- Adding alias for.*iz.*issue list", "iz: issue list"}, - - {`ii 'issue list --author="$1" --label="$2"'`, - `- Adding alias for.*ii.*issue list --author="\$1" --label="\$2"`, - `ii: issue list --author="\$1" --label="\$2"`}, - - {`ix "issue list --author='\$1' --label='\$2'"`, - `- Adding alias for.*ix.*issue list --author='\$1' --label='\$2'`, - `ix: issue list --author='\$1' --label='\$2'`}, - } - - for _, c := range cases { - t.Run(c.Cmd, func(t *testing.T) { - cfg := config.NewFromString(``) - - output, err := runCommand(cfg, true, c.Cmd, "") - if err != nil { - t.Fatalf("got unexpected error running %s: %s", c.Cmd, err) + _, err = cmd.ExecuteC() + if tt.wantErr { + assert.EqualError(t, err, tt.errMsg) + return } - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), c.ExpectedOutputLine) - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, mainBuf.String(), c.ExpectedConfigLine) + assert.NoError(t, err) + assert.Equal(t, tt.output.Name, gotOpts.Name) + assert.Equal(t, tt.output.Expansion, gotOpts.Expansion) + assert.Equal(t, tt.output.IsShell, gotOpts.IsShell) + assert.Equal(t, tt.output.OverwriteExisting, gotOpts.OverwriteExisting) }) } } -func TestAliasSet_init_alias_cfg(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(heredoc.Doc(` - editor: vim - `)) - - output, err := runCommand(cfg, true, "diff 'pr diff'", "") - require.NoError(t, err) - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - expected := `editor: vim -aliases: - diff: pr diff -` - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Adding alias for.*diff.*pr diff", "Added alias.") - assert.Equal(t, expected, mainBuf.String()) -} - -func TestAliasSet_existing_aliases(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(heredoc.Doc(` - aliases: - foo: bar - `)) - - output, err := runCommand(cfg, true, "view 'pr view'", "") - require.NoError(t, err) - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - expected := `aliases: - foo: bar - view: pr view -` - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Adding alias for.*view.*pr view", "Added alias.") - assert.Equal(t, expected, mainBuf.String()) - -} - -func TestAliasSet_invalid_command(t *testing.T) { - cfg := config.NewFromString(``) - - _, err := runCommand(cfg, true, "co 'pe checkout'", "") - assert.EqualError(t, err, "could not create alias: pe checkout does not correspond to a gh command, extension, or alias") -} - -func TestShellAlias_flag(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(``) - - output, err := runCommand(cfg, true, "--shell igrep 'gh issue list | grep'", "") - if err != nil { - t.Fatalf("unexpected error: %s", err) +func TestSetRun(t *testing.T) { + tests := []struct { + name string + tty bool + opts *SetOptions + stdin string + wantExpansion string + wantStdout string + wantStderr string + wantErrMsg string + }{ + { + name: "creates alias tty", + tty: true, + opts: &SetOptions{ + Name: "foo", + Expansion: "bar", + }, + wantExpansion: "bar", + wantStderr: "- Creating alias for foo: bar\n✓ Added alias foo\n", + }, + { + name: "creates alias", + opts: &SetOptions{ + Name: "foo", + Expansion: "bar", + }, + wantExpansion: "bar", + }, + { + name: "creates shell alias tty", + tty: true, + opts: &SetOptions{ + Name: "igrep", + Expansion: "!gh issue list | grep", + }, + wantExpansion: "!gh issue list | grep", + wantStderr: "- Creating alias for igrep: !gh issue list | grep\n✓ Added alias igrep\n", + }, + { + name: "creates shell alias", + opts: &SetOptions{ + Name: "igrep", + Expansion: "!gh issue list | grep", + }, + wantExpansion: "!gh issue list | grep", + }, + { + name: "creates shell alias using flag tty", + tty: true, + opts: &SetOptions{ + Name: "igrep", + Expansion: "gh issue list | grep", + IsShell: true, + }, + wantExpansion: "!gh issue list | grep", + wantStderr: "- Creating alias for igrep: !gh issue list | grep\n✓ Added alias igrep\n", + }, + { + name: "creates shell alias using flag", + opts: &SetOptions{ + Name: "igrep", + Expansion: "gh issue list | grep", + IsShell: true, + }, + wantExpansion: "!gh issue list | grep", + }, + { + name: "creates alias where expansion has args tty", + tty: true, + opts: &SetOptions{ + Name: "foo", + Expansion: "bar baz --author='$1' --label='$2'", + }, + wantExpansion: "bar baz --author='$1' --label='$2'", + wantStderr: "- Creating alias for foo: bar baz --author='$1' --label='$2'\n✓ Added alias foo\n", + }, + { + name: "creates alias where expansion has args", + opts: &SetOptions{ + Name: "foo", + Expansion: "bar baz --author='$1' --label='$2'", + }, + wantExpansion: "bar baz --author='$1' --label='$2'", + }, + { + name: "creates alias from stdin tty", + tty: true, + opts: &SetOptions{ + Name: "foo", + Expansion: "-", + }, + stdin: `bar baz --author="$1" --label="$2"`, + wantExpansion: `bar baz --author="$1" --label="$2"`, + wantStderr: "- Creating alias for foo: bar baz --author=\"$1\" --label=\"$2\"\n✓ Added alias foo\n", + }, + { + name: "creates alias from stdin", + opts: &SetOptions{ + Name: "foo", + Expansion: "-", + }, + stdin: `bar baz --author="$1" --label="$2"`, + wantExpansion: `bar baz --author="$1" --label="$2"`, + }, + { + name: "overwrites existing alias tty", + tty: true, + opts: &SetOptions{ + Name: "co", + Expansion: "bar", + OverwriteExisting: true, + }, + wantExpansion: "bar", + wantStderr: "- Creating alias for co: bar\n! Changed alias co\n", + }, + { + name: "overwrites existing alias", + opts: &SetOptions{ + Name: "co", + Expansion: "bar", + OverwriteExisting: true, + }, + wantExpansion: "bar", + }, + { + name: "fails when alias name is an existing alias tty", + tty: true, + opts: &SetOptions{ + Name: "co", + Expansion: "bar", + }, + wantExpansion: "pr checkout", + wantErrMsg: "X Could not create alias co: name already taken, use the --clobber flag to overwrite it", + wantStderr: "- Creating alias for co: bar\n", + }, + { + name: "fails when alias name is an existing alias", + opts: &SetOptions{ + Name: "co", + Expansion: "bar", + }, + wantExpansion: "pr checkout", + wantErrMsg: "X Could not create alias co: name already taken, use the --clobber flag to overwrite it", + }, + { + name: "fails when alias expansion is not an existing command tty", + tty: true, + opts: &SetOptions{ + Name: "foo", + Expansion: "baz", + }, + wantErrMsg: "X Could not create alias foo: expansion does not correspond to a gh command, extension, or alias", + wantStderr: "- Creating alias for foo: baz\n", + }, + { + name: "fails when alias expansion is not an existing command", + opts: &SetOptions{ + Name: "foo", + Expansion: "baz", + }, + wantErrMsg: "X Could not create alias foo: expansion does not correspond to a gh command, extension, or alias", + }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rootCmd := &cobra.Command{} + barCmd := &cobra.Command{Use: "bar"} + barCmd.AddCommand(&cobra.Command{Use: "baz"}) + rootCmd.AddCommand(barCmd) + coCmd := &cobra.Command{Use: "co"} + rootCmd.AddCommand(coCmd) - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) + tt.opts.validAliasName = shared.ValidAliasNameFunc(rootCmd) + tt.opts.validAliasExpansion = shared.ValidAliasExpansionFunc(rootCmd) - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") + ios, stdin, stdout, stderr := iostreams.Test() + ios.SetStdinTTY(tt.tty) + ios.SetStdoutTTY(tt.tty) + ios.SetStderrTTY(tt.tty) + tt.opts.IO = ios - expected := `aliases: - igrep: '!gh issue list | grep' -` - assert.Equal(t, expected, mainBuf.String()) + if tt.stdin != "" { + fmt.Fprint(stdin, tt.stdin) + } + + cfg := config.NewBlankConfig() + cfg.WriteFunc = func() error { + return nil + } + tt.opts.Config = func() (config.Config, error) { + return cfg, nil + } + + err := setRun(tt.opts) + if tt.wantErrMsg != "" { + assert.EqualError(t, err, tt.wantErrMsg) + writeCalls := cfg.WriteCalls() + assert.Equal(t, 0, len(writeCalls)) + } else { + assert.NoError(t, err) + writeCalls := cfg.WriteCalls() + assert.Equal(t, 1, len(writeCalls)) + } + + ac := cfg.Aliases() + expansion, _ := ac.Get(tt.opts.Name) + assert.Equal(t, tt.wantExpansion, expansion) + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + }) + } } -func TestShellAlias_bang(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(``) - - output, err := runCommand(cfg, true, "igrep '!gh issue list | grep'", "") - require.NoError(t, err) - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") - - expected := `aliases: - igrep: '!gh issue list | grep' -` - assert.Equal(t, expected, mainBuf.String()) -} - -func TestShellAlias_from_stdin(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(``) - - output, err := runCommand(cfg, true, "users -", `api graphql -F name="$1" -f query=' - query ($name: String!) { - user(login: $name) { - name - } - }'`) - - require.NoError(t, err) - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Adding alias for.*users") - - expected := `aliases: - users: |- - api graphql -F name="$1" -f query=' - query ($name: String!) { - user(login: $name) { - name - } - }' -` - - assert.Equal(t, expected, mainBuf.String()) -} - -func TestShellAlias_getExpansion(t *testing.T) { +func TestGetExpansion(t *testing.T) { tests := []struct { name string want string From e4cddb562edc1436258dc1a70cf8c8c2ca08eb29 Mon Sep 17 00:00:00 2001 From: Jamie Tanna Date: Tue, 15 Aug 2023 18:40:09 +0100 Subject: [PATCH 16/23] Remove GHE handling for `workflow` scope (#7841) According to [0] we appear to no longer support GHE 2.x, as the latest release was deprecated in early 2022. We can therefore remove this custom handling. [0]: https://docs.github.com/en/enterprise-server@3.7/admin/all-releases --- pkg/cmd/auth/shared/login_flow.go | 8 ++------ pkg/cmd/auth/shared/login_flow_test.go | 24 ++++++------------------ 2 files changed, 8 insertions(+), 24 deletions(-) diff --git a/pkg/cmd/auth/shared/login_flow.go b/pkg/cmd/auth/shared/login_flow.go index 7c2ff1639..48cccd3b2 100644 --- a/pkg/cmd/auth/shared/login_flow.go +++ b/pkg/cmd/auth/shared/login_flow.go @@ -162,7 +162,7 @@ func Login(opts *LoginOptions) error { fmt.Fprint(opts.IO.ErrOut, heredoc.Docf(` Tip: you can generate a Personal Access Token here https://%s/settings/tokens The minimum required scopes are %s. - `, hostname, scopesSentence(minimumScopes, ghinstance.IsEnterprise(hostname)))) + `, hostname, scopesSentence(minimumScopes))) var err error authToken, err = opts.Prompter.AuthToken() @@ -216,14 +216,10 @@ func Login(opts *LoginOptions) error { return nil } -func scopesSentence(scopes []string, isEnterprise bool) string { +func scopesSentence(scopes []string) string { quoted := make([]string, len(scopes)) for i, s := range scopes { quoted[i] = fmt.Sprintf("'%s'", s) - if s == "workflow" && isEnterprise { - // remove when GHE 2.x reaches EOL - quoted[i] += " (GHE 3.0+)" - } } return strings.Join(quoted, ", ") } diff --git a/pkg/cmd/auth/shared/login_flow_test.go b/pkg/cmd/auth/shared/login_flow_test.go index 92ae75915..15cc3d2d6 100644 --- a/pkg/cmd/auth/shared/login_flow_test.go +++ b/pkg/cmd/auth/shared/login_flow_test.go @@ -117,8 +117,7 @@ func TestLogin_ssh(t *testing.T) { func Test_scopesSentence(t *testing.T) { type args struct { - scopes []string - isEnterprise bool + scopes []string } tests := []struct { name string @@ -128,39 +127,28 @@ func Test_scopesSentence(t *testing.T) { { name: "basic scopes", args: args{ - scopes: []string{"repo", "read:org"}, - isEnterprise: false, + scopes: []string{"repo", "read:org"}, }, want: "'repo', 'read:org'", }, { name: "empty", args: args{ - scopes: []string(nil), - isEnterprise: false, + scopes: []string(nil), }, want: "", }, { - name: "workflow scope for dotcom", + name: "workflow scope", args: args{ - scopes: []string{"repo", "workflow"}, - isEnterprise: false, + scopes: []string{"repo", "workflow"}, }, want: "'repo', 'workflow'", }, - { - name: "workflow scope for GHE", - args: args{ - scopes: []string{"repo", "workflow"}, - isEnterprise: true, - }, - want: "'repo', 'workflow' (GHE 3.0+)", - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := scopesSentence(tt.args.scopes, tt.args.isEnterprise); got != tt.want { + if got := scopesSentence(tt.args.scopes); got != tt.want { t.Errorf("scopesSentence() = %q, want %q", got, tt.want) } }) From 4a57a812f5db9db72812030f19b214c93b18b297 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Aug 2023 10:37:58 -0700 Subject: [PATCH 17/23] Upgrade to Go 1.21 (#7843) --- .github/CONTRIBUTING.md | 2 +- .github/workflows/deployment.yml | 8 ++++---- .github/workflows/go.yml | 4 ++-- .github/workflows/lint.yml | 6 +++--- docs/source.md | 2 +- go.mod | 2 +- go.sum | 4 ++++ internal/codespaces/api/api_test.go | 6 ++++++ pkg/cmd/label/create.go | 4 ++-- pkg/cmd/repo/credits/credits.go | 12 ++++++------ pkg/cmd/repo/garden/garden.go | 12 ++++++------ 11 files changed, 36 insertions(+), 26 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index eed468389..10f17a2fc 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -23,7 +23,7 @@ Please avoid: ## Building the project Prerequisites: -- Go 1.19+ +- Go 1.21+ Build with: * Unix-like systems: `make` diff --git a/.github/workflows/deployment.yml b/.github/workflows/deployment.yml index 3974737aa..b15b23806 100644 --- a/.github/workflows/deployment.yml +++ b/.github/workflows/deployment.yml @@ -1,6 +1,6 @@ name: Deployment -concurrency: +concurrency: group: ${{ github.workflow }}-${{ github.ref_name }} cancel-in-progress: true @@ -17,7 +17,7 @@ on: default: production type: environment go_version: - default: "1.19" + default: "1.21" type: string platforms: default: "linux,macos,windows" @@ -61,7 +61,7 @@ jobs: dist/*.tar.gz dist/*.rpm dist/*.deb - + macos: runs-on: macos-latest environment: ${{ inputs.environment }} @@ -118,7 +118,7 @@ jobs: path: | dist/*.tar.gz dist/*.zip - + windows: runs-on: windows-latest environment: ${{ inputs.environment }} diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 1a3a33318..a375e2d8f 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -13,10 +13,10 @@ jobs: runs-on: ${{ matrix.os }} steps: - - name: Set up Go 1.19 + - name: Set up Go 1.21 uses: actions/setup-go@v4 with: - go-version: 1.19 + go-version: 1.21 - name: Check out code uses: actions/checkout@v3 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index dd1fede0d..c8defbf3a 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -19,10 +19,10 @@ jobs: runs-on: ubuntu-latest steps: - - name: Set up Go 1.19 + - name: Set up Go 1.21 uses: actions/setup-go@v4 with: - go-version: 1.19 + go-version: 1.21 - name: Check out code uses: actions/checkout@v3 @@ -40,7 +40,7 @@ jobs: go mod verify go mod download - LINT_VERSION=1.50.1 + LINT_VERSION=1.54.1 curl -fsSL https://github.com/golangci/golangci-lint/releases/download/v${LINT_VERSION}/golangci-lint-${LINT_VERSION}-linux-amd64.tar.gz | \ tar xz --strip-components 1 --wildcards \*/golangci-lint mkdir -p bin && mv golangci-lint bin/ diff --git a/docs/source.md b/docs/source.md index 9034e9de5..cc1605b8c 100644 --- a/docs/source.md +++ b/docs/source.md @@ -1,6 +1,6 @@ # Installation from source -1. Verify that you have Go 1.19+ installed +1. Verify that you have Go 1.21+ installed ```sh $ go version diff --git a/go.mod b/go.mod index 2602fc496..48632a1e3 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/cli/cli/v2 -go 1.19 +go 1.21 require ( github.com/AlecAivazis/survey/v2 v2.3.7 diff --git a/go.sum b/go.sum index 47e5159ae..14af85480 100644 --- a/go.sum +++ b/go.sum @@ -91,6 +91,7 @@ github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwA github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= +github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY= @@ -182,6 +183,7 @@ golang.org/x/net v0.0.0-20220923203811-8be639271d50/go.mod h1:YDH+HFinaLZZlnHAfS golang.org/x/net v0.9.0 h1:aWJ/m6xSmxWBx+V0XRHTlrYrPG56jKsLdTFmsSsCzOM= golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns= golang.org/x/oauth2 v0.4.0 h1:NF0gk8LVPg1Ml7SSbGyySuoxdsXitj7TvgvuRxIMc/M= +golang.org/x/oauth2 v0.4.0/go.mod h1:RznEsdpjGAINPTOF0UH/t+xJ75L18YO3Ho6Pyn+uRec= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -221,11 +223,13 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM= +golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.6.7 h1:FZR1q0exgwxzPzp/aF+VccGrSfxfPpkBqjIIEq3ru6c= +google.golang.org/appengine v1.6.7/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= google.golang.org/genproto v0.0.0-20230110181048-76db0878b65f h1:BWUVssLB0HVOSY78gIdvk1dTVYtT1y8SBWtPYuTJ/6w= google.golang.org/genproto v0.0.0-20230110181048-76db0878b65f/go.mod h1:RGgjbofJ8xD9Sq1VVhDM1Vok1vRONV+rg+CjzG4SZKM= google.golang.org/grpc v1.53.0 h1:LAv2ds7cmFV/XTS3XG1NneeENYrXGmorPxsBbptIjNc= diff --git a/internal/codespaces/api/api_test.go b/internal/codespaces/api/api_test.go index 75f82c39e..65852cc79 100644 --- a/internal/codespaces/api/api_test.go +++ b/internal/codespaces/api/api_test.go @@ -136,6 +136,7 @@ func createHttpClient() (*http.Client, error) { func TestNew_APIURL_dotcomConfig(t *testing.T) { t.Setenv("GITHUB_API_URL", "") + t.Setenv("GITHUB_SERVER_URL", "https://github.com") cfg := &config.ConfigMock{ AuthenticationFunc: func() *config.AuthConfig { return &config.AuthConfig{} @@ -158,6 +159,7 @@ func TestNew_APIURL_dotcomConfig(t *testing.T) { func TestNew_APIURL_customConfig(t *testing.T) { t.Setenv("GITHUB_API_URL", "") + t.Setenv("GITHUB_SERVER_URL", "https://github.mycompany.com") cfg := &config.ConfigMock{ AuthenticationFunc: func() *config.AuthConfig { authCfg := &config.AuthConfig{} @@ -182,6 +184,7 @@ func TestNew_APIURL_customConfig(t *testing.T) { func TestNew_APIURL_env(t *testing.T) { t.Setenv("GITHUB_API_URL", "https://api.mycompany.com") + t.Setenv("GITHUB_SERVER_URL", "https://mycompany.com") cfg := &config.ConfigMock{ AuthenticationFunc: func() *config.AuthConfig { return &config.AuthConfig{} @@ -218,6 +221,7 @@ func TestNew_APIURL_dotcomFallback(t *testing.T) { func TestNew_ServerURL_dotcomConfig(t *testing.T) { t.Setenv("GITHUB_SERVER_URL", "") + t.Setenv("GITHUB_API_URL", "https://api.github.com") cfg := &config.ConfigMock{ AuthenticationFunc: func() *config.AuthConfig { return &config.AuthConfig{} @@ -240,6 +244,7 @@ func TestNew_ServerURL_dotcomConfig(t *testing.T) { func TestNew_ServerURL_customConfig(t *testing.T) { t.Setenv("GITHUB_SERVER_URL", "") + t.Setenv("GITHUB_API_URL", "https://github.mycompany.com/api/v3") cfg := &config.ConfigMock{ AuthenticationFunc: func() *config.AuthConfig { authCfg := &config.AuthConfig{} @@ -264,6 +269,7 @@ func TestNew_ServerURL_customConfig(t *testing.T) { func TestNew_ServerURL_env(t *testing.T) { t.Setenv("GITHUB_SERVER_URL", "https://mycompany.com") + t.Setenv("GITHUB_API_URL", "https://api.mycompany.com") cfg := &config.ConfigMock{ AuthenticationFunc: func() *config.AuthConfig { return &config.AuthConfig{} diff --git a/pkg/cmd/label/create.go b/pkg/cmd/label/create.go index f9c63f6e6..0a88fbcab 100644 --- a/pkg/cmd/label/create.go +++ b/pkg/cmd/label/create.go @@ -102,8 +102,8 @@ func createRun(opts *createOptions) error { } if opts.Color == "" { - rand.Seed(time.Now().UnixNano()) - opts.Color = randomColors[rand.Intn(len(randomColors)-1)] + r := rand.New(rand.NewSource(time.Now().UnixNano())) + opts.Color = randomColors[r.Intn(len(randomColors)-1)] } opts.IO.StartProgressIndicator() diff --git a/pkg/cmd/repo/credits/credits.go b/pkg/cmd/repo/credits/credits.go index 37390bcb1..4b7b9fb28 100644 --- a/pkg/cmd/repo/credits/credits.go +++ b/pkg/cmd/repo/credits/credits.go @@ -178,7 +178,7 @@ func creditsRun(opts *CreditsOptions) error { return nil } - rand.Seed(time.Now().UnixNano()) + r := rand.New(rand.NewSource(time.Now().UnixNano())) lines := []string{} @@ -199,13 +199,13 @@ func creditsRun(opts *CreditsOptions) error { starLinesLeft := []string{} for x := 0; x < len(lines); x++ { - starLinesLeft = append(starLinesLeft, starLine(margin)) + starLinesLeft = append(starLinesLeft, starLine(r, margin)) } starLinesRight := []string{} for x := 0; x < len(lines); x++ { lineWidth := termWidth - (margin + len(lines[x])) - starLinesRight = append(starLinesRight, starLine(lineWidth)) + starLinesRight = append(starLinesRight, starLine(r, lineWidth)) } loop := true @@ -245,13 +245,13 @@ func creditsRun(opts *CreditsOptions) error { return nil } -func starLine(width int) string { +func starLine(r *rand.Rand, width int) string { line := "" starChance := 0.1 for y := 0; y < width; y++ { - chance := rand.Float64() + chance := r.Float64() if chance <= starChance { - charRoll := rand.Float64() + charRoll := r.Float64() switch { case charRoll < 0.3: line += "." diff --git a/pkg/cmd/repo/garden/garden.go b/pkg/cmd/repo/garden/garden.go index c9c59d420..840d5adca 100644 --- a/pkg/cmd/repo/garden/garden.go +++ b/pkg/cmd/repo/garden/garden.go @@ -169,7 +169,7 @@ func gardenRun(opts *GardenOptions) error { } seed := computeSeed(ghrepo.FullName(toView)) - rand.Seed(seed) + r := rand.New(rand.NewSource(seed)) termWidth, termHeight, err := utils.TerminalSize(out) if err != nil { @@ -198,7 +198,7 @@ func gardenRun(opts *GardenOptions) error { } player := &Player{0, 0, cs.Bold("@"), geo, 0} - garden := plantGarden(commits, geo) + garden := plantGarden(r, commits, geo) if len(garden) < geo.Height { geo.Height = len(garden) } @@ -334,11 +334,11 @@ func isQuit(b []byte) bool { return rune(b[0]) == 'q' || bytes.Equal(b, ctrlC) } -func plantGarden(commits []*Commit, geo *Geometry) [][]*Cell { +func plantGarden(r *rand.Rand, commits []*Commit, geo *Geometry) [][]*Cell { cellIx := 0 grassCell := &Cell{RGB(0, 200, 0, ","), "You're standing on a patch of grass in a field of wildflowers."} garden := [][]*Cell{} - streamIx := rand.Intn(geo.Width - 1) + streamIx := r.Intn(geo.Width - 1) if streamIx == geo.Width/2 { streamIx-- } @@ -363,7 +363,7 @@ func plantGarden(commits []*Commit, geo *Geometry) [][]*Cell { }) tint += 15 streamIx-- - if rand.Float64() < 0.5 { + if r.Float64() < 0.5 { streamIx++ } if streamIx < 0 { @@ -393,7 +393,7 @@ func plantGarden(commits []*Commit, geo *Geometry) [][]*Cell { continue } - chance := rand.Float64() + chance := r.Float64() if chance <= geo.Density { commit := commits[cellIx] garden[y] = append(garden[y], &Cell{ From 719c9579ba835e52b65e0f9d73104462e2db6b94 Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Thu, 10 Aug 2023 18:03:46 -0700 Subject: [PATCH 18/23] switch to prompter in workflow commands --- pkg/cmd/run/list/list.go | 10 +++- pkg/cmd/workflow/disable/disable.go | 8 ++- pkg/cmd/workflow/disable/disable_test.go | 38 ++++++------ pkg/cmd/workflow/enable/enable.go | 8 ++- pkg/cmd/workflow/enable/enable_test.go | 38 ++++++------ pkg/cmd/workflow/run/run.go | 74 +++++++++++++---------- pkg/cmd/workflow/run/run_test.go | 76 ++++++++++++++---------- pkg/cmd/workflow/shared/shared.go | 23 +++---- pkg/cmd/workflow/view/view.go | 8 ++- 9 files changed, 169 insertions(+), 114 deletions(-) diff --git a/pkg/cmd/run/list/list.go b/pkg/cmd/run/list/list.go index af84b089e..394af5dcb 100644 --- a/pkg/cmd/run/list/list.go +++ b/pkg/cmd/run/list/list.go @@ -24,6 +24,7 @@ type ListOptions struct { IO *iostreams.IOStreams HttpClient func() (*http.Client, error) BaseRepo func() (ghrepo.Interface, error) + Prompter iprompter Exporter cmdutil.Exporter @@ -38,10 +39,15 @@ type ListOptions struct { now time.Time } +type iprompter interface { + Select(string, string, []string) (int, error) +} + func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { opts := &ListOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + Prompter: f.Prompter, now: time.Now(), } @@ -103,7 +109,9 @@ func listRun(opts *ListOptions) error { opts.IO.StartProgressIndicator() if opts.WorkflowSelector != "" { states := []workflowShared.WorkflowState{workflowShared.Active} - if workflow, err := workflowShared.ResolveWorkflow(opts.IO, client, baseRepo, false, opts.WorkflowSelector, states); err == nil { + if workflow, err := workflowShared.ResolveWorkflow( + opts.Prompter, opts.IO, client, baseRepo, false, opts.WorkflowSelector, + states); err == nil { filters.WorkflowID = workflow.ID filters.WorkflowName = workflow.Name } else { diff --git a/pkg/cmd/workflow/disable/disable.go b/pkg/cmd/workflow/disable/disable.go index ed3155fbd..8b2fb62d3 100644 --- a/pkg/cmd/workflow/disable/disable.go +++ b/pkg/cmd/workflow/disable/disable.go @@ -17,15 +17,21 @@ type DisableOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) + Prompter iprompter Selector string Prompt bool } +type iprompter interface { + Select(string, string, []string) (int, error) +} + func NewCmdDisable(f *cmdutil.Factory, runF func(*DisableOptions) error) *cobra.Command { opts := &DisableOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -69,7 +75,7 @@ func runDisable(opts *DisableOptions) error { states := []shared.WorkflowState{shared.Active} workflow, err := shared.ResolveWorkflow( - opts.IO, client, repo, opts.Prompt, opts.Selector, states) + opts.Prompter, opts.IO, client, repo, opts.Prompt, opts.Selector, states) if err != nil { var fae shared.FilteredAllError if errors.As(err, &fae) { diff --git a/pkg/cmd/workflow/disable/disable_test.go b/pkg/cmd/workflow/disable/disable_test.go index 4bbf766e7..d8ba3fcd9 100644 --- a/pkg/cmd/workflow/disable/disable_test.go +++ b/pkg/cmd/workflow/disable/disable_test.go @@ -7,11 +7,11 @@ import ( "testing" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/workflow/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -91,14 +91,14 @@ func TestNewCmdDisable(t *testing.T) { func TestDisableRun(t *testing.T) { tests := []struct { - name string - opts *DisableOptions - httpStubs func(*httpmock.Registry) - askStubs func(*prompt.AskStubber) - tty bool - wantOut string - wantErrOut string - wantErr bool + name string + opts *DisableOptions + httpStubs func(*httpmock.Registry) + promptStubs func(*prompter.MockPrompter) + tty bool + wantOut string + wantErrOut string + wantErr bool }{ { name: "tty no arg", @@ -120,8 +120,10 @@ func TestDisableRun(t *testing.T) { httpmock.REST("PUT", "repos/OWNER/REPO/actions/workflows/789/disable"), httpmock.StatusStringResponse(204, "{}")) }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Select a workflow").AnswerWith("another workflow (another.yml)") + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow", []string{"a workflow (flow.yml)", "another workflow (another.yml)"}, func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "another workflow (another.yml)") + }) }, wantOut: "✓ Disabled another workflow\n", }, @@ -169,8 +171,10 @@ func TestDisableRun(t *testing.T) { httpmock.REST("PUT", "repos/OWNER/REPO/actions/workflows/1011/disable"), httpmock.StatusStringResponse(204, "{}")) }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Which workflow do you mean?").AnswerWith("another workflow (yetanother.yml)") + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Which workflow do you mean?", []string{"another workflow (another.yml)", "another workflow (yetanother.yml)"}, func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "another workflow (yetanother.yml)") + }) }, wantOut: "✓ Disabled another workflow\n", }, @@ -269,10 +273,10 @@ func TestDisableRun(t *testing.T) { } t.Run(tt.name, func(t *testing.T) { - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - if tt.askStubs != nil { - tt.askStubs(as) + pm := prompter.NewMockPrompter(t) + tt.opts.Prompter = pm + if tt.promptStubs != nil { + tt.promptStubs(pm) } err := runDisable(tt.opts) diff --git a/pkg/cmd/workflow/enable/enable.go b/pkg/cmd/workflow/enable/enable.go index 06922a46f..93e8ac007 100644 --- a/pkg/cmd/workflow/enable/enable.go +++ b/pkg/cmd/workflow/enable/enable.go @@ -17,15 +17,21 @@ type EnableOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) + Prompter iprompter Selector string Prompt bool } +type iprompter interface { + Select(string, string, []string) (int, error) +} + func NewCmdEnable(f *cmdutil.Factory, runF func(*EnableOptions) error) *cobra.Command { opts := &EnableOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -68,7 +74,7 @@ func runEnable(opts *EnableOptions) error { } states := []shared.WorkflowState{shared.DisabledManually, shared.DisabledInactivity} - workflow, err := shared.ResolveWorkflow( + workflow, err := shared.ResolveWorkflow(opts.Prompter, opts.IO, client, repo, opts.Prompt, opts.Selector, states) if err != nil { var fae shared.FilteredAllError diff --git a/pkg/cmd/workflow/enable/enable_test.go b/pkg/cmd/workflow/enable/enable_test.go index b626f794d..905965087 100644 --- a/pkg/cmd/workflow/enable/enable_test.go +++ b/pkg/cmd/workflow/enable/enable_test.go @@ -7,11 +7,11 @@ import ( "testing" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/workflow/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -91,14 +91,14 @@ func TestNewCmdEnable(t *testing.T) { func TestEnableRun(t *testing.T) { tests := []struct { - name string - opts *EnableOptions - httpStubs func(*httpmock.Registry) - askStubs func(*prompt.AskStubber) - tty bool - wantOut string - wantErrOut string - wantErr bool + name string + opts *EnableOptions + httpStubs func(*httpmock.Registry) + promptStubs func(*prompter.MockPrompter) + tty bool + wantOut string + wantErrOut string + wantErr bool }{ { name: "tty no arg", @@ -120,8 +120,10 @@ func TestEnableRun(t *testing.T) { httpmock.REST("PUT", "repos/OWNER/REPO/actions/workflows/456/enable"), httpmock.StatusStringResponse(204, "{}")) }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Select a workflow").AnswerWith("a disabled workflow (disabled.yml)") + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow", []string{"a disabled workflow (disabled.yml)"}, func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "a disabled workflow (disabled.yml)") + }) }, wantOut: "✓ Enabled a disabled workflow\n", }, @@ -169,8 +171,10 @@ func TestEnableRun(t *testing.T) { httpmock.REST("PUT", "repos/OWNER/REPO/actions/workflows/1213/enable"), httpmock.StatusStringResponse(204, "{}")) }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Which workflow do you mean?").AnswerWith("a disabled workflow (anotherDisabled.yml)") + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Which workflow do you mean?", []string{"a disabled workflow (disabled.yml)", "a disabled workflow (anotherDisabled.yml)"}, func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "a disabled workflow (anotherDisabled.yml)") + }) }, wantOut: "✓ Enabled a disabled workflow\n", }, @@ -309,10 +313,10 @@ func TestEnableRun(t *testing.T) { } t.Run(tt.name, func(t *testing.T) { - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - if tt.askStubs != nil { - tt.askStubs(as) + pm := prompter.NewMockPrompter(t) + tt.opts.Prompter = pm + if tt.promptStubs != nil { + tt.promptStubs(pm) } err := runEnable(tt.opts) diff --git a/pkg/cmd/workflow/run/run.go b/pkg/cmd/workflow/run/run.go index 564b41985..4ba0a2ab4 100644 --- a/pkg/cmd/workflow/run/run.go +++ b/pkg/cmd/workflow/run/run.go @@ -11,14 +11,12 @@ import ( "sort" "strings" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/workflow/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/spf13/cobra" "gopkg.in/yaml.v3" ) @@ -27,6 +25,7 @@ type RunOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) + Prompter iprompter Selector string Ref string @@ -39,10 +38,16 @@ type RunOptions struct { Prompt bool } +type iprompter interface { + Input(string, string) (string, error) + Select(string, string, []string) (int, error) +} + func NewCmdRun(f *cmdutil.Factory, runF func(*RunOptions) error) *cobra.Command { opts := &RunOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -198,7 +203,7 @@ func (ia *InputAnswer) WriteAnswer(name string, value interface{}) error { return fmt.Errorf("unexpected value type: %v", value) } -func collectInputs(yamlContent []byte) (map[string]string, error) { +func collectInputs(p iprompter, yamlContent []byte) (map[string]string, error) { inputs, err := findInputs(yamlContent) if err != nil { return nil, err @@ -210,32 +215,24 @@ func collectInputs(yamlContent []byte) (map[string]string, error) { return providedInputs, nil } - qs := []*survey.Question{} - for inputName, input := range inputs { - q := &survey.Question{ - Name: inputName, - Prompt: &survey.Input{ - Message: inputName, - Default: input.Default, - }, - } + for _, input := range inputs { + var answer string if input.Required { - q.Validate = survey.Required + for answer == "" { + answer, err = p.Input(input.Name+" (required)", input.Default) + if err != nil { + break + } + } + } else { + answer, err = p.Input(input.Name, input.Default) } - qs = append(qs, q) - } - sort.Slice(qs, func(i, j int) bool { - return qs[i].Name < qs[j].Name - }) + if err != nil { + return nil, err + } - inputAnswer := InputAnswer{ - providedInputs: providedInputs, - } - //nolint:staticcheck // SA1019: prompt.SurveyAsk is deprecated: use Prompter - err = prompt.SurveyAsk(qs, &inputAnswer) - if err != nil { - return nil, err + providedInputs[input.Name] = answer } return providedInputs, nil @@ -263,7 +260,7 @@ func runRun(opts *RunOptions) error { } states := []shared.WorkflowState{shared.Active} - workflow, err := shared.ResolveWorkflow( + workflow, err := shared.ResolveWorkflow(opts.Prompter, opts.IO, client, repo, opts.Prompt, opts.Selector, states) if err != nil { var fae shared.FilteredAllError @@ -290,7 +287,7 @@ func runRun(opts *RunOptions) error { if err != nil { return fmt.Errorf("unable to fetch workflow file content: %w", err) } - providedInputs, err = collectInputs(yamlContent) + providedInputs, err = collectInputs(opts.Prompter, yamlContent) if err != nil { return err } @@ -330,12 +327,13 @@ func runRun(opts *RunOptions) error { } type WorkflowInput struct { + Name string Required bool Default string Description string } -func findInputs(yamlContent []byte) (map[string]WorkflowInput, error) { +func findInputs(yamlContent []byte) ([]WorkflowInput, error) { var rootNode yaml.Node err := yaml.Unmarshal(yamlContent, &rootNode) if err != nil { @@ -400,16 +398,32 @@ func findInputs(yamlContent []byte) (map[string]WorkflowInput, error) { return nil, errors.New("unable to manually run a workflow without a workflow_dispatch event") } - out := map[string]WorkflowInput{} + out := []WorkflowInput{} + + m := map[string]WorkflowInput{} if inputsKeyNode == nil || inputsMapNode == nil { return out, nil } - err = inputsMapNode.Decode(&out) + err = inputsMapNode.Decode(&m) if err != nil { return nil, fmt.Errorf("could not decode workflow inputs: %w", err) } + for name, input := range m { + input.Name = name + out = append(out, WorkflowInput{ + Name: name, + Default: input.Default, + Description: input.Description, + Required: input.Required, + }) + } + + sort.Slice(out, func(i, j int) bool { + return out[i].Name < out[j].Name + }) + return out, nil } diff --git a/pkg/cmd/workflow/run/run_test.go b/pkg/cmd/workflow/run/run_test.go index c90c9a09e..70fea8aa7 100644 --- a/pkg/cmd/workflow/run/run_test.go +++ b/pkg/cmd/workflow/run/run_test.go @@ -12,11 +12,11 @@ import ( "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/workflow/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -215,7 +215,7 @@ func Test_findInputs(t *testing.T) { YAML []byte wantErr bool errMsg string - wantOut map[string]WorkflowInput + wantOut []WorkflowInput }{ { name: "blank", @@ -244,12 +244,12 @@ func Test_findInputs(t *testing.T) { { name: "short syntax", YAML: []byte("name: workflow\non: workflow_dispatch"), - wantOut: map[string]WorkflowInput{}, + wantOut: []WorkflowInput{}, }, { name: "array of events", YAML: []byte("name: workflow\non: [pull_request, workflow_dispatch]\n"), - wantOut: map[string]WorkflowInput{}, + wantOut: []WorkflowInput{}, }, { name: "inputs", @@ -274,18 +274,22 @@ jobs: - name: echo run: | echo "echo"`), - wantOut: map[string]WorkflowInput{ - "foo": { + wantOut: []WorkflowInput{ + { + Name: "bar", + Default: "boo", + }, + { + Name: "baz", + Description: "it's baz", + }, + { + Name: "foo", Required: true, Description: "good foo", }, - "bar": { - Default: "boo", - }, - "baz": { - Description: "it's baz", - }, - "quux": { + { + Name: "quux", Required: true, Default: "cool", }, @@ -359,15 +363,15 @@ jobs: } tests := []struct { - name string - opts *RunOptions - tty bool - wantErr bool - errOut string - wantOut string - wantBody map[string]interface{} - httpStubs func(*httpmock.Registry) - askStubs func(*prompt.AskStubber) + name string + opts *RunOptions + tty bool + wantErr bool + errOut string + wantOut string + wantBody map[string]interface{} + httpStubs func(*httpmock.Registry) + promptStubs func(*prompter.MockPrompter) }{ { name: "bad JSON", @@ -577,8 +581,10 @@ jobs: httpmock.REST("POST", "repos/OWNER/REPO/actions/workflows/1/dispatches"), httpmock.StatusStringResponse(204, "cool")) }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Select a workflow").AnswerDefault() + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow", []string{"minimal workflow (minimal.yml)"}, func(_, _ string, opts []string) (int, error) { + return 0, nil + }) }, wantBody: map[string]interface{}{ "inputs": map[string]interface{}{}, @@ -614,10 +620,16 @@ jobs: httpmock.REST("POST", "repos/OWNER/REPO/actions/workflows/12345/dispatches"), httpmock.StatusStringResponse(204, "cool")) }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Select a workflow").AnswerDefault() - as.StubPrompt("greeting").AnswerWith("hi") - as.StubPrompt("name").AnswerWith("scully") + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow", []string{"a workflow (workflow.yml)"}, func(_, _ string, opts []string) (int, error) { + return 0, nil + }) + pm.RegisterInput("greeting", func(_, _ string) (string, error) { + return "hi", nil + }) + pm.RegisterInput("name (required)", func(_, _ string) (string, error) { + return "scully", nil + }) }, wantBody: map[string]interface{}{ "inputs": map[string]interface{}{ @@ -652,10 +664,10 @@ jobs: } t.Run(tt.name, func(t *testing.T) { - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - if tt.askStubs != nil { - tt.askStubs(as) + pm := prompter.NewMockPrompter(t) + tt.opts.Prompter = pm + if tt.promptStubs != nil { + tt.promptStubs(pm) } err := runRun(tt.opts) diff --git a/pkg/cmd/workflow/shared/shared.go b/pkg/cmd/workflow/shared/shared.go index cfb1ff86e..6d887f927 100644 --- a/pkg/cmd/workflow/shared/shared.go +++ b/pkg/cmd/workflow/shared/shared.go @@ -11,11 +11,9 @@ import ( "strconv" "strings" - "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/cli/go-gh/v2/pkg/asciisanitizer" "golang.org/x/text/transform" ) @@ -26,6 +24,10 @@ const ( DisabledInactivity WorkflowState = "disabled_inactivity" ) +type iprompter interface { + Select(string, string, []string) (int, error) +} + type WorkflowState string type Workflow struct { @@ -90,7 +92,7 @@ type FilteredAllError struct { error } -func SelectWorkflow(workflows []Workflow, promptMsg string, states []WorkflowState) (*Workflow, error) { +func selectWorkflow(p iprompter, workflows []Workflow, promptMsg string, states []WorkflowState) (*Workflow, error) { filtered := []Workflow{} candidates := []string{} for _, workflow := range workflows { @@ -107,14 +109,7 @@ func SelectWorkflow(workflows []Workflow, promptMsg string, states []WorkflowSta return nil, FilteredAllError{errors.New("")} } - var selected int - - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne(&survey.Select{ - Message: promptMsg, - Options: candidates, - PageSize: 15, - }, &selected) + selected, err := p.Select(promptMsg, "", candidates) if err != nil { return nil, err } @@ -182,7 +177,7 @@ func getWorkflowsByName(client *api.Client, repo ghrepo.Interface, name string, return filtered, nil } -func ResolveWorkflow(io *iostreams.IOStreams, client *api.Client, repo ghrepo.Interface, prompt bool, workflowSelector string, states []WorkflowState) (*Workflow, error) { +func ResolveWorkflow(p iprompter, io *iostreams.IOStreams, client *api.Client, repo ghrepo.Interface, prompt bool, workflowSelector string, states []WorkflowState) (*Workflow, error) { if prompt { workflows, err := GetWorkflows(client, repo, 0) if len(workflows) == 0 { @@ -198,7 +193,7 @@ func ResolveWorkflow(io *iostreams.IOStreams, client *api.Client, repo ghrepo.In return nil, fmt.Errorf("could not fetch workflows for %s: %w", ghrepo.FullName(repo), err) } - return SelectWorkflow(workflows, "Select a workflow", states) + return selectWorkflow(p, workflows, "Select a workflow", states) } workflows, err := FindWorkflow(client, repo, workflowSelector, states) @@ -222,7 +217,7 @@ func ResolveWorkflow(io *iostreams.IOStreams, client *api.Client, repo ghrepo.In return nil, errors.New(errMsg) } - return SelectWorkflow(workflows, "Which workflow do you mean?", states) + return selectWorkflow(p, workflows, "Which workflow do you mean?", states) } func GetWorkflowContent(client *api.Client, repo ghrepo.Interface, workflow Workflow, ref string) ([]byte, error) { diff --git a/pkg/cmd/workflow/view/view.go b/pkg/cmd/workflow/view/view.go index f45024828..23b148942 100644 --- a/pkg/cmd/workflow/view/view.go +++ b/pkg/cmd/workflow/view/view.go @@ -26,6 +26,7 @@ type ViewOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser + Prompter iprompter Selector string Ref string @@ -37,11 +38,16 @@ type ViewOptions struct { now time.Time } +type iprompter interface { + Select(string, string, []string) (int, error) +} + func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { opts := &ViewOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, Browser: f.Browser, + Prompter: f.Prompter, now: time.Now(), } @@ -104,7 +110,7 @@ func runView(opts *ViewOptions) error { var workflow *shared.Workflow states := []shared.WorkflowState{shared.Active} - workflow, err = shared.ResolveWorkflow(opts.IO, client, repo, opts.Prompt, opts.Selector, states) + workflow, err = shared.ResolveWorkflow(opts.Prompter, opts.IO, client, repo, opts.Prompt, opts.Selector, states) if err != nil { return err } From 2ddfc3827def5430a21ff38a47870ddb6861568d Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Wed, 16 Aug 2023 16:02:14 -0500 Subject: [PATCH 19/23] use new prompter in repo fork --- pkg/cmd/repo/fork/fork.go | 17 +++++---- pkg/cmd/repo/fork/fork_test.go | 70 ++++++++++++++++++---------------- 2 files changed, 47 insertions(+), 40 deletions(-) diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index 087d7219e..0d2d1da69 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -19,13 +19,16 @@ import ( "github.com/cli/cli/v2/pkg/cmd/repo/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/spf13/cobra" "github.com/spf13/pflag" ) const defaultRemoteName = "origin" +type iprompter interface { + Confirm(string, bool) (bool, error) +} + type ForkOptions struct { HttpClient func() (*http.Client, error) GitClient *git.Client @@ -35,6 +38,7 @@ type ForkOptions struct { Remotes func() (ghContext.Remotes, error) Since func(time.Time) time.Duration BackOff backoff.BackOff + Prompter iprompter GitArgs []string Repository string @@ -65,6 +69,7 @@ func NewCmdFork(f *cmdutil.Factory, runF func(*ForkOptions) error) *cobra.Comman Config: f.Config, BaseRepo: f.BaseRepo, Remotes: f.Remotes, + Prompter: f.Prompter, Since: time.Since, } @@ -273,10 +278,9 @@ func forkRun(opts *ForkOptions) error { remoteDesired := opts.Remote if opts.PromptRemote { - //nolint:staticcheck // SA1019: prompt.Confirm is deprecated: use Prompter - err = prompt.Confirm("Would you like to add a remote for the fork?", &remoteDesired) + remoteDesired, err = opts.Prompter.Confirm("Would you like to add a remote for the fork?", false) if err != nil { - return fmt.Errorf("failed to prompt: %w", err) + return err } } @@ -317,10 +321,9 @@ func forkRun(opts *ForkOptions) error { } else { cloneDesired := opts.Clone if opts.PromptClone { - //nolint:staticcheck // SA1019: prompt.Confirm is deprecated: use Prompter - err = prompt.Confirm("Would you like to clone the fork?", &cloneDesired) + cloneDesired, err = opts.Prompter.Confirm("Would you like to clone the fork?", false) if err != nil { - return fmt.Errorf("failed to prompt: %w", err) + return err } } if cloneDesired { diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 5833acb5d..f59fba1b7 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -14,11 +14,11 @@ import ( "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -205,18 +205,18 @@ func TestRepoFork(t *testing.T) { } tests := []struct { - name string - opts *ForkOptions - tty bool - httpStubs func(*httpmock.Registry) - execStubs func(*run.CommandStubber) - askStubs func(*prompt.AskStubber) - cfgStubs func(*config.ConfigMock) - remotes []*context.Remote - wantOut string - wantErrOut string - wantErr bool - errMsg string + name string + opts *ForkOptions + tty bool + httpStubs func(*httpmock.Registry) + execStubs func(*run.CommandStubber) + promptStubs func(*prompter.MockPrompter) + cfgStubs func(*config.ConfigMock) + remotes []*context.Remote + wantOut string + wantErrOut string + wantErr bool + errMsg string }{ { name: "implicit match, configured protocol overrides provided", @@ -272,9 +272,10 @@ func TestRepoFork(t *testing.T) { RemoteName: defaultRemoteName, }, httpStubs: forkPost, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(false) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterConfirm("Would you like to add a remote for the fork?", func(_ string, _ bool) (bool, error) { + return false, nil + }) }, wantErrOut: "✓ Created fork someone/REPO\n", }, @@ -291,9 +292,10 @@ func TestRepoFork(t *testing.T) { cs.Register("git remote rename origin upstream", 0, "") cs.Register(`git remote add origin https://github.com/someone/REPO.git`, 0, "") }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(true) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterConfirm("Would you like to add a remote for the fork?", func(_ string, _ bool) (bool, error) { + return true, nil + }) }, wantErrOut: "✓ Created fork someone/REPO\n✓ Added remote origin\n", }, @@ -497,9 +499,10 @@ func TestRepoFork(t *testing.T) { PromptClone: true, }, httpStubs: forkPost, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(false) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterConfirm("Would you like to clone the fork?", func(_ string, _ bool) (bool, error) { + return false, nil + }) }, wantErrOut: "✓ Created fork someone/REPO\n", }, @@ -511,9 +514,10 @@ func TestRepoFork(t *testing.T) { PromptClone: true, }, httpStubs: forkPost, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(true) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterConfirm("Would you like to clone the fork?", func(_ string, _ bool) (bool, error) { + return true, nil + }) }, execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "") @@ -533,9 +537,10 @@ func TestRepoFork(t *testing.T) { }, }, httpStubs: forkPost, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(true) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterConfirm("Would you like to clone the fork?", func(_ string, _ bool) (bool, error) { + return true, nil + }) }, execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "") @@ -746,11 +751,10 @@ func TestRepoFork(t *testing.T) { GitPath: "some/path/git", } - //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber - as, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(as) + pm := prompter.NewMockPrompter(t) + tt.opts.Prompter = pm + if tt.promptStubs != nil { + tt.promptStubs(pm) } cs, restoreRun := run.Stub() From f2e5ad6dcd9a692fe97badd0b3485f1177a7592e Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Wed, 16 Aug 2023 21:26:37 -0500 Subject: [PATCH 20/23] fix RegisterPassword --- internal/prompter/test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/prompter/test.go b/internal/prompter/test.go index c376e9c83..0dea18e5a 100644 --- a/internal/prompter/test.go +++ b/internal/prompter/test.go @@ -258,7 +258,7 @@ func (m *MockPrompter) RegisterInputHostname(stub func() (string, error)) { } func (m *MockPrompter) RegisterPassword(prompt string, stub func(string) (string, error)) { - m.PasswordStubs = append(m.PasswordStubs, PasswordStub{Fn: stub}) + m.PasswordStubs = append(m.PasswordStubs, PasswordStub{Prompt: prompt, Fn: stub}) } func (m *MockPrompter) RegisterConfirmDeletion(prompt string, stub func(string) error) { From fc73c16fe82c925e72b8f017172315456922a9ce Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Wed, 16 Aug 2023 21:26:57 -0500 Subject: [PATCH 21/23] use prompter in secret set --- pkg/cmd/secret/set/set.go | 14 +++++++------- pkg/cmd/secret/set/set_test.go | 12 +++++++----- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index 86dd35258..95f80f8b1 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -9,7 +9,6 @@ import ( "os" "strings" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/config" @@ -17,7 +16,6 @@ import ( "github.com/cli/cli/v2/pkg/cmd/secret/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/hashicorp/go-multierror" "github.com/joho/godotenv" "github.com/spf13/cobra" @@ -29,6 +27,7 @@ type SetOptions struct { IO *iostreams.IOStreams Config func() (config.Config, error) BaseRepo func() (ghrepo.Interface, error) + Prompter iprompter RandomOverride func() io.Reader @@ -44,11 +43,16 @@ type SetOptions struct { Application string } +type iprompter interface { + Password(string) (string, error) +} + func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command { opts := &SetOptions{ IO: f.IOStreams, Config: f.Config, HttpClient: f.HttpClient, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -375,11 +379,7 @@ func getBody(opts *SetOptions) ([]byte, error) { } if opts.IO.CanPrompt() { - var bodyInput string - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne(&survey.Password{ - Message: "Paste your secret", - }, &bodyInput) + bodyInput, err := opts.Prompter.Password("Paste your secret") if err != nil { return nil, err } diff --git a/pkg/cmd/secret/set/set_test.go b/pkg/cmd/secret/set/set_test.go index 592527c5f..376ef2d53 100644 --- a/pkg/cmd/secret/set/set_test.go +++ b/pkg/cmd/secret/set/set_test.go @@ -12,11 +12,11 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/secret/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -595,12 +595,14 @@ func Test_getBodyPrompt(t *testing.T) { ios.SetStdinTTY(true) ios.SetStdoutTTY(true) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("Paste your secret").AnswerWith("cool secret") + pm := prompter.NewMockPrompter(t) + pm.RegisterPassword("Paste your secret", func(_ string) (string, error) { + return "cool secret", nil + }) body, err := getBody(&SetOptions{ - IO: ios, + IO: ios, + Prompter: pm, }) assert.NoError(t, err) assert.Equal(t, string(body), "cool secret") From 0d3b7db495d8082df3ade134e85ecb1541750efd Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Wed, 16 Aug 2023 21:48:30 -0500 Subject: [PATCH 22/23] use prompter in issue delete --- pkg/cmd/issue/delete/delete.go | 25 +++++++----------- pkg/cmd/issue/delete/delete_test.go | 39 ++++++++++++++--------------- 2 files changed, 28 insertions(+), 36 deletions(-) diff --git a/pkg/cmd/issue/delete/delete.go b/pkg/cmd/issue/delete/delete.go index f0c68de42..cee367cb8 100644 --- a/pkg/cmd/issue/delete/delete.go +++ b/pkg/cmd/issue/delete/delete.go @@ -3,16 +3,13 @@ package delete import ( "fmt" "net/http" - "strconv" - "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/issue/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/shurcooL/githubv4" "github.com/spf13/cobra" ) @@ -22,16 +19,22 @@ type DeleteOptions struct { Config func() (config.Config, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) + Prompter iprompter SelectorArg string Confirmed bool } +type iprompter interface { + ConfirmDeletion(string) error +} + func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Command { opts := &DeleteOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, Config: f.Config, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -79,22 +82,12 @@ func deleteRun(opts *DeleteOptions) error { // When executed in an interactive shell, require confirmation, unless // already provided. Otherwise skip confirmation. if opts.IO.CanPrompt() && !opts.Confirmed { - answer := "" - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne( - &survey.Input{ - Message: fmt.Sprintf("You're going to delete issue #%d. This action cannot be reversed. To confirm, type the issue number:", issue.Number), - }, - &answer, - ) + cs := opts.IO.ColorScheme() + fmt.Printf("%s Deleted issues cannot be recovered.\n", cs.WarningIcon()) + err := opts.Prompter.ConfirmDeletion(fmt.Sprintf("%d", issue.Number)) if err != nil { return err } - answerInt, err := strconv.Atoi(answer) - if err != nil || answerInt != issue.Number { - fmt.Fprintf(opts.IO.Out, "Issue #%d was not deleted.\n", issue.Number) - return nil - } } if err := apiDelete(httpClient, baseRepo, issue.ID); err != nil { diff --git a/pkg/cmd/issue/delete/delete_test.go b/pkg/cmd/issue/delete/delete_test.go index e3b2613ce..acc52af65 100644 --- a/pkg/cmd/issue/delete/delete_test.go +++ b/pkg/cmd/issue/delete/delete_test.go @@ -2,6 +2,7 @@ package delete import ( "bytes" + "errors" "io" "net/http" "regexp" @@ -9,16 +10,16 @@ import ( "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/cli/cli/v2/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) -func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { +func runCommand(rt http.RoundTripper, pm *prompter.MockPrompter, isTTY bool, cli string) (*test.CmdOut, error) { ios, _, stdout, stderr := iostreams.Test() ios.SetStdoutTTY(isTTY) ios.SetStdinTTY(isTTY) @@ -26,6 +27,7 @@ func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, err factory := &cmdutil.Factory{ IOStreams: ios, + Prompter: pm, HttpClient: func() (*http.Client, error) { return &http.Client{Transport: rt}, nil }, @@ -76,11 +78,10 @@ func TestIssueDelete(t *testing.T) { }), ) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("You're going to delete issue #13. This action cannot be reversed. To confirm, type the issue number:").AnswerWith("13") + pm := prompter.NewMockPrompter(t) + pm.RegisterConfirmDeletion("13", func(_ string) error { return nil }) - output, err := runCommand(httpRegistry, true, "13") + output, err := runCommand(httpRegistry, pm, true, "13") if err != nil { t.Fatalf("error running command `issue delete`: %v", err) } @@ -112,7 +113,7 @@ func TestIssueDelete_confirm(t *testing.T) { }), ) - output, err := runCommand(httpRegistry, true, "13 --confirm") + output, err := runCommand(httpRegistry, nil, true, "13 --confirm") if err != nil { t.Fatalf("error running command `issue delete`: %v", err) } @@ -137,19 +138,17 @@ func TestIssueDelete_cancel(t *testing.T) { } } }`), ) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("You're going to delete issue #13. This action cannot be reversed. To confirm, type the issue number:").AnswerWith("14") + pm := prompter.NewMockPrompter(t) + pm.RegisterConfirmDeletion("13", func(_ string) error { + return errors.New("You entered 14") + }) - output, err := runCommand(httpRegistry, true, "13") - if err != nil { - t.Fatalf("error running command `issue delete`: %v", err) + _, err := runCommand(httpRegistry, pm, true, "13") + if err == nil { + t.Fatalf("expected error") } - - r := regexp.MustCompile(`Issue #13 was not deleted`) - - if !r.MatchString(output.String()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.String()) + if err.Error() != "You entered 14" { + t.Fatalf("got unexpected error '%s'", err) } } @@ -166,7 +165,7 @@ func TestIssueDelete_doesNotExist(t *testing.T) { `), ) - _, err := runCommand(httpRegistry, true, "13") + _, err := runCommand(httpRegistry, nil, true, "13") if err == nil || err.Error() != "GraphQL: Could not resolve to an Issue with the number of 13." { t.Errorf("error running command `issue delete`: %v", err) } @@ -199,7 +198,7 @@ func TestIssueDelete_issuesDisabled(t *testing.T) { }`), ) - _, err := runCommand(httpRegistry, true, "13") + _, err := runCommand(httpRegistry, nil, true, "13") if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { t.Fatalf("got error: %v", err) } From fe3eeb481dcc5fc8681928265ad1aaf281a62dfc Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Thu, 17 Aug 2023 17:14:56 -0500 Subject: [PATCH 23/23] linter appeasement --- pkg/cmd/workflow/run/run.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/cmd/workflow/run/run.go b/pkg/cmd/workflow/run/run.go index 4ba0a2ab4..009c48182 100644 --- a/pkg/cmd/workflow/run/run.go +++ b/pkg/cmd/workflow/run/run.go @@ -412,7 +412,6 @@ func findInputs(yamlContent []byte) ([]WorkflowInput, error) { } for name, input := range m { - input.Name = name out = append(out, WorkflowInput{ Name: name, Default: input.Default,