From 0da4e86639f2833568bfacc0477af39fb2cc518d Mon Sep 17 00:00:00 2001 From: David Sansome Date: Tue, 29 Sep 2015 18:01:06 +1000 Subject: [PATCH 1/3] Wait for goroutines to finish before leaving test functions. VerifyTailOutput is started in a goroutine, but the test function doesn't wait for this goroutine to exit before returning, so any errors raised by VerifyTailOutput after the function returns are lost. Also add a failing test for tailing a file with a relative path. --- tail_test.go | 91 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 30 deletions(-) diff --git a/tail_test.go b/tail_test.go index 6095f4f..89158d1 100644 --- a/tail_test.go +++ b/tail_test.go @@ -6,15 +6,14 @@ package tail import ( - _ "fmt" "io/ioutil" "os" "strings" "testing" "time" - "./watch" "github.com/ActiveState/tail/ratelimiter" + "github.com/ActiveState/tail/watch" ) func init() { @@ -44,6 +43,40 @@ func TestMustExist(t *testing.T) { tail.Cleanup() } +func TestWaitsForFileToExist(_t *testing.T) { + t := NewTailTest("waits-for-file-to-exist", _t) + tail := t.StartTail("test.txt", Config{}) + go t.VerifyTailOutput(tail, []string{"hello", "world"}) + + <-time.After(100 * time.Millisecond) + t.CreateFile("test.txt", "hello\nworld\n") + t.Cleanup(tail) +} + +func TestWaitsForFileToExistRelativePath(_t *testing.T) { + t := NewTailTest("waits-for-file-to-exist-relative", _t) + + oldWD, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + os.Chdir(t.path) + defer os.Chdir(oldWD) + + tail, err := TailFile("test.txt", Config{}) + if err != nil { + t.Fatal(err) + } + + go t.VerifyTailOutput(tail, []string{"hello", "world"}) + + <-time.After(100 * time.Millisecond) + if err := ioutil.WriteFile("test.txt", []byte("hello\nworld\n"), 0600); err != nil { + t.Fatal(err) + } + t.Cleanup(tail) +} + func TestStop(t *testing.T) { tail, err := TailFile("_no_such_file", Config{Follow: true, MustExist: false}) if err != nil { @@ -65,8 +98,7 @@ func MaxLineSizeT(_t *testing.T, follow bool, fileContent string, expected []str // to read all lines. <-time.After(100 * time.Millisecond) t.RemoveFile("test.txt") - tail.Stop() - tail.Cleanup() + t.Cleanup(tail) } func TestMaxLineSizeFollow(_t *testing.T) { @@ -89,9 +121,9 @@ func TestOver4096ByteLine(_t *testing.T) { // to read all lines. <-time.After(100 * time.Millisecond) t.RemoveFile("test.txt") - tail.Stop() - tail.Cleanup() + t.Cleanup(tail) } + func TestOver4096ByteLineWithSetMaxLineSize(_t *testing.T) { t := NewTailTest("Over4096ByteLineMaxLineSize", _t) testString := strings.Repeat("a", 4097) @@ -103,8 +135,7 @@ func TestOver4096ByteLineWithSetMaxLineSize(_t *testing.T) { // to read all lines. <-time.After(100 * time.Millisecond) t.RemoveFile("test.txt") - // tail.Stop() - tail.Cleanup() + t.Cleanup(tail) } func TestLocationFull(_t *testing.T) { @@ -117,8 +148,7 @@ func TestLocationFull(_t *testing.T) { // to read all lines. <-time.After(100 * time.Millisecond) t.RemoveFile("test.txt") - tail.Stop() - tail.Cleanup() + t.Cleanup(tail) } func TestLocationFullDontFollow(_t *testing.T) { @@ -132,8 +162,7 @@ func TestLocationFullDontFollow(_t *testing.T) { t.AppendFile("test.txt", "more\ndata\n") <-time.After(100 * time.Millisecond) - tail.Stop() - tail.Cleanup() + t.Cleanup(tail) } func TestLocationEnd(_t *testing.T) { @@ -149,8 +178,7 @@ func TestLocationEnd(_t *testing.T) { // to read all lines. <-time.After(100 * time.Millisecond) t.RemoveFile("test.txt") - tail.Stop() - tail.Cleanup() + t.Cleanup(tail) } func TestLocationMiddle(_t *testing.T) { @@ -167,8 +195,7 @@ func TestLocationMiddle(_t *testing.T) { // to read all lines. <-time.After(100 * time.Millisecond) t.RemoveFile("test.txt") - tail.Stop() - tail.Cleanup() + t.Cleanup(tail) } func _TestReOpen(_t *testing.T, poll bool) { @@ -199,7 +226,7 @@ func _TestReOpen(_t *testing.T, poll bool) { <-time.After(delay) t.RenameFile("test.txt", "test.txt.rotated") <-time.After(delay) - t.CreateFile("test.txt", "endofworld") + t.CreateFile("test.txt", "endofworld\n") // Delete after a reasonable delay, to give tail sufficient time // to read all lines. @@ -207,11 +234,7 @@ func _TestReOpen(_t *testing.T, poll bool) { t.RemoveFile("test.txt") <-time.After(delay) - // Do not bother with stopping as it could kill the tomb during - // the reading of data written above. Timings can vary based on - // test environment. - // tail.Stop() - tail.Cleanup() + t.Cleanup(tail) } // The use of polling file watcher could affect file rotation @@ -250,11 +273,7 @@ func _TestReSeek(_t *testing.T, poll bool) { <-time.After(100 * time.Millisecond) t.RemoveFile("test.txt") - // Do not bother with stopping as it could kill the tomb during - // the reading of data written above. Timings can vary based on - // test environment. - // tail.Stop() - tail.Cleanup() + t.Cleanup(tail) } // The use of polling file watcher could affect file rotation @@ -293,8 +312,7 @@ func TestRateLimiting(_t *testing.T) { <-time.After(100 * time.Millisecond) t.RemoveFile("test.txt") - // tail.Stop() - tail.Cleanup() + t.Cleanup(tail) } func TestTell(_t *testing.T) { @@ -336,11 +354,12 @@ func TestTell(_t *testing.T) { type TailTest struct { Name string path string + done chan struct{} *testing.T } func NewTailTest(name string, t *testing.T) TailTest { - tt := TailTest{name, ".test/" + name, t} + tt := TailTest{name, ".test/" + name, make(chan struct{}), t} err := os.MkdirAll(tt.path, os.ModeTemporary|0700) if err != nil { tt.Fatal(err) @@ -408,7 +427,19 @@ func (t TailTest) StartTail(name string, config Config) *Tail { return tail } +func (t TailTest) Cleanup(tail *Tail) { + <-time.After(100 * time.Millisecond) + tail.Stop() + tail.Cleanup() + + // VerifyTailOutput runs in a goroutine so ensure it's finished before ending + // the test, otherwise its failures will be ignored. + <-t.done +} + func (t TailTest) VerifyTailOutput(tail *Tail, lines []string) { + defer close(t.done) + for idx, line := range lines { tailedLine, ok := <-tail.Lines if !ok { From 760f3e6edcb693e49ce66eba2c0229b05acc73de Mon Sep 17 00:00:00 2001 From: David Sansome Date: Tue, 29 Sep 2015 18:02:16 +1000 Subject: [PATCH 2/3] Fix BlockUntilExists for relative paths. filepath.Dir("foo") returns ".", so the inotify events are of the form "./foo", which is not equal to the filename being watched ("foo") --- watch/inotify.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/watch/inotify.go b/watch/inotify.go index f263b56..55c8fab 100644 --- a/watch/inotify.go +++ b/watch/inotify.go @@ -46,7 +46,7 @@ func (fw *InotifyFileWatcher) BlockUntilExists(t *tomb.Tomb) error { case evt, ok := <-fw.w.Event: if !ok { return fmt.Errorf("inotify watcher has been closed") - } else if evt.Name == fw.Filename { + } else if filepath.Base(evt.Name) == filepath.Base(fw.Filename) { return nil } case <-t.Dying(): From e6815324582c19ed8d618808a330333805f9d54f Mon Sep 17 00:00:00 2001 From: David Sansome Date: Tue, 29 Sep 2015 18:04:25 +1000 Subject: [PATCH 3/3] Watch the *directory* for file deletions instead of the file itself. inotify works on the inode, not the path. Deleting the file doesn't touch the inode, so doesn't generate an event on the file. Fixes #57 --- watch/inotify.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/watch/inotify.go b/watch/inotify.go index 55c8fab..dde26b7 100644 --- a/watch/inotify.go +++ b/watch/inotify.go @@ -59,15 +59,22 @@ func (fw *InotifyFileWatcher) BlockUntilExists(t *tomb.Tomb) error { func (fw *InotifyFileWatcher) ChangeEvents(t *tomb.Tomb, fi os.FileInfo) *FileChanges { changes := NewFileChanges() - err := fw.w.Watch(fw.Filename) - if err != nil { + if err := fw.w.Watch(fw.Filename); err != nil { util.Fatal("Error watching %v: %v", fw.Filename, err) } + // Watch the directory to be notified when the file is deleted since the file + // watch is on the inode, not the path. + dirname := filepath.Dir(fw.Filename) + if err := fw.w.WatchFlags(dirname, fsnotify.FSN_DELETE); err != nil { + util.Fatal("Error watching %v: %v", dirname, err) + } + fw.Size = fi.Size() go func() { defer fw.w.RemoveWatch(fw.Filename) + defer fw.w.RemoveWatch(dirname) defer changes.Close() for { @@ -87,6 +94,9 @@ func (fw *InotifyFileWatcher) ChangeEvents(t *tomb.Tomb, fi os.FileInfo) *FileCh switch { case evt.IsDelete(): + if filepath.Base(evt.Name) != filepath.Base(fw.Filename) { + continue + } fallthrough case evt.IsRename():