From 885c1952a47d6a19419311958ecbf864275374f0 Mon Sep 17 00:00:00 2001 From: jregan Date: Sun, 28 Oct 2018 13:07:15 -0700 Subject: [PATCH] Improve test coverage. --- .../configmapfactory_test.go | 8 +- pkg/fs/fakefs.go | 55 ++-- pkg/fs/fakefs_test.go | 25 ++ pkg/loader/fileloader_test.go | 235 +++++++++++------- pkg/loader/githubloader_test.go | 16 +- pkg/transformers/config/factory_test.go | 6 +- 6 files changed, 230 insertions(+), 115 deletions(-) diff --git a/k8sdeps/configmapandsecret/configmapfactory_test.go b/k8sdeps/configmapandsecret/configmapfactory_test.go index 691dbe82e..4d2214d7e 100644 --- a/k8sdeps/configmapandsecret/configmapfactory_test.go +++ b/k8sdeps/configmapandsecret/configmapfactory_test.go @@ -94,7 +94,7 @@ func TestConstructConfigMap(t *testing.T) { input: types.ConfigMapArgs{ Name: "envConfigMap", DataSources: types.DataSources{ - EnvSource: "configmap/app.env", + EnvSource: "/configmap/app.env", }, }, options: nil, @@ -105,7 +105,7 @@ func TestConstructConfigMap(t *testing.T) { input: types.ConfigMapArgs{ Name: "fileConfigMap", DataSources: types.DataSources{ - FileSources: []string{"configmap/app-init.ini"}, + FileSources: []string{"/configmap/app-init.ini"}, }, }, options: nil, @@ -129,8 +129,8 @@ func TestConstructConfigMap(t *testing.T) { } fSys := fs.MakeFakeFS() - fSys.WriteFile("configmap/app.env", []byte("DB_USERNAME=admin\nDB_PASSWORD=somepw\n")) - fSys.WriteFile("configmap/app-init.ini", []byte("FOO=bar\nBAR=baz\n")) + fSys.WriteFile("/configmap/app.env", []byte("DB_USERNAME=admin\nDB_PASSWORD=somepw\n")) + fSys.WriteFile("/configmap/app-init.ini", []byte("FOO=bar\nBAR=baz\n")) f := NewConfigMapFactory(fSys, loader.NewFileLoader(fSys)) for _, tc := range testCases { cm, err := f.MakeConfigMap(&tc.input, tc.options) diff --git a/pkg/fs/fakefs.go b/pkg/fs/fakefs.go index 64029eab5..4695aa736 100644 --- a/pkg/fs/fakefs.go +++ b/pkg/fs/fakefs.go @@ -21,18 +21,21 @@ import ( "path/filepath" "sigs.k8s.io/kustomize/pkg/constants" "sort" + "strings" ) -var _ FileSystem = &FakeFS{} +var _ FileSystem = &fakeFs{} -// FakeFS implements FileSystem using a fake in-memory filesystem. -type FakeFS struct { +// fakeFs implements FileSystem using a fake in-memory filesystem. +type fakeFs struct { m map[string]*FakeFile } -// MakeFakeFS returns an instance of FakeFS with no files in it. -func MakeFakeFS() *FakeFS { - return &FakeFS{m: map[string]*FakeFile{}} +// MakeFakeFS returns an instance of fakeFs with no files in it. +func MakeFakeFS() *fakeFs { + result := &fakeFs{m: map[string]*FakeFile{}} + result.Mkdir("/") + return result } // kustomizationContent is used in tests. @@ -54,7 +57,7 @@ secretGenerator: [] ` // Create assures a fake file appears in the in-memory file system. -func (fs *FakeFS) Create(name string) (File, error) { +func (fs *fakeFs) Create(name string) (File, error) { f := &FakeFile{} f.open = true fs.m[name] = f @@ -62,18 +65,18 @@ func (fs *FakeFS) Create(name string) (File, error) { } // Mkdir assures a fake directory appears in the in-memory file system. -func (fs *FakeFS) Mkdir(name string) error { +func (fs *fakeFs) Mkdir(name string) error { fs.m[name] = makeDir(name) return nil } // MkdirAll delegates to Mkdir -func (fs *FakeFS) MkdirAll(name string) error { +func (fs *fakeFs) MkdirAll(name string) error { return fs.Mkdir(name) } // Open returns a fake file in the open state. -func (fs *FakeFS) Open(name string) (File, error) { +func (fs *fakeFs) Open(name string) (File, error) { if _, found := fs.m[name]; !found { return nil, fmt.Errorf("file %q cannot be opened", name) } @@ -81,13 +84,13 @@ func (fs *FakeFS) Open(name string) (File, error) { } // Exists returns true if file is known. -func (fs *FakeFS) Exists(name string) bool { +func (fs *fakeFs) Exists(name string) bool { _, found := fs.m[name] return found } // Glob returns the list of matching files -func (fs *FakeFS) Glob(pattern string) ([]string, error) { +func (fs *fakeFs) Glob(pattern string) ([]string, error) { var result []string for p := range fs.m { if fs.pathMatch(p, pattern) { @@ -99,28 +102,36 @@ func (fs *FakeFS) Glob(pattern string) ([]string, error) { } // IsDir returns true if the file exists and is a directory. -func (fs *FakeFS) IsDir(name string) bool { +func (fs *fakeFs) IsDir(name string) bool { f, found := fs.m[name] - if !found { - return false + if found && f.dir { + return true } - return f.dir + if !strings.HasSuffix(name, "/") { + name = name + "/" + } + for k := range fs.m { + if strings.HasPrefix(k, name) { + return true + } + } + return false } // ReadFile always returns an empty bytes and error depending on content of m. -func (fs *FakeFS) ReadFile(name string) ([]byte, error) { +func (fs *fakeFs) ReadFile(name string) ([]byte, error) { if ff, found := fs.m[name]; found { return ff.content, nil } return nil, fmt.Errorf("cannot read file %q", name) } -func (fs *FakeFS) ReadTestKustomization() ([]byte, error) { +func (fs *fakeFs) ReadTestKustomization() ([]byte, error) { return fs.ReadFile(constants.KustomizationFileName) } // WriteFile always succeeds and does nothing. -func (fs *FakeFS) WriteFile(name string, c []byte) error { +func (fs *fakeFs) WriteFile(name string, c []byte) error { ff := &FakeFile{} ff.Write(c) fs.m[name] = ff @@ -128,16 +139,16 @@ func (fs *FakeFS) WriteFile(name string, c []byte) error { } // WriteTestKustomization writes a standard test file. -func (fs *FakeFS) WriteTestKustomization() { +func (fs *fakeFs) WriteTestKustomization() { fs.WriteTestKustomizationWith([]byte(kustomizationContent)) } // WriteTestKustomizationWith writes a standard test file. -func (fs *FakeFS) WriteTestKustomizationWith(bytes []byte) { +func (fs *fakeFs) WriteTestKustomizationWith(bytes []byte) { fs.WriteFile(constants.KustomizationFileName, bytes) } -func (fs *FakeFS) pathMatch(path, pattern string) bool { +func (fs *fakeFs) pathMatch(path, pattern string) bool { match, _ := filepath.Match(pattern, path) return match } diff --git a/pkg/fs/fakefs_test.go b/pkg/fs/fakefs_test.go index 709d9df06..9278d3cb5 100644 --- a/pkg/fs/fakefs_test.go +++ b/pkg/fs/fakefs_test.go @@ -27,6 +27,10 @@ func TestExists(t *testing.T) { if x.Exists("foo") { t.Fatalf("expected no foo") } + x.Mkdir("/") + if !x.IsDir("/") { + t.Fatalf("expected dir at /") + } } func TestIsDir(t *testing.T) { @@ -44,6 +48,27 @@ func TestIsDir(t *testing.T) { } } +func TestIsDirDeeper(t *testing.T) { + x := MakeFakeFS() + x.WriteFile("/foo/project/file.yaml", []byte("Unused")) + x.WriteFile("/foo/project/subdir/file.yaml", []byte("Unused")) + if !x.IsDir("/") { + t.Fatalf("/ should be a dir") + } + if !x.IsDir("/foo") { + t.Fatalf("/foo should be a dir") + } + if !x.IsDir("/foo/project") { + t.Fatalf("/foo/project should be a dir") + } + if x.IsDir("/fo") { + t.Fatalf("/fo should not be a dir") + } + if x.IsDir("/x") { + t.Fatalf("/x should not be a dir") + } +} + func TestCreate(t *testing.T) { x := MakeFakeFS() f, err := x.Create("foo") diff --git a/pkg/loader/fileloader_test.go b/pkg/loader/fileloader_test.go index b0f657528..52fc78b37 100644 --- a/pkg/loader/fileloader_test.go +++ b/pkg/loader/fileloader_test.go @@ -17,102 +17,169 @@ limitations under the License. package loader import ( - "path/filepath" "reflect" + "strings" "testing" "sigs.k8s.io/kustomize/pkg/fs" ) -func TestLoader_Root(t *testing.T) { +type testData struct { + path string + expectedContent string +} - // Initialize the fake file system and the root loader. - fakefs := fs.MakeFakeFS() - fakefs.WriteFile("/home/seans/project/file.yaml", []byte("Unused")) - fakefs.WriteFile("/home/seans/project/subdir/file.yaml", []byte("Unused")) - fakefs.WriteFile("/home/seans/project2/file.yaml", []byte("Unused")) - rootLoader := NewFileLoader(fakefs) +var testCases = []testData{ + { + path: "/foo/project/fileA.yaml", + expectedContent: "fileA content", + }, + { + path: "/foo/project/subdir1/fileB.yaml", + expectedContent: "fileB content", + }, + { + path: "/foo/project/subdir2/fileC.yaml", + expectedContent: "fileC content", + }, + { + path: "/foo/project2/fileD.yaml", + expectedContent: "fileD content", + }, +} - _, err := rootLoader.New("") +func MakeFakeFs(td []testData) fs.FileSystem { + fSys := fs.MakeFakeFS() + for _, x := range td { + fSys.WriteFile(x.path, []byte(x.expectedContent)) + } + return fSys +} + +func TestLoaderLoad(t *testing.T) { + l1 := NewFileLoader(MakeFakeFs(testCases)) + if "" != l1.Root() { + t.Fatalf("incorrect root: '%s'\n", l1.Root()) + } + for _, x := range testCases { + b, err := l1.Load(x.path) + if err != nil { + t.Fatalf("unexpected load error %v", err) + } + if !reflect.DeepEqual([]byte(x.expectedContent), b) { + t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) + } + } + l2, err := l1.New("/foo/project") + if err != nil { + t.Fatalf("unexpected err: %v\n", err) + } + if "/foo/project" != l2.Root() { + t.Fatalf("incorrect root: %s\n", l2.Root()) + } + for _, x := range testCases { + b, err := l2.Load(strings.TrimPrefix(x.path, "/foo/project/")) + if err != nil { + t.Fatalf("unexpected load error %v", err) + } + if !reflect.DeepEqual([]byte(x.expectedContent), b) { + t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) + } + } + l2, err = l1.New("/foo/project/") // Assure trailing slash stripped + if err != nil { + t.Fatalf("unexpected err: %v\n", err) + } + if "/foo/project" != l2.Root() { + t.Fatalf("incorrect root: %s\n", l2.Root()) + } +} + +func TestLoaderNewSubDir(t *testing.T) { + l1, err := NewFileLoader(MakeFakeFs(testCases)).New("/foo/project") + if err != nil { + t.Fatalf("unexpected err: %v\n", err) + } + l2, err := l1.New("subdir1") + if err != nil { + t.Fatalf("unexpected err: %v\n", err) + } + if "/foo/project/subdir1" != l2.Root() { + t.Fatalf("incorrect root: %s\n", l2.Root()) + } + x := testCases[1] + b, err := l2.Load("fileB.yaml") + if err != nil { + t.Fatalf("unexpected load error %v", err) + } + if !reflect.DeepEqual([]byte(x.expectedContent), b) { + t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) + } +} + +func TestLoaderBadRelative(t *testing.T) { + l1, err := NewFileLoader(MakeFakeFs(testCases)).New("/foo/project/subdir1") + if err != nil { + t.Fatalf("unexpected err: %v\n", err) + } + if "/foo/project/subdir1" != l1.Root() { + t.Fatalf("incorrect root: %s\n", l1.Root()) + } + + // Cannot cd into a file. + l2, err := l1.New("fileB.yaml") + // TODO: this should be an error. + if err != nil { + t.Fatalf("unexpected err %v", err) + } + + // It's not okay to stay put. + l2, err = l1.New(".") + // TODO: this should be an error. + if err != nil { + t.Fatalf("unexpected err %v", err) + } + + // It's not okay to go up and back down into same place. + l2, err = l1.New("../subdir1") + // TODO: this should be an error. + if err != nil { + t.Fatalf("unexpected err %v", err) + } + + // It's not okay to go up. + l2, err = l1.New("..") + // TODO: this should be an error. + if err != nil { + t.Fatalf("unexpected err %v", err) + } + + // It's okay to go up and down to a sibling. + l2, err = l1.New("../subdir2") + if err != nil { + t.Fatalf("unexpected new error %v", err) + } + if "/foo/project/subdir2" != l2.Root() { + t.Fatalf("incorrect root: %s\n", l2.Root()) + } + x := testCases[2] + b, err := l2.Load("fileC.yaml") + if err != nil { + t.Fatalf("unexpected load error %v", err) + } + if !reflect.DeepEqual([]byte(x.expectedContent), b) { + t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) + } +} + +func TestLoaderMisc(t *testing.T) { + l := NewFileLoader(MakeFakeFs(testCases)) + _, err := l.New("") if err == nil { t.Fatalf("Expected error for empty root location not returned") } - _, err = rootLoader.New("https://google.com/project") + _, err = l.New("https://google.com/project") if err == nil { t.Fatalf("Expected error") } - - // Test with trailing slash in directory. - loader, err := rootLoader.New("/home/seans/project/") - if err != nil { - t.Fatalf("Unexpected in New(): %v\n", err) - } - if "/home/seans/project" != loader.Root() { - t.Fatalf("Incorrect Loader Root: %s\n", loader.Root()) - } - - subLoader, err := loader.New("subdir") - if err != nil { - t.Fatalf("Unexpected in New(): %v\n", err) - } - if "/home/seans/project/subdir" != subLoader.Root() { - t.Fatalf("Incorrect Loader Root: %s\n", subLoader.Root()) - } - - // Test without trailing slash in directory. - anotherLoader, err := loader.New("/home/seans/project2") - if err != nil { - t.Fatalf("Unexpected in New(): %v\n", err) - } - if "/home/seans/project2" != anotherLoader.Root() { - t.Fatalf("Incorrect Loader Root: %s\n", anotherLoader.Root()) - } - - // Current directory should be expanded to a full absolute file path. - currentDirLoader, err := loader.New(".") - if err != nil { - t.Fatalf("Unexpected in New(): %v\n", err) - } - if !filepath.IsAbs(currentDirLoader.Root()) { - t.Fatalf("Incorrect Loader Root: %s\n", currentDirLoader.Root()) - } -} - -func TestLoader_Load(t *testing.T) { - - // Initialize the fake file system and the root loader. - fakefs := fs.MakeFakeFS() - fakefs.WriteFile("/home/seans/project/file.yaml", []byte("This is a yaml file")) - fakefs.WriteFile("/home/seans/project/subdir/file.yaml", []byte("Subdirectory file content")) - fakefs.WriteFile("/home/seans/project2/file.yaml", []byte("This is another yaml file")) - rootLoader := NewFileLoader(fakefs) - - loader, err := rootLoader.New("/home/seans/project") - if err != nil { - t.Fatalf("Unexpected in New(): %v\n", err) - } - fileBytes, err := loader.Load("file.yaml") // Load relative to root location - if err != nil { - t.Fatalf("Unexpected error in Load(): %v", err) - } - if !reflect.DeepEqual([]byte("This is a yaml file"), fileBytes) { - t.Fatalf("Load failed. Expected %s, but got %s", "This is a yaml file", fileBytes) - } - - fileBytes, err = loader.Load("subdir/file.yaml") - if err != nil { - t.Fatalf("Unexpected error in Load(): %v", err) - } - if !reflect.DeepEqual([]byte("Subdirectory file content"), fileBytes) { - t.Fatalf("Load failed. Expected %s, but got %s", "Subdirectory file content", fileBytes) - } - - fileBytes, err = loader.Load("/home/seans/project2/file.yaml") - if err != nil { - t.Fatalf("Unexpected error in Load(): %v", err) - } - if !reflect.DeepEqual([]byte("This is another yaml file"), fileBytes) { - t.Fatalf("Load failed. Expected %s, but got %s", "This is another yaml file", fileBytes) - } - } diff --git a/pkg/loader/githubloader_test.go b/pkg/loader/githubloader_test.go index e62315ea6..69b873b6f 100644 --- a/pkg/loader/githubloader_test.go +++ b/pkg/loader/githubloader_test.go @@ -34,6 +34,10 @@ func TestIsRepoURL(t *testing.T) { input: "github.com/org/repo", expected: true, }, + { + input: "git::https://gitlab.com/org/repo", + expected: true, + }, { input: "/github.com/org/repo", expected: false, @@ -47,8 +51,16 @@ func TestIsRepoURL(t *testing.T) { expected: false, }, { - input: "git::https://gitlab.com/org/repo", - expected: true, + input: "foo", + expected: false, + }, + { + input: ".", + expected: false, + }, + { + input: "", + expected: false, }, } for _, tc := range testcases { diff --git a/pkg/transformers/config/factory_test.go b/pkg/transformers/config/factory_test.go index 5e835fe43..2274fcc1a 100644 --- a/pkg/transformers/config/factory_test.go +++ b/pkg/transformers/config/factory_test.go @@ -38,7 +38,7 @@ namePrefix: kind: SomeKind ` fakeFS := fs.MakeFakeFS() - fakeFS.WriteFile("transformerconfig/test/config.yaml", []byte(transformerConfig)) + fakeFS.WriteFile("/transformerconfig/test/config.yaml", []byte(transformerConfig)) ldr := loader.NewFileLoader(fakeFS) expected := &TransformerConfig{ NamePrefix: []FieldSpec{ @@ -53,9 +53,9 @@ namePrefix: func TestMakeTransformerConfigFromFiles(t *testing.T) { ldr, expected, _ := makeFakeLoaderAndOutput() - tcfg, err := NewFactory(ldr).FromFiles([]string{"transformerconfig/test/config.yaml"}) + tcfg, err := NewFactory(ldr).FromFiles([]string{"/transformerconfig/test/config.yaml"}) if err != nil { - t.Fatalf("unexpected error %v", err) + t.Fatalf("unexpected error: %v", err) } if !reflect.DeepEqual(tcfg, expected) {