From ed6c4088fa94eb15084689ed88cc47affee392ed Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Wed, 9 Dec 2015 16:11:17 +0000 Subject: [PATCH] Fix up tests. --- common/fs/fs.go | 45 ++++++- .../procspy/benchmark_internal_test.go | 2 +- probe/endpoint/procspy/proc.go | 40 +----- probe/endpoint/procspy/proc_internal_test.go | 13 +- probe/endpoint/reporter_test.go | 4 +- probe/process/walker_linux.go | 15 +-- probe/process/walker_linux_test.go | 127 +++++++++--------- test/fs/fs.go | 18 ++- 8 files changed, 138 insertions(+), 126 deletions(-) diff --git a/common/fs/fs.go b/common/fs/fs.go index 50e41a3ab..8655f1843 100644 --- a/common/fs/fs.go +++ b/common/fs/fs.go @@ -2,10 +2,51 @@ package fs import ( "io" + "io/ioutil" "os" + "syscall" ) -// Open is a mockable version of os.Open -var Open = func(path string) (io.ReadWriteCloser, error) { +// T is the filesystem interface type. +type T interface { + ReadDir(string) ([]os.FileInfo, error) + ReadFile(string) ([]byte, error) + Lstat(string, *syscall.Stat_t) error + Stat(string, *syscall.Stat_t) error + Open(string) (io.ReadWriteCloser, error) +} + +type realFS struct{} + +// FS is the way you should access the filesystem. +var FS T = realFS{} + +// Mock is used to switch out the filesystem for a mock. +func Mock(fs T) { + FS = fs +} + +// Restore puts back the real filesystem. +func Restore() { + FS = realFS{} +} + +func (realFS) ReadDir(path string) ([]os.FileInfo, error) { + return ioutil.ReadDir(path) +} + +func (realFS) ReadFile(path string) ([]byte, error) { + return ioutil.ReadFile(path) +} + +func (realFS) Lstat(path string, stat *syscall.Stat_t) error { + return syscall.Lstat(path, stat) +} + +func (realFS) Stat(path string, stat *syscall.Stat_t) error { + return syscall.Stat(path, stat) +} + +func (realFS) Open(path string) (io.ReadWriteCloser, error) { return os.Open(path) } diff --git a/probe/endpoint/procspy/benchmark_internal_test.go b/probe/endpoint/procspy/benchmark_internal_test.go index c1606f895..ec1dea59c 100644 --- a/probe/endpoint/procspy/benchmark_internal_test.go +++ b/probe/endpoint/procspy/benchmark_internal_test.go @@ -21,7 +21,7 @@ func benchmarkConnections(b *testing.B) { b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { - cbConnections(false) + cbConnections(false, nil) } } diff --git a/probe/endpoint/procspy/proc.go b/probe/endpoint/procspy/proc.go index 0dc03325a..3b834a50a 100644 --- a/probe/endpoint/procspy/proc.go +++ b/probe/endpoint/procspy/proc.go @@ -4,8 +4,6 @@ package procspy import ( "bytes" - "io/ioutil" - "os" "path/filepath" "strconv" "syscall" @@ -23,14 +21,6 @@ func SetProcRoot(root string) { procRoot = root } -// made variables for mocking -var ( - readDir = ioutil.ReadDir - lstat = syscall.Lstat - stat = syscall.Stat - open = fs.Open -) - // walkProcPid walks over all numerical (PID) /proc entries, and sees if their // ./fd/* files are symlink to sockets. Returns a map from socket ID (inode) // to PID. Will return an error if /proc isn't there. @@ -44,7 +34,7 @@ func walkProcPid(buf *bytes.Buffer, walker process.Walker) (map[uint64]Proc, err walker.Walk(func(p process.Process) { dirName := strconv.Itoa(p.PID) fdBase := filepath.Join(procRoot, dirName, "fd") - fds, err := readDir(fdBase) + fds, err := fs.FS.ReadDir(fdBase) if err != nil { // Process is be gone by now, or we don't have access. return @@ -52,7 +42,7 @@ func walkProcPid(buf *bytes.Buffer, walker process.Walker) (map[uint64]Proc, err // Read network namespace, and if we haven't seen it before, // read /proc//net/tcp - err = lstat(filepath.Join(procRoot, dirName, "/ns/net"), &statT) + err = fs.FS.Lstat(filepath.Join(procRoot, dirName, "/ns/net"), &statT) if err != nil { return } @@ -65,7 +55,7 @@ func walkProcPid(buf *bytes.Buffer, walker process.Walker) (map[uint64]Proc, err for _, fd := range fds { // Direct use of syscall.Stat() to save garbage. - err = stat(filepath.Join(fdBase, fd.Name()), &statT) + err = fs.FS.Stat(filepath.Join(fdBase, fd.Name()), &statT) if err != nil { continue } @@ -85,33 +75,11 @@ func walkProcPid(buf *bytes.Buffer, walker process.Walker) (map[uint64]Proc, err return res, nil } -// procName does a pid->name lookup. -func procName(base string) string { - fh, err := open(filepath.Join(base, "/comm")) - if err != nil { - return "" - } - - name := make([]byte, 64) - l, err := fh.Read(name) - fh.Close() - if err != nil { - return "" - } - - if l < 2 { - return "" - } - - // drop trailing "\n" - return string(name[:l-1]) -} - // readFile reads an arbitrary file into a buffer. It's a variable so it can // be overwritten for benchmarks. That's bad practice and we should change it // to be a dependency. var readFile = func(filename string, buf *bytes.Buffer) error { - f, err := os.Open(filename) + f, err := fs.FS.Open(filename) if err != nil { return err } diff --git a/probe/endpoint/procspy/proc_internal_test.go b/probe/endpoint/procspy/proc_internal_test.go index 0dbf4882d..4ebd00552 100644 --- a/probe/endpoint/procspy/proc_internal_test.go +++ b/probe/endpoint/procspy/proc_internal_test.go @@ -6,6 +6,8 @@ import ( "syscall" "testing" + fs_hook "github.com/weaveworks/scope/common/fs" + "github.com/weaveworks/scope/probe/process" "github.com/weaveworks/scope/test/fs" ) @@ -31,17 +33,20 @@ var mockFS = fs.Dir("", FStat: syscall.Stat_t{}, }, ), + fs.File{ + FName: "stat", + FContents: "1 na R 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1", + }, ), ), ) func TestWalkProcPid(t *testing.T) { - oldReadDir, oldLstat, oldStat, oldOpen := readDir, lstat, stat, open - defer func() { readDir, lstat, stat, open = oldReadDir, oldLstat, oldStat, oldOpen }() - readDir, lstat, stat, open = mockFS.ReadDir, mockFS.Lstat, mockFS.Stat, mockFS.Open + fs_hook.Mock(mockFS) + defer fs_hook.Restore() buf := bytes.Buffer{} - have, err := walkProcPid(&buf) + have, err := walkProcPid(&buf, process.NewWalker(procRoot)) if err != nil { t.Fatal(err) } diff --git a/probe/endpoint/reporter_test.go b/probe/endpoint/reporter_test.go index 9da3e4cac..a4a8c83d0 100644 --- a/probe/endpoint/reporter_test.go +++ b/probe/endpoint/reporter_test.go @@ -71,7 +71,7 @@ func TestSpyNoProcesses(t *testing.T) { nodeName = "frenchs-since-1904" // TODO rename to hostNmae ) - reporter := endpoint.NewReporter(nodeID, nodeName, false, false) + reporter := endpoint.NewReporter(nodeID, nodeName, false, false, nil) r, _ := reporter.Report() //buf, _ := json.MarshalIndent(r, "", " ") //t.Logf("\n%s\n", buf) @@ -107,7 +107,7 @@ func TestSpyWithProcesses(t *testing.T) { nodeName = "fishermans-friend" // TODO rename to hostNmae ) - reporter := endpoint.NewReporter(nodeID, nodeName, true, false) + reporter := endpoint.NewReporter(nodeID, nodeName, true, false, nil) r, _ := reporter.Report() // buf, _ := json.MarshalIndent(r, "", " ") ; t.Logf("\n%s\n", buf) diff --git a/probe/process/walker_linux.go b/probe/process/walker_linux.go index f84ded6a6..ee7c17861 100644 --- a/probe/process/walker_linux.go +++ b/probe/process/walker_linux.go @@ -2,16 +2,11 @@ package process import ( "bytes" - "io/ioutil" "path" "strconv" "strings" -) -// Hooks exposed for mocking -var ( - ReadDir = ioutil.ReadDir - ReadFile = ioutil.ReadFile + "github.com/weaveworks/scope/common/fs" ) type walker struct { @@ -28,7 +23,7 @@ func NewWalker(procRoot string) Walker { // passes one-by-one to the supplied function. Walk is only made public // so that is can be tested. func (w *walker) Walk(f func(Process)) error { - dirEntries, err := ReadDir(w.procRoot) + dirEntries, err := fs.FS.ReadDir(w.procRoot) if err != nil { return err } @@ -40,7 +35,7 @@ func (w *walker) Walk(f func(Process)) error { continue } - stat, err := ReadFile(path.Join(w.procRoot, filename, "stat")) + stat, err := fs.FS.ReadFile(path.Join(w.procRoot, filename, "stat")) if err != nil { continue } @@ -56,13 +51,13 @@ func (w *walker) Walk(f func(Process)) error { } cmdline := "" - if cmdlineBuf, err := ReadFile(path.Join(w.procRoot, filename, "cmdline")); err == nil { + if cmdlineBuf, err := fs.FS.ReadFile(path.Join(w.procRoot, filename, "cmdline")); err == nil { cmdlineBuf = bytes.Replace(cmdlineBuf, []byte{'\000'}, []byte{' '}, -1) cmdline = string(cmdlineBuf) } comm := "(unknown)" - if commBuf, err := ReadFile(path.Join(w.procRoot, filename, "comm")); err == nil { + if commBuf, err := fs.FS.ReadFile(path.Join(w.procRoot, filename, "comm")); err == nil { comm = strings.TrimSpace(string(commBuf)) } diff --git a/probe/process/walker_linux_test.go b/probe/process/walker_linux_test.go index 6b449edd9..3f56a41aa 100644 --- a/probe/process/walker_linux_test.go +++ b/probe/process/walker_linux_test.go @@ -1,75 +1,80 @@ package process_test import ( - "fmt" - "os" "reflect" - "strconv" - "strings" "testing" - "time" + fs_hook "github.com/weaveworks/scope/common/fs" "github.com/weaveworks/scope/probe/process" "github.com/weaveworks/scope/test" + "github.com/weaveworks/scope/test/fs" ) -type mockProcess struct { - name, comm, cmdline string -} - -func (p mockProcess) Name() string { return p.name } -func (p mockProcess) Size() int64 { return 0 } -func (p mockProcess) Mode() os.FileMode { return 0 } -func (p mockProcess) ModTime() time.Time { return time.Now() } -func (p mockProcess) IsDir() bool { return true } -func (p mockProcess) Sys() interface{} { return nil } +var mockFS = fs.Dir("", + fs.Dir("proc", + fs.Dir("3", + fs.File{ + FName: "comm", + FContents: "curl\n", + }, + fs.File{ + FName: "cmdline", + FContents: "curl\000google.com", + }, + fs.File{ + FName: "stat", + FContents: "3 na R 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1", + }, + ), + fs.Dir("2", + fs.File{ + FName: "comm", + FContents: "bash\n", + }, + fs.File{ + FName: "cmdline", + FContents: "", + }, + fs.File{ + FName: "stat", + FContents: "2 na R 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1", + }, + ), + fs.Dir("4", + fs.File{ + FName: "comm", + FContents: "apache\n", + }, + fs.File{ + FName: "cmdline", + FContents: "", + }, + fs.File{ + FName: "stat", + FContents: "4 na R 3 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1", + }, + ), + fs.Dir("notapid"), + fs.Dir("1", + fs.File{ + FName: "comm", + FContents: "init\n", + }, + fs.File{ + FName: "cmdline", + FContents: "", + }, + fs.File{ + FName: "stat", + FContents: "1 na R 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1", + }, + ), + ), +) func TestWalker(t *testing.T) { - oldReadDir, oldReadFile := process.ReadDir, process.ReadFile - defer func() { - process.ReadDir = oldReadDir - process.ReadFile = oldReadFile - }() - - processes := map[string]mockProcess{ - "3": {name: "3", comm: "curl\n", cmdline: "curl\000google.com"}, - "2": {name: "2", comm: "bash\n"}, - "4": {name: "4", comm: "apache\n"}, - "notapid": {name: "notapid"}, - "1": {name: "1", comm: "init\n"}, - } - - process.ReadDir = func(path string) ([]os.FileInfo, error) { - result := []os.FileInfo{} - for _, p := range processes { - result = append(result, p) - } - return result, nil - } - - process.ReadFile = func(path string) ([]byte, error) { - splits := strings.Split(path, "/") - - pid := splits[len(splits)-2] - process, ok := processes[pid] - if !ok { - return nil, fmt.Errorf("not found") - } - - file := splits[len(splits)-1] - switch file { - case "comm": - return []byte(process.comm), nil - case "stat": - pid, _ := strconv.Atoi(splits[len(splits)-2]) - parent := pid - 1 - return []byte(fmt.Sprintf("%d na R %d 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1", pid, parent)), nil - case "cmdline": - return []byte(process.cmdline), nil - } - - return nil, fmt.Errorf("not found") - } + fs_hook.Mock(mockFS) + defer fs_hook.Restore() want := map[int]process.Process{ 3: {PID: 3, PPID: 2, Comm: "curl", Cmdline: "curl google.com", Threads: 1}, @@ -79,7 +84,7 @@ func TestWalker(t *testing.T) { } have := map[int]process.Process{} - walker := process.NewWalker("unused") + walker := process.NewWalker("/proc") err := walker.Walk(func(p process.Process) { have[p.PID] = p }) diff --git a/test/fs/fs.go b/test/fs/fs.go index 2a47a602e..b20f61d39 100644 --- a/test/fs/fs.go +++ b/test/fs/fs.go @@ -9,6 +9,8 @@ import ( "strings" "syscall" "time" + + "github.com/weaveworks/scope/common/fs" ) type mockInode struct{} @@ -16,7 +18,7 @@ type mockInode struct{} type dir struct { mockInode name string - entries map[string]FS + entries map[string]Entry stat syscall.Stat_t } @@ -28,21 +30,17 @@ type File struct { FStat syscall.Stat_t } -// FS is a mock filesystem -type FS interface { +// Entry is an entry in the mock filesystem +type Entry interface { os.FileInfo - ReadDir(string) ([]os.FileInfo, error) - ReadFile(string) ([]byte, error) - Lstat(string, *syscall.Stat_t) error - Stat(string, *syscall.Stat_t) error - Open(string) (io.ReadWriteCloser, error) + fs.T } // Dir creates a new directory with the given entries. -func Dir(name string, entries ...FS) FS { +func Dir(name string, entries ...Entry) Entry { result := dir{ name: name, - entries: map[string]FS{}, + entries: map[string]Entry{}, } for _, entry := range entries {