diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 400a56f3d..c11e635f6 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -807,7 +807,6 @@ type EditCodespaceParams struct { func (a *API) EditCodespace(ctx context.Context, codespaceName string, params *EditCodespaceParams) (*Codespace, error) { requestBody, err := json.Marshal(params) - if err != nil { return nil, fmt.Errorf("error marshaling request: %w", err) } @@ -818,7 +817,7 @@ func (a *API) EditCodespace(ctx context.Context, codespaceName string, params *E } a.setHeaders(req) - resp, err := a.do(ctx, req, "/user/codespaces") + resp, err := a.do(ctx, req, "/user/codespaces/*") if err != nil { return nil, fmt.Errorf("error making request: %w", err) } diff --git a/pkg/cmd/codespace/edit.go b/pkg/cmd/codespace/edit.go index 522a2263a..d07173623 100644 --- a/pkg/cmd/codespace/edit.go +++ b/pkg/cmd/codespace/edit.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" ) @@ -22,34 +23,30 @@ func newEditCmd(app *App) *cobra.Command { Short: "Edit a codespace", Args: noArgsConstraint, RunE: func(cmd *cobra.Command, args []string) error { + if opts.displayName == "" && opts.machine == "" { + return cmdutil.FlagErrorf("must provide `--display-name` or `--machine`") + } + return app.Edit(cmd.Context(), opts) }, } editCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "Name of the codespace") - editCmd.Flags().StringVarP(&opts.displayName, "displayName", "d", "", "display name") - editCmd.Flags().StringVarP(&opts.machine, "machine", "m", "", "hardware specifications for the VM") + editCmd.Flags().StringVarP(&opts.displayName, "display-name", "d", "", "Set the display name") + editCmd.Flags().StringVar(&opts.displayName, "displayName", "", "display name") + if err := editCmd.Flags().MarkDeprecated("displayName", "use `--display-name` instead"); err != nil { + fmt.Fprintf(app.io.ErrOut, "error marking flag as deprecated: %v\n", err) + } + editCmd.Flags().StringVarP(&opts.machine, "machine", "m", "", "Set hardware specifications for the VM") return editCmd } // Edits a codespace func (a *App) Edit(ctx context.Context, opts editOptions) error { - userInputs := struct { - CodespaceName string - DisplayName string - SKU string - }{ - CodespaceName: opts.codespaceName, - DisplayName: opts.displayName, - SKU: opts.machine, - } + codespaceName := opts.codespaceName - if userInputs.DisplayName == "" && userInputs.SKU == "" { - return fmt.Errorf("at least one property has to be edited") - } - - if userInputs.CodespaceName == "" { + if codespaceName == "" { selectedCodespace, err := chooseCodespace(ctx, a.apiClient) if err != nil { if err == errNoCodespaces { @@ -57,14 +54,13 @@ func (a *App) Edit(ctx context.Context, opts editOptions) error { } return fmt.Errorf("error choosing codespace: %w", err) } - - userInputs.CodespaceName = selectedCodespace.Name + codespaceName = selectedCodespace.Name } a.StartProgressIndicatorWithLabel("Editing codespace") - _, err := a.apiClient.EditCodespace(ctx, userInputs.CodespaceName, &api.EditCodespaceParams{ - DisplayName: userInputs.DisplayName, - Machine: userInputs.SKU, + _, err := a.apiClient.EditCodespace(ctx, codespaceName, &api.EditCodespaceParams{ + DisplayName: opts.displayName, + Machine: opts.machine, }) a.StopProgressIndicator() if err != nil { diff --git a/pkg/cmd/codespace/edit_test.go b/pkg/cmd/codespace/edit_test.go index eac08ff32..b5e0e3cd1 100644 --- a/pkg/cmd/codespace/edit_test.go +++ b/pkg/cmd/codespace/edit_test.go @@ -2,7 +2,6 @@ package codespace import ( "context" - "fmt" "testing" "github.com/cli/cli/v2/internal/codespaces/api" @@ -10,15 +9,16 @@ import ( ) func TestEdit(t *testing.T) { - tests := []struct { name string opts editOptions - codespaces []*api.Codespace + cliArgs []string // alternative to opts; will test command dispatcher + wantEdits *api.EditCodespaceParams mockCodespace *api.Codespace - editErr error - wantErr bool wantStdout string + wantStderr string + wantErr bool + errMsg string }{ { name: "edit codespace display name", @@ -27,6 +27,9 @@ func TestEdit(t *testing.T) { displayName: "hubot-changed", machine: "", }, + wantEdits: &api.EditCodespaceParams{ + DisplayName: "hubot-changed", + }, mockCodespace: &api.Codespace{ Name: "hubot", DisplayName: "hubot-changed", @@ -34,6 +37,20 @@ func TestEdit(t *testing.T) { wantStdout: "", wantErr: false, }, + { + name: "CLI legacy --displayName", + cliArgs: []string{"--codespace", "hubot", "--displayName", "hubot-changed"}, + wantEdits: &api.EditCodespaceParams{ + DisplayName: "hubot-changed", + }, + mockCodespace: &api.Codespace{ + Name: "hubot", + DisplayName: "hubot-changed", + }, + wantStdout: "", + wantStderr: "Flag --displayName has been deprecated, use `--display-name` instead\n", + wantErr: false, + }, { name: "edit codespace machine", opts: editOptions{ @@ -41,6 +58,9 @@ func TestEdit(t *testing.T) { displayName: "", machine: "machine", }, + wantEdits: &api.EditCodespaceParams{ + Machine: "machine", + }, mockCodespace: &api.Codespace{ Name: "hubot", Machine: api.CodespaceMachine{ @@ -51,56 +71,69 @@ func TestEdit(t *testing.T) { wantErr: false, }, { - name: "trying to edit a codespace without anything to edit should return an error", - opts: editOptions{ - codespaceName: "hubot", - displayName: "", - machine: "", - }, - editErr: fmt.Errorf("at least one property has to be edited"), + name: "no CLI arguments", + cliArgs: []string{}, wantErr: true, + errMsg: "must provide `--display-name` or `--machine`", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - + var gotEdits *api.EditCodespaceParams apiMock := &apiClientMock{ EditCodespaceFunc: func(_ context.Context, codespaceName string, params *api.EditCodespaceParams) (*api.Codespace, error) { - if tt.editErr != nil { - return tt.mockCodespace, tt.editErr - } + gotEdits = params return tt.mockCodespace, nil }, } - if tt.opts.codespaceName == "" { - apiMock.ListCodespacesFunc = func(_ context.Context, num int) ([]*api.Codespace, error) { - return tt.codespaces, nil - } - } - - opts := tt.opts - - ios, _, stdout, _ := iostreams.Test() - ios.SetStdinTTY(true) - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) + ios, _, stdout, stderr := iostreams.Test() a := NewApp(ios, nil, apiMock, nil) - err := a.Edit(context.Background(), opts) + var err error + if tt.cliArgs == nil { + err = a.Edit(context.Background(), tt.opts) + } else { + cmd := newEditCmd(a) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + cmd.SetOut(ios.ErrOut) + cmd.SetErr(ios.ErrOut) + cmd.SetArgs(tt.cliArgs) + _, err = cmd.ExecuteC() + } - if (err != nil) != tt.wantErr { - t.Errorf("App.Edit() error = %v, wantErr %v", err, tt.wantErr) + if tt.wantErr { + if err == nil { + t.Error("Edit() expected error, got nil") + } else if err.Error() != tt.errMsg { + t.Errorf("Edit() error = %q, want %q", err, tt.errMsg) + } + } else if err != nil { + t.Errorf("Edit() expected no error, got %v", err) } if out := stdout.String(); out != tt.wantStdout { t.Errorf("stdout = %q, want %q", out, tt.wantStdout) } - - if tt.wantErr && err.Error() != tt.editErr.Error() { - t.Errorf("stderr = %v, expected error %v", err, tt.editErr) + if out := stderr.String(); out != tt.wantStderr { + t.Errorf("stderr = %q, want %q", out, tt.wantStderr) } + if tt.wantEdits != nil { + if gotEdits == nil { + t.Fatalf("EditCodespace() never called") + } + if tt.wantEdits.DisplayName != gotEdits.DisplayName { + t.Errorf("edited display name %q, want %q", gotEdits.DisplayName, tt.wantEdits.DisplayName) + } + if tt.wantEdits.Machine != gotEdits.Machine { + t.Errorf("edited machine type %q, want %q", gotEdits.Machine, tt.wantEdits.Machine) + } + if tt.wantEdits.IdleTimeoutMinutes != gotEdits.IdleTimeoutMinutes { + t.Errorf("edited idle timeout minutes %d, want %d", gotEdits.IdleTimeoutMinutes, tt.wantEdits.IdleTimeoutMinutes) + } + } }) } }