Merge pull request #1809 from tejal29/refactor_walk

Add tests for skaffold init walk flow.

pkg/skaffold/initializer/init.go

@@ -77,35 +77,7 @@ func DoInit(out io.Writer, c Config) error {
 		}
 	}
 
-	var potentialConfigs, dockerfiles []string
-
-	err := filepath.Walk(rootDir, func(path string, f os.FileInfo, e error) error {
-		if f.IsDir() && util.IsHiddenDir(f.Name()) {
-			logrus.Debugf("skip walking hidden dir %s", f.Name())
-			return filepath.SkipDir
-		}
-		if f.IsDir() || util.IsHiddenFile(f.Name()) {
-			return nil
-		}
-		if IsSkaffoldConfig(path) {
-			if !c.Force {
-				return fmt.Errorf("pre-existing %s found", path)
-			}
-			logrus.Debugf("%s is a valid skaffold configuration: continuing since --force=true", path)
-			return nil
-		}
-		if IsSupportedKubernetesFileExtension(path) {
-			potentialConfigs = append(potentialConfigs, path)
-			return nil
-		}
-		// try and parse dockerfile
-		if docker.ValidateDockerfile(path) {
-			logrus.Infof("existing dockerfile found: %s", path)
-			dockerfiles = append(dockerfiles, path)
-		}
-		return nil
-	})
-
+	potentialConfigs, dockerfiles, err := walk(rootDir, c.Force, docker.ValidateDockerfile)
 	if err != nil {
 		return err
 	}
@@ -312,3 +284,37 @@ type dockerfilePair struct {
 	Dockerfile string
 	ImageName  string
 }
+
+func walk(dir string, force bool, validateDockerfile func(string) bool) ([]string, []string, error) {
+	var dockerfiles, potentialConfigs []string
+	err := filepath.Walk(dir, func(path string, f os.FileInfo, e error) error {
+		if f.IsDir() && util.IsHiddenDir(f.Name()) {
+			logrus.Debugf("skip walking hidden dir %s", f.Name())
+			return filepath.SkipDir
+		}
+		if f.IsDir() || util.IsHiddenFile(f.Name()) {
+			return nil
+		}
+		if IsSkaffoldConfig(path) {
+			if !force {
+				return fmt.Errorf("pre-existing %s found", path)
+			}
+			logrus.Debugf("%s is a valid skaffold configuration: continuing since --force=true", path)
+			return nil
+		}
+		if IsSupportedKubernetesFileExtension(path) {
+			potentialConfigs = append(potentialConfigs, path)
+			return nil
+		}
+		// try and parse dockerfile
+		if validateDockerfile(path) {
+			logrus.Infof("existing dockerfile found: %s", path)
+			dockerfiles = append(dockerfiles, path)
+		}
+		return nil
+	})
+	if err != nil {
+		return nil, nil, err
+	}
+	return potentialConfigs, dockerfiles, nil
+}

pkg/skaffold/initializer/init_test.go

@@ -18,46 +18,163 @@ package initializer
 
 import (
 	"bytes"
+	"strings"
 	"testing"
 
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
 func TestPrintAnalyzeJSON(t *testing.T) {
 	tests := []struct {
-		name        string
+		description string
 		dockerfiles []string
 		images      []string
 		skipBuild   bool
 		shouldErr   bool
 		expected    string
 	}{
 		{
-			name:        "dockerfile and image",
+			description: "dockerfile and image",
 			dockerfiles: []string{"Dockerfile", "Dockerfile_2"},
 			images:      []string{"image1", "image2"},
 			expected:    "{\"dockerfiles\":[\"Dockerfile\",\"Dockerfile_2\"],\"images\":[\"image1\",\"image2\"]}",
 		},
 		{
-			name:      "no dockerfile, skip build",
-			images:    []string{"image1", "image2"},
-			skipBuild: true,
-			expected:  "{\"images\":[\"image1\",\"image2\"]}"},
+			description: "no dockerfile, skip build",
+			images:      []string{"image1", "image2"},
+			skipBuild:   true,
+			expected:    "{\"images\":[\"image1\",\"image2\"]}"},
 		{
-			name:      "no dockerfile",
-			images:    []string{"image1", "image2"},
-			shouldErr: true,
+			description: "no dockerfile",
+			images:      []string{"image1", "image2"},
+			shouldErr:   true,
 		},
 		{
-			name:      "no dockerfiles or images",
-			shouldErr: true,
+			description: "no dockerfiles or images",
+			shouldErr:   true,
 		},
 	}
 	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
+		t.Run(test.description, func(t *testing.T) {
 			out := bytes.NewBuffer([]byte{})
 			err := printAnalyzeJSON(out, test.skipBuild, test.dockerfiles, test.images)
 			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, out.String())
 		})
 	}
 }
