diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index 99d1f339c..fc70febee 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -114,10 +114,11 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace, i }, } + prompter := &Prompter{} var answers struct { Codespace int } - if err := ask(csSurvey, &answers); err != nil { + if err := prompter.Ask(csSurvey, &answers); err != nil { return nil, fmt.Errorf("error getting answers: %w", err) } @@ -145,9 +146,15 @@ func safeClose(closer io.Closer, err *error) { // It is not portable to assume stdin/stdout are fds 0 and 1. var hasTTY = term.IsTerminal(int(os.Stdin.Fd())) && term.IsTerminal(int(os.Stdout.Fd())) +type SurveyPrompter interface { + Ask(qs []*survey.Question, response interface{}) error +} + +type Prompter struct{} + // ask asks survey questions on the terminal, using standard options. // It fails unless hasTTY, but ideally callers should avoid calling it in that case. -func ask(qs []*survey.Question, response interface{}) error { +func (p *Prompter) Ask(qs []*survey.Question, response interface{}) error { if !hasTTY { return fmt.Errorf("no terminal") } diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 821faa10b..67eb5461e 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -140,6 +140,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { return a.browser.Browse(fmt.Sprintf("%s/codespaces/new", a.apiClient.ServerURL())) } + prompter := &Prompter{} promptForRepoAndBranch := userInputs.Repository == "" && !opts.useWeb if promptForRepoAndBranch { var defaultRepo string @@ -167,7 +168,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { Validate: survey.Required, }, } - if err := ask(repoQuestions, &userInputs); err != nil { + if err := prompter.Ask(repoQuestions, &userInputs); err != nil { return fmt.Errorf("failed to prompt: %w", err) } } @@ -212,7 +213,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { }, } - if err := ask(branchQuestions, &userInputs); err != nil { + if err := prompter.Ask(branchQuestions, &userInputs); err != nil { return fmt.Errorf("failed to prompt: %w", err) } } @@ -259,7 +260,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { }, } - if err := ask([]*survey.Question{devContainerPathQuestion}, &devContainerPath); err != nil { + if err := prompter.Ask([]*survey.Question{devContainerPathQuestion}, &devContainerPath); err != nil { return fmt.Errorf("failed to prompt: %w", err) } } @@ -277,7 +278,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { // web UI also provide a way to select machine type // therefore we let the user choose from the web UI instead of prompting from CLI if !(opts.useWeb && opts.machine == "") { - machine, err = getMachineName(ctx, a.apiClient, repository.ID, opts.machine, branch, userInputs.Location, devContainerPath) + machine, err = getMachineName(ctx, a.apiClient, prompter, repository.ID, opts.machine, branch, userInputs.Location, devContainerPath) if err != nil { return fmt.Errorf("error getting machine type: %w", err) } @@ -320,7 +321,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { return fmt.Errorf("error creating codespace: %w", err) } - codespace, err = a.handleAdditionalPermissions(ctx, createParams, aerr.AllowPermissionsURL) + codespace, err = a.handleAdditionalPermissions(ctx, prompter, createParams, aerr.AllowPermissionsURL) if err != nil { // this error could be a cmdutil.SilentError (in the case that the user opened the browser) so we don't want to wrap it return err @@ -344,7 +345,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { return nil } -func (a *App) handleAdditionalPermissions(ctx context.Context, createParams *api.CreateCodespaceParams, allowPermissionsURL string) (*api.Codespace, error) { +func (a *App) handleAdditionalPermissions(ctx context.Context, prompter SurveyPrompter, createParams *api.CreateCodespaceParams, allowPermissionsURL string) (*api.Codespace, error) { var ( isInteractive = a.io.CanPrompt() cs = a.io.ColorScheme() @@ -379,7 +380,7 @@ func (a *App) handleAdditionalPermissions(ctx context.Context, createParams *api Accept string } - if err := ask(permsSurvey, &answers); err != nil { + if err := prompter.Ask(permsSurvey, &answers); err != nil { return nil, fmt.Errorf("error getting answers: %w", err) } @@ -393,12 +394,12 @@ func (a *App) handleAdditionalPermissions(ctx context.Context, createParams *api if err := a.pollForPermissions(ctx, createParams); err != nil { return nil, fmt.Errorf("error polling for permissions: %w", err) } + } else { + // If the user chose to create the codespace without the permissions, + // we can continue with the create opting out of the additional permissions + createParams.PermissionsOptOut = true } - // if the user chose to create the codespace without the permissions, - // we can continue with the create opting out of the additional permissions - createParams.PermissionsOptOut = true - var codespace *api.Codespace err := a.RunWithProgress("Creating codespace", func() (err error) { codespace, err = a.apiClient.CreateCodespace(ctx, createParams) @@ -510,7 +511,7 @@ func (a *App) showStatus(ctx context.Context, codespace *api.Codespace) error { } // getMachineName prompts the user to select the machine type, or validates the machine if non-empty. -func getMachineName(ctx context.Context, apiClient apiClient, repoID int, machine, branch, location string, devcontainerPath string) (string, error) { +func getMachineName(ctx context.Context, apiClient apiClient, prompter SurveyPrompter, repoID int, machine, branch, location string, devcontainerPath string) (string, error) { machines, err := apiClient.GetCodespacesMachines(ctx, repoID, branch, location, devcontainerPath) if err != nil { return "", fmt.Errorf("error requesting machine instance types: %w", err) @@ -561,7 +562,7 @@ func getMachineName(ctx context.Context, apiClient apiClient, repoID int, machin } var machineAnswers struct{ Machine string } - if err := ask(machineSurvey, &machineAnswers); err != nil { + if err := prompter.Ask(machineSurvey, &machineAnswers); err != nil { return "", fmt.Errorf("error getting machine: %w", err) } diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index 78039db0d..6466d9be7 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/codespaces/api" "github.com/cli/cli/v2/pkg/cmdutil" @@ -703,6 +704,127 @@ func TestBuildDisplayName(t *testing.T) { } } +type MockSurveyPrompter struct { + AskFunc func(qs []*survey.Question, response interface{}) error +} + +func (m *MockSurveyPrompter) Ask(qs []*survey.Question, response interface{}) error { + return m.AskFunc(qs, response) +} + +type MockBrowser struct { + Err error +} + +func (b *MockBrowser) Browse(url string) error { + if b.Err != nil { + return b.Err + } + + return nil +} + +func TestHandleAdditionalPermissions(t *testing.T) { + tests := []struct { + name string + isInteractive bool + accept string + permissionsOptOut bool + browserErr error + pollForPermissionsErr error + createCodespaceErr error + wantErr bool + }{ + { + name: "non-interactive", + isInteractive: false, + permissionsOptOut: false, + wantErr: true, + }, + { + name: "interactive, continue in browser, browser error", + isInteractive: true, + accept: "Continue in browser to review and authorize additional permissions (Recommended)", + permissionsOptOut: false, + browserErr: fmt.Errorf("browser error"), + wantErr: true, + }, + { + name: "interactive, continue in browser, poll for permissions error", + isInteractive: true, + accept: "Continue in browser to review and authorize additional permissions (Recommended)", + permissionsOptOut: false, + pollForPermissionsErr: fmt.Errorf("poll for permissions error"), + wantErr: true, + }, + { + name: "interactive, continue in browser, create codespace error", + isInteractive: true, + accept: "Continue in browser to review and authorize additional permissions (Recommended)", + permissionsOptOut: false, + createCodespaceErr: fmt.Errorf("create codespace error"), + wantErr: true, + }, + { + name: "interactive, continue without authorizing", + isInteractive: true, + accept: "Continue without authorizing additional permissions", + permissionsOptOut: true, + createCodespaceErr: fmt.Errorf("create codespace error"), + wantErr: true, + }, + { + name: "interactive, continue without authorizing, create codespace success", + isInteractive: true, + accept: "Continue without authorizing additional permissions", + permissionsOptOut: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + a := &App{ + io: ios, + browser: &MockBrowser{ + Err: tt.browserErr, + }, + apiClient: &apiClientMock{ + CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { + return nil, tt.createCodespaceErr + }, + GetCodespacesPermissionsCheckFunc: func(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) { + if tt.pollForPermissionsErr != nil { + return false, tt.pollForPermissionsErr + } + return true, nil + }, + }, + } + + if tt.isInteractive { + a.io.SetStdinTTY(true) + a.io.SetStdoutTTY(true) + a.io.SetStderrTTY(true) + } + + params := &api.CreateCodespaceParams{} + _, err := a.handleAdditionalPermissions(context.Background(), &MockSurveyPrompter{ + AskFunc: func(qs []*survey.Question, response interface{}) error { + *response.(*struct{ Accept string }) = struct{ Accept string }{Accept: tt.accept} + return nil + }, + }, params, "http://example.com") + if (err != nil) != tt.wantErr { + t.Errorf("handleAdditionalPermissions() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.permissionsOptOut != params.PermissionsOptOut { + t.Errorf("handleAdditionalPermissions() permissionsOptOut = %v, want %v", params.PermissionsOptOut, tt.permissionsOptOut) + } + }) + } +} + func apiCreateDefaults(c *apiClientMock) *apiClientMock { if c.GetRepositoryFunc == nil { c.GetRepositoryFunc = func(ctx context.Context, nwo string) (*api.Repository, error) { diff --git a/pkg/cmd/codespace/delete.go b/pkg/cmd/codespace/delete.go index e54bf09fb..5da24a634 100644 --- a/pkg/cmd/codespace/delete.go +++ b/pkg/cmd/codespace/delete.go @@ -220,6 +220,7 @@ func confirmDeletion(p prompter, apiCodespace *api.Codespace, isInteractive bool type surveyPrompter struct{} func (p *surveyPrompter) Confirm(message string) (bool, error) { + prompter := &Prompter{} var confirmed struct { Confirmed bool } @@ -231,7 +232,7 @@ func (p *surveyPrompter) Confirm(message string) (bool, error) { }, }, } - if err := ask(q, &confirmed); err != nil { + if err := prompter.Ask(q, &confirmed); err != nil { return false, fmt.Errorf("failed to prompt: %w", err) }