From 68f4cad1af18a1a10f8bea2ef8a3e78a11df6567 Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Thu, 16 Sep 2021 16:42:53 +0200 Subject: [PATCH 01/19] implement delete all with thresold Signed-off-by: Raffaele Di Fazio --- cmd/ghcs/delete.go | 47 +++++++++++- internal/api/api.go | 49 +++++++++---- internal/api/api_test.go | 155 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 234 insertions(+), 17 deletions(-) create mode 100644 internal/api/api_test.go diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 961310675..a07559cfe 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -15,10 +15,11 @@ import ( func newDeleteCmd() *cobra.Command { var ( - codespace string - allCodespaces bool - repo string - force bool + codespace string + allCodespaces bool + repo string + force bool + keepThresholdDays int ) log := output.NewLogger(os.Stdout, os.Stderr, false) @@ -29,6 +30,8 @@ func newDeleteCmd() *cobra.Command { switch { case allCodespaces && repo != "": return errors.New("both --all and --repo is not supported.") + case allCodespaces && keepThresholdDays != 0: + return deleteWithThreshold(log, keepThresholdDays) case allCodespaces: return deleteAll(log, force) case repo != "": @@ -43,6 +46,7 @@ func newDeleteCmd() *cobra.Command { deleteCmd.Flags().BoolVar(&allCodespaces, "all", false, "Delete all codespaces") deleteCmd.Flags().StringVarP(&repo, "repo", "r", "", "Delete all codespaces for a repository") deleteCmd.Flags().BoolVarP(&force, "force", "f", false, "Delete codespaces with unsaved changes without confirmation") + deleteCmd.Flags().IntVar(&keepThresholdDays, "days", 0, "Value of threshold for codespaces to keep") return deleteCmd } @@ -199,3 +203,38 @@ func confirmDeletion(codespace *api.Codespace, force bool) (bool, error) { return confirmed.Confirmed, nil } + +func deleteWithThreshold(log *output.Logger, keepThresholdDays int) error { + apiClient := api.New(os.Getenv("GITHUB_TOKEN")) + ctx := context.Background() + + user, err := apiClient.GetUser(ctx) + if err != nil { + return fmt.Errorf("error getting user: %v", err) + } + + codespaces, err := apiClient.ListCodespaces(ctx, user) + if err != nil { + return fmt.Errorf("error getting codespaces: %v", err) + } + + codespacesToDelete, err := apiClient.FilterCodespacesToDelete(codespaces, keepThresholdDays) + if err != nil { + return err + } + + for _, c := range codespacesToDelete { + token, err := apiClient.GetCodespaceToken(ctx, user.Login, c.Name) + if err != nil { + return fmt.Errorf("error getting codespace token: %v", err) + } + + if err := apiClient.DeleteCodespace(ctx, user, token, c.Name); err != nil { + return fmt.Errorf("error deleting codespace: %v", err) + } + + log.Printf("Codespace deleted: %s\n", c.Name) + } + + return list(&listOptions{}) +} diff --git a/internal/api/api.go b/internal/api/api.go index 1246389e8..e0d506c02 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -35,19 +35,23 @@ import ( "net/http" "strconv" "strings" + "time" "github.com/opentracing/opentracing-go" ) const githubAPI = "https://api.github.com" +var now func() time.Time = time.Now + type API struct { - token string - client *http.Client + token string + client *http.Client + githubAPI string } func New(token string) *API { - return &API{token, &http.Client{}} + return &API{token, &http.Client{}, githubAPI} } type User struct { @@ -55,7 +59,7 @@ type User struct { } func (a *API) GetUser(ctx context.Context) (*User, error) { - req, err := http.NewRequest(http.MethodGet, githubAPI+"/user", nil) + req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/user", nil) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) } @@ -100,7 +104,7 @@ type Repository struct { } func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error) { - req, err := http.NewRequest(http.MethodGet, githubAPI+"/repos/"+strings.ToLower(nwo), nil) + req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/repos/"+strings.ToLower(nwo), nil) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) } @@ -133,6 +137,7 @@ type Codespace struct { Name string `json:"name"` GUID string `json:"guid"` CreatedAt string `json:"created_at"` + LastUsedAt string `json:"last_used_at"` Branch string `json:"branch"` RepositoryName string `json:"repository_name"` RepositoryNWO string `json:"repository_nwo"` @@ -168,7 +173,7 @@ type CodespaceEnvironmentConnection struct { func (a *API) ListCodespaces(ctx context.Context, user *User) ([]*Codespace, error) { req, err := http.NewRequest( - http.MethodGet, githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces", nil, + http.MethodGet, a.githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces", nil, ) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) @@ -215,7 +220,7 @@ func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName s req, err := http.NewRequest( http.MethodPost, - githubAPI+"/vscs_internal/user/"+ownerLogin+"/codespaces/"+codespaceName+"/token", + a.githubAPI+"/vscs_internal/user/"+ownerLogin+"/codespaces/"+codespaceName+"/token", bytes.NewBuffer(reqBody), ) if err != nil { @@ -249,7 +254,7 @@ func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName s func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) (*Codespace, error) { req, err := http.NewRequest( http.MethodGet, - githubAPI+"/vscs_internal/user/"+owner+"/codespaces/"+codespace, + a.githubAPI+"/vscs_internal/user/"+owner+"/codespaces/"+codespace, nil, ) if err != nil { @@ -283,7 +288,7 @@ func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codespace) error { req, err := http.NewRequest( http.MethodPost, - githubAPI+"/vscs_internal/proxy/environments/"+codespace.GUID+"/start", + a.githubAPI+"/vscs_internal/proxy/environments/"+codespace.GUID+"/start", nil, ) if err != nil { @@ -357,7 +362,7 @@ type SKU struct { } func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Repository, branch, location string) ([]*SKU, error) { - req, err := http.NewRequest(http.MethodGet, githubAPI+"/vscs_internal/user/"+user.Login+"/skus", nil) + req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/vscs_internal/user/"+user.Login+"/skus", nil) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) } @@ -407,7 +412,7 @@ func (a *API) CreateCodespace(ctx context.Context, user *User, repository *Repos return nil, fmt.Errorf("error marshaling request: %w", err) } - req, err := http.NewRequest(http.MethodPost, githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces", bytes.NewBuffer(requestBody)) + req, err := http.NewRequest(http.MethodPost, a.githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces", bytes.NewBuffer(requestBody)) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) } @@ -437,7 +442,7 @@ func (a *API) CreateCodespace(ctx context.Context, user *User, repository *Repos } func (a *API) DeleteCodespace(ctx context.Context, user *User, token, codespaceName string) error { - req, err := http.NewRequest(http.MethodDelete, githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces/"+codespaceName, nil) + req, err := http.NewRequest(http.MethodDelete, a.githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces/"+codespaceName, nil) if err != nil { return fmt.Errorf("error creating request: %w", err) } @@ -465,7 +470,7 @@ type getCodespaceRepositoryContentsResponse struct { } func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Codespace, path string) ([]byte, error) { - req, err := http.NewRequest(http.MethodGet, githubAPI+"/repos/"+codespace.RepositoryNWO+"/contents/"+path, nil) + req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/repos/"+codespace.RepositoryNWO+"/contents/"+path, nil) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) } @@ -507,6 +512,24 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod return decoded, nil } +func (a *API) FilterCodespacesToDelete(codespaces []*Codespace, keepThresholdDays int) ([]*Codespace, error) { + if keepThresholdDays < 0 { + return nil, fmt.Errorf("invalid value for threshold: %d", keepThresholdDays) + } + codespacesToDelete := []*Codespace{} + for _, codespace := range codespaces { + // get a date from a string representation + t, err := time.Parse(time.RFC3339, codespace.LastUsedAt) + if err != nil { + return nil, fmt.Errorf("error parsing last used at date: %v", err) + } + if t.Before(now().AddDate(0, 0, -keepThresholdDays)) && codespace.Environment.State == "Shutdown" { + codespacesToDelete = append(codespacesToDelete, codespace) + } + } + return codespacesToDelete, nil +} + func (a *API) do(ctx context.Context, req *http.Request, spanName string) (*http.Response, error) { // TODO(adonovan): use NewRequestWithContext(ctx) and drop ctx parameter. span, ctx := opentracing.StartSpanFromContext(ctx, spanName) diff --git a/internal/api/api_test.go b/internal/api/api_test.go new file mode 100644 index 000000000..6653248e1 --- /dev/null +++ b/internal/api/api_test.go @@ -0,0 +1,155 @@ +package api + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" +) + +func TestListCodespaces(t *testing.T) { + user := &User{ + Login: "testuser", + } + + codespaces := []*Codespace{ + { + Name: "testcodespace", + CreatedAt: "2021-08-09T10:10:24+02:00", + LastUsedAt: "2021-08-09T13:10:24+02:00", + }, + } + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + response := struct { + Codespaces []*Codespace `json:"codespaces"` + }{ + Codespaces: codespaces, + } + data, _ := json.Marshal(response) + fmt.Fprint(w, string(data)) + })) + defer svr.Close() + + api := API{ + githubAPI: svr.URL, + client: &http.Client{}, + token: "faketoken", + } + ctx := context.TODO() + codespaces, err := api.ListCodespaces(ctx, user) + if err != nil { + t.Fatal(err) + } + + if len(codespaces) != 1 { + t.Fatalf("expected 1 codespace, got %d", len(codespaces)) + } + + if codespaces[0].Name != "testcodespace" { + t.Fatalf("expected testcodespace, got %s", codespaces[0].Name) + } + +} + +func TestCleanupUnusedCodespaces(t *testing.T) { + type args struct { + codespaces []*Codespace + thresholdDays int + } + tests := []struct { + name string + now time.Time + args args + wantErr bool + deleted []*Codespace + }{ + { + name: "no codespaces is to be deleted", + + args: args{ + codespaces: []*Codespace{ + { + Name: "testcodespace", + CreatedAt: "2021-08-09T10:10:24+02:00", + LastUsedAt: "2021-08-09T13:10:24+02:00", + Environment: CodespaceEnvironment{ + State: "Shutdown", + }, + }, + }, + thresholdDays: 1, + }, + now: time.Date(2021, 8, 9, 20, 10, 24, 0, time.UTC), + deleted: []*Codespace{}, + }, + { + name: "one codespace is to be deleted", + + args: args{ + codespaces: []*Codespace{ + { + Name: "testcodespace", + CreatedAt: "2021-08-09T10:10:24+02:00", + LastUsedAt: "2021-08-09T13:10:24+02:00", + Environment: CodespaceEnvironment{ + State: "Shutdown", + }, + }, + }, + thresholdDays: 1, + }, + now: time.Date(2021, 8, 15, 20, 12, 24, 0, time.UTC), + deleted: []*Codespace{ + { + Name: "testcodespace", + CreatedAt: "2021-08-09T10:10:24+02:00", + LastUsedAt: "2021-08-09T13:10:24+02:00", + }, + }, + }, + { + name: "threshold is invalid", + + args: args{ + codespaces: []*Codespace{ + { + Name: "testcodespace", + CreatedAt: "2021-08-09T10:10:24+02:00", + LastUsedAt: "2021-08-09T13:10:24+02:00", + Environment: CodespaceEnvironment{ + State: "Shutdown", + }, + }, + }, + thresholdDays: -1, + }, + now: time.Date(2021, 8, 15, 20, 12, 24, 0, time.UTC), + wantErr: true, + deleted: []*Codespace{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + now = func() time.Time { + return tt.now + } + + a := &API{ + token: "testtoken", + client: &http.Client{}, + } + codespaces, err := a.FilterCodespacesToDelete(tt.args.codespaces, tt.args.thresholdDays) + if (err != nil) != tt.wantErr { + t.Errorf("API.CleanupUnusedCodespaces() error = %v, wantErr %v", err, tt.wantErr) + } + + if len(codespaces) != len(tt.deleted) { + t.Errorf("expected %d deleted codespaces, got %d", len(tt.deleted), len(codespaces)) + } + }) + } +} From 5cd90fea889c9a3a496fa4b16030933a5b58bcfe Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Thu, 16 Sep 2021 16:45:07 +0200 Subject: [PATCH 02/19] fix linter Signed-off-by: Raffaele Di Fazio --- cmd/ghcs/delete.go | 8 ++++---- internal/api/api.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index a07559cfe..cb718d47a 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -210,12 +210,12 @@ func deleteWithThreshold(log *output.Logger, keepThresholdDays int) error { user, err := apiClient.GetUser(ctx) if err != nil { - return fmt.Errorf("error getting user: %v", err) + return fmt.Errorf("error getting user: %w", err) } codespaces, err := apiClient.ListCodespaces(ctx, user) if err != nil { - return fmt.Errorf("error getting codespaces: %v", err) + return fmt.Errorf("error getting codespaces: %w", err) } codespacesToDelete, err := apiClient.FilterCodespacesToDelete(codespaces, keepThresholdDays) @@ -226,11 +226,11 @@ func deleteWithThreshold(log *output.Logger, keepThresholdDays int) error { for _, c := range codespacesToDelete { token, err := apiClient.GetCodespaceToken(ctx, user.Login, c.Name) if err != nil { - return fmt.Errorf("error getting codespace token: %v", err) + return fmt.Errorf("error getting codespace token: %w", err) } if err := apiClient.DeleteCodespace(ctx, user, token, c.Name); err != nil { - return fmt.Errorf("error deleting codespace: %v", err) + return fmt.Errorf("error deleting codespace: %w", err) } log.Printf("Codespace deleted: %s\n", c.Name) diff --git a/internal/api/api.go b/internal/api/api.go index e0d506c02..db26191a8 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -521,7 +521,7 @@ func (a *API) FilterCodespacesToDelete(codespaces []*Codespace, keepThresholdDay // get a date from a string representation t, err := time.Parse(time.RFC3339, codespace.LastUsedAt) if err != nil { - return nil, fmt.Errorf("error parsing last used at date: %v", err) + return nil, fmt.Errorf("error parsing last used at date: %w", err) } if t.Before(now().AddDate(0, 0, -keepThresholdDays)) && codespace.Environment.State == "Shutdown" { codespacesToDelete = append(codespacesToDelete, codespace) From 35e0f95243e1048033748de94afb932d6fa401e8 Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Thu, 16 Sep 2021 18:42:41 +0200 Subject: [PATCH 03/19] Update cmd/ghcs/delete.go Co-authored-by: CamiloGarciaLaRotta --- cmd/ghcs/delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index cb718d47a..8f3027f6f 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -46,7 +46,7 @@ func newDeleteCmd() *cobra.Command { deleteCmd.Flags().BoolVar(&allCodespaces, "all", false, "Delete all codespaces") deleteCmd.Flags().StringVarP(&repo, "repo", "r", "", "Delete all codespaces for a repository") deleteCmd.Flags().BoolVarP(&force, "force", "f", false, "Delete codespaces with unsaved changes without confirmation") - deleteCmd.Flags().IntVar(&keepThresholdDays, "days", 0, "Value of threshold for codespaces to keep") + deleteCmd.Flags().IntVar(&keepThresholdDays, "days", 0, "Minimum number of days that a codespace has to have to be deleted. Only shutdown codespaces will be considered for deletion.") return deleteCmd } From 22e9da790c92e6e65b5ec531a466856bda0fc2a3 Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Thu, 16 Sep 2021 18:43:16 +0200 Subject: [PATCH 04/19] Update internal/api/api_test.go Co-authored-by: CamiloGarciaLaRotta --- internal/api/api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 6653248e1..22e9ac234 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -54,7 +54,7 @@ func TestListCodespaces(t *testing.T) { } -func TestCleanupUnusedCodespaces(t *testing.T) { +func TestDeleteCodespacesByAge(t *testing.T) { type args struct { codespaces []*Codespace thresholdDays int From 455dabb484f818cb88e1e204b98ff86e2cf5cb8f Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Thu, 16 Sep 2021 18:49:44 +0200 Subject: [PATCH 05/19] use named params Signed-off-by: Raffaele Di Fazio --- internal/api/api.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/api/api.go b/internal/api/api.go index db26191a8..d7aee0f66 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -51,7 +51,11 @@ type API struct { } func New(token string) *API { - return &API{token, &http.Client{}, githubAPI} + return &API{ + token: token, + client: &http.Client{}, + githubAPI: githubAPI, + } } type User struct { From 29c2a17866cc03f3c701b7503aa26b90d0950c97 Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Fri, 17 Sep 2021 08:55:54 +0200 Subject: [PATCH 06/19] Update cmd/ghcs/delete.go Co-authored-by: Jose Garcia --- cmd/ghcs/delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 8f3027f6f..6aeedb4c5 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -46,7 +46,7 @@ func newDeleteCmd() *cobra.Command { deleteCmd.Flags().BoolVar(&allCodespaces, "all", false, "Delete all codespaces") deleteCmd.Flags().StringVarP(&repo, "repo", "r", "", "Delete all codespaces for a repository") deleteCmd.Flags().BoolVarP(&force, "force", "f", false, "Delete codespaces with unsaved changes without confirmation") - deleteCmd.Flags().IntVar(&keepThresholdDays, "days", 0, "Minimum number of days that a codespace has to have to be deleted. Only shutdown codespaces will be considered for deletion.") + deleteCmd.Flags().IntVar(&keepThresholdDays, "days", 0, "Minimum number of days since the codespace was created") return deleteCmd } From 054fec0ba117508cb761e527bcf7a30d449b9a89 Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Fri, 17 Sep 2021 14:45:08 +0200 Subject: [PATCH 07/19] address code comments Signed-off-by: Raffaele Di Fazio --- cmd/ghcs/delete.go | 55 +++++++++------------ internal/api/api.go | 24 +--------- internal/api/api_test.go | 101 --------------------------------------- 3 files changed, 23 insertions(+), 157 deletions(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 6aeedb4c5..4ad776cf9 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "strings" + "time" "github.com/AlecAivazis/survey/v2" "github.com/github/ghcs/cmd/ghcs/output" @@ -13,6 +14,8 @@ import ( "github.com/spf13/cobra" ) +var now func() time.Time = time.Now + func newDeleteCmd() *cobra.Command { var ( codespace string @@ -30,10 +33,8 @@ func newDeleteCmd() *cobra.Command { switch { case allCodespaces && repo != "": return errors.New("both --all and --repo is not supported.") - case allCodespaces && keepThresholdDays != 0: - return deleteWithThreshold(log, keepThresholdDays) case allCodespaces: - return deleteAll(log, force) + return deleteAll(log, force, keepThresholdDays) case repo != "": return deleteByRepo(log, repo, force) default: @@ -87,7 +88,7 @@ func delete_(log *output.Logger, codespaceName string, force bool) error { return list(&listOptions{}) } -func deleteAll(log *output.Logger, force bool) error { +func deleteAll(log *output.Logger, force bool, keepThresholdDays int) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() @@ -101,7 +102,12 @@ func deleteAll(log *output.Logger, force bool) error { return fmt.Errorf("error getting codespaces: %w", err) } - for _, c := range codespaces { + codespacesToDelete, err := filterCodespacesToDelete(codespaces, keepThresholdDays) + if err != nil { + return err + } + + for _, c := range codespacesToDelete { confirmed, err := confirmDeletion(c, force) if err != nil { return fmt.Errorf("deletion could not be confirmed: %w", err) @@ -204,37 +210,20 @@ func confirmDeletion(codespace *api.Codespace, force bool) (bool, error) { return confirmed.Confirmed, nil } -func deleteWithThreshold(log *output.Logger, keepThresholdDays int) error { - apiClient := api.New(os.Getenv("GITHUB_TOKEN")) - ctx := context.Background() - - user, err := apiClient.GetUser(ctx) - if err != nil { - return fmt.Errorf("error getting user: %w", err) +func filterCodespacesToDelete(codespaces []*api.Codespace, keepThresholdDays int) ([]*api.Codespace, error) { + if keepThresholdDays < 0 { + return nil, fmt.Errorf("invalid value for threshold: %d", keepThresholdDays) } - - codespaces, err := apiClient.ListCodespaces(ctx, user) - if err != nil { - return fmt.Errorf("error getting codespaces: %w", err) - } - - codespacesToDelete, err := apiClient.FilterCodespacesToDelete(codespaces, keepThresholdDays) - if err != nil { - return err - } - - for _, c := range codespacesToDelete { - token, err := apiClient.GetCodespaceToken(ctx, user.Login, c.Name) + codespacesToDelete := []*api.Codespace{} + for _, codespace := range codespaces { + // get a date from a string representation + t, err := time.Parse(time.RFC3339, codespace.LastUsedAt) if err != nil { - return fmt.Errorf("error getting codespace token: %w", err) + return nil, fmt.Errorf("error parsing last used at date: %w", err) } - - if err := apiClient.DeleteCodespace(ctx, user, token, c.Name); err != nil { - return fmt.Errorf("error deleting codespace: %w", err) + if t.Before(now().AddDate(0, 0, -keepThresholdDays)) && codespace.Environment.State == "Shutdown" { + codespacesToDelete = append(codespacesToDelete, codespace) } - - log.Printf("Codespace deleted: %s\n", c.Name) } - - return list(&listOptions{}) + return codespacesToDelete, nil } diff --git a/internal/api/api.go b/internal/api/api.go index d7aee0f66..823e3cf86 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -31,19 +31,15 @@ import ( "encoding/json" "errors" "fmt" + "github.com/opentracing/opentracing-go" "io/ioutil" "net/http" "strconv" "strings" - "time" - - "github.com/opentracing/opentracing-go" ) const githubAPI = "https://api.github.com" -var now func() time.Time = time.Now - type API struct { token string client *http.Client @@ -516,24 +512,6 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod return decoded, nil } -func (a *API) FilterCodespacesToDelete(codespaces []*Codespace, keepThresholdDays int) ([]*Codespace, error) { - if keepThresholdDays < 0 { - return nil, fmt.Errorf("invalid value for threshold: %d", keepThresholdDays) - } - codespacesToDelete := []*Codespace{} - for _, codespace := range codespaces { - // get a date from a string representation - t, err := time.Parse(time.RFC3339, codespace.LastUsedAt) - if err != nil { - return nil, fmt.Errorf("error parsing last used at date: %w", err) - } - if t.Before(now().AddDate(0, 0, -keepThresholdDays)) && codespace.Environment.State == "Shutdown" { - codespacesToDelete = append(codespacesToDelete, codespace) - } - } - return codespacesToDelete, nil -} - func (a *API) do(ctx context.Context, req *http.Request, spanName string) (*http.Response, error) { // TODO(adonovan): use NewRequestWithContext(ctx) and drop ctx parameter. span, ctx := opentracing.StartSpanFromContext(ctx, spanName) diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 22e9ac234..c1f4e5c19 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -7,7 +7,6 @@ import ( "net/http" "net/http/httptest" "testing" - "time" ) func TestListCodespaces(t *testing.T) { @@ -53,103 +52,3 @@ func TestListCodespaces(t *testing.T) { } } - -func TestDeleteCodespacesByAge(t *testing.T) { - type args struct { - codespaces []*Codespace - thresholdDays int - } - tests := []struct { - name string - now time.Time - args args - wantErr bool - deleted []*Codespace - }{ - { - name: "no codespaces is to be deleted", - - args: args{ - codespaces: []*Codespace{ - { - Name: "testcodespace", - CreatedAt: "2021-08-09T10:10:24+02:00", - LastUsedAt: "2021-08-09T13:10:24+02:00", - Environment: CodespaceEnvironment{ - State: "Shutdown", - }, - }, - }, - thresholdDays: 1, - }, - now: time.Date(2021, 8, 9, 20, 10, 24, 0, time.UTC), - deleted: []*Codespace{}, - }, - { - name: "one codespace is to be deleted", - - args: args{ - codespaces: []*Codespace{ - { - Name: "testcodespace", - CreatedAt: "2021-08-09T10:10:24+02:00", - LastUsedAt: "2021-08-09T13:10:24+02:00", - Environment: CodespaceEnvironment{ - State: "Shutdown", - }, - }, - }, - thresholdDays: 1, - }, - now: time.Date(2021, 8, 15, 20, 12, 24, 0, time.UTC), - deleted: []*Codespace{ - { - Name: "testcodespace", - CreatedAt: "2021-08-09T10:10:24+02:00", - LastUsedAt: "2021-08-09T13:10:24+02:00", - }, - }, - }, - { - name: "threshold is invalid", - - args: args{ - codespaces: []*Codespace{ - { - Name: "testcodespace", - CreatedAt: "2021-08-09T10:10:24+02:00", - LastUsedAt: "2021-08-09T13:10:24+02:00", - Environment: CodespaceEnvironment{ - State: "Shutdown", - }, - }, - }, - thresholdDays: -1, - }, - now: time.Date(2021, 8, 15, 20, 12, 24, 0, time.UTC), - wantErr: true, - deleted: []*Codespace{}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - - now = func() time.Time { - return tt.now - } - - a := &API{ - token: "testtoken", - client: &http.Client{}, - } - codespaces, err := a.FilterCodespacesToDelete(tt.args.codespaces, tt.args.thresholdDays) - if (err != nil) != tt.wantErr { - t.Errorf("API.CleanupUnusedCodespaces() error = %v, wantErr %v", err, tt.wantErr) - } - - if len(codespaces) != len(tt.deleted) { - t.Errorf("expected %d deleted codespaces, got %d", len(tt.deleted), len(codespaces)) - } - }) - } -} From c6b5fb5ba336cc3160c637bab413342c15ffcfc5 Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Fri, 17 Sep 2021 14:55:50 +0200 Subject: [PATCH 08/19] add the tests Signed-off-by: Raffaele Di Fazio --- cmd/ghcs/delete_test.go | 103 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 cmd/ghcs/delete_test.go diff --git a/cmd/ghcs/delete_test.go b/cmd/ghcs/delete_test.go new file mode 100644 index 000000000..efb59124e --- /dev/null +++ b/cmd/ghcs/delete_test.go @@ -0,0 +1,103 @@ +package main + +import ( + "github.com/github/ghcs/internal/api" + "testing" + "time" +) + +func TestFilterCodespacesToDelete(t *testing.T) { + type args struct { + codespaces []*api.Codespace + thresholdDays int + } + tests := []struct { + name string + now time.Time + args args + wantErr bool + deleted []*api.Codespace + }{ + { + name: "no codespaces is to be deleted", + + args: args{ + codespaces: []*api.Codespace{ + { + Name: "testcodespace", + CreatedAt: "2021-08-09T10:10:24+02:00", + LastUsedAt: "2021-08-09T13:10:24+02:00", + Environment: api.CodespaceEnvironment{ + State: "Shutdown", + }, + }, + }, + thresholdDays: 1, + }, + now: time.Date(2021, 8, 9, 20, 10, 24, 0, time.UTC), + deleted: []*api.Codespace{}, + }, + { + name: "one codespace is to be deleted", + + args: args{ + codespaces: []*api.Codespace{ + { + Name: "testcodespace", + CreatedAt: "2021-08-09T10:10:24+02:00", + LastUsedAt: "2021-08-09T13:10:24+02:00", + Environment: api.CodespaceEnvironment{ + State: "Shutdown", + }, + }, + }, + thresholdDays: 1, + }, + now: time.Date(2021, 8, 15, 20, 12, 24, 0, time.UTC), + deleted: []*api.Codespace{ + { + Name: "testcodespace", + CreatedAt: "2021-08-09T10:10:24+02:00", + LastUsedAt: "2021-08-09T13:10:24+02:00", + }, + }, + }, + { + name: "threshold is invalid", + + args: args{ + codespaces: []*api.Codespace{ + { + Name: "testcodespace", + CreatedAt: "2021-08-09T10:10:24+02:00", + LastUsedAt: "2021-08-09T13:10:24+02:00", + Environment: api.CodespaceEnvironment{ + State: "Shutdown", + }, + }, + }, + thresholdDays: -1, + }, + now: time.Date(2021, 8, 15, 20, 12, 24, 0, time.UTC), + wantErr: true, + deleted: []*api.Codespace{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + now = func() time.Time { + return tt.now + } + + codespaces, err := filterCodespacesToDelete(tt.args.codespaces, tt.args.thresholdDays) + if (err != nil) != tt.wantErr { + t.Errorf("API.CleanupUnusedCodespaces() error = %v, wantErr %v", err, tt.wantErr) + } + + if len(codespaces) != len(tt.deleted) { + t.Errorf("expected %d deleted codespaces, got %d", len(tt.deleted), len(codespaces)) + } + }) + } +} From 11024f71fabc349404b1115884eb26e0ecf5ea2b Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Mon, 20 Sep 2021 10:27:29 +0200 Subject: [PATCH 09/19] force is not used in delete by repo Signed-off-by: Raffaele Di Fazio --- cmd/ghcs/delete.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 97864664a..5aff182c5 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -37,7 +37,7 @@ func newDeleteCmd() *cobra.Command { case allCodespaces: return deleteAll(log, force, keepThresholdDays) case repo != "": - return deleteByRepo(log, repo, force) + return deleteByRepo(log, repo) default: return delete_(log, codespace, force) } @@ -133,7 +133,7 @@ func deleteAll(log *output.Logger, force bool, keepThresholdDays int) error { return list(&listOptions{}) } -func deleteByRepo(log *output.Logger, repo string, force bool) error { +func deleteByRepo(log *output.Logger, repo string) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() From 4721e7004be64656c693901ae87a236ee646cd51 Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Mon, 20 Sep 2021 11:10:44 +0200 Subject: [PATCH 10/19] add threshold to delete by repo Signed-off-by: Raffaele Di Fazio --- cmd/ghcs/delete.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 5aff182c5..b91962f92 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -37,7 +37,7 @@ func newDeleteCmd() *cobra.Command { case allCodespaces: return deleteAll(log, force, keepThresholdDays) case repo != "": - return deleteByRepo(log, repo) + return deleteByRepo(log, repo, keepThresholdDays) default: return delete_(log, codespace, force) } @@ -133,7 +133,7 @@ func deleteAll(log *output.Logger, force bool, keepThresholdDays int) error { return list(&listOptions{}) } -func deleteByRepo(log *output.Logger, repo string) error { +func deleteByRepo(log *output.Logger, repo string, keepThresholdDays int) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() @@ -147,6 +147,11 @@ func deleteByRepo(log *output.Logger, repo string) error { return fmt.Errorf("error getting codespaces: %w", err) } + codespaces, err = filterCodespacesToDelete(codespaces, keepThresholdDays) + if err != nil { + return err + } + delete := func(name string) error { token, err := apiClient.GetCodespaceToken(ctx, user.Login, name) if err != nil { From c4f0eda96d18199431b66c83c75ab954e13af685 Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Mon, 20 Sep 2021 11:54:30 +0200 Subject: [PATCH 11/19] force was actually needed by a next commit Signed-off-by: Raffaele Di Fazio --- cmd/ghcs/delete.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 621a2050b..c5bd53f98 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -37,7 +37,7 @@ func newDeleteCmd() *cobra.Command { case allCodespaces: return deleteAll(log, force, keepThresholdDays) case repo != "": - return deleteByRepo(log, repo, keepThresholdDays) + return deleteByRepo(log, repo, force, keepThresholdDays) default: return delete_(log, codespace, force) } @@ -133,7 +133,7 @@ func deleteAll(log *output.Logger, force bool, keepThresholdDays int) error { return list(&listOptions{}) } -func deleteByRepo(log *output.Logger, repo string, keepThresholdDays int) error { +func deleteByRepo(log *output.Logger, repo string, force bool, keepThresholdDays int) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() From c222c3d696ef599229a47b20a023aa2ca2ecfdea Mon Sep 17 00:00:00 2001 From: Raffaele Di Fazio Date: Mon, 20 Sep 2021 18:23:00 +0200 Subject: [PATCH 12/19] drop check on shut down Signed-off-by: Raffaele Di Fazio --- cmd/ghcs/delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index c5bd53f98..dbbbae814 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -261,7 +261,7 @@ func filterCodespacesToDelete(codespaces []*api.Codespace, keepThresholdDays int if err != nil { return nil, fmt.Errorf("error parsing last used at date: %w", err) } - if t.Before(now().AddDate(0, 0, -keepThresholdDays)) && codespace.Environment.State == "Shutdown" { + if t.Before(now().AddDate(0, 0, -keepThresholdDays)) { codespacesToDelete = append(codespacesToDelete, codespace) } } From b894d3e1340da8aeaf8b83df77a88cc85b1f169a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 20 Sep 2021 18:37:00 +0200 Subject: [PATCH 13/19] Simplify delete implementation --- cmd/ghcs/common.go | 3 + cmd/ghcs/delete.go | 263 +++++++++++++-------------------------------- 2 files changed, 79 insertions(+), 187 deletions(-) diff --git a/cmd/ghcs/common.go b/cmd/ghcs/common.go index e71e3dfe4..46a4f8c0b 100644 --- a/cmd/ghcs/common.go +++ b/cmd/ghcs/common.go @@ -22,7 +22,10 @@ func chooseCodespace(ctx context.Context, apiClient *api.API, user *api.User) (* if err != nil { return nil, fmt.Errorf("error getting codespaces: %w", err) } + return chooseCodespaceFromList(ctx, codespaces) +} +func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace) (*api.Codespace, error) { if len(codespaces) == 0 { return nil, errNoCodespaces } diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index c5bd53f98..fdb813c83 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -2,53 +2,57 @@ package main import ( "context" - "errors" "fmt" "os" "strings" - "sync" "time" "github.com/AlecAivazis/survey/v2" "github.com/github/ghcs/cmd/ghcs/output" "github.com/github/ghcs/internal/api" "github.com/spf13/cobra" + "golang.org/x/sync/errgroup" ) -var now func() time.Time = time.Now +type deleteOptions struct { + deleteAll bool + skipConfirm bool + isInteractive bool + codespaceName string + repoFilter string + keepDays uint16 + now func() time.Time + apiClient *api.API +} func newDeleteCmd() *cobra.Command { - var ( - codespace string - allCodespaces bool - repo string - force bool - keepThresholdDays int - ) + opts := deleteOptions{ + apiClient: api.New(os.Getenv("GITHUB_TOKEN")), + now: time.Now, + isInteractive: hasTTY, + } - log := output.NewLogger(os.Stdout, os.Stderr, false) deleteCmd := &cobra.Command{ Use: "delete", Short: "Delete a codespace", RunE: func(cmd *cobra.Command, args []string) error { - switch { - case allCodespaces && repo != "": - return errors.New("both --all and --repo is not supported") - case allCodespaces: - return deleteAll(log, force, keepThresholdDays) - case repo != "": - return deleteByRepo(log, repo, force, keepThresholdDays) - default: - return delete_(log, codespace, force) - } + // switch { + // case allCodespaces && repo != "": + // return errors.New("both --all and --repo is not supported") + // case allCodespaces: + // return deleteAll(log, force, keepThresholdDays) + // case repo != "": + // return deleteByRepo(log, repo, force, keepThresholdDays) + log := output.NewLogger(os.Stdout, os.Stderr, false) + return delete(context.Background(), log, opts) }, } - deleteCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") - deleteCmd.Flags().BoolVar(&allCodespaces, "all", false, "Delete all codespaces") - deleteCmd.Flags().StringVarP(&repo, "repo", "r", "", "Delete all codespaces for a repository") - deleteCmd.Flags().BoolVarP(&force, "force", "f", false, "Delete codespaces with unsaved changes without confirmation") - deleteCmd.Flags().IntVar(&keepThresholdDays, "days", 0, "Minimum number of days since the codespace was created") + deleteCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "Delete codespace by `name`") + deleteCmd.Flags().BoolVar(&opts.deleteAll, "all", false, "Delete all codespaces") + deleteCmd.Flags().StringVarP(&opts.repoFilter, "repo", "r", "", "Delete codespaces for a repository") + deleteCmd.Flags().BoolVarP(&opts.skipConfirm, "force", "f", false, "Skip confirmation for codespaces that contain unsaved changes") + deleteCmd.Flags().Uint16Var(&opts.keepDays, "days", 0, "Delete codespaces older than `N` days") return deleteCmd } @@ -57,175 +61,78 @@ func init() { rootCmd.AddCommand(newDeleteCmd()) } -func delete_(log *output.Logger, codespaceName string, force bool) error { - apiClient := api.New(os.Getenv("GITHUB_TOKEN")) - ctx := context.Background() - - user, err := apiClient.GetUser(ctx) +func delete(ctx context.Context, log *output.Logger, opts deleteOptions) error { + user, err := opts.apiClient.GetUser(ctx) if err != nil { return fmt.Errorf("error getting user: %w", err) } - codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, codespaceName) - if err != nil { - return fmt.Errorf("get or choose codespace: %w", err) - } - - confirmed, err := confirmDeletion(codespace, force) - if err != nil { - return fmt.Errorf("deletion could not be confirmed: %w", err) - } - - if !confirmed { - return nil - } - - if err := apiClient.DeleteCodespace(ctx, user, token, codespace.Name); err != nil { - return fmt.Errorf("error deleting codespace: %w", err) - } - - log.Println("Codespace deleted.") - - return list(&listOptions{}) -} - -func deleteAll(log *output.Logger, force bool, keepThresholdDays int) error { - apiClient := api.New(os.Getenv("GITHUB_TOKEN")) - ctx := context.Background() - - user, err := apiClient.GetUser(ctx) - if err != nil { - return fmt.Errorf("error getting user: %w", err) - } - - codespaces, err := apiClient.ListCodespaces(ctx, user) + codespaces, err := opts.apiClient.ListCodespaces(ctx, user) if err != nil { return fmt.Errorf("error getting codespaces: %w", err) } - codespacesToDelete, err := filterCodespacesToDelete(codespaces, keepThresholdDays) - if err != nil { - return err - } - - for _, c := range codespacesToDelete { - confirmed, err := confirmDeletion(c, force) + nameFilter := opts.codespaceName + if nameFilter == "" && !opts.deleteAll && opts.repoFilter == "" { + c, err := chooseCodespaceFromList(ctx, codespaces) if err != nil { - return fmt.Errorf("deletion could not be confirmed: %w", err) + return fmt.Errorf("error choosing codespace: %w", err) } - - if !confirmed { - continue - } - - token, err := apiClient.GetCodespaceToken(ctx, user.Login, c.Name) - if err != nil { - return fmt.Errorf("error getting codespace token: %w", err) - } - - if err := apiClient.DeleteCodespace(ctx, user, token, c.Name); err != nil { - return fmt.Errorf("error deleting codespace: %w", err) - } - - log.Printf("Codespace deleted: %s\n", c.Name) + nameFilter = c.Name } - return list(&listOptions{}) -} - -func deleteByRepo(log *output.Logger, repo string, force bool, keepThresholdDays int) error { - apiClient := api.New(os.Getenv("GITHUB_TOKEN")) - ctx := context.Background() - - user, err := apiClient.GetUser(ctx) - if err != nil { - return fmt.Errorf("error getting user: %w", err) - } - - codespaces, err := apiClient.ListCodespaces(ctx, user) - if err != nil { - return fmt.Errorf("error getting codespaces: %w", err) - } - - codespaces, err = filterCodespacesToDelete(codespaces, keepThresholdDays) - if err != nil { - return err - } - - delete := func(name string) error { - token, err := apiClient.GetCodespaceToken(ctx, user.Login, name) - if err != nil { - return fmt.Errorf("error getting codespace token: %w", err) - } - - if err := apiClient.DeleteCodespace(ctx, user, token, name); err != nil { - return fmt.Errorf("error deleting codespace: %w", err) - } - - return nil - } - - // Perform deletions in parallel, for performance, - // and to ensure all are attempted even if any one fails. - var ( - found bool - mu sync.Mutex // guards errs, logger - errs []error - wg sync.WaitGroup - ) + var codespacesToDelete []*api.Codespace + lastUpdatedCutoffTime := opts.now().AddDate(0, 0, -int(opts.keepDays)) for _, c := range codespaces { - if !strings.EqualFold(c.RepositoryNWO, repo) { + if nameFilter != "" && c.Name != nameFilter { continue } - - confirmed, err := confirmDeletion(c, force) - if err != nil { - mu.Lock() - errs = append(errs, fmt.Errorf("deletion could not be confirmed: %w", err)) - mu.Unlock() + if opts.repoFilter != "" && !strings.EqualFold(c.RepositoryNWO, opts.repoFilter) { continue } - - if !confirmed { - continue - } - - found = true - c := c - wg.Add(1) - go func() { - defer wg.Done() - err := delete(c.Name) - mu.Lock() - defer mu.Unlock() + if opts.keepDays > 0 { + t, err := time.Parse(time.RFC3339, c.LastUsedAt) if err != nil { - errs = append(errs, err) - } else { - log.Printf("Codespace deleted: %s\n", c.Name) + return fmt.Errorf("error parsing last_used_at timestamp %q: %w", c.LastUsedAt, err) + } + if t.After(lastUpdatedCutoffTime) { + continue } - }() - } - if !found { - return fmt.Errorf("no codespace was found for repository: %s", repo) - } - wg.Wait() - - // Return first error, plus count of others. - if errs != nil { - err := errs[0] - if others := len(errs) - 1; others > 0 { - err = fmt.Errorf("%w (+%d more)", err, others) } - return err + if nameFilter == "" || !opts.skipConfirm { + confirmed, err := confirmDeletion(c) + if err != nil { + return fmt.Errorf("deletion could not be confirmed: %w", err) + } + if !confirmed { + continue + } + } + codespacesToDelete = append(codespacesToDelete, c) } - return nil + g := errgroup.Group{} + for _, c := range codespacesToDelete { + codespaceName := c.Name + g.Go(func() error { + token, err := opts.apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) + if err != nil { + return fmt.Errorf("error getting codespace token: %w", err) + } + if err := opts.apiClient.DeleteCodespace(ctx, user, token, codespaceName); err != nil { + return fmt.Errorf("error deleting codespace: %w", err) + } + return nil + }) + } + + return g.Wait() } -func confirmDeletion(codespace *api.Codespace, force bool) (bool, error) { +func confirmDeletion(codespace *api.Codespace) (bool, error) { gs := codespace.Environment.GitStatus hasUnsavedChanges := gs.HasUncommitedChanges || gs.HasUnpushedChanges - if force || !hasUnsavedChanges { + if !hasUnsavedChanges { return true, nil } if !hasTTY { @@ -249,21 +156,3 @@ func confirmDeletion(codespace *api.Codespace, force bool) (bool, error) { return confirmed.Confirmed, nil } - -func filterCodespacesToDelete(codespaces []*api.Codespace, keepThresholdDays int) ([]*api.Codespace, error) { - if keepThresholdDays < 0 { - return nil, fmt.Errorf("invalid value for threshold: %d", keepThresholdDays) - } - codespacesToDelete := []*api.Codespace{} - for _, codespace := range codespaces { - // get a date from a string representation - t, err := time.Parse(time.RFC3339, codespace.LastUsedAt) - if err != nil { - return nil, fmt.Errorf("error parsing last used at date: %w", err) - } - if t.Before(now().AddDate(0, 0, -keepThresholdDays)) && codespace.Environment.State == "Shutdown" { - codespacesToDelete = append(codespacesToDelete, codespace) - } - } - return codespacesToDelete, nil -} From 678da44c28506629da1feb53b34efbe59d38b7f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 21 Sep 2021 21:09:26 +0200 Subject: [PATCH 14/19] Simplify delete further --- cmd/ghcs/common.go | 2 +- cmd/ghcs/delete.go | 63 ++++++++++++++++------------ cmd/ghcs/delete_test.go | 91 ++++------------------------------------ cmd/ghcs/list.go | 2 +- internal/api/api.go | 16 ++++--- internal/api/api_test.go | 6 +-- 6 files changed, 58 insertions(+), 122 deletions(-) diff --git a/cmd/ghcs/common.go b/cmd/ghcs/common.go index a3963d22f..4ebc89a2d 100644 --- a/cmd/ghcs/common.go +++ b/cmd/ghcs/common.go @@ -19,7 +19,7 @@ import ( var errNoCodespaces = errors.New("you have no codespaces") func chooseCodespace(ctx context.Context, apiClient *api.API, user *api.User) (*api.Codespace, error) { - codespaces, err := apiClient.ListCodespaces(ctx, user) + codespaces, err := apiClient.ListCodespaces(ctx, user.Login) if err != nil { return nil, fmt.Errorf("error getting codespaces: %w", err) } diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index a8005f1e9..c08344163 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -2,13 +2,13 @@ package ghcs import ( "context" + "errors" "fmt" "os" "strings" "time" "github.com/AlecAivazis/survey/v2" - "github.com/github/ghcs/cmd/ghcs/output" "github.com/github/ghcs/internal/api" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" @@ -17,19 +17,32 @@ import ( type deleteOptions struct { deleteAll bool skipConfirm bool - isInteractive bool codespaceName string repoFilter string keepDays uint16 + + isInteractive bool now func() time.Time - apiClient *api.API + apiClient apiClient + prompter prompter +} + +type prompter interface { + Confirm(message string) (bool, error) +} + +type apiClient interface { + GetUser(ctx context.Context) (*api.User, error) + ListCodespaces(ctx context.Context, user string) ([]*api.Codespace, error) + DeleteCodespace(ctx context.Context, user, name string) error } func newDeleteCmd() *cobra.Command { opts := deleteOptions{ - apiClient: api.New(os.Getenv("GITHUB_TOKEN")), - now: time.Now, isInteractive: hasTTY, + now: time.Now, + apiClient: api.New(os.Getenv("GITHUB_TOKEN")), + prompter: &surveyPrompter{}, } deleteCmd := &cobra.Command{ @@ -37,15 +50,10 @@ func newDeleteCmd() *cobra.Command { Short: "Delete a codespace", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - // switch { - // case allCodespaces && repo != "": - // return errors.New("both --all and --repo is not supported") - // case allCodespaces: - // return deleteAll(log, force, keepThresholdDays) - // case repo != "": - // return deleteByRepo(log, repo, force, keepThresholdDays) - log := output.NewLogger(os.Stdout, os.Stderr, false) - return delete(context.Background(), log, opts) + if opts.deleteAll && opts.repoFilter != "" { + return errors.New("both --all and --repo is not supported") + } + return delete(context.Background(), opts) }, } @@ -58,13 +66,13 @@ func newDeleteCmd() *cobra.Command { return deleteCmd } -func delete(ctx context.Context, log *output.Logger, opts deleteOptions) error { +func delete(ctx context.Context, opts deleteOptions) error { user, err := opts.apiClient.GetUser(ctx) if err != nil { return fmt.Errorf("error getting user: %w", err) } - codespaces, err := opts.apiClient.ListCodespaces(ctx, user) + codespaces, err := opts.apiClient.ListCodespaces(ctx, user.Login) if err != nil { return fmt.Errorf("error getting codespaces: %w", err) } @@ -78,7 +86,7 @@ func delete(ctx context.Context, log *output.Logger, opts deleteOptions) error { nameFilter = c.Name } - var codespacesToDelete []*api.Codespace + codespacesToDelete := make([]*api.Codespace, 0, len(codespaces)) lastUpdatedCutoffTime := opts.now().AddDate(0, 0, -int(opts.keepDays)) for _, c := range codespaces { if nameFilter != "" && c.Name != nameFilter { @@ -97,9 +105,9 @@ func delete(ctx context.Context, log *output.Logger, opts deleteOptions) error { } } if nameFilter == "" || !opts.skipConfirm { - confirmed, err := confirmDeletion(c) + confirmed, err := confirmDeletion(opts.prompter, c, opts.isInteractive) if err != nil { - return fmt.Errorf("deletion could not be confirmed: %w", err) + return fmt.Errorf("unable to confirm: %w", err) } if !confirmed { continue @@ -112,11 +120,7 @@ func delete(ctx context.Context, log *output.Logger, opts deleteOptions) error { for _, c := range codespacesToDelete { codespaceName := c.Name g.Go(func() error { - token, err := opts.apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) - if err != nil { - return fmt.Errorf("error getting codespace token: %w", err) - } - if err := opts.apiClient.DeleteCodespace(ctx, user, token, codespaceName); err != nil { + if err := opts.apiClient.DeleteCodespace(ctx, user.Login, codespaceName); err != nil { return fmt.Errorf("error deleting codespace: %w", err) } return nil @@ -126,16 +130,21 @@ func delete(ctx context.Context, log *output.Logger, opts deleteOptions) error { return g.Wait() } -func confirmDeletion(codespace *api.Codespace) (bool, error) { +func confirmDeletion(p prompter, codespace *api.Codespace, isInteractive bool) (bool, error) { gs := codespace.Environment.GitStatus hasUnsavedChanges := gs.HasUncommitedChanges || gs.HasUnpushedChanges if !hasUnsavedChanges { return true, nil } - if !hasTTY { + if !isInteractive { return false, fmt.Errorf("codespace %s has unsaved changes (use --force to override)", codespace.Name) } + return p.Confirm(fmt.Sprintf("Codespace %s has unsaved changes. OK to delete?", codespace.Name)) +} +type surveyPrompter struct{} + +func (p *surveyPrompter) Confirm(message string) (bool, error) { var confirmed struct { Confirmed bool } @@ -143,7 +152,7 @@ func confirmDeletion(codespace *api.Codespace) (bool, error) { { Name: "confirmed", Prompt: &survey.Confirm{ - Message: fmt.Sprintf("Codespace %s has unsaved changes. OK to delete?", codespace.Name), + Message: message, }, }, } diff --git a/cmd/ghcs/delete_test.go b/cmd/ghcs/delete_test.go index c43331bea..783ad80e6 100644 --- a/cmd/ghcs/delete_test.go +++ b/cmd/ghcs/delete_test.go @@ -1,103 +1,28 @@ package ghcs import ( + "context" "testing" - "time" - - "github.com/github/ghcs/internal/api" ) -func TestFilterCodespacesToDelete(t *testing.T) { - type args struct { - codespaces []*api.Codespace - thresholdDays int - } +func TestDelete(t *testing.T) { tests := []struct { name string - now time.Time - args args + opts deleteOptions wantErr bool - deleted []*api.Codespace }{ { - name: "no codespaces is to be deleted", - - args: args{ - codespaces: []*api.Codespace{ - { - Name: "testcodespace", - CreatedAt: "2021-08-09T10:10:24+02:00", - LastUsedAt: "2021-08-09T13:10:24+02:00", - Environment: api.CodespaceEnvironment{ - State: "Shutdown", - }, - }, - }, - thresholdDays: 1, + name: "by name", + opts: deleteOptions{ + codespaceName: "foo-bar-123", }, - now: time.Date(2021, 8, 9, 20, 10, 24, 0, time.UTC), - deleted: []*api.Codespace{}, - }, - { - name: "one codespace is to be deleted", - - args: args{ - codespaces: []*api.Codespace{ - { - Name: "testcodespace", - CreatedAt: "2021-08-09T10:10:24+02:00", - LastUsedAt: "2021-08-09T13:10:24+02:00", - Environment: api.CodespaceEnvironment{ - State: "Shutdown", - }, - }, - }, - thresholdDays: 1, - }, - now: time.Date(2021, 8, 15, 20, 12, 24, 0, time.UTC), - deleted: []*api.Codespace{ - { - Name: "testcodespace", - CreatedAt: "2021-08-09T10:10:24+02:00", - LastUsedAt: "2021-08-09T13:10:24+02:00", - }, - }, - }, - { - name: "threshold is invalid", - - args: args{ - codespaces: []*api.Codespace{ - { - Name: "testcodespace", - CreatedAt: "2021-08-09T10:10:24+02:00", - LastUsedAt: "2021-08-09T13:10:24+02:00", - Environment: api.CodespaceEnvironment{ - State: "Shutdown", - }, - }, - }, - thresholdDays: -1, - }, - now: time.Date(2021, 8, 15, 20, 12, 24, 0, time.UTC), - wantErr: true, - deleted: []*api.Codespace{}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - - now = func() time.Time { - return tt.now - } - - codespaces, err := filterCodespacesToDelete(tt.args.codespaces, tt.args.thresholdDays) + err := delete(context.Background(), tt.opts) if (err != nil) != tt.wantErr { - t.Errorf("API.CleanupUnusedCodespaces() error = %v, wantErr %v", err, tt.wantErr) - } - - if len(codespaces) != len(tt.deleted) { - t.Errorf("expected %d deleted codespaces, got %d", len(tt.deleted), len(codespaces)) + t.Errorf("delete() error = %v, wantErr %v", err, tt.wantErr) } }) } diff --git a/cmd/ghcs/list.go b/cmd/ghcs/list.go index 7ee156012..85eabaef5 100644 --- a/cmd/ghcs/list.go +++ b/cmd/ghcs/list.go @@ -40,7 +40,7 @@ func list(opts *listOptions) error { return fmt.Errorf("error getting user: %w", err) } - codespaces, err := apiClient.ListCodespaces(ctx, user) + codespaces, err := apiClient.ListCodespaces(ctx, user.Login) if err != nil { return fmt.Errorf("error getting codespaces: %w", err) } diff --git a/internal/api/api.go b/internal/api/api.go index 12d5a7263..4d4078c9c 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -32,11 +32,12 @@ import ( "encoding/json" "errors" "fmt" - "github.com/opentracing/opentracing-go" "io/ioutil" "net/http" "strconv" "strings" + + "github.com/opentracing/opentracing-go" ) const githubAPI = "https://api.github.com" @@ -172,9 +173,9 @@ type CodespaceEnvironmentConnection struct { RelaySAS string `json:"relaySas"` } -func (a *API) ListCodespaces(ctx context.Context, user *User) ([]*Codespace, error) { +func (a *API) ListCodespaces(ctx context.Context, user string) ([]*Codespace, error) { req, err := http.NewRequest( - http.MethodGet, a.githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces", nil, + http.MethodGet, a.githubAPI+"/vscs_internal/user/"+user+"/codespaces", nil, ) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) @@ -442,8 +443,13 @@ func (a *API) CreateCodespace(ctx context.Context, user *User, repository *Repos return &response, nil } -func (a *API) DeleteCodespace(ctx context.Context, user *User, token, codespaceName string) error { - req, err := http.NewRequest(http.MethodDelete, a.githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces/"+codespaceName, nil) +func (a *API) DeleteCodespace(ctx context.Context, user string, codespaceName string) error { + token, err := a.GetCodespaceToken(ctx, user, codespaceName) + if err != nil { + return fmt.Errorf("error getting codespace token: %w", err) + } + + req, err := http.NewRequest(http.MethodDelete, a.githubAPI+"/vscs_internal/user/"+user+"/codespaces/"+codespaceName, nil) if err != nil { return fmt.Errorf("error creating request: %w", err) } diff --git a/internal/api/api_test.go b/internal/api/api_test.go index c1f4e5c19..6fb162030 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -10,10 +10,6 @@ import ( ) func TestListCodespaces(t *testing.T) { - user := &User{ - Login: "testuser", - } - codespaces := []*Codespace{ { Name: "testcodespace", @@ -38,7 +34,7 @@ func TestListCodespaces(t *testing.T) { token: "faketoken", } ctx := context.TODO() - codespaces, err := api.ListCodespaces(ctx, user) + codespaces, err := api.ListCodespaces(ctx, "testuser") if err != nil { t.Fatal(err) } From cb7b535b917ffddc38f12069b32fbce5a4034eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 22 Sep 2021 16:11:34 +0200 Subject: [PATCH 15/19] Add tests for delete --- cmd/ghcs/delete.go | 10 ++- cmd/ghcs/delete_test.go | 169 +++++++++++++++++++++++++++++++++-- cmd/ghcs/mock_api.go | 180 ++++++++++++++++++++++++++++++++++++++ cmd/ghcs/mock_prompter.go | 73 ++++++++++++++++ 4 files changed, 425 insertions(+), 7 deletions(-) create mode 100644 cmd/ghcs/mock_api.go create mode 100644 cmd/ghcs/mock_prompter.go diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index c08344163..3408f08d7 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -27,10 +27,12 @@ type deleteOptions struct { prompter prompter } +//go:generate moq -fmt goimports -rm -out mock_prompter.go . prompter type prompter interface { Confirm(message string) (bool, error) } +//go:generate moq -fmt goimports -rm -out mock_api.go . apiClient type apiClient interface { GetUser(ctx context.Context) (*api.User, error) ListCodespaces(ctx context.Context, user string) ([]*api.Codespace, error) @@ -57,9 +59,9 @@ func newDeleteCmd() *cobra.Command { }, } - deleteCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "Delete codespace by `name`") + deleteCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "The `name` of the codespace to delete") deleteCmd.Flags().BoolVar(&opts.deleteAll, "all", false, "Delete all codespaces") - deleteCmd.Flags().StringVarP(&opts.repoFilter, "repo", "r", "", "Delete codespaces for a repository") + deleteCmd.Flags().StringVarP(&opts.repoFilter, "repo", "r", "", "Delete codespaces for a `repository`") deleteCmd.Flags().BoolVarP(&opts.skipConfirm, "force", "f", false, "Skip confirmation for codespaces that contain unsaved changes") deleteCmd.Flags().Uint16Var(&opts.keepDays, "days", 0, "Delete codespaces older than `N` days") @@ -116,6 +118,10 @@ func delete(ctx context.Context, opts deleteOptions) error { codespacesToDelete = append(codespacesToDelete, c) } + if len(codespacesToDelete) == 0 { + return errors.New("no codespaces to delete") + } + g := errgroup.Group{} for _, c := range codespacesToDelete { codespaceName := c.Name diff --git a/cmd/ghcs/delete_test.go b/cmd/ghcs/delete_test.go index 783ad80e6..754254494 100644 --- a/cmd/ghcs/delete_test.go +++ b/cmd/ghcs/delete_test.go @@ -2,28 +2,187 @@ package ghcs import ( "context" + "fmt" + "sort" "testing" + "time" + + "github.com/github/ghcs/internal/api" ) func TestDelete(t *testing.T) { + user := &api.User{Login: "hubot"} + now, _ := time.Parse(time.RFC3339, "2021-09-22T00:00:00Z") + daysAgo := func(n int) string { + return now.Add(time.Hour * -time.Duration(24*n)).Format(time.RFC3339) + } + tests := []struct { - name string - opts deleteOptions - wantErr bool + name string + opts deleteOptions + codespaces []*api.Codespace + confirms map[string]bool + wantErr bool + wantDeleted []string }{ { name: "by name", opts: deleteOptions{ - codespaceName: "foo-bar-123", + codespaceName: "hubot-robawt-abc", }, + codespaces: []*api.Codespace{ + { + Name: "monalisa-spoonknife-123", + }, + { + Name: "hubot-robawt-abc", + }, + }, + wantDeleted: []string{"hubot-robawt-abc"}, + }, + { + name: "by repo", + opts: deleteOptions{ + repoFilter: "monalisa/spoon-knife", + }, + codespaces: []*api.Codespace{ + { + Name: "monalisa-spoonknife-123", + RepositoryNWO: "monalisa/Spoon-Knife", + }, + { + Name: "hubot-robawt-abc", + RepositoryNWO: "hubot/ROBAWT", + }, + { + Name: "monalisa-spoonknife-c4f3", + RepositoryNWO: "monalisa/Spoon-Knife", + }, + }, + wantDeleted: []string{"monalisa-spoonknife-123", "monalisa-spoonknife-c4f3"}, + }, + { + 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"}, + }, + { + name: "with confirm", + opts: deleteOptions{ + isInteractive: true, + deleteAll: true, + skipConfirm: false, + }, + codespaces: []*api.Codespace{ + { + Name: "monalisa-spoonknife-123", + Environment: api.CodespaceEnvironment{ + GitStatus: api.CodespaceEnvironmentGitStatus{ + HasUnpushedChanges: true, + }, + }, + }, + { + Name: "hubot-robawt-abc", + Environment: api.CodespaceEnvironment{ + GitStatus: api.CodespaceEnvironmentGitStatus{ + HasUncommitedChanges: true, + }, + }, + }, + { + Name: "monalisa-spoonknife-c4f3", + Environment: api.CodespaceEnvironment{ + GitStatus: api.CodespaceEnvironmentGitStatus{ + HasUnpushedChanges: false, + HasUncommitedChanges: false, + }, + }, + }, + }, + confirms: map[string]bool{ + "Codespace monalisa-spoonknife-123 has unsaved changes. OK to delete?": false, + "Codespace hubot-robawt-abc has unsaved changes. OK to delete?": true, + }, + wantDeleted: []string{"hubot-robawt-abc", "monalisa-spoonknife-c4f3"}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := delete(context.Background(), tt.opts) + apiMock := &apiClientMock{ + GetUserFunc: func(_ context.Context) (*api.User, error) { + return user, nil + }, + ListCodespacesFunc: func(_ context.Context, userLogin string) ([]*api.Codespace, error) { + if userLogin != user.Login { + return nil, fmt.Errorf("unexpected user %q", userLogin) + } + return tt.codespaces, nil + }, + DeleteCodespaceFunc: func(_ context.Context, userLogin, name string) error { + if userLogin != user.Login { + return fmt.Errorf("unexpected user %q", userLogin) + } + return nil + }, + } + opts := tt.opts + opts.apiClient = apiMock + opts.now = func() time.Time { return now } + opts.prompter = &prompterMock{ + ConfirmFunc: func(msg string) (bool, error) { + res, found := tt.confirms[msg] + if !found { + return false, fmt.Errorf("unexpected prompt %q", msg) + } + return res, nil + }, + } + + err := delete(context.Background(), opts) if (err != nil) != tt.wantErr { t.Errorf("delete() error = %v, wantErr %v", err, tt.wantErr) } + if n := len(apiMock.GetUserCalls()); n != 1 { + t.Errorf("GetUser invoked %d times, expected %d", n, 1) + } + var gotDeleted []string + for _, delArgs := range apiMock.DeleteCodespaceCalls() { + gotDeleted = append(gotDeleted, delArgs.Name) + } + sort.Strings(gotDeleted) + if !sliceEquals(gotDeleted, tt.wantDeleted) { + t.Errorf("deleted %q, want %q", gotDeleted, tt.wantDeleted) + } }) } } + +func sliceEquals(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/cmd/ghcs/mock_api.go b/cmd/ghcs/mock_api.go new file mode 100644 index 000000000..46edd2835 --- /dev/null +++ b/cmd/ghcs/mock_api.go @@ -0,0 +1,180 @@ +// Code generated by moq; DO NOT EDIT. +// github.com/matryer/moq + +package ghcs + +import ( + "context" + "sync" + + "github.com/github/ghcs/internal/api" +) + +// Ensure, that apiClientMock does implement apiClient. +// If this is not the case, regenerate this file with moq. +var _ apiClient = &apiClientMock{} + +// apiClientMock is a mock implementation of apiClient. +// +// func TestSomethingThatUsesapiClient(t *testing.T) { +// +// // make and configure a mocked apiClient +// mockedapiClient := &apiClientMock{ +// DeleteCodespaceFunc: func(ctx context.Context, user string, name string) error { +// panic("mock out the DeleteCodespace method") +// }, +// GetUserFunc: func(ctx context.Context) (*api.User, error) { +// panic("mock out the GetUser method") +// }, +// ListCodespacesFunc: func(ctx context.Context, user string) ([]*api.Codespace, error) { +// panic("mock out the ListCodespaces method") +// }, +// } +// +// // use mockedapiClient in code that requires apiClient +// // and then make assertions. +// +// } +type apiClientMock struct { + // DeleteCodespaceFunc mocks the DeleteCodespace method. + DeleteCodespaceFunc func(ctx context.Context, user string, name string) error + + // GetUserFunc mocks the GetUser method. + GetUserFunc func(ctx context.Context) (*api.User, error) + + // ListCodespacesFunc mocks the ListCodespaces method. + ListCodespacesFunc func(ctx context.Context, user string) ([]*api.Codespace, error) + + // calls tracks calls to the methods. + calls struct { + // DeleteCodespace holds details about calls to the DeleteCodespace method. + DeleteCodespace []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // User is the user argument value. + User string + // Name is the name argument value. + Name string + } + // GetUser holds details about calls to the GetUser method. + GetUser []struct { + // Ctx is the ctx argument value. + Ctx context.Context + } + // ListCodespaces holds details about calls to the ListCodespaces method. + ListCodespaces []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // User is the user argument value. + User string + } + } + lockDeleteCodespace sync.RWMutex + lockGetUser sync.RWMutex + lockListCodespaces sync.RWMutex +} + +// DeleteCodespace calls DeleteCodespaceFunc. +func (mock *apiClientMock) DeleteCodespace(ctx context.Context, user string, name string) error { + if mock.DeleteCodespaceFunc == nil { + panic("apiClientMock.DeleteCodespaceFunc: method is nil but apiClient.DeleteCodespace was just called") + } + callInfo := struct { + Ctx context.Context + User string + Name string + }{ + Ctx: ctx, + User: user, + Name: name, + } + mock.lockDeleteCodespace.Lock() + mock.calls.DeleteCodespace = append(mock.calls.DeleteCodespace, callInfo) + mock.lockDeleteCodespace.Unlock() + return mock.DeleteCodespaceFunc(ctx, user, name) +} + +// DeleteCodespaceCalls gets all the calls that were made to DeleteCodespace. +// Check the length with: +// len(mockedapiClient.DeleteCodespaceCalls()) +func (mock *apiClientMock) DeleteCodespaceCalls() []struct { + Ctx context.Context + User string + Name string +} { + var calls []struct { + Ctx context.Context + User string + Name string + } + mock.lockDeleteCodespace.RLock() + calls = mock.calls.DeleteCodespace + mock.lockDeleteCodespace.RUnlock() + return calls +} + +// GetUser calls GetUserFunc. +func (mock *apiClientMock) GetUser(ctx context.Context) (*api.User, error) { + if mock.GetUserFunc == nil { + panic("apiClientMock.GetUserFunc: method is nil but apiClient.GetUser was just called") + } + callInfo := struct { + Ctx context.Context + }{ + Ctx: ctx, + } + mock.lockGetUser.Lock() + mock.calls.GetUser = append(mock.calls.GetUser, callInfo) + mock.lockGetUser.Unlock() + return mock.GetUserFunc(ctx) +} + +// GetUserCalls gets all the calls that were made to GetUser. +// Check the length with: +// len(mockedapiClient.GetUserCalls()) +func (mock *apiClientMock) GetUserCalls() []struct { + Ctx context.Context +} { + var calls []struct { + Ctx context.Context + } + mock.lockGetUser.RLock() + calls = mock.calls.GetUser + mock.lockGetUser.RUnlock() + return calls +} + +// ListCodespaces calls ListCodespacesFunc. +func (mock *apiClientMock) ListCodespaces(ctx context.Context, user string) ([]*api.Codespace, error) { + if mock.ListCodespacesFunc == nil { + panic("apiClientMock.ListCodespacesFunc: method is nil but apiClient.ListCodespaces was just called") + } + callInfo := struct { + Ctx context.Context + User string + }{ + Ctx: ctx, + User: user, + } + mock.lockListCodespaces.Lock() + mock.calls.ListCodespaces = append(mock.calls.ListCodespaces, callInfo) + mock.lockListCodespaces.Unlock() + return mock.ListCodespacesFunc(ctx, user) +} + +// ListCodespacesCalls gets all the calls that were made to ListCodespaces. +// Check the length with: +// len(mockedapiClient.ListCodespacesCalls()) +func (mock *apiClientMock) ListCodespacesCalls() []struct { + Ctx context.Context + User string +} { + var calls []struct { + Ctx context.Context + User string + } + mock.lockListCodespaces.RLock() + calls = mock.calls.ListCodespaces + mock.lockListCodespaces.RUnlock() + return calls +} diff --git a/cmd/ghcs/mock_prompter.go b/cmd/ghcs/mock_prompter.go new file mode 100644 index 000000000..e15209c03 --- /dev/null +++ b/cmd/ghcs/mock_prompter.go @@ -0,0 +1,73 @@ +// Code generated by moq; DO NOT EDIT. +// github.com/matryer/moq + +package ghcs + +import ( + "sync" +) + +// Ensure, that prompterMock does implement prompter. +// If this is not the case, regenerate this file with moq. +var _ prompter = &prompterMock{} + +// prompterMock is a mock implementation of prompter. +// +// func TestSomethingThatUsesprompter(t *testing.T) { +// +// // make and configure a mocked prompter +// mockedprompter := &prompterMock{ +// ConfirmFunc: func(message string) (bool, error) { +// panic("mock out the Confirm method") +// }, +// } +// +// // use mockedprompter in code that requires prompter +// // and then make assertions. +// +// } +type prompterMock struct { + // ConfirmFunc mocks the Confirm method. + ConfirmFunc func(message string) (bool, error) + + // calls tracks calls to the methods. + calls struct { + // Confirm holds details about calls to the Confirm method. + Confirm []struct { + // Message is the message argument value. + Message string + } + } + lockConfirm sync.RWMutex +} + +// Confirm calls ConfirmFunc. +func (mock *prompterMock) Confirm(message string) (bool, error) { + if mock.ConfirmFunc == nil { + panic("prompterMock.ConfirmFunc: method is nil but prompter.Confirm was just called") + } + callInfo := struct { + Message string + }{ + Message: message, + } + mock.lockConfirm.Lock() + mock.calls.Confirm = append(mock.calls.Confirm, callInfo) + mock.lockConfirm.Unlock() + return mock.ConfirmFunc(message) +} + +// ConfirmCalls gets all the calls that were made to Confirm. +// Check the length with: +// len(mockedprompter.ConfirmCalls()) +func (mock *prompterMock) ConfirmCalls() []struct { + Message string +} { + var calls []struct { + Message string + } + mock.lockConfirm.RLock() + calls = mock.calls.Confirm + mock.lockConfirm.RUnlock() + return calls +} From 32d3a38465ef15e8e7b305dccfef31dbc05c07f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 22 Sep 2021 16:39:50 +0200 Subject: [PATCH 16/19] Name of the codespace --- cmd/ghcs/delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 3408f08d7..4f1c71842 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -59,7 +59,7 @@ func newDeleteCmd() *cobra.Command { }, } - deleteCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "The `name` of the codespace to delete") + deleteCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "Name of the codespace") deleteCmd.Flags().BoolVar(&opts.deleteAll, "all", false, "Delete all codespaces") deleteCmd.Flags().StringVarP(&opts.repoFilter, "repo", "r", "", "Delete codespaces for a `repository`") deleteCmd.Flags().BoolVarP(&opts.skipConfirm, "force", "f", false, "Skip confirmation for codespaces that contain unsaved changes") From e8212a80a9dcdbecb698f47bd45176ad1703bff1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 23 Sep 2021 17:14:25 +0200 Subject: [PATCH 17/19] Print `delete` failures as they occur --- cmd/ghcs/delete.go | 18 ++++++++++++++---- cmd/ghcs/delete_test.go | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 4f1c71842..b4c0bcbcf 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -9,6 +9,7 @@ import ( "time" "github.com/AlecAivazis/survey/v2" + "github.com/github/ghcs/cmd/ghcs/output" "github.com/github/ghcs/internal/api" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" @@ -55,7 +56,8 @@ func newDeleteCmd() *cobra.Command { if opts.deleteAll && opts.repoFilter != "" { return errors.New("both --all and --repo is not supported") } - return delete(context.Background(), opts) + log := output.NewLogger(cmd.OutOrStdout(), cmd.ErrOrStderr(), !opts.isInteractive) + return delete(context.Background(), log, opts) }, } @@ -68,7 +70,11 @@ func newDeleteCmd() *cobra.Command { return deleteCmd } -func delete(ctx context.Context, opts deleteOptions) error { +type logger interface { + Errorf(format string, v ...interface{}) (int, error) +} + +func delete(ctx context.Context, log logger, opts deleteOptions) error { user, err := opts.apiClient.GetUser(ctx) if err != nil { return fmt.Errorf("error getting user: %w", err) @@ -127,13 +133,17 @@ func delete(ctx context.Context, opts deleteOptions) error { codespaceName := c.Name g.Go(func() error { if err := opts.apiClient.DeleteCodespace(ctx, user.Login, codespaceName); err != nil { - return fmt.Errorf("error deleting codespace: %w", err) + log.Errorf("error deleting codespace %q: %v", codespaceName, err) + return err } return nil }) } - return g.Wait() + if err := g.Wait(); err != nil { + return errors.New("some codespaces failed to delete") + } + return nil } func confirmDeletion(p prompter, codespace *api.Codespace, isInteractive bool) (bool, error) { diff --git a/cmd/ghcs/delete_test.go b/cmd/ghcs/delete_test.go index 754254494..beb371dd4 100644 --- a/cmd/ghcs/delete_test.go +++ b/cmd/ghcs/delete_test.go @@ -156,7 +156,7 @@ func TestDelete(t *testing.T) { }, } - err := delete(context.Background(), opts) + err := delete(context.Background(), nil, opts) if (err != nil) != tt.wantErr { t.Errorf("delete() error = %v, wantErr %v", err, tt.wantErr) } From 75c1dfdf49e4b43c31704639d5d33b8361fb58e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 23 Sep 2021 18:57:22 +0200 Subject: [PATCH 18/19] Fetch codespace by name directly if name argument given --- cmd/ghcs/delete.go | 44 +++++++++---- cmd/ghcs/delete_test.go | 33 +++++++--- cmd/ghcs/mock_api.go | 126 +++++++++++++++++++++++++++++++++++--- cmd/ghcs/mock_prompter.go | 4 -- 4 files changed, 174 insertions(+), 33 deletions(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index b4c0bcbcf..0c21c1674 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -28,14 +28,16 @@ type deleteOptions struct { prompter prompter } -//go:generate moq -fmt goimports -rm -out mock_prompter.go . prompter +//go:generate moq -fmt goimports -rm -skip-ensure -out mock_prompter.go . prompter type prompter interface { Confirm(message string) (bool, error) } -//go:generate moq -fmt goimports -rm -out mock_api.go . apiClient +//go:generate moq -fmt goimports -rm -skip-ensure -out mock_api.go . apiClient type apiClient interface { GetUser(ctx context.Context) (*api.User, error) + GetCodespaceToken(ctx context.Context, user, name string) (string, error) + GetCodespace(ctx context.Context, token, user, name string) (*api.Codespace, error) ListCodespaces(ctx context.Context, user string) ([]*api.Codespace, error) DeleteCodespace(ctx context.Context, user, name string) error } @@ -80,18 +82,34 @@ func delete(ctx context.Context, log logger, opts deleteOptions) error { return fmt.Errorf("error getting user: %w", err) } - codespaces, err := opts.apiClient.ListCodespaces(ctx, user.Login) - if err != nil { - return fmt.Errorf("error getting codespaces: %w", err) - } - + var codespaces []*api.Codespace nameFilter := opts.codespaceName - if nameFilter == "" && !opts.deleteAll && opts.repoFilter == "" { - c, err := chooseCodespaceFromList(ctx, codespaces) + if nameFilter == "" { + codespaces, err = opts.apiClient.ListCodespaces(ctx, user.Login) if err != nil { - return fmt.Errorf("error choosing codespace: %w", err) + return fmt.Errorf("error getting codespaces: %w", err) } - nameFilter = c.Name + + if !opts.deleteAll && opts.repoFilter == "" { + c, err := chooseCodespaceFromList(ctx, codespaces) + if err != nil { + return fmt.Errorf("error choosing codespace: %w", err) + } + nameFilter = c.Name + } + } else { + // TODO: this token is discarded and then re-requested later in DeleteCodespace + token, err := opts.apiClient.GetCodespaceToken(ctx, user.Login, nameFilter) + if err != nil { + return fmt.Errorf("error getting codespace token: %w", err) + } + + codespace, err := opts.apiClient.GetCodespace(ctx, token, user.Login, nameFilter) + if err != nil { + return fmt.Errorf("error fetching codespace information: %w", err) + } + + codespaces = []*api.Codespace{codespace} } codespacesToDelete := make([]*api.Codespace, 0, len(codespaces)) @@ -112,7 +130,7 @@ func delete(ctx context.Context, log logger, opts deleteOptions) error { continue } } - if nameFilter == "" || !opts.skipConfirm { + if !opts.skipConfirm { confirmed, err := confirmDeletion(opts.prompter, c, opts.isInteractive) if err != nil { return fmt.Errorf("unable to confirm: %w", err) @@ -133,7 +151,7 @@ func delete(ctx context.Context, log logger, opts deleteOptions) error { codespaceName := c.Name g.Go(func() error { if err := opts.apiClient.DeleteCodespace(ctx, user.Login, codespaceName); err != nil { - log.Errorf("error deleting codespace %q: %v", codespaceName, err) + _, _ = log.Errorf("error deleting codespace %q: %v", codespaceName, err) return err } return nil diff --git a/cmd/ghcs/delete_test.go b/cmd/ghcs/delete_test.go index beb371dd4..7d90a3bd2 100644 --- a/cmd/ghcs/delete_test.go +++ b/cmd/ghcs/delete_test.go @@ -31,9 +31,6 @@ func TestDelete(t *testing.T) { codespaceName: "hubot-robawt-abc", }, codespaces: []*api.Codespace{ - { - Name: "monalisa-spoonknife-123", - }, { Name: "hubot-robawt-abc", }, @@ -130,12 +127,6 @@ func TestDelete(t *testing.T) { GetUserFunc: func(_ context.Context) (*api.User, error) { return user, nil }, - ListCodespacesFunc: func(_ context.Context, userLogin string) ([]*api.Codespace, error) { - if userLogin != user.Login { - return nil, fmt.Errorf("unexpected user %q", userLogin) - } - return tt.codespaces, nil - }, DeleteCodespaceFunc: func(_ context.Context, userLogin, name string) error { if userLogin != user.Login { return fmt.Errorf("unexpected user %q", userLogin) @@ -143,6 +134,30 @@ func TestDelete(t *testing.T) { return nil }, } + if tt.opts.codespaceName == "" { + apiMock.ListCodespacesFunc = func(_ context.Context, userLogin string) ([]*api.Codespace, error) { + if userLogin != user.Login { + return nil, fmt.Errorf("unexpected user %q", userLogin) + } + return tt.codespaces, nil + } + } else { + apiMock.GetCodespaceTokenFunc = func(_ context.Context, userLogin, name string) (string, error) { + if userLogin != user.Login { + return "", fmt.Errorf("unexpected user %q", userLogin) + } + return "CS_TOKEN", nil + } + apiMock.GetCodespaceFunc = func(_ context.Context, token, userLogin, name string) (*api.Codespace, error) { + if userLogin != user.Login { + return nil, fmt.Errorf("unexpected user %q", userLogin) + } + if token != "CS_TOKEN" { + return nil, fmt.Errorf("unexpected token %q", token) + } + return tt.codespaces[0], nil + } + } opts := tt.opts opts.apiClient = apiMock opts.now = func() time.Time { return now } diff --git a/cmd/ghcs/mock_api.go b/cmd/ghcs/mock_api.go index 46edd2835..256a30ec3 100644 --- a/cmd/ghcs/mock_api.go +++ b/cmd/ghcs/mock_api.go @@ -10,10 +10,6 @@ import ( "github.com/github/ghcs/internal/api" ) -// Ensure, that apiClientMock does implement apiClient. -// If this is not the case, regenerate this file with moq. -var _ apiClient = &apiClientMock{} - // apiClientMock is a mock implementation of apiClient. // // func TestSomethingThatUsesapiClient(t *testing.T) { @@ -23,6 +19,12 @@ var _ apiClient = &apiClientMock{} // DeleteCodespaceFunc: func(ctx context.Context, user string, name string) error { // panic("mock out the DeleteCodespace method") // }, +// GetCodespaceFunc: func(ctx context.Context, token string, user string, name string) (*api.Codespace, error) { +// panic("mock out the GetCodespace method") +// }, +// GetCodespaceTokenFunc: func(ctx context.Context, user string, name string) (string, error) { +// panic("mock out the GetCodespaceToken method") +// }, // GetUserFunc: func(ctx context.Context) (*api.User, error) { // panic("mock out the GetUser method") // }, @@ -39,6 +41,12 @@ type apiClientMock struct { // DeleteCodespaceFunc mocks the DeleteCodespace method. DeleteCodespaceFunc func(ctx context.Context, user string, name string) error + // GetCodespaceFunc mocks the GetCodespace method. + GetCodespaceFunc func(ctx context.Context, token string, user string, name string) (*api.Codespace, error) + + // GetCodespaceTokenFunc mocks the GetCodespaceToken method. + GetCodespaceTokenFunc func(ctx context.Context, user string, name string) (string, error) + // GetUserFunc mocks the GetUser method. GetUserFunc func(ctx context.Context) (*api.User, error) @@ -56,6 +64,26 @@ type apiClientMock struct { // Name is the name argument value. Name string } + // GetCodespace holds details about calls to the GetCodespace method. + GetCodespace []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // Token is the token argument value. + Token string + // User is the user argument value. + User string + // Name is the name argument value. + Name string + } + // GetCodespaceToken holds details about calls to the GetCodespaceToken method. + GetCodespaceToken []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // User is the user argument value. + User string + // Name is the name argument value. + Name string + } // GetUser holds details about calls to the GetUser method. GetUser []struct { // Ctx is the ctx argument value. @@ -69,9 +97,11 @@ type apiClientMock struct { User string } } - lockDeleteCodespace sync.RWMutex - lockGetUser sync.RWMutex - lockListCodespaces sync.RWMutex + lockDeleteCodespace sync.RWMutex + lockGetCodespace sync.RWMutex + lockGetCodespaceToken sync.RWMutex + lockGetUser sync.RWMutex + lockListCodespaces sync.RWMutex } // DeleteCodespace calls DeleteCodespaceFunc. @@ -113,6 +143,88 @@ func (mock *apiClientMock) DeleteCodespaceCalls() []struct { return calls } +// GetCodespace calls GetCodespaceFunc. +func (mock *apiClientMock) GetCodespace(ctx context.Context, token string, user string, name string) (*api.Codespace, error) { + if mock.GetCodespaceFunc == nil { + panic("apiClientMock.GetCodespaceFunc: method is nil but apiClient.GetCodespace was just called") + } + callInfo := struct { + Ctx context.Context + Token string + User string + Name string + }{ + Ctx: ctx, + Token: token, + User: user, + Name: name, + } + mock.lockGetCodespace.Lock() + mock.calls.GetCodespace = append(mock.calls.GetCodespace, callInfo) + mock.lockGetCodespace.Unlock() + return mock.GetCodespaceFunc(ctx, token, user, name) +} + +// GetCodespaceCalls gets all the calls that were made to GetCodespace. +// Check the length with: +// len(mockedapiClient.GetCodespaceCalls()) +func (mock *apiClientMock) GetCodespaceCalls() []struct { + Ctx context.Context + Token string + User string + Name string +} { + var calls []struct { + Ctx context.Context + Token string + User string + Name string + } + mock.lockGetCodespace.RLock() + calls = mock.calls.GetCodespace + mock.lockGetCodespace.RUnlock() + return calls +} + +// GetCodespaceToken calls GetCodespaceTokenFunc. +func (mock *apiClientMock) GetCodespaceToken(ctx context.Context, user string, name string) (string, error) { + if mock.GetCodespaceTokenFunc == nil { + panic("apiClientMock.GetCodespaceTokenFunc: method is nil but apiClient.GetCodespaceToken was just called") + } + callInfo := struct { + Ctx context.Context + User string + Name string + }{ + Ctx: ctx, + User: user, + Name: name, + } + mock.lockGetCodespaceToken.Lock() + mock.calls.GetCodespaceToken = append(mock.calls.GetCodespaceToken, callInfo) + mock.lockGetCodespaceToken.Unlock() + return mock.GetCodespaceTokenFunc(ctx, user, name) +} + +// GetCodespaceTokenCalls gets all the calls that were made to GetCodespaceToken. +// Check the length with: +// len(mockedapiClient.GetCodespaceTokenCalls()) +func (mock *apiClientMock) GetCodespaceTokenCalls() []struct { + Ctx context.Context + User string + Name string +} { + var calls []struct { + Ctx context.Context + User string + Name string + } + mock.lockGetCodespaceToken.RLock() + calls = mock.calls.GetCodespaceToken + mock.lockGetCodespaceToken.RUnlock() + return calls +} + // GetUser calls GetUserFunc. func (mock *apiClientMock) GetUser(ctx context.Context) (*api.User, error) { if mock.GetUserFunc == nil { diff --git a/cmd/ghcs/mock_prompter.go b/cmd/ghcs/mock_prompter.go index e15209c03..56581b64d 100644 --- a/cmd/ghcs/mock_prompter.go +++ b/cmd/ghcs/mock_prompter.go @@ -7,10 +7,6 @@ import ( "sync" ) -// Ensure, that prompterMock does implement prompter. -// If this is not the case, regenerate this file with moq. -var _ prompter = &prompterMock{} - // prompterMock is a mock implementation of prompter. // // func TestSomethingThatUsesprompter(t *testing.T) { From 3d017b282484617a73ca185d27dfcefedefe2e46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 24 Sep 2021 15:09:41 +0200 Subject: [PATCH 19/19] Fix stderr output on delete errors --- cmd/ghcs/delete.go | 2 +- cmd/ghcs/delete_test.go | 38 +++++++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 0c21c1674..94aaaf214 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -151,7 +151,7 @@ func delete(ctx context.Context, log logger, opts deleteOptions) error { codespaceName := c.Name g.Go(func() error { if err := opts.apiClient.DeleteCodespace(ctx, user.Login, codespaceName); err != nil { - _, _ = log.Errorf("error deleting codespace %q: %v", codespaceName, err) + _, _ = log.Errorf("error deleting codespace %q: %v\n", codespaceName, err) return err } return nil diff --git a/cmd/ghcs/delete_test.go b/cmd/ghcs/delete_test.go index 7d90a3bd2..47e6a4d6c 100644 --- a/cmd/ghcs/delete_test.go +++ b/cmd/ghcs/delete_test.go @@ -1,12 +1,15 @@ package ghcs import ( + "bytes" "context" + "errors" "fmt" "sort" "testing" "time" + "github.com/github/ghcs/cmd/ghcs/output" "github.com/github/ghcs/internal/api" ) @@ -22,8 +25,11 @@ func TestDelete(t *testing.T) { opts deleteOptions codespaces []*api.Codespace confirms map[string]bool + deleteErr error wantErr bool wantDeleted []string + wantStdout string + wantStderr string }{ { name: "by name", @@ -80,6 +86,24 @@ func TestDelete(t *testing.T) { }, wantDeleted: []string{"hubot-robawt-abc", "monalisa-spoonknife-c4f3"}, }, + { + 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: "error deleting codespace \"hubot-robawt-abc\": aborted by test\nerror deleting codespace \"monalisa-spoonknife-123\": aborted by test\n", + }, { name: "with confirm", opts: deleteOptions{ @@ -131,6 +155,9 @@ func TestDelete(t *testing.T) { if userLogin != user.Login { return fmt.Errorf("unexpected user %q", userLogin) } + if tt.deleteErr != nil { + return tt.deleteErr + } return nil }, } @@ -171,7 +198,10 @@ func TestDelete(t *testing.T) { }, } - err := delete(context.Background(), nil, opts) + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + log := output.NewLogger(stdout, stderr, false) + err := delete(context.Background(), log, opts) if (err != nil) != tt.wantErr { t.Errorf("delete() error = %v, wantErr %v", err, tt.wantErr) } @@ -186,6 +216,12 @@ func TestDelete(t *testing.T) { if !sliceEquals(gotDeleted, tt.wantDeleted) { t.Errorf("deleted %q, want %q", gotDeleted, tt.wantDeleted) } + 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) + } }) } }