Skip to content
This repository has been archived by the owner on Jul 12, 2022. It is now read-only.

Adding flags, delete confirmation and new tests to delete repo #908

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion functional/stdin/stdin_feature.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"steps": [
{
"key": "",
"value": "{\"provider\":\"Github\", \"name\":\"Leonidas\", \"url\":\"https://github.com/viniciussousazup/ritchie-formulas\", \"version\":\"0.0.3\",\"priority\":999}",
"value": "{\"provider\":\"Github\", \"name\":\"Leonidas\", \"url\":\"https://github.com/ZupIT/ritchie-formulas\", \"version\":\"2.14.2\",\"priority\":999}",
"action": "echo"
},
{
Expand Down
104 changes: 82 additions & 22 deletions pkg/cmd/delete_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
package cmd

import (
"errors"
"fmt"
"os"
"reflect"

"github.com/spf13/cobra"

Expand All @@ -28,55 +30,63 @@ import (
)

const (
deleteSuccessMsg = "Repository %q was deleted with success"
deleteSuccessMsg = "Repository %q was deleted with success"
repoFlagDescription = "Repository name to delete"
nameFlag = "name"
)

var deleteRepoFlags = flags{
{
name: nameFlag,
kind: reflect.String,
defValue: "",
description: repoFlagDescription,
},
}

type deleteRepoCmd struct {
formula.RepositoryLister
prompt.InputList
prompt.InputBool
formula.RepositoryDeleter
}

func NewDeleteRepoCmd(rl formula.RepositoryLister, il prompt.InputList, rd formula.RepositoryDeleter) *cobra.Command {
dr := deleteRepoCmd{rl, il, rd}
func NewDeleteRepoCmd(
rl formula.RepositoryLister,
il prompt.InputList,
ib prompt.InputBool,
rd formula.RepositoryDeleter,
) *cobra.Command {
dr := deleteRepoCmd{rl, il, ib, rd}

cmd := &cobra.Command{
Use: "repo",
Short: "Delete a repository",
Example: "rit delete repo",
RunE: RunFuncE(dr.runStdin(), dr.runFunc()),
RunE: RunFuncE(dr.runStdin(), dr.runCmd()),
ValidArgs: []string{""},
Args: cobra.OnlyValidArgs,
}

addReservedFlags(cmd.Flags(), deleteRepoFlags)

return cmd
}

func (dr deleteRepoCmd) runFunc() CommandRunnerFunc {
func (dr deleteRepoCmd) runCmd() CommandRunnerFunc {
return func(cmd *cobra.Command, args []string) error {
repos, err := dr.RepositoryLister.List()
name, err := dr.resolveInput(cmd)
if err != nil {
return err
}

if len(repos) == 0 {
prompt.Warning("You don't have any repositories")
} else if name == "" {
return nil
}

var reposNames []string
for _, r := range repos {
reposNames = append(reposNames, r.Name.String())
}

repo, err := dr.InputList.List("Repository:", reposNames)
if err != nil {
return err
}

if err = dr.Delete(formula.RepoName(repo)); err != nil {
if err := dr.Delete(formula.RepoName(name)); err != nil {
return err
}

prompt.Success(fmt.Sprintf(deleteSuccessMsg, repo))
prompt.Success(fmt.Sprintf(deleteSuccessMsg, name))
return nil
}
}
Expand All @@ -99,3 +109,53 @@ func (dr deleteRepoCmd) runStdin() CommandRunnerFunc {
return nil
}
}

func (dr *deleteRepoCmd) resolveInput(cmd *cobra.Command) (string, error) {
if IsFlagInput(cmd) {
return dr.resolveFlags(cmd)
}
return dr.resolvePrompt()
}

func (dr *deleteRepoCmd) resolvePrompt() (string, error) {
repos, err := dr.RepositoryLister.List()
if err != nil {
return "", err
}

if len(repos) == 0 {
prompt.Warning("You don't have any repositories")
return "", nil
}

reposNames := make([]string, 0, len(repos))
for _, r := range repos {
reposNames = append(reposNames, r.Name.String())
}

repo, err := dr.InputList.List("Repository:", reposNames)
if err != nil {
return "", err
}

question := "Are you sure you want to delete this repo?"
ans, err := dr.InputBool.Bool(question, []string{"no", "yes"})
if err != nil {
return "", err
}
if !ans {
return "", nil
}
return repo, nil
}

func (dr *deleteRepoCmd) resolveFlags(cmd *cobra.Command) (string, error) {
name, err := cmd.Flags().GetString(nameFlag)
if err != nil {
return "", err
} else if name == "" {
return "", errors.New(missingFlagText(nameFlag))
}

return name, nil
}
104 changes: 90 additions & 14 deletions pkg/cmd/delete_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,36 @@ package cmd

import (
"errors"
"os"
"path/filepath"
"testing"

"github.com/ZupIT/ritchie-cli/internal/mocks"
"github.com/ZupIT/ritchie-cli/pkg/formula"
"github.com/ZupIT/ritchie-cli/pkg/formula/repo"
"github.com/ZupIT/ritchie-cli/pkg/stream"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)

func TestNewDeleteRepo(t *testing.T) {
tmpDir := os.TempDir()
ritHomeName := ".rit"
ritHome := filepath.Join(tmpDir, ritHomeName)
reposPath := filepath.Join(ritHome, "repos")
repoName := "repoName"
repoPath := filepath.Join(reposPath, repoName)

type in struct {
args []string
repoList formula.Repos
repoListErr error
inputListString string
inputListErr error
repoDeleteErr error
args []string
repoList formula.Repos
repoListErr error
inputListString string
inputListErr error
inputBool bool
existingRepoIsDeleted bool
inputBoolErr error
}

tests := []struct {
Expand All @@ -43,7 +56,7 @@ func TestNewDeleteRepo(t *testing.T) {
want error
}{
{
name: "success",
name: "success prompt",
in: in{
args: []string{},
repoList: formula.Repos{
Expand All @@ -52,7 +65,22 @@ func TestNewDeleteRepo(t *testing.T) {
Priority: 0,
},
},
inputListString: "repoName",
inputListString: "repoName",
inputBool: true,
existingRepoIsDeleted: true,
},
},
{
name: "success flag",
in: in{
args: []string{"--name=repoName"},
repoList: formula.Repos{
{
Name: "repoName",
Priority: 0,
},
},
existingRepoIsDeleted: true,
},
},
{
Expand All @@ -78,7 +106,7 @@ func TestNewDeleteRepo(t *testing.T) {
want: errors.New("error to input list"),
},
{
name: "error to delete repo",
name: "error to input bool",
in: in{
args: []string{},
repoList: formula.Repos{
Expand All @@ -88,30 +116,78 @@ func TestNewDeleteRepo(t *testing.T) {
},
},
inputListString: "repoName",
repoDeleteErr: errors.New("error to delete repo"),
inputBoolErr: errors.New("error to input bool"),
},
want: errors.New("error to delete repo"),
want: errors.New("error to input bool"),
},
{
name: "do not accept delete selected repo",
in: in{
args: []string{},
repoList: formula.Repos{
{
Name: "repoName",
Priority: 0,
},
},
inputListString: "repoName",
inputBool: false,
},
},
{
name: "error on empty flag",
in: in{
args: []string{"--name="},
},
want: errors.New("please provide a value for 'name'"),
},
{
name: "error to delete repo with wrong name",
in: in{
args: []string{"--name=wrongName"},
repoList: formula.Repos{
{
Name: "repoName",
Priority: 0,
},
},
},
want: errors.New("no repository with this name"),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_ = os.MkdirAll(repoPath, os.ModePerm)
defer os.RemoveAll(ritHome)
fileManager := stream.NewFileManager()
dirManager := stream.NewDirManager(fileManager)

inputListMock := new(mocks.InputListMock)
inputListMock.On("List", mock.Anything, mock.Anything, mock.Anything).Return(tt.in.inputListString, tt.in.inputListErr)
inputBoolMock := new(mocks.InputBoolMock)
inputBoolMock.On("Bool", mock.Anything, mock.Anything, mock.Anything).Return(tt.in.inputBool, tt.in.inputBoolErr)
repoManagerMock := new(mocks.RepoManager)
repoManagerMock.On("Delete", mock.Anything).Return(tt.in.repoDeleteErr)
repoManagerMock.On("List", mock.Anything, mock.Anything, mock.Anything).Return(tt.in.repoList, tt.in.repoListErr)
repoManagerMock.On("Write", mock.Anything).Return(nil)

repoDeleter := repo.NewDeleter(ritHome, repoManagerMock, dirManager)

cmd := NewDeleteRepoCmd(
repoManagerMock,
inputListMock,
repoManagerMock,
inputBoolMock,
repoDeleter,
)
// TODO: remove stdin flag after deprecation
cmd.PersistentFlags().Bool("stdin", false, "input by stdin")
cmd.SetArgs(tt.in.args)

got := cmd.Execute()
brunasilvazup marked this conversation as resolved.
Show resolved Hide resolved
if tt.in.existingRepoIsDeleted {
assert.NoDirExists(t, repoPath)
} else {
assert.DirExists(t, repoPath)
}
assert.Equal(t, tt.want, got)
})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func Build() *cobra.Command {

updateRepoCmd := cmd.NewUpdateRepoCmd(http.DefaultClient, repoListUpdater, repoProviders, inputText, inputPassword, inputURL, inputList, inputBool, inputInt)
listRepoCmd := cmd.NewListRepoCmd(repoLister, tutorialFinder)
deleteRepoCmd := cmd.NewDeleteRepoCmd(repoLister, inputList, repoDeleter)
deleteRepoCmd := cmd.NewDeleteRepoCmd(repoLister, inputList, inputBool, repoDeleter)
listWorkspaceCmd := cmd.NewListWorkspaceCmd(formulaWorkspace, tutorialFinder)
setPriorityCmd := cmd.NewSetPriorityCmd(inputList, inputInt, repoLister, repoPrioritySetter)
autocompleteZsh := cmd.NewAutocompleteZsh(autocompleteGen)
Expand Down
6 changes: 6 additions & 0 deletions pkg/formula/repo/deleter.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
package repo

import (
"errors"
"os"
"path/filepath"

"github.com/ZupIT/ritchie-cli/pkg/formula"
Expand Down Expand Up @@ -53,6 +55,10 @@ func (dm DeleteManager) Delete(repoName formula.RepoName) error {

func (dm DeleteManager) deleteRepoDir(repoName formula.RepoName) error {
repoPath := filepath.Join(dm.ritHome, reposDirName, repoName.String())
if _, err := os.Stat(repoPath); os.IsNotExist(err) {
return errors.New("no repository with this name")
}

Comment on lines +58 to +61
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about adding tests on the deleter_test.go on this rule?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like a great idea, but at the same time it feels like it is already being tested.

deleter_test.go already checks for generic errors inside Delete function. And aiming to check for this specific rule tests should be refactored similarly to what happened to delete_repo_test.go.

On the other hand, on deleter_test.go we assure that any removal error is sent to callers and we certify it on "error to delete repo with wrong name" (delete_repo_test.go line 142).

In this fashion, I am not sure if deleter_test.go should get a refactor issue so this rule could be tested as noted.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any way deleter_test will still have to be refactored, maybe not necessary on this PR, but we should keep in mind

if err := dm.dir.Remove(repoPath); err != nil {
return err
}
Expand Down