Merge pull request #956 from Kasita-Inc/ISSUE-955

Issue 955

path/strip.go

@@ -8,18 +8,9 @@ import (
 	"github.com/Masterminds/glide/msg"
 )
 
-// StripVendor removes nested vendor and Godeps/_workspace/ directories.
-func StripVendor() error {
-	searchPath, _ := Vendor()
-	if _, err := os.Stat(searchPath); err != nil {
-		if os.IsNotExist(err) {
-			msg.Debug("Vendor directory does not exist.")
-		}
-
-		return err
-	}
-
-	err := filepath.Walk(searchPath, func(path string, info os.FileInfo, err error) error {
+func getWalkFunction(searchPath string, removeAll func(p string) error) func(path string,
+	info os.FileInfo, err error) error {
+	return func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
@@ -29,20 +20,31 @@ func StripVendor() error {
 			return nil
 		}
 
-		name := info.Name()
-		if name == "vendor" {
-			if _, err := os.Stat(path); err == nil {
-				if info.IsDir() {
-					msg.Info("Removing: %s", path)
-					return CustomRemoveAll(path)
-				}
-
-				msg.Debug("%s is not a directory. Skipping removal", path)
-				return nil
+		if info.Name() == "vendor" && info.IsDir() {
+			msg.Info("Removing: %s", path)
+			err = removeAll(path)
+			if nil != err {
+				return err
 			}
+			return filepath.SkipDir
 		}
 		return nil
-	})
+	}
+}
+
+// StripVendor removes nested vendor and Godeps/_workspace/ directories.
+func StripVendor() error {
+	searchPath, _ := Vendor()
+	if _, err := os.Stat(searchPath); err != nil {
+		if os.IsNotExist(err) {
+			msg.Debug("Vendor directory does not exist.")
+		}
+
+		return err
+	}
+
+	err := filepath.Walk(searchPath, getWalkFunction(searchPath, CustomRemoveAll))
+
 	if err != nil {
 		return err
 	}

path/strip_int_test.go

@@ -0,0 +1,44 @@
+package path
+
+import (
+	"io/ioutil"
+	"os"
+	"path"
+	"testing"
+)
+
+func generateTestDirectory(t *testing.T) string {
+	baseDir, err := ioutil.TempDir(os.TempDir(), "mgt")
+	if nil != err {
+		t.Error("Unable to create temp directory: ", err.Error())
+	}
+	paths := map[string][]string{
+		"github.com/fake/log":                                                                            {"log.go"},
+		"github.com/phoney/foo":                                                                          {"bar.go"},
+		"github.com/phoney/foo/vendor":                                                                   {"test.go", "foo.bar"},
+		"github.com/aws/aws-sdk-go/awsmigrate/awsmigrate-renamer/vendor":                                 {},
+		"github.com/aws/aws-sdk-go/awsmigrate/awsmigrate-renamer/vendor/golang.org/x/tools/go/buildutil": {"allpackages.go", "tags.go", "fakecontext.go"},
+		"github.com/aws/aws-sdk-go/vendor":                                                               {"key_test.go", "key.go"},
+		"github.com/aws/aws-sdk-go/vendor/github.com/go-ini/ini":                                         {"struct_test.go", "error.go", "ini_test.go"},
+	}
+	os.OpenFile(path.Join(baseDir, "glide.yaml"), os.O_RDONLY|os.O_CREATE, 0666)
+	for p, files := range paths {
+		p = path.Join(baseDir, "vendor", p)
+		if err = os.MkdirAll(p, 0777); nil != err {
+			t.Errorf("Unable to create vendor dir: %s\n%s", p, err.Error())
+		}
+		for _, f := range files {
+			os.OpenFile(path.Join(p, f), os.O_RDONLY|os.O_CREATE, 0666)
+		}
+	}
+	return baseDir
+}
+
+func TestNestVendorNoError(t *testing.T) {
+	workingDir := generateTestDirectory(t)
+	os.Chdir(workingDir)
+	err := StripVendor()
+	if nil != err {
+		t.Errorf("Unexpected error in StripVendor: %s", err.Error())
+	}
+}

path/strip_test.go

@@ -0,0 +1,141 @@
+package path
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"reflect"
+	"testing"
+	"time"
+)
+
+type mockFileInfo struct {
+	name  string
+	isDir bool
+}
+
+func (mfi *mockFileInfo) Name() string {
+	return mfi.name
+}
+
+func (mfi *mockFileInfo) Size() int64 {
+	panic("not implemented")
+}
+
+func (mfi *mockFileInfo) Mode() os.FileMode {
+	panic("not implemented")
+}
+
+func (mfi *mockFileInfo) ModTime() time.Time {
+	panic("not implemented")
+}
+
+func (mfi *mockFileInfo) IsDir() bool {
+	return mfi.isDir
+}
+
+func (mfi *mockFileInfo) Sys() interface{} {
+	panic("not implemented")
+}
+
+type removeAll struct {
+	calledWith string
+	err        error
+}
+
+func (rah *removeAll) removeAll(p string) error {
+	rah.calledWith = p
+	return rah.err
+}
+
+func TestWalkFunction(t *testing.T) {
+	type args struct {
+		searchPath string
+		removeAll  *removeAll
+		path       string
+		info       os.FileInfo
+		err        error
+	}
+	tests := []struct {
+		name           string
+		args           args
+		want           error
+		wantCalledWith string
+	}{
+		{
+			name: "WalkFunctionSkipsNonVendor",
+			args: args{searchPath: "foo",
+				removeAll: &removeAll{},
+				path:      "foo/bar",
+				info:      &mockFileInfo{name: "bar", isDir: true},
+				err:       nil,
+			},
+			want:           nil,
+			wantCalledWith: "",
+		},
+		{
+			name: "WalkFunctionSkipsNonDir",
+			args: args{searchPath: "foo",
+				removeAll: &removeAll{},
+				path:      "foo/vendor",
+				info:      &mockFileInfo{name: "vendor", isDir: false},
+				err:       nil,
+			},
+			want:           nil,
+			wantCalledWith: "",
+		},
+		{
+			name: "WalkFunctionDeletesVendor",
+			args: args{searchPath: "foo",
+				removeAll: &removeAll{},
+				path:      "foo/vendor",
+				info:      &mockFileInfo{name: "vendor", isDir: true},
+				err:       nil,
+			},
+			want:           filepath.SkipDir,
+			wantCalledWith: "foo/vendor",
+		},
+		{
+			name: "WalkFunctionReturnsPassedError",
+			args: args{searchPath: "foo",
+				removeAll: &removeAll{},
+				path:      "foo/vendor",
+				info:      &mockFileInfo{name: "vendor", isDir: true},
+				err:       errors.New("expected"),
+			},
+			want:           errors.New("expected"),
+			wantCalledWith: "",
+		},
+		{
+			name: "WalkFunctionReturnsRemoveAllError",
+			args: args{searchPath: "foo",
+				removeAll: &removeAll{err: errors.New("expected")},
+				path:      "foo/vendor",
+				info:      &mockFileInfo{name: "vendor", isDir: true},
+				err:       nil,
+			},
+			want:           errors.New("expected"),
+			wantCalledWith: "foo/vendor",
+		},
+		{
+			name: "WalkFunctionSkipsBaseDir",
+			args: args{searchPath: "vendor",
+				removeAll: &removeAll{},
+				path:      "vendor",
+				info:      &mockFileInfo{name: "vendor", isDir: true},
+				err:       nil,
+			},
+			want:           nil,
+			wantCalledWith: "",
+		},
+	}
+	for _, test := range tests {
+		walkFunction := getWalkFunction(test.args.searchPath, test.args.removeAll.removeAll)
+		if actual := walkFunction(test.args.path, test.args.info, test.args.err); !reflect.DeepEqual(actual, test.want) {
+			t.Errorf("walkFunction() = %v, want %v", actual, test.want)
+		}
+		if test.args.removeAll.calledWith != test.wantCalledWith {
+			t.Errorf("removeAll argument = \"%s\", want \"%s\"", test.args.removeAll.calledWith, test.wantCalledWith)
+		}
+	}
+}