Repository: cli/cli
Pull request #8570 (2024-01-12 21:22:14)

Align devcontainer Go version with go.mod
Update the devcontainer configuration to use the correct version of Go.

https://github.com/cli/cli/blob/72b6dc5d8c70525a6409abe6b1cbdf8d667f6193/go.mod#L3


Patch:
diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
index c9de19da2..e69a804ee 100644
--- a/.devcontainer/devcontainer.json
+++ b/.devcontainer/devcontainer.json
@@ -1,5 +1,5 @@
 {
-	"image": "mcr.microsoft.com/devcontainers/go:1.19",
+	"image": "mcr.microsoft.com/devcontainers/go:1.21",
 	"features": {
 		"ghcr.io/devcontainers/features/sshd:1": {}
 	},
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/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)
 	}
 


Test patch:
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) {