From 1084fd149bd7c72fe94c67bc608b9207d4493fda Mon Sep 17 00:00:00 2001 From: Ashwin Jeyaseelan <8gitbrix@github.com> Date: Tue, 24 May 2022 17:44:45 +0000 Subject: [PATCH 1/5] Corrected code to update codespace from selected codespace name if chosen from list of codespaces --- pkg/cmd/codespace/edit.go | 2 +- pkg/cmd/codespace/edit_test.go | 185 +++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 pkg/cmd/codespace/edit_test.go diff --git a/pkg/cmd/codespace/edit.go b/pkg/cmd/codespace/edit.go index 5fa5b00e5..110f841d1 100644 --- a/pkg/cmd/codespace/edit.go +++ b/pkg/cmd/codespace/edit.go @@ -62,7 +62,7 @@ func (a *App) Edit(ctx context.Context, opts editOptions) error { return fmt.Errorf("error choosing codespace: %w", err) } - opts.codespaceName = selectedCodespace.Name + userInputs.CodespaceName = selectedCodespace.Name } a.StartProgressIndicatorWithLabel("Editing codespace") diff --git a/pkg/cmd/codespace/edit_test.go b/pkg/cmd/codespace/edit_test.go new file mode 100644 index 000000000..df9817f4a --- /dev/null +++ b/pkg/cmd/codespace/edit_test.go @@ -0,0 +1,185 @@ +package codespace + +import ( + "context" + "errors" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/pkg/iostreams" +) + +func TestEdit(t *testing.T) { + + tests := []struct { + name string + opts deleteOptions + codespaces []*api.Codespace + deleteErr error + wantErr bool + wantDeleted []string + wantStdout string + wantStderr string + }{ + { + name: "by name", + opts: deleteOptions{ + codespaceName: "hubot-robawt-abc", + }, + codespaces: []*api.Codespace{ + { + Name: "hubot-robawt-abc", + }, + }, + wantDeleted: []string{"hubot-robawt-abc"}, + wantStdout: "", + }, + { + name: "by repo", + opts: deleteOptions{ + repoFilter: "monalisa/spoon-knife", + }, + codespaces: []*api.Codespace{ + { + Name: "monalisa-spoonknife-123", + Repository: api.Repository{ + FullName: "monalisa/Spoon-Knife", + }, + }, + { + Name: "hubot-robawt-abc", + Repository: api.Repository{ + FullName: "hubot/ROBAWT", + }, + }, + { + Name: "monalisa-spoonknife-c4f3", + Repository: api.Repository{ + FullName: "monalisa/Spoon-Knife", + }, + }, + }, + wantDeleted: []string{"monalisa-spoonknife-123", "monalisa-spoonknife-c4f3"}, + wantStdout: "", + }, + { + name: "unused", + opts: deleteOptions{ + deleteAll: true, + keepDays: 3, + }, + codespaces: []*api.Codespace{ + { + Name: "monalisa-spoonknife-123", + LastUsedAt: daysAgo(1), + }, + { + Name: "hubot-robawt-abc", + LastUsedAt: daysAgo(4), + }, + { + Name: "monalisa-spoonknife-c4f3", + LastUsedAt: daysAgo(10), + }, + }, + wantDeleted: []string{"hubot-robawt-abc", "monalisa-spoonknife-c4f3"}, + wantStdout: "", + }, + { + name: "deletion failed", + opts: deleteOptions{ + deleteAll: true, + }, + codespaces: []*api.Codespace{ + { + Name: "monalisa-spoonknife-123", + }, + { + Name: "hubot-robawt-abc", + }, + }, + deleteErr: errors.New("aborted by test"), + wantErr: true, + wantDeleted: []string{"hubot-robawt-abc", "monalisa-spoonknife-123"}, + wantStderr: heredoc.Doc(` + error deleting codespace "hubot-robawt-abc": aborted by test + error deleting codespace "monalisa-spoonknife-123": aborted by test + `), + }, + { + name: "with confirm", + opts: deleteOptions{ + isInteractive: true, + deleteAll: true, + skipConfirm: false, + }, + codespaces: []*api.Codespace{ + { + Name: "monalisa-spoonknife-123", + GitStatus: api.CodespaceGitStatus{ + HasUnpushedChanges: true, + }, + }, + { + Name: "hubot-robawt-abc", + GitStatus: api.CodespaceGitStatus{ + HasUncommitedChanges: true, + }, + }, + { + Name: "monalisa-spoonknife-c4f3", + GitStatus: api.CodespaceGitStatus{ + HasUnpushedChanges: false, + HasUncommitedChanges: false, + }, + }, + }, + wantDeleted: []string{"hubot-robawt-abc", "monalisa-spoonknife-c4f3"}, + wantStdout: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + apiMock := &apiClientMock{ + GetUserFunc: func(_ context.Context) (*api.User, error) { + return user, nil + }, + DeleteCodespaceFunc: func(_ context.Context, name string) error { + if tt.deleteErr != nil { + return tt.deleteErr + } + return nil + }, + } + if tt.opts.codespaceName == "" { + apiMock.ListCodespacesFunc = func(_ context.Context, num int) ([]*api.Codespace, error) { + return tt.codespaces, nil + } + } else { + apiMock.GetCodespaceFunc = func(_ context.Context, name string, includeConnection bool) (*api.Codespace, error) { + return tt.codespaces[0], nil + } + } + + opts := tt.opts + + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdinTTY(true) + ios.SetStdoutTTY(true) + app := NewApp(ios, nil, apiMock, nil) + err := app.Delete(context.Background(), opts) + if (err != nil) != tt.wantErr { + t.Errorf("delete() error = %v, wantErr %v", err, tt.wantErr) + } + var gotDeleted []string + for _, delArgs := range apiMock.DeleteCodespaceCalls() { + gotDeleted = append(gotDeleted, delArgs.Name) + } + + if out := stdout.String(); out != tt.wantStdout { + t.Errorf("stdout = %q, want %q", out, tt.wantStdout) + } + }) + } +} From 2e84804f958c9b2d931c166917eecd6f95b14633 Mon Sep 17 00:00:00 2001 From: Ashwin Jeyaseelan <8gitbrix@github.com> Date: Tue, 24 May 2022 17:46:30 +0000 Subject: [PATCH 2/5] Removed incorrect test file --- pkg/cmd/codespace/edit_test.go | 185 --------------------------------- 1 file changed, 185 deletions(-) delete mode 100644 pkg/cmd/codespace/edit_test.go diff --git a/pkg/cmd/codespace/edit_test.go b/pkg/cmd/codespace/edit_test.go deleted file mode 100644 index df9817f4a..000000000 --- a/pkg/cmd/codespace/edit_test.go +++ /dev/null @@ -1,185 +0,0 @@ -package codespace - -import ( - "context" - "errors" - "testing" - - "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/v2/internal/codespaces/api" - "github.com/cli/cli/v2/pkg/iostreams" -) - -func TestEdit(t *testing.T) { - - tests := []struct { - name string - opts deleteOptions - codespaces []*api.Codespace - deleteErr error - wantErr bool - wantDeleted []string - wantStdout string - wantStderr string - }{ - { - name: "by name", - opts: deleteOptions{ - codespaceName: "hubot-robawt-abc", - }, - codespaces: []*api.Codespace{ - { - Name: "hubot-robawt-abc", - }, - }, - wantDeleted: []string{"hubot-robawt-abc"}, - wantStdout: "", - }, - { - name: "by repo", - opts: deleteOptions{ - repoFilter: "monalisa/spoon-knife", - }, - codespaces: []*api.Codespace{ - { - Name: "monalisa-spoonknife-123", - Repository: api.Repository{ - FullName: "monalisa/Spoon-Knife", - }, - }, - { - Name: "hubot-robawt-abc", - Repository: api.Repository{ - FullName: "hubot/ROBAWT", - }, - }, - { - Name: "monalisa-spoonknife-c4f3", - Repository: api.Repository{ - FullName: "monalisa/Spoon-Knife", - }, - }, - }, - wantDeleted: []string{"monalisa-spoonknife-123", "monalisa-spoonknife-c4f3"}, - wantStdout: "", - }, - { - name: "unused", - opts: deleteOptions{ - deleteAll: true, - keepDays: 3, - }, - codespaces: []*api.Codespace{ - { - Name: "monalisa-spoonknife-123", - LastUsedAt: daysAgo(1), - }, - { - Name: "hubot-robawt-abc", - LastUsedAt: daysAgo(4), - }, - { - Name: "monalisa-spoonknife-c4f3", - LastUsedAt: daysAgo(10), - }, - }, - wantDeleted: []string{"hubot-robawt-abc", "monalisa-spoonknife-c4f3"}, - wantStdout: "", - }, - { - name: "deletion failed", - opts: deleteOptions{ - deleteAll: true, - }, - codespaces: []*api.Codespace{ - { - Name: "monalisa-spoonknife-123", - }, - { - Name: "hubot-robawt-abc", - }, - }, - deleteErr: errors.New("aborted by test"), - wantErr: true, - wantDeleted: []string{"hubot-robawt-abc", "monalisa-spoonknife-123"}, - wantStderr: heredoc.Doc(` - error deleting codespace "hubot-robawt-abc": aborted by test - error deleting codespace "monalisa-spoonknife-123": aborted by test - `), - }, - { - name: "with confirm", - opts: deleteOptions{ - isInteractive: true, - deleteAll: true, - skipConfirm: false, - }, - codespaces: []*api.Codespace{ - { - Name: "monalisa-spoonknife-123", - GitStatus: api.CodespaceGitStatus{ - HasUnpushedChanges: true, - }, - }, - { - Name: "hubot-robawt-abc", - GitStatus: api.CodespaceGitStatus{ - HasUncommitedChanges: true, - }, - }, - { - Name: "monalisa-spoonknife-c4f3", - GitStatus: api.CodespaceGitStatus{ - HasUnpushedChanges: false, - HasUncommitedChanges: false, - }, - }, - }, - wantDeleted: []string{"hubot-robawt-abc", "monalisa-spoonknife-c4f3"}, - wantStdout: "", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - apiMock := &apiClientMock{ - GetUserFunc: func(_ context.Context) (*api.User, error) { - return user, nil - }, - DeleteCodespaceFunc: func(_ context.Context, name string) error { - if tt.deleteErr != nil { - return tt.deleteErr - } - return nil - }, - } - if tt.opts.codespaceName == "" { - apiMock.ListCodespacesFunc = func(_ context.Context, num int) ([]*api.Codespace, error) { - return tt.codespaces, nil - } - } else { - apiMock.GetCodespaceFunc = func(_ context.Context, name string, includeConnection bool) (*api.Codespace, error) { - return tt.codespaces[0], nil - } - } - - opts := tt.opts - - ios, _, stdout, stderr := iostreams.Test() - ios.SetStdinTTY(true) - ios.SetStdoutTTY(true) - app := NewApp(ios, nil, apiMock, nil) - err := app.Delete(context.Background(), opts) - if (err != nil) != tt.wantErr { - t.Errorf("delete() error = %v, wantErr %v", err, tt.wantErr) - } - var gotDeleted []string - for _, delArgs := range apiMock.DeleteCodespaceCalls() { - gotDeleted = append(gotDeleted, delArgs.Name) - } - - if out := stdout.String(); out != tt.wantStdout { - t.Errorf("stdout = %q, want %q", out, tt.wantStdout) - } - }) - } -} From 81893b60f22f560eacb1e6d110ccb6bca9dd863f Mon Sep 17 00:00:00 2001 From: Ashwin Jeyaseelan <8gitbrix@github.com> Date: Wed, 25 May 2022 01:08:20 +0000 Subject: [PATCH 3/5] Added tests --- pkg/cmd/codespace/edit_test.go | 127 +++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 pkg/cmd/codespace/edit_test.go diff --git a/pkg/cmd/codespace/edit_test.go b/pkg/cmd/codespace/edit_test.go new file mode 100644 index 000000000..76bf7b439 --- /dev/null +++ b/pkg/cmd/codespace/edit_test.go @@ -0,0 +1,127 @@ +package codespace + +import ( + "context" + "testing" + + "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/pkg/iostreams" +) + +func TestEdit(t *testing.T) { + + tests := []struct { + name string + opts editOptions + codespaces []*api.Codespace + mockCodespace *api.Codespace + editErr error + wantErr bool + wantCodespace string + wantStdout string + wantStderr string + }{ + { + name: "edit codespace display name", + opts: editOptions{ + codespaceName: "hubot", + displayName: "hubot-changed", + machine: "", + }, + mockCodespace: &api.Codespace{ + Name: "hubot", + DisplayName: "hubot-changed", + }, + wantStdout: "", + wantErr: false, + }, + { + name: "edit codespace machine", + opts: editOptions{ + codespaceName: "hubot", + displayName: "", + machine: "machine", + }, + mockCodespace: &api.Codespace{ + Name: "hubot", + Machine: api.CodespaceMachine{ + Name: "machine", + }, + }, + wantStdout: "", + wantErr: false, + }, + { + name: "trying to edit a codespace without anything to edit should return an error", + opts: editOptions{ + codespaceName: "hubot", + displayName: "", + machine: "", + }, + wantStderr: "please pass in at least one valid argument to be edited", + wantErr: true, + }, + { + name: "select codespace to edit when no codespace input is given", + opts: editOptions{ + codespaceName: "", + displayName: "monalisa-new", + machine: "", + }, + codespaces: []*api.Codespace{ + { + Name: "monalisa-123", + DisplayName: "monalisa-old", + }, + { + Name: "hubot-robawt-abc", + DisplayName: "hubot", + }, + { + Name: "monalisa-spoonknife-c4f3", + DisplayName: "c4f3", + }, + }, + wantCodespace: "monalisa-123", + wantStdout: "", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + apiMock := &apiClientMock{ + EditCodespaceFunc: func(_ context.Context, codespaceName string, params *api.EditCodespaceParams) (*api.Codespace, error) { + if tt.editErr != nil { + return tt.mockCodespace, tt.editErr + } + 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, stderr := iostreams.Test() + ios.SetStdinTTY(true) + ios.SetStdoutTTY(true) + a := NewApp(ios, nil, apiMock, nil) + + if err := a.Edit(context.Background(), opts); (err != nil) != tt.wantErr { + t.Errorf("App.Edit() error = %v, wantErr %v", err, tt.wantErr) + } + + if out := stdout.String(); out != tt.wantStdout { + t.Errorf("stdout = %q, want %q", out, tt.wantStdout) + } + if out := stderr.String(); out != tt.wantStderr { + t.Errorf("stderr = %q, want %q", out, tt.wantStderr) + } + }) + } +} From f22183ef2793e7691e347d9707d09489f3abfb3d Mon Sep 17 00:00:00 2001 From: Ashwin Jeyaseelan <8gitbrix@github.com> Date: Wed, 25 May 2022 17:19:36 +0000 Subject: [PATCH 4/5] WIP tests --- go | 0 pkg/cmd/codespace/edit.go | 16 ++++++---------- pkg/cmd/codespace/edit_test.go | 22 ++++++++++++++++------ 3 files changed, 22 insertions(+), 16 deletions(-) create mode 100644 go diff --git a/go b/go new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/cmd/codespace/edit.go b/pkg/cmd/codespace/edit.go index 110f841d1..223dc6285 100644 --- a/pkg/cmd/codespace/edit.go +++ b/pkg/cmd/codespace/edit.go @@ -46,20 +46,16 @@ func (a *App) Edit(ctx context.Context, opts editOptions) error { } if userInputs.DisplayName == "" && userInputs.SKU == "" { - return fmt.Errorf("please pass in at least one valid argument to be edited") + return fmt.Errorf("at least one property has to be edited") } if userInputs.CodespaceName == "" { - a.StartProgressIndicatorWithLabel("Fetching codespaces") - codespaces, err := a.apiClient.ListCodespaces(ctx, -1) - a.StopProgressIndicator() + selectedCodespace, err := chooseCodespace(ctx, a.apiClient) if err != nil { - return fmt.Errorf("error getting codespaces to select from: %w", err) - } - - selectedCodespace, err := chooseCodespaceFromList(ctx, codespaces) - if err != nil { - return fmt.Errorf("error choosing codespace: %w", err) + if err == errNoCodespaces { + return err + } + return fmt.Errorf("choosing codespace: %w", err) } userInputs.CodespaceName = selectedCodespace.Name diff --git a/pkg/cmd/codespace/edit_test.go b/pkg/cmd/codespace/edit_test.go index 76bf7b439..a974d7cdc 100644 --- a/pkg/cmd/codespace/edit_test.go +++ b/pkg/cmd/codespace/edit_test.go @@ -17,7 +17,6 @@ func TestEdit(t *testing.T) { mockCodespace *api.Codespace editErr error wantErr bool - wantCodespace string wantStdout string wantStderr string }{ @@ -58,7 +57,7 @@ func TestEdit(t *testing.T) { displayName: "", machine: "", }, - wantStderr: "please pass in at least one valid argument to be edited", + wantStderr: "at least one property has to be edited", wantErr: true, }, { @@ -82,9 +81,15 @@ func TestEdit(t *testing.T) { DisplayName: "c4f3", }, }, - wantCodespace: "monalisa-123", - wantStdout: "", - wantErr: false, + mockCodespace: &api.Codespace{ + Name: "hubot", + Machine: api.CodespaceMachine{ + Name: "monalisa-123", + DisplayName: "monalisa-new", + }, + }, + wantStdout: "", + wantErr: false, }, } for _, tt := range tests { @@ -110,18 +115,23 @@ func TestEdit(t *testing.T) { ios, _, stdout, stderr := iostreams.Test() ios.SetStdinTTY(true) ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) a := NewApp(ios, nil, apiMock, nil) - if err := a.Edit(context.Background(), opts); (err != nil) != tt.wantErr { + err := a.Edit(context.Background(), opts) + + if (err != nil) != tt.wantErr { t.Errorf("App.Edit() error = %v, wantErr %v", err, tt.wantErr) } if out := stdout.String(); out != tt.wantStdout { t.Errorf("stdout = %q, want %q", out, tt.wantStdout) } + if out := stderr.String(); out != tt.wantStderr { t.Errorf("stderr = %q, want %q", out, tt.wantStderr) } + }) } } From a1053c9c43262a830bbf48fdc3a5915815a999d2 Mon Sep 17 00:00:00 2001 From: Ashwin Jeyaseelan <8gitbrix@github.com> Date: Wed, 25 May 2022 18:55:36 +0000 Subject: [PATCH 5/5] Fixed error checking in test instead of incorrectly using stderr --- pkg/cmd/codespace/edit_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/codespace/edit_test.go b/pkg/cmd/codespace/edit_test.go index a974d7cdc..b35a7fb6a 100644 --- a/pkg/cmd/codespace/edit_test.go +++ b/pkg/cmd/codespace/edit_test.go @@ -2,6 +2,7 @@ package codespace import ( "context" + "fmt" "testing" "github.com/cli/cli/v2/internal/codespaces/api" @@ -18,7 +19,6 @@ func TestEdit(t *testing.T) { editErr error wantErr bool wantStdout string - wantStderr string }{ { name: "edit codespace display name", @@ -57,8 +57,8 @@ func TestEdit(t *testing.T) { displayName: "", machine: "", }, - wantStderr: "at least one property has to be edited", - wantErr: true, + editErr: fmt.Errorf("at least one property has to be edited"), + wantErr: true, }, { name: "select codespace to edit when no codespace input is given", @@ -112,7 +112,7 @@ func TestEdit(t *testing.T) { opts := tt.opts - ios, _, stdout, stderr := iostreams.Test() + ios, _, stdout, _ := iostreams.Test() ios.SetStdinTTY(true) ios.SetStdoutTTY(true) ios.SetStderrTTY(true) @@ -128,8 +128,8 @@ func TestEdit(t *testing.T) { t.Errorf("stdout = %q, want %q", out, tt.wantStdout) } - if out := stderr.String(); out != tt.wantStderr { - t.Errorf("stderr = %q, want %q", out, tt.wantStderr) + if tt.wantErr && err.Error() != tt.editErr.Error() { + t.Errorf("stderr = %v, expected error %v", err, tt.editErr) } })