Skip to content

Commit

Permalink
fix(selector): should have image select option when no Dockerfile fou…
Browse files Browse the repository at this point in the history
…nd (#1659)

<!-- Provide summary of changes -->
fixes #1656.
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
  • Loading branch information
iamhopaul123 authored Nov 11, 2020
1 parent beae041 commit 8b8e0ad
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 59 deletions.
3 changes: 1 addition & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@ require (
github.com/fatih/color v1.10.0
github.com/fatih/structs v1.1.0
github.com/gobuffalo/packd v1.0.0
github.com/gobuffalo/packr/v2 v2.8.0
github.com/gobuffalo/packr/v2 v2.8.1
github.com/golang/mock v1.4.4
github.com/google/go-cmp v0.5.1 // indirect
github.com/google/shlex v0.0.0-20150127133951-6f45313302b9
github.com/google/uuid v1.1.2
github.com/hinshun/vt10x v0.0.0-20180809195222-d55458df857c // indirect
github.com/imdario/mergo v0.3.9
github.com/karrick/godirwalk v1.15.6 // indirect
github.com/lnquy/cron v1.0.1
github.com/moby/buildkit v0.7.2
github.com/onsi/ginkgo v1.14.2
Expand Down
9 changes: 4 additions & 5 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ github.com/gobuffalo/logger v1.0.3 h1:YaXOTHNPCvkqqA7w05A4v0k2tCdpr+sgFlgINbQ6gq
github.com/gobuffalo/logger v1.0.3/go.mod h1:SoeejUwldiS7ZsyCBphOGURmWdwUFXs0J7TCjEhjKxM=
github.com/gobuffalo/packd v1.0.0 h1:6ERZvJHfe24rfFmA9OaoKBdC7+c9sydrytMg8SdFGBM=
github.com/gobuffalo/packd v1.0.0/go.mod h1:6VTc4htmJRFB7u1m/4LeMTWjFoYrUiBkU9Fdec9hrhI=
github.com/gobuffalo/packr/v2 v2.8.0 h1:IULGd15bQL59ijXLxEvA5wlMxsmx/ZkQv9T282zNVIY=
github.com/gobuffalo/packr/v2 v2.8.0/go.mod h1:PDk2k3vGevNE3SwVyVRgQCCXETC9SaONCNSXT1Q8M1g=
github.com/gobuffalo/packr/v2 v2.8.1 h1:tkQpju6i3EtMXJ9uoF5GT6kB+LMTimDWD8Xvbz6zDVA=
github.com/gobuffalo/packr/v2 v2.8.1/go.mod h1:c/PLlOuTU+p3SybaJATW3H6lX/iK7xEz5OeMf+NnJpg=
github.com/godbus/dbus v0.0.0-20190422162347-ade71ed3457e/go.mod h1:bBOAhwG1umN6/6ZUMtDFBMQR8jRg9O75tm9K00oMsK4=
github.com/godbus/dbus/v5 v5.0.3/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
github.com/gofrs/flock v0.7.0/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU=
Expand Down Expand Up @@ -249,9 +249,8 @@ github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1
github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo=
github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU=
github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w=
github.com/karrick/godirwalk v1.15.3/go.mod h1:j4mkqPuvaLI8mp1DroR3P6ad7cyYd4c1qeJ3RV7ULlk=
github.com/karrick/godirwalk v1.15.6 h1:Yf2mmR8TJy+8Fa0SuQVto5SYap6IF7lNVX4Jdl8G1qA=
github.com/karrick/godirwalk v1.15.6/go.mod h1:j4mkqPuvaLI8mp1DroR3P6ad7cyYd4c1qeJ3RV7ULlk=
github.com/karrick/godirwalk v1.15.8 h1:7+rWAZPn9zuRxaIqqT8Ohs2Q2Ac0msBqwRdxNCr2VVs=
github.com/karrick/godirwalk v1.15.8/go.mod h1:j4mkqPuvaLI8mp1DroR3P6ad7cyYd4c1qeJ3RV7ULlk=
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
Expand Down
34 changes: 16 additions & 18 deletions internal/pkg/term/selector/selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ const (
)

const (
// dockerfilePromptUseCustom is the option for using Dockerfile with custom path.
dockerfilePromptUseCustom = "Enter custom path for your Dockerfile"
// DockerfilePromptUseImage is the option for using existing image instead of Dockerfile.
DockerfilePromptUseImage = "Use an existing image instead"

Expand Down Expand Up @@ -515,27 +517,23 @@ func (s *WorkspaceSelect) retrieveWorkspaceWorkloads() ([]string, error) {
// directory or one level down. If no dockerfiles are found, it asks for a custom path.
func (s *WorkspaceSelect) Dockerfile(selPrompt, notFoundPrompt, selHelp, notFoundHelp string, pathValidator prompt.ValidatorFunc) (string, error) {
dockerfiles, err := s.ws.ListDockerfiles()
// If Dockerfiles are found in the current directory or subdirectory one level down, ask the user to select one.
if err != nil {
return "", fmt.Errorf("list Dockerfiles: %w", err)
}
var sel string
if err == nil {
dockerfiles = append(dockerfiles, DockerfilePromptUseImage)
sel, err = s.prompt.SelectOne(
selPrompt,
selHelp,
dockerfiles,
prompt.WithFinalMessage("Dockerfile:"),
)
if err != nil {
return "", fmt.Errorf("select Dockerfile: %w", err)
}
return sel, nil
dockerfiles = append(dockerfiles, []string{dockerfilePromptUseCustom, DockerfilePromptUseImage}...)
sel, err = s.prompt.SelectOne(
selPrompt,
selHelp,
dockerfiles,
prompt.WithFinalMessage("Dockerfile:"),
)
if err != nil {
return "", fmt.Errorf("select Dockerfile: %w", err)
}

var notExistErr *workspace.ErrDockerfileNotFound
if !errors.As(err, &notExistErr) {
return "", err
if sel != dockerfilePromptUseCustom {
return sel, nil
}
// If no Dockerfiles were found, prompt user for custom path.
sel, err = s.prompt.Get(
notFoundPrompt,
notFoundHelp,
Expand Down
52 changes: 37 additions & 15 deletions internal/pkg/term/selector/selector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1207,11 +1207,18 @@ func TestSelect_Application(t *testing.T) {
}

func TestWorkspaceSelect_Dockerfile(t *testing.T) {
var dockerfiles = []string{
dockerfiles := []string{
"./Dockerfile",
"backend/Dockerfile",
"frontend/Dockerfile",
}
dockerfileOptions := []string{
"./Dockerfile",
"backend/Dockerfile",
"frontend/Dockerfile",
"Enter custom path for your Dockerfile",
"Use an existing image instead",
}
testCases := map[string]struct {
mockWs func(retriever *mocks.MockWorkspaceRetriever)
mockPrompt func(*mocks.MockPrompter)
Expand All @@ -1226,23 +1233,26 @@ func TestWorkspaceSelect_Dockerfile(t *testing.T) {
mockPrompt: func(m *mocks.MockPrompter) {
m.EXPECT().SelectOne(
gomock.Any(), gomock.Any(),
gomock.Eq([]string{
"./Dockerfile",
"backend/Dockerfile",
"frontend/Dockerfile",
"Use an existing image instead",
}),
gomock.Eq(dockerfileOptions),
gomock.Any(),
).Return("frontend/Dockerfile", nil)
},
wantedErr: nil,
wantedDockerfile: "frontend/Dockerfile",
},
"prompts user for custom path if fail to find Dockerfiles": {
"prompts user for custom path": {
mockWs: func(m *mocks.MockWorkspaceRetriever) {
m.EXPECT().ListDockerfiles().Return(nil, &workspace.ErrDockerfileNotFound{})
m.EXPECT().ListDockerfiles().Return([]string{}, nil)
},
mockPrompt: func(m *mocks.MockPrompter) {
m.EXPECT().SelectOne(
gomock.Any(), gomock.Any(),
gomock.Eq([]string{
"Enter custom path for your Dockerfile",
"Use an existing image instead",
}),
gomock.Any(),
).Return("Enter custom path for your Dockerfile", nil)
m.EXPECT().Get(
gomock.Any(),
gomock.Any(),
Expand All @@ -1253,33 +1263,45 @@ func TestWorkspaceSelect_Dockerfile(t *testing.T) {
wantedErr: nil,
wantedDockerfile: "crazy/path/Dockerfile",
},
"returns an error if fail to get custom Dockerfile path": {
"returns an error if fail to list Dockerfile": {
mockWs: func(m *mocks.MockWorkspaceRetriever) {
m.EXPECT().ListDockerfiles().Return(nil, errors.New("some error"))
},
mockPrompt: func(m *mocks.MockPrompter) {},
wantedErr: fmt.Errorf("list Dockerfiles: some error"),
},
"returns an error if fail to select Dockerfile": {
mockWs: func(m *mocks.MockWorkspaceRetriever) {
m.EXPECT().ListDockerfiles().Return(nil, &workspace.ErrDockerfileNotFound{})
m.EXPECT().ListDockerfiles().Return(dockerfiles, nil)
},
mockPrompt: func(m *mocks.MockPrompter) {
m.EXPECT().Get(
m.EXPECT().SelectOne(
gomock.Any(),
gomock.Any(),
gomock.Any(),
gomock.Any(),
).Return("", errors.New("some error"))
},
wantedErr: fmt.Errorf("get custom Dockerfile path: some error"),
wantedErr: fmt.Errorf("select Dockerfile: some error"),
},
"returns an error if fail to select Dockerfile": {
"returns an error if fail to get custom Dockerfile path": {
mockWs: func(m *mocks.MockWorkspaceRetriever) {
m.EXPECT().ListDockerfiles().Return(dockerfiles, nil)
},
mockPrompt: func(m *mocks.MockPrompter) {
m.EXPECT().SelectOne(
gomock.Any(), gomock.Any(),
gomock.Eq(dockerfileOptions),
gomock.Any(),
).Return("Enter custom path for your Dockerfile", nil)
m.EXPECT().Get(
gomock.Any(),
gomock.Any(),
gomock.Any(),
gomock.Any(),
).Return("", errors.New("some error"))
},
wantedErr: fmt.Errorf("select Dockerfile: some error"),
wantedErr: fmt.Errorf("get custom Dockerfile path: some error"),
},
}
for name, tc := range testCases {
Expand Down
15 changes: 0 additions & 15 deletions internal/pkg/workspace/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,11 +468,6 @@ func (ws *Workspace) ListDockerfiles() ([]string, error) {
}
}
}
if len(directories) == 0 {
return nil, &ErrDockerfileNotFound{
dir: ws.workingDir,
}
}
sort.Strings(directories)
dockerfiles := make([]string, 0, len(directories))
for _, dir := range directories {
Expand All @@ -482,16 +477,6 @@ func (ws *Workspace) ListDockerfiles() ([]string, error) {
return dockerfiles, nil
}

// ErrDockerfileNotFound is returned when no Dockerfiles could be found in the current
// working directory or in any directories one level down from it.
type ErrDockerfileNotFound struct {
dir string
}

func (e *ErrDockerfileNotFound) Error() string {
return fmt.Sprintf("no Dockerfiles found within %s or a sub-directory level below", e.dir)
}

// RelPath returns the path relative to the current working directory.
func RelPath(fullPath string) (string, error) {
wkdir, err := os.Getwd()
Expand Down
10 changes: 6 additions & 4 deletions internal/pkg/workspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,11 +820,12 @@ func TestWorkspace_DeleteWorkspaceFile(t *testing.T) {
}
}

func TestValidateDockerfiles(t *testing.T) {
func TestWorkspace_ListDockerfiles(t *testing.T) {
wantedDockerfiles := []string{"./Dockerfile", "backend/Dockerfile", "frontend/Dockerfile"}
testCases := map[string]struct {
mockFileSystem func(mockFS afero.Fs)
err error
dockerfiles []string
}{
"find Dockerfiles": {
mockFileSystem: func(mockFS afero.Fs) {
Expand All @@ -835,11 +836,12 @@ func TestValidateDockerfiles(t *testing.T) {
afero.WriteFile(mockFS, "frontend/Dockerfile", []byte("FROM nginx"), 0644)
afero.WriteFile(mockFS, "backend/Dockerfile", []byte("FROM nginx"), 0644)
},
err: nil,
err: nil,
dockerfiles: wantedDockerfiles,
},
"no Dockerfiles": {
mockFileSystem: func(mockFS afero.Fs) {},
err: fmt.Errorf("no Dockerfiles found within / or a sub-directory level below"),
dockerfiles: []string{},
},
}

Expand All @@ -861,7 +863,7 @@ func TestValidateDockerfiles(t *testing.T) {
require.EqualError(t, err, tc.err.Error())
} else {
require.NoError(t, err)
require.Equal(t, wantedDockerfiles, got)
require.Equal(t, tc.dockerfiles, got)
}
})
}
Expand Down

0 comments on commit 8b8e0ad

Please sign in to comment.