+
+func TestWalk(t *testing.T) {
+	emptyFile := ""
+	tests := []struct {
+		description         string
+		filesWithContents   map[string]string
+		expectedConfigs     []string
+		expectedDockerfiles []string
+		force               bool
+		shouldErr           bool
+	}{
+		{
+			description: "should return correct k8 configs and dockerfiles",
+			filesWithContents: map[string]string{
+				"config/test.yaml":  emptyFile,
+				"k8pod.yml":         emptyFile,
+				"README":            emptyFile,
+				"deploy/Dockerfile": emptyFile,
+				"Dockerfile":        emptyFile,
+			},
+			force: false,
+			expectedConfigs: []string{
+				"config/test.yaml",
+				"k8pod.yml",
+			},
+			expectedDockerfiles: []string{
+				"Dockerfile",
+				"deploy/Dockerfile",
+			},
+			shouldErr: false,
+		},
+		{
+			description: "should skip hidden dir",
+			filesWithContents: map[string]string{
+				".hidden/test.yaml":  emptyFile,
+				"k8pod.yml":          emptyFile,
+				"README":             emptyFile,
+				".hidden/Dockerfile": emptyFile,
+				"Dockerfile":         emptyFile,
+			},
+			force: false,
+			expectedConfigs: []string{
+				"k8pod.yml",
+			},
+			expectedDockerfiles: []string{
+				"Dockerfile",
+			},
+			shouldErr: false,
+		},
+		{
+			description: "should not error when skaffold.config present and force = true",
+			filesWithContents: map[string]string{
+				"skaffold.yaml": `apiVersion: skaffold/v1beta6
+kind: Config
+deploy:
+  kustomize: {}`,
+				"config/test.yaml":  emptyFile,
+				"k8pod.yml":         emptyFile,
+				"README":            emptyFile,
+				"deploy/Dockerfile": emptyFile,
+				"Dockerfile":        emptyFile,
+			},
+			force: true,
+			expectedConfigs: []string{
+				"config/test.yaml",
+				"k8pod.yml",
+			},
+			expectedDockerfiles: []string{
+				"Dockerfile",
+				"deploy/Dockerfile",
+			},
+			shouldErr: false,
+		},
+		{
+			description: "should  error when skaffold.config present and force = false",
+			filesWithContents: map[string]string{
+				"config/test.yaml":  emptyFile,
+				"k8pod.yml":         emptyFile,
+				"README":            emptyFile,
+				"deploy/Dockerfile": emptyFile,
+				"Dockerfile":        emptyFile,
+				"skaffold.yaml": `apiVersion: skaffold/v1beta6
+kind: Config
+deploy:
+  kustomize: {}`,
+			},
+			force:               false,
+			expectedConfigs:     nil,
+			expectedDockerfiles: nil,
+			shouldErr:           true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			testDir, cleanUp := testutil.NewTempDir(t)
+			defer cleanUp()
+			rootDir := testDir.Root()
+			writeAllFiles(testDir, test.filesWithContents)
+			potentialConfigs, dockerfiles, err := walk(rootDir, test.force, testValidDocker)
+			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err,
+				testDir.Paths(test.expectedConfigs), potentialConfigs)
+			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err,
+				testDir.Paths(test.expectedDockerfiles), dockerfiles)
+		})
+	}
+}
+
+func testValidDocker(path string) bool {
+	return strings.HasSuffix(path, "Dockerfile")
+}
+
+func writeAllFiles(tmpDir *testutil.TempDir, filesWithContents map[string]string) {
+	for file, contents := range filesWithContents {
+		tmpDir.Write(file, contents)
+	}
+}

testutil/tmp.go

@@ -128,3 +128,12 @@ func (h *TempDir) failIfErr(err error) *TempDir {
 	}
 	return h
 }
+
+// Paths returns the paths to a list of files in the temp directory.
+func (h *TempDir) Paths(files []string) []string {
+	var paths []string
+	for _, file := range files {
+		paths = append(paths, h.Path(file))
+	}
+	return paths
+}