From e7b42f8a778d796ed47488916ea086ab0b8ea364 Mon Sep 17 00:00:00 2001 From: Michael Taufen Date: Mon, 7 May 2018 11:13:04 -0700 Subject: [PATCH] Improve test coverage of Kubelet file utils Improves from 30.9% to 77.8%. --- pkg/kubelet/kubeletconfig/util/files/files.go | 6 +- .../kubeletconfig/util/files/files_test.go | 183 ++++++++++++++++++ 2 files changed, 187 insertions(+), 2 deletions(-) diff --git a/pkg/kubelet/kubeletconfig/util/files/files.go b/pkg/kubelet/kubeletconfig/util/files/files.go index 8fd19ce860b..fc42151a729 100644 --- a/pkg/kubelet/kubeletconfig/util/files/files.go +++ b/pkg/kubelet/kubeletconfig/util/files/files.go @@ -67,6 +67,7 @@ func EnsureFile(fs utilfs.Filesystem, path string) error { } // WriteTmpFile creates a temporary file at `path`, writes `data` into it, and fsyncs the file +// Expects the parent directory to exist. func WriteTmpFile(fs utilfs.Filesystem, path string, data []byte) (tmpPath string, retErr error) { dir := filepath.Dir(path) prefix := tmptag + filepath.Base(path) @@ -103,7 +104,8 @@ func WriteTmpFile(fs utilfs.Filesystem, path string, data []byte) (tmpPath strin } // ReplaceFile replaces the contents of the file at `path` with `data` by writing to a tmp file in the same -// dir as `path` and renaming the tmp file over `path`. The file does not have to exist to use ReplaceFile. +// dir as `path` and renaming the tmp file over `path`. The file does not have to exist to use ReplaceFile, +// but the parent directory must exist. // Note ReplaceFile calls fsync. func ReplaceFile(fs utilfs.Filesystem, path string, data []byte) error { // write data to a temporary file @@ -121,7 +123,7 @@ func DirExists(fs utilfs.Filesystem, path string) (bool, error) { if info.IsDir() { return true, nil } - return false, fmt.Errorf("expected dir at %q, but mode is is %q", path, info.Mode().String()) + return false, fmt.Errorf("expected dir at %q, but mode is %q", path, info.Mode().String()) } else if os.IsNotExist(err) { return false, nil } else { diff --git a/pkg/kubelet/kubeletconfig/util/files/files_test.go b/pkg/kubelet/kubeletconfig/util/files/files_test.go index ba81b82ec3e..4917679abbc 100644 --- a/pkg/kubelet/kubeletconfig/util/files/files_test.go +++ b/pkg/kubelet/kubeletconfig/util/files/files_test.go @@ -209,6 +209,189 @@ func TestHelpers(t *testing.T) { } } +func TestFileExists(t *testing.T) { + fn := func(fs utilfs.Filesystem, dir string, c *test) []error { + ok, err := FileExists(fs, filepath.Join(dir, "foo")) + if err != nil { + return []error{err} + } + if !ok { + return []error{fmt.Errorf("does not exist (test)")} + } + return nil + } + cases := []test{ + { + fn: fn, + desc: "file exists", + writes: []file{{name: "foo"}}, + }, + { + fn: fn, + desc: "file does not exist", + err: "does not exist (test)", + }, + { + fn: fn, + desc: "object has non-file mode", + writes: []file{{name: "foo", mode: os.ModeDir}}, + err: "expected regular file", + }, + } + for _, c := range cases { + t.Run(c.desc, func(t *testing.T) { + c.run(t, utilfs.DefaultFs{}) + }) + } +} + +func TestEnsureFile(t *testing.T) { + fn := func(fs utilfs.Filesystem, dir string, c *test) []error { + var errs []error + for _, f := range c.expects { + if err := EnsureFile(fs, filepath.Join(dir, f.name)); err != nil { + errs = append(errs, err) + } + } + return errs + } + cases := []test{ + { + fn: fn, + desc: "file exists", + writes: []file{{name: "foo"}}, + expects: []file{{name: "foo"}}, + }, + { + fn: fn, + desc: "file does not exist", + expects: []file{{name: "bar"}}, + }, + { + fn: fn, + desc: "neither parent nor file exists", + expects: []file{{name: "baz/quux"}}, + }, + } + for _, c := range cases { + t.Run(c.desc, func(t *testing.T) { + c.run(t, utilfs.DefaultFs{}) + }) + } +} + +// Note: This transitively tests WriteTmpFile +func TestReplaceFile(t *testing.T) { + fn := func(fs utilfs.Filesystem, dir string, c *test) []error { + var errs []error + for _, f := range c.expects { + if err := ReplaceFile(fs, filepath.Join(dir, f.name), []byte(f.data)); err != nil { + errs = append(errs, err) + } + } + return errs + } + cases := []test{ + { + fn: fn, + desc: "file exists", + writes: []file{{name: "foo"}}, + expects: []file{{name: "foo", data: "bar"}}, + }, + { + fn: fn, + desc: "file does not exist", + expects: []file{{name: "foo", data: "bar"}}, + }, + { + fn: func(fs utilfs.Filesystem, dir string, c *test) []error { + if err := ReplaceFile(fs, filepath.Join(dir, "foo/bar"), []byte("")); err != nil { + return []error{err} + } + return nil + }, + desc: "neither parent nor file exists", + err: "no such file or directory", + }, + } + for _, c := range cases { + t.Run(c.desc, func(t *testing.T) { + c.run(t, utilfs.DefaultFs{}) + }) + } +} + +func TestDirExists(t *testing.T) { + fn := func(fs utilfs.Filesystem, dir string, c *test) []error { + ok, err := DirExists(fs, filepath.Join(dir, "foo")) + if err != nil { + return []error{err} + } + if !ok { + return []error{fmt.Errorf("does not exist (test)")} + } + return nil + } + cases := []test{ + { + fn: fn, + desc: "dir exists", + writes: []file{{name: "foo", mode: os.ModeDir}}, + }, + { + fn: fn, + desc: "dir does not exist", + err: "does not exist (test)", + }, + { + fn: fn, + desc: "object has non-dir mode", + writes: []file{{name: "foo"}}, + err: "expected dir", + }, + } + for _, c := range cases { + t.Run(c.desc, func(t *testing.T) { + c.run(t, utilfs.DefaultFs{}) + }) + } +} + +func TestEnsureDir(t *testing.T) { + fn := func(fs utilfs.Filesystem, dir string, c *test) []error { + var errs []error + for _, f := range c.expects { + if err := EnsureDir(fs, filepath.Join(dir, f.name)); err != nil { + errs = append(errs, err) + } + } + return errs + } + cases := []test{ + { + fn: fn, + desc: "dir exists", + writes: []file{{name: "foo", mode: os.ModeDir}}, + expects: []file{{name: "foo", mode: os.ModeDir}}, + }, + { + fn: fn, + desc: "dir does not exist", + expects: []file{{name: "bar", mode: os.ModeDir}}, + }, + { + fn: fn, + desc: "neither parent nor dir exists", + expects: []file{{name: "baz/quux", mode: os.ModeDir}}, + }, + } + for _, c := range cases { + t.Run(c.desc, func(t *testing.T) { + c.run(t, utilfs.DefaultFs{}) + }) + } +} + func TestWriteTempDir(t *testing.T) { // writing a tmp dir is covered by TestReplaceDir, but we additionally test filename validation here c := test{