From e549066853c9fb37b359af442777126dc1f748d7 Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Sat, 19 Dec 2020 16:13:54 +0300 Subject: [PATCH 01/10] Remove unneded goroutines, remove samefile abstraction and checking for the samefile. --- service/reload/samefile.go | 9 -- service/reload/samefile_windows.go | 12 --- service/reload/service.go | 35 +++---- service/reload/service_test.go | 1 - service/reload/watcher.go | 150 ++++++++++++----------------- service/reload/watcher_test.go | 124 ++++++++++++------------ 6 files changed, 140 insertions(+), 191 deletions(-) delete mode 100644 service/reload/samefile.go delete mode 100644 service/reload/samefile_windows.go delete mode 100644 service/reload/service_test.go diff --git a/service/reload/samefile.go b/service/reload/samefile.go deleted file mode 100644 index 80df04318..000000000 --- a/service/reload/samefile.go +++ /dev/null @@ -1,9 +0,0 @@ -// +build !windows - -package reload - -import "os" - -func sameFile(fi1, fi2 os.FileInfo) bool { - return os.SameFile(fi1, fi2) -} diff --git a/service/reload/samefile_windows.go b/service/reload/samefile_windows.go deleted file mode 100644 index 5f70d327c..000000000 --- a/service/reload/samefile_windows.go +++ /dev/null @@ -1,12 +0,0 @@ -// +build windows - -package reload - -import "os" - -func sameFile(fi1, fi2 os.FileInfo) bool { - return fi1.ModTime() == fi2.ModTime() && - fi1.Size() == fi2.Size() && - fi1.Mode() == fi2.Mode() && - fi1.IsDir() == fi2.IsDir() -} diff --git a/service/reload/service.go b/service/reload/service.go index 9c615e0b5..c065d95d4 100644 --- a/service/reload/service.go +++ b/service/reload/service.go @@ -2,12 +2,13 @@ package reload import ( "errors" - "github.com/sirupsen/logrus" - "github.com/spiral/roadrunner" - "github.com/spiral/roadrunner/service" "os" "strings" "time" + + "github.com/sirupsen/logrus" + "github.com/spiral/roadrunner" + "github.com/spiral/roadrunner/service" ) // ID contains default service name. @@ -51,10 +52,10 @@ func (s *Service) Init(cfg *Config, log *logrus.Logger, c service.Container) (bo return false, err } configs = append(configs, WatcherConfig{ - serviceName: serviceName, - recursive: config.Recursive, - directories: config.Dirs, - filterHooks: func(filename string, patterns []string) error { + ServiceName: serviceName, + Recursive: config.Recursive, + Directories: config.Dirs, + FilterHooks: func(filename string, patterns []string) error { for i := 0; i < len(patterns); i++ { if strings.Contains(filename, patterns[i]) { return nil @@ -62,9 +63,9 @@ func (s *Service) Init(cfg *Config, log *logrus.Logger, c service.Container) (bo } return ErrorSkip }, - files: make(map[string]os.FileInfo), - ignored: ignored, - filePatterns: append(config.Patterns, cfg.Patterns...), + Files: make(map[string]os.FileInfo), + Ignored: ignored, + FilePatterns: append(config.Patterns, cfg.Patterns...), }) } @@ -91,7 +92,7 @@ func (s *Service) Serve() error { }, 100) // use the same interval - ticker := time.NewTicker(s.cfg.Interval) + timer := time.NewTimer(s.cfg.Interval) // drain channel in case of leaved messages defer func() { @@ -120,15 +121,15 @@ func (s *Service) Serve() error { case config := <-treshholdc: // replace previous value in map by more recent without adding new one updated[config.service] = config.serviceConfig - // stop ticker - ticker.Stop() + // stop timer + timer.Stop() // restart // logic is following: // if we getting a lot of events, we should't restart particular service on each of it (user doing bug move or very fast typing) - // instead, we are resetting the ticker and wait for Interval time + // instead, we are resetting the timer and wait for Interval time // If there is no more events, we restart service only once - ticker = time.NewTicker(s.cfg.Interval) - case <-ticker.C: + timer.Reset(s.cfg.Interval) + case <-timer.C: if len(updated) > 0 { for k, v := range updated { sv := *v.service @@ -142,7 +143,7 @@ func (s *Service) Serve() error { updated = make(map[string]ServiceConfig, 100) } case <-s.stopc: - ticker.Stop() + timer.Stop() return } } diff --git a/service/reload/service_test.go b/service/reload/service_test.go deleted file mode 100644 index 7cad4a5d8..000000000 --- a/service/reload/service_test.go +++ /dev/null @@ -1 +0,0 @@ -package reload diff --git a/service/reload/watcher.go b/service/reload/watcher.go index 721495c30..1397dfa5d 100644 --- a/service/reload/watcher.go +++ b/service/reload/watcher.go @@ -19,33 +19,33 @@ type SimpleHook func(filename string, pattern []string) error // changes occur. It includes the os.FileInfo of the changed file or // directory and the type of event that's occurred and the full path of the file. type Event struct { - path string - info os.FileInfo + Path string + Info os.FileInfo service string // type of service, http, grpc, etc... } type WatcherConfig struct { // service name - serviceName string + ServiceName string - // recursive or just add by singe directory - recursive bool + // Recursive or just add by singe directory + Recursive bool - // directories used per-service - directories []string + // Directories used per-service + Directories []string // simple hook, just CONTAINS - filterHooks func(filename string, pattern []string) error + FilterHooks func(filename string, pattern []string) error - // path to file with files - files map[string]os.FileInfo + // path to file with Files + Files map[string]os.FileInfo - // ignored directories, used map for O(1) amortized get - ignored map[string]struct{} + // Ignored Directories, used map for O(1) amortized get + Ignored map[string]struct{} - // filePatterns to ignore - filePatterns []string + // FilePatterns to ignore + FilePatterns []string } type Watcher struct { @@ -53,7 +53,7 @@ type Watcher struct { Event chan Event close chan struct{} - //============================= + // ============================= mu *sync.Mutex // indicates is walker started or not @@ -73,7 +73,7 @@ func NewWatcher(configs []WatcherConfig, options ...Options) (*Watcher, error) { Event: make(chan Event), mu: &sync.Mutex{}, - close: make(chan struct{}), + close: make(chan struct{}, 1), //workingDir: workDir, watcherConfigs: make(map[string]WatcherConfig), @@ -81,7 +81,7 @@ func NewWatcher(configs []WatcherConfig, options ...Options) (*Watcher, error) { // add watcherConfigs by service names for _, v := range configs { - w.watcherConfigs[v.serviceName] = v + w.watcherConfigs[v.ServiceName] = v } // apply options @@ -105,7 +105,7 @@ func (w *Watcher) initFs() error { } // workaround. in golang you can't assign to map in struct field tmp := w.watcherConfigs[srvName] - tmp.files = fileList + tmp.Files = fileList w.watcherConfigs[srvName] = tmp } return nil @@ -127,14 +127,13 @@ func ConvertIgnored(ignored []string) (map[string]struct{}, error) { } return ign, nil - } // GetAllFiles returns all files initialized for particular company func (w *Watcher) GetAllFiles(serviceName string) []os.FileInfo { var ret []os.FileInfo - for _, v := range w.watcherConfigs[serviceName].files { + for _, v := range w.watcherConfigs[serviceName].Files { ret = append(ret, v) } @@ -146,12 +145,12 @@ func (w *Watcher) GetAllFiles(serviceName string) []os.FileInfo { // In case of file watch errors, this value can be increased system-wide // For linux: set --> fs.inotify.max_user_watches = 600000 (under /etc/.conf) // Add apply: sudo sysctl -p --system -//func SetMaxFileEvents(events int) Options { +// func SetMaxFileEvents(events int) Options { // return func(watcher *Watcher) { // watcher.maxFileWatchEvents = events // } // -//} +// } // pass map from outside func (w *Watcher) retrieveFilesSingle(serviceName, path string) (map[string]os.FileInfo, error) { @@ -178,13 +177,13 @@ func (w *Watcher) retrieveFilesSingle(serviceName, path string) (map[string]os.F outer: for i := 0; i < len(fileInfoList); i++ { // if file in ignored --> continue - if _, ignored := w.watcherConfigs[serviceName].ignored[path]; ignored { + if _, ignored := w.watcherConfigs[serviceName].Ignored[path]; ignored { continue } // if filename does not contain pattern --> ignore that file - if w.watcherConfigs[serviceName].filePatterns != nil && w.watcherConfigs[serviceName].filterHooks != nil { - err = w.watcherConfigs[serviceName].filterHooks(fileInfoList[i].Name(), w.watcherConfigs[serviceName].filePatterns) + if w.watcherConfigs[serviceName].FilePatterns != nil && w.watcherConfigs[serviceName].FilterHooks != nil { + err = w.watcherConfigs[serviceName].FilterHooks(fileInfoList[i].Name(), w.watcherConfigs[serviceName].FilePatterns) if err == ErrorSkip { continue outer } @@ -223,27 +222,23 @@ func (w *Watcher) waitEvent(d time.Duration) error { // this is not very effective way // because we have to wait on Lock // better is to listen files in parallel, but, since that would be used in debug... TODO - for serviceName, config := range w.watcherConfigs { - go func(sn string, c WatcherConfig) { - fileList, _ := w.retrieveFileList(sn, c) - w.pollEvents(c.serviceName, fileList) - }(serviceName, config) + for serviceName := range w.watcherConfigs { + // TODO sync approach + fileList, _ := w.retrieveFileList(serviceName, w.watcherConfigs[serviceName]) + w.pollEvents(w.watcherConfigs[serviceName].ServiceName, fileList) } } } - } // retrieveFileList get file list for service func (w *Watcher) retrieveFileList(serviceName string, config WatcherConfig) (map[string]os.FileInfo, error) { - w.mu.Lock() - defer w.mu.Unlock() fileList := make(map[string]os.FileInfo) - if config.recursive { + if config.Recursive { // walk through directories recursively - for _, dir := range config.directories { + for i := 0; i < len(config.Directories); i++ { // full path is workdir/relative_path - fullPath, err := filepath.Abs(dir) + fullPath, err := filepath.Abs(config.Directories[i]) if err != nil { return nil, err } @@ -252,16 +247,16 @@ func (w *Watcher) retrieveFileList(serviceName string, config WatcherConfig) (ma return nil, err } - for k, v := range list { - fileList[k] = v + for k := range list { + fileList[k] = list[k] } } return fileList, nil } - for _, dir := range config.directories { + for i := 0; i < len(config.Directories); i++ { // full path is workdir/relative_path - fullPath, err := filepath.Abs(dir) + fullPath, err := filepath.Abs(config.Directories[i]) if err != nil { return nil, err } @@ -290,7 +285,7 @@ func (w *Watcher) retrieveFilesRecursive(serviceName, root string) (map[string]o // If path is ignored and it's a directory, skip the directory. If it's // ignored and it's a single file, skip the file. - _, ignored := w.watcherConfigs[serviceName].ignored[path] + _, ignored := w.watcherConfigs[serviceName].Ignored[path] if ignored { if info.IsDir() { // if it's dir, ignore whole @@ -300,7 +295,7 @@ func (w *Watcher) retrieveFilesRecursive(serviceName, root string) (map[string]o } // if filename does not contain pattern --> ignore that file - err = w.watcherConfigs[serviceName].filterHooks(info.Name(), w.watcherConfigs[serviceName].filePatterns) + err = w.watcherConfigs[serviceName].FilterHooks(info.Name(), w.watcherConfigs[serviceName].FilePatterns) if err == ErrorSkip { return nil } @@ -320,76 +315,53 @@ func (w *Watcher) pollEvents(serviceName string, files map[string]os.FileInfo) { removes := make(map[string]os.FileInfo) // Check for removed files. - for pth, info := range w.watcherConfigs[serviceName].files { + for pth := range w.watcherConfigs[serviceName].Files { if _, found := files[pth]; !found { - removes[pth] = info + removes[pth] = w.watcherConfigs[serviceName].Files[pth] } } // Check for created files, writes and chmods. - for pth, info := range files { - if info.IsDir() { + for pth := range files { + if files[pth].IsDir() { continue } - oldInfo, found := w.watcherConfigs[serviceName].files[pth] + oldInfo, found := w.watcherConfigs[serviceName].Files[pth] if !found { // A file was created. - creates[pth] = info + creates[pth] = files[pth] continue } - if oldInfo.ModTime() != info.ModTime() { - w.watcherConfigs[serviceName].files[pth] = info - w.Event <- Event{ - path: pth, - info: info, - service: serviceName, - } - } - if oldInfo.Mode() != info.Mode() { - w.watcherConfigs[serviceName].files[pth] = info + + if oldInfo.ModTime() != files[pth].ModTime() || oldInfo.Mode() != files[pth].Mode() { + w.watcherConfigs[serviceName].Files[pth] = files[pth] w.Event <- Event{ - path: pth, - info: info, + Path: pth, + Info: files[pth], service: serviceName, } } } - //Check for renames and moves. - for path1, info1 := range removes { - for path2, info2 := range creates { - if sameFile(info1, info2) { - e := Event{ - path: path2, - info: info2, - service: serviceName, - } + // Send all the remaining create and remove events. + for pth := range creates { + // add file to the plugin watch files + w.watcherConfigs[serviceName].Files[pth] = creates[pth] - // remove initial path - delete(w.watcherConfigs[serviceName].files, path1) - // update with new - w.watcherConfigs[serviceName].files[path2] = info2 - - - w.Event <- e - } - } - } - - //Send all the remaining create and remove events. - for pth, info := range creates { - w.watcherConfigs[serviceName].files[pth] = info w.Event <- Event{ - path: pth, - info: info, + Path: pth, + Info: creates[pth], service: serviceName, } } - for pth, info := range removes { - delete(w.watcherConfigs[serviceName].files, pth) + + for pth := range removes { + // delete path from the config + delete(w.watcherConfigs[serviceName].Files, pth) + w.Event <- Event{ - path: pth, - info: info, + Path: pth, + Info: removes[pth], service: serviceName, } } diff --git a/service/reload/watcher_test.go b/service/reload/watcher_test.go index 9683d2de8..9eaed3ec7 100644 --- a/service/reload/watcher_test.go +++ b/service/reload/watcher_test.go @@ -10,6 +10,8 @@ import ( "strings" "testing" "time" + + "github.com/stretchr/testify/assert" ) var testServiceName = "test" @@ -34,13 +36,13 @@ func Test_Correct_Watcher_Init(t *testing.T) { } wc := WatcherConfig{ - serviceName: testServiceName, - recursive: false, - directories: []string{tempDir}, - filterHooks: nil, - files: make(map[string]os.FileInfo), - ignored: nil, - filePatterns: nil, + ServiceName: testServiceName, + Recursive: false, + Directories: []string{tempDir}, + FilterHooks: nil, + Files: make(map[string]os.FileInfo), + Ignored: nil, + FilePatterns: nil, } w, err := NewWatcher([]WatcherConfig{wc}) @@ -91,13 +93,13 @@ func Test_Get_FileEvent(t *testing.T) { } wc := WatcherConfig{ - serviceName: testServiceName, - recursive: false, - directories: []string{tempDir}, - filterHooks: nil, - files: make(map[string]os.FileInfo), - ignored: nil, - filePatterns: []string{"aaa", "txt"}, + ServiceName: testServiceName, + Recursive: false, + Directories: []string{tempDir}, + FilterHooks: nil, + Files: make(map[string]os.FileInfo), + Ignored: nil, + FilePatterns: []string{"aaa", "txt"}, } w, err := NewWatcher([]WatcherConfig{wc}) @@ -110,7 +112,7 @@ func Test_Get_FileEvent(t *testing.T) { t.Fatal("incorrect directories len") } - go limitTime(time.Second * 10, t.Name(), c) + go limitTime(time.Second*10, t.Name(), c) go func() { go func() { @@ -125,7 +127,7 @@ func Test_Get_FileEvent(t *testing.T) { go func() { for e := range w.Event { - if e.path != "file2.txt" { + if e.Path != "file2.txt" { panic("didn't handle event when write file2") } w.Stop() @@ -176,10 +178,10 @@ func Test_FileExtensionFilter(t *testing.T) { t.Fatal(err) } wc := WatcherConfig{ - serviceName: testServiceName, - recursive: false, - directories: []string{tempDir}, - filterHooks: func(filename string, patterns []string) error { + ServiceName: testServiceName, + Recursive: false, + Directories: []string{tempDir}, + FilterHooks: func(filename string, patterns []string) error { for i := 0; i < len(patterns); i++ { if strings.Contains(filename, patterns[i]) { return nil @@ -187,9 +189,9 @@ func Test_FileExtensionFilter(t *testing.T) { } return ErrorSkip }, - files: make(map[string]os.FileInfo), - ignored: nil, - filePatterns: []string{"aaa", "bbb"}, + Files: make(map[string]os.FileInfo), + Ignored: nil, + FilePatterns: []string{"aaa", "bbb"}, } w, err := NewWatcher([]WatcherConfig{wc}) @@ -203,7 +205,7 @@ func Test_FileExtensionFilter(t *testing.T) { t.Fatalf("incorrect directories len, len is: %d", dirLen) } - go limitTime(time.Second * 5, t.Name(), c) + go limitTime(time.Second*5, t.Name(), c) go func() { go func() { @@ -218,7 +220,7 @@ func Test_FileExtensionFilter(t *testing.T) { go func() { for e := range w.Event { - fmt.Println(e.info.Name()) + fmt.Println(e.Info.Name()) panic("handled event from filtered file") } }() @@ -275,10 +277,10 @@ func Test_Recursive_Support(t *testing.T) { } wc := WatcherConfig{ - serviceName: testServiceName, - recursive: true, - directories: []string{tempDir}, - filterHooks: func(filename string, patterns []string) error { + ServiceName: testServiceName, + Recursive: true, + Directories: []string{tempDir}, + FilterHooks: func(filename string, patterns []string) error { for i := 0; i < len(patterns); i++ { if strings.Contains(filename, patterns[i]) { return nil @@ -286,9 +288,9 @@ func Test_Recursive_Support(t *testing.T) { } return ErrorSkip }, - files: make(map[string]os.FileInfo), - ignored: nil, - filePatterns: []string{"aaa", "bbb"}, + Files: make(map[string]os.FileInfo), + Ignored: nil, + FilePatterns: []string{"aaa", "bbb"}, } w, err := NewWatcher([]WatcherConfig{wc}) @@ -313,7 +315,7 @@ func Test_Recursive_Support(t *testing.T) { } go func() { for e := range w.Event { - if e.info.Name() != "file4.aaa" { + if e.Info.Name() != "file4.aaa" { panic("wrong handled event from watcher in nested dir") } w.Stop() @@ -332,10 +334,10 @@ func Test_Wrong_Dir(t *testing.T) { wrongDir := "askdjfhaksdlfksdf" wc := WatcherConfig{ - serviceName: testServiceName, - recursive: true, - directories: []string{wrongDir}, - filterHooks: func(filename string, patterns []string) error { + ServiceName: testServiceName, + Recursive: true, + Directories: []string{wrongDir}, + FilterHooks: func(filename string, patterns []string) error { for i := 0; i < len(patterns); i++ { if strings.Contains(filename, patterns[i]) { return nil @@ -343,9 +345,9 @@ func Test_Wrong_Dir(t *testing.T) { } return ErrorSkip }, - files: make(map[string]os.FileInfo), - ignored: nil, - filePatterns: []string{"aaa", "bbb"}, + Files: make(map[string]os.FileInfo), + Ignored: nil, + FilePatterns: []string{"aaa", "bbb"}, } _, err := NewWatcher([]WatcherConfig{wc}) @@ -401,10 +403,10 @@ func Test_Filter_Directory(t *testing.T) { t.Fatal(err) } wc := WatcherConfig{ - serviceName: testServiceName, - recursive: true, - directories: []string{tempDir}, - filterHooks: func(filename string, patterns []string) error { + ServiceName: testServiceName, + Recursive: true, + Directories: []string{tempDir}, + FilterHooks: func(filename string, patterns []string) error { for i := 0; i < len(patterns); i++ { if strings.Contains(filename, patterns[i]) { return nil @@ -412,9 +414,9 @@ func Test_Filter_Directory(t *testing.T) { } return ErrorSkip }, - files: make(map[string]os.FileInfo), - ignored: ignored, - filePatterns: []string{"aaa", "bbb", "txt"}, + Files: make(map[string]os.FileInfo), + Ignored: ignored, + FilePatterns: []string{"aaa", "bbb", "txt"}, } w, err := NewWatcher([]WatcherConfig{wc}) @@ -439,7 +441,7 @@ func Test_Filter_Directory(t *testing.T) { go func() { for e := range w.Event { - fmt.Println("file: " + e.info.Name()) + fmt.Println("file: " + e.Info.Name()) panic("handled event from watcher in nested dir") } }() @@ -470,6 +472,8 @@ func Test_Copy_Directory(t *testing.T) { c <- struct{}{} }() + go limitTime(time.Second*20, t.Name(), c) + nestedDir, err := ioutil.TempDir(tempDir, "nested") if err != nil { t.Fatal(err) @@ -504,10 +508,10 @@ func Test_Copy_Directory(t *testing.T) { } wc := WatcherConfig{ - serviceName: testServiceName, - recursive: true, - directories: []string{tempDir}, - filterHooks: func(filename string, patterns []string) error { + ServiceName: testServiceName, + Recursive: true, + Directories: []string{tempDir}, + FilterHooks: func(filename string, patterns []string) error { for i := 0; i < len(patterns); i++ { if strings.Contains(filename, patterns[i]) { return nil @@ -515,9 +519,9 @@ func Test_Copy_Directory(t *testing.T) { } return ErrorSkip }, - files: make(map[string]os.FileInfo), - ignored: ignored, - filePatterns: []string{"aaa", "bbb", "txt"}, + Files: make(map[string]os.FileInfo), + Ignored: ignored, + FilePatterns: []string{"aaa", "bbb", "txt"}, } w, err := NewWatcher([]WatcherConfig{wc}) @@ -531,17 +535,11 @@ func Test_Copy_Directory(t *testing.T) { t.Fatalf("incorrect directories len, len is: %d", dirLen) } - go limitTime(time.Second*10, t.Name(), c) - go func() { go func() { + time.Sleep(time.Second) err2 := copyDir(nestedDir, filepath.Join(tempDir, "copyTo")) - if err2 != nil { - panic(err2) - } - - // exit from current goroutine - runtime.Goexit() + assert.NoError(t, err2) }() go func() { From d8762242ff7c33851056ce237860c7a93dccdf94 Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Sat, 19 Dec 2020 16:16:29 +0300 Subject: [PATCH 02/10] Update pipe_factory error message. --- pipe_factory.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pipe_factory.go b/pipe_factory.go index 9696a474b..e9e750c4e 100644 --- a/pipe_factory.go +++ b/pipe_factory.go @@ -2,10 +2,11 @@ package roadrunner import ( "fmt" - "github.com/pkg/errors" - "github.com/spiral/goridge/v2" "io" "os/exec" + + "github.com/pkg/errors" + "github.com/spiral/goridge/v2" ) // PipeFactory connects to workers using standard @@ -50,7 +51,7 @@ func (f *PipeFactory) SpawnWorker(cmd *exec.Cmd) (w *Worker, err error) { err := w.Kill() if err != nil { // there is no logger here, how to handle error in goroutines ? - fmt.Println(fmt.Sprintf("error killing the worker with PID number %d, Created: %s", w.Pid, w.Created)) + fmt.Printf("error killing the worker with PID number %d, Created: %s", w.Pid, w.Created) } }(w) From 31c74fa9454b9060ec9bf089d7dc288d227219cf Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Sat, 19 Dec 2020 16:18:29 +0300 Subject: [PATCH 03/10] Update CI --- .github/workflows/build.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 92b55666a..242ad184b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -3,6 +3,12 @@ name: build on: push: pull_request: + branches: + # Branches from forks have the form 'user:branch-name' so we only run + # this job on pull_request events for branches that look like fork + # branches. Without this we would end up running this job twice for non + # forked PRs, once for the push and then once for opening the PR. + - '**:**' jobs: php: From 104513f4c57a647ca91cd23adfdf4d080eea034a Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Sat, 19 Dec 2020 16:41:25 +0300 Subject: [PATCH 04/10] Add time to concurrent run on 2 gorountines --- service/reload/watcher_test.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/service/reload/watcher_test.go b/service/reload/watcher_test.go index 9eaed3ec7..e3a284d95 100644 --- a/service/reload/watcher_test.go +++ b/service/reload/watcher_test.go @@ -209,13 +209,10 @@ func Test_FileExtensionFilter(t *testing.T) { go func() { go func() { - err2 := ioutil.WriteFile(filepath.Join(tempDir, "file3.txt"), + time.Sleep(time.Second) + err := ioutil.WriteFile(filepath.Join(tempDir, "file3.txt"), []byte{1, 1, 1}, 0755) - if err2 != nil { - panic(err2) - } - - runtime.Goexit() + assert.NoError(t, err) }() go func() { @@ -224,8 +221,8 @@ func Test_FileExtensionFilter(t *testing.T) { panic("handled event from filtered file") } }() + time.Sleep(time.Second) w.Stop() - runtime.Goexit() }() err = w.StartPolling(time.Second) From 1e5bea6f98591bbde9b9791df476768f07e81766 Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Sat, 19 Dec 2020 16:49:18 +0300 Subject: [PATCH 05/10] Correct reload plugin tests --- service/reload/watcher_test.go | 110 ++++++++++----------------------- 1 file changed, 33 insertions(+), 77 deletions(-) diff --git a/service/reload/watcher_test.go b/service/reload/watcher_test.go index e3a284d95..89e9a6e40 100644 --- a/service/reload/watcher_test.go +++ b/service/reload/watcher_test.go @@ -205,7 +205,7 @@ func Test_FileExtensionFilter(t *testing.T) { t.Fatalf("incorrect directories len, len is: %d", dirLen) } - go limitTime(time.Second*5, t.Name(), c) + go limitTime(time.Second*10, t.Name(), c) go func() { go func() { @@ -226,9 +226,7 @@ func Test_FileExtensionFilter(t *testing.T) { }() err = w.StartPolling(time.Second) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) } // nested @@ -240,38 +238,26 @@ func Test_Recursive_Support(t *testing.T) { tempDir, err := ioutil.TempDir(".", "") defer func() { err = freeResources(tempDir) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) }() nestedDir, err := ioutil.TempDir(tempDir, "nested") - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(tempDir, "file1.aaa"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(tempDir, "file2.bbb"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(nestedDir, "file3.txt"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(nestedDir, "file4.aaa"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) wc := WatcherConfig{ ServiceName: testServiceName, @@ -291,9 +277,7 @@ func Test_Recursive_Support(t *testing.T) { } w, err := NewWatcher([]WatcherConfig{wc}) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) dirLen := len(w.GetAllFiles(testServiceName)) // should be 3 files (2 from root dir, and 1 from nested), filtered txt @@ -307,10 +291,9 @@ func Test_Recursive_Support(t *testing.T) { // change file in nested directory err = ioutil.WriteFile(filepath.Join(nestedDir, "file4.aaa"), []byte{1, 1, 1}, 0755) - if err != nil { - panic(err) - } + assert.NoError(t, err) go func() { + time.Sleep(time.Second) for e := range w.Event { if e.Info.Name() != "file4.aaa" { panic("wrong handled event from watcher in nested dir") @@ -321,9 +304,7 @@ func Test_Recursive_Support(t *testing.T) { }() err = w.StartPolling(time.Second) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) } func Test_Wrong_Dir(t *testing.T) { @@ -348,9 +329,7 @@ func Test_Wrong_Dir(t *testing.T) { } _, err := NewWatcher([]WatcherConfig{wc}) - if err == nil { - t.Fatal(err) - } + assert.NoError(t, err) } func Test_Filter_Directory(t *testing.T) { @@ -368,32 +347,22 @@ func Test_Filter_Directory(t *testing.T) { go limitTime(time.Second*10, t.Name(), c) nestedDir, err := ioutil.TempDir(tempDir, "nested") - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(tempDir, "file1.aaa"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(tempDir, "file2.bbb"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(nestedDir, "file3.txt"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(nestedDir, "file4.aaa"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) ignored, err := ConvertIgnored([]string{nestedDir}) if err != nil { @@ -429,14 +398,13 @@ func Test_Filter_Directory(t *testing.T) { go func() { go func() { - err2 := ioutil.WriteFile(filepath.Join(nestedDir, "file4.aaa"), + err := ioutil.WriteFile(filepath.Join(nestedDir, "file4.aaa"), []byte{1, 1, 1}, 0755) - if err2 != nil { - panic(err2) - } + assert.NoError(t, err) }() go func() { + time.Sleep(time.Second) for e := range w.Event { fmt.Println("file: " + e.Info.Name()) panic("handled event from watcher in nested dir") @@ -472,37 +440,25 @@ func Test_Copy_Directory(t *testing.T) { go limitTime(time.Second*20, t.Name(), c) nestedDir, err := ioutil.TempDir(tempDir, "nested") - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(tempDir, "file1.aaa"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(tempDir, "file2.bbb"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(nestedDir, "file3.txt"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(nestedDir, "file4.aaa"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) ignored, err := ConvertIgnored([]string{nestedDir}) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) wc := WatcherConfig{ ServiceName: testServiceName, @@ -522,9 +478,7 @@ func Test_Copy_Directory(t *testing.T) { } w, err := NewWatcher([]WatcherConfig{wc}) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) dirLen := len(w.GetAllFiles(testServiceName)) // should be 2 files (2 from root dir), filtered other @@ -535,8 +489,8 @@ func Test_Copy_Directory(t *testing.T) { go func() { go func() { time.Sleep(time.Second) - err2 := copyDir(nestedDir, filepath.Join(tempDir, "copyTo")) - assert.NoError(t, err2) + err := copyDir(nestedDir, filepath.Join(tempDir, "copyTo")) + assert.NoError(t, err) }() go func() { @@ -574,7 +528,9 @@ func copyFile(src, dst string) (err error) { if err != nil { return } - defer in.Close() + defer func() { + _ = in.Close() + }() out, err := os.Create(dst) if err != nil { From bed3e3fea64a2ec18da9fb7dfb1b1701b853f83a Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Sat, 19 Dec 2020 17:06:11 +0300 Subject: [PATCH 06/10] Mistake in assertion --- service/reload/watcher_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/reload/watcher_test.go b/service/reload/watcher_test.go index 89e9a6e40..c0b8da6b4 100644 --- a/service/reload/watcher_test.go +++ b/service/reload/watcher_test.go @@ -329,7 +329,7 @@ func Test_Wrong_Dir(t *testing.T) { } _, err := NewWatcher([]WatcherConfig{wc}) - assert.NoError(t, err) + assert.Error(t, err) } func Test_Filter_Directory(t *testing.T) { From ace5f47196e0a5d0e926f5a956cd89574083c19a Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Sat, 19 Dec 2020 17:11:06 +0300 Subject: [PATCH 07/10] Add codecov --- codecov.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 codecov.yml diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 000000000..672717e37 --- /dev/null +++ b/codecov.yml @@ -0,0 +1,12 @@ +coverage: + status: + project: + default: + target: auto + threshold: 0% + informational: true + patch: + default: + target: auto + threshold: 0% + informational: true From a73279446750b753255375226f53e9cc10478066 Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Sat, 19 Dec 2020 17:43:36 +0300 Subject: [PATCH 08/10] Correct Test_FileExtensionFilter test --- service/reload/watcher_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/service/reload/watcher_test.go b/service/reload/watcher_test.go index c0b8da6b4..4507bdcd8 100644 --- a/service/reload/watcher_test.go +++ b/service/reload/watcher_test.go @@ -208,17 +208,21 @@ func Test_FileExtensionFilter(t *testing.T) { go limitTime(time.Second*10, t.Name(), c) go func() { + stop := make(chan struct{}, 1) go func() { time.Sleep(time.Second) err := ioutil.WriteFile(filepath.Join(tempDir, "file3.txt"), []byte{1, 1, 1}, 0755) assert.NoError(t, err) + stop <- struct{}{} }() go func() { - for e := range w.Event { - fmt.Println(e.Info.Name()) + select { + case <-w.Event: panic("handled event from filtered file") + case <-stop: + return } }() time.Sleep(time.Second) From 53702786cc76cbd6843faedf19d0ee2b06ce0182 Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Sat, 19 Dec 2020 18:03:57 +0300 Subject: [PATCH 09/10] Remove panics from test --- service/reload/watcher_test.go | 139 ++++++++++++--------------------- 1 file changed, 52 insertions(+), 87 deletions(-) diff --git a/service/reload/watcher_test.go b/service/reload/watcher_test.go index 4507bdcd8..584238723 100644 --- a/service/reload/watcher_test.go +++ b/service/reload/watcher_test.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "os" "path/filepath" - "runtime" "strings" "testing" "time" @@ -61,36 +60,21 @@ func Test_Correct_Watcher_Init(t *testing.T) { // change file and see, if event had come to handler func Test_Get_FileEvent(t *testing.T) { tempDir, err := ioutil.TempDir(".", "") - c := make(chan struct{}) defer func(name string) { err = freeResources(name) - if err != nil { - c <- struct{}{} - t.Fatal(err) - } - c <- struct{}{} + assert.NoError(t, err) }(tempDir) - - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(tempDir, "file1.txt"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } - + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(tempDir, "file2.txt"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(tempDir, "file3.txt"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) wc := WatcherConfig{ ServiceName: testServiceName, @@ -103,42 +87,42 @@ func Test_Get_FileEvent(t *testing.T) { } w, err := NewWatcher([]WatcherConfig{wc}) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) // should be 3 files and directory if len(w.GetAllFiles(testServiceName)) != 4 { t.Fatal("incorrect directories len") } - go limitTime(time.Second*10, t.Name(), c) - go func() { + stop := make(chan struct{}, 1) go func() { - time.Sleep(time.Second) - err2 := ioutil.WriteFile(filepath.Join(tempDir, "file2.txt"), + time.Sleep(time.Second * 2) + err := ioutil.WriteFile(filepath.Join(tempDir, "file2.txt"), []byte{1, 1, 1}, 0755) - if err2 != nil { - panic(err2) - } - runtime.Goexit() + assert.NoError(t, err) + time.Sleep(time.Second) + stop <- struct{}{} }() go func() { - for e := range w.Event { - if e.Path != "file2.txt" { - panic("didn't handle event when write file2") + for { + select { + case e := <-w.Event: + if e.Path != "file2.txt" { + assert.Fail(t, "didn't handle event when write file2") + } + w.Stop() + case <-stop: + return } - w.Stop() } + }() }() err = w.StartPolling(time.Second) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) } // scenario @@ -205,8 +189,6 @@ func Test_FileExtensionFilter(t *testing.T) { t.Fatalf("incorrect directories len, len is: %d", dirLen) } - go limitTime(time.Second*10, t.Name(), c) - go func() { stop := make(chan struct{}, 1) go func() { @@ -220,7 +202,7 @@ func Test_FileExtensionFilter(t *testing.T) { go func() { select { case <-w.Event: - panic("handled event from filtered file") + assert.Fail(t, "handled event from filtered file") case <-stop: return } @@ -290,21 +272,31 @@ func Test_Recursive_Support(t *testing.T) { } go func() { + stop := make(chan struct{}, 1) // time sleep is used here because StartPolling is blocking operation time.Sleep(time.Second * 5) // change file in nested directory err = ioutil.WriteFile(filepath.Join(nestedDir, "file4.aaa"), []byte{1, 1, 1}, 0755) assert.NoError(t, err) + go func() { time.Sleep(time.Second) - for e := range w.Event { - if e.Info.Name() != "file4.aaa" { - panic("wrong handled event from watcher in nested dir") + for { + select { + case e := <-w.Event: + if e.Info.Name() != "file4.aaa" { + assert.Fail(t, "wrong handled event from watcher in nested dir") + } + case <-stop: + w.Stop() + return } - w.Stop() } }() + + time.Sleep(time.Second) + stop <- struct{}{} }() err = w.StartPolling(time.Second) @@ -338,18 +330,11 @@ func Test_Wrong_Dir(t *testing.T) { func Test_Filter_Directory(t *testing.T) { tempDir, err := ioutil.TempDir(".", "") - c := make(chan struct{}) defer func(name string) { err = freeResources(name) - if err != nil { - c <- struct{}{} - t.Fatal(err) - } - c <- struct{}{} + assert.NoError(t, err) }(tempDir) - go limitTime(time.Second*10, t.Name(), c) - nestedDir, err := ioutil.TempDir(tempDir, "nested") assert.NoError(t, err) @@ -401,24 +386,28 @@ func Test_Filter_Directory(t *testing.T) { } go func() { + stop := make(chan struct{}, 1) go func() { + time.Sleep(time.Second) err := ioutil.WriteFile(filepath.Join(nestedDir, "file4.aaa"), []byte{1, 1, 1}, 0755) assert.NoError(t, err) }() go func() { - time.Sleep(time.Second) - for e := range w.Event { + select { + case e := <-w.Event: fmt.Println("file: " + e.Info.Name()) - panic("handled event from watcher in nested dir") + assert.Fail(t, "handled event from watcher in nested dir") + case <-stop: + w.Stop() + return } }() // time sleep is used here because StartPolling is blocking operation time.Sleep(time.Second * 5) - w.Stop() - + stop <- struct{}{} }() err = w.StartPolling(time.Second) @@ -431,18 +420,12 @@ func Test_Filter_Directory(t *testing.T) { // should fire an event func Test_Copy_Directory(t *testing.T) { tempDir, err := ioutil.TempDir(".", "") - c := make(chan struct{}) + defer func() { err = freeResources(tempDir) - if err != nil { - c <- struct{}{} - t.Fatal(err) - } - c <- struct{}{} + assert.NoError(t, err) }() - go limitTime(time.Second*20, t.Name(), c) - nestedDir, err := ioutil.TempDir(tempDir, "nested") assert.NoError(t, err) @@ -492,7 +475,7 @@ func Test_Copy_Directory(t *testing.T) { go func() { go func() { - time.Sleep(time.Second) + time.Sleep(time.Second * 2) err := copyDir(nestedDir, filepath.Join(tempDir, "copyTo")) assert.NoError(t, err) }() @@ -506,25 +489,7 @@ func Test_Copy_Directory(t *testing.T) { }() err = w.StartPolling(time.Second) - if err != nil { - t.Fatal(err) - } -} - -func limitTime(d time.Duration, name string, free chan struct{}) { - go func() { - ticket := time.NewTicker(d) - for { - select { - case <-ticket.C: - ticket.Stop() - panic("timeout exceed, test: " + name) - case <-free: - ticket.Stop() - return - } - } - }() + assert.NoError(t, err) } func copyFile(src, dst string) (err error) { From 52ac2c31107b396433042c1e46de2ddb07546bec Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Sat, 19 Dec 2020 18:14:21 +0300 Subject: [PATCH 10/10] Fix freezing test --- service/reload/watcher_test.go | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/service/reload/watcher_test.go b/service/reload/watcher_test.go index 584238723..ac5d37402 100644 --- a/service/reload/watcher_test.go +++ b/service/reload/watcher_test.go @@ -131,36 +131,24 @@ func Test_Get_FileEvent(t *testing.T) { // change file with txt extension, and see, if event had not come to handler because it was filtered func Test_FileExtensionFilter(t *testing.T) { tempDir, err := ioutil.TempDir(".", "") - c := make(chan struct{}) + assert.NoError(t, err) + defer func(name string) { err = freeResources(name) - if err != nil { - c <- struct{}{} - t.Fatal(err) - } - c <- struct{}{} + assert.NoError(t, err) }(tempDir) - if err != nil { - t.Fatal(err) - } err = ioutil.WriteFile(filepath.Join(tempDir, "file1.aaa"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(tempDir, "file2.bbb"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) err = ioutil.WriteFile(filepath.Join(tempDir, "file3.txt"), []byte{}, 0755) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) wc := WatcherConfig{ ServiceName: testServiceName, Recursive: false, @@ -179,9 +167,7 @@ func Test_FileExtensionFilter(t *testing.T) { } w, err := NewWatcher([]WatcherConfig{wc}) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) dirLen := len(w.GetAllFiles(testServiceName)) // should be 2 files (one filtered) and directory @@ -191,6 +177,7 @@ func Test_FileExtensionFilter(t *testing.T) { go func() { stop := make(chan struct{}, 1) + go func() { time.Sleep(time.Second) err := ioutil.WriteFile(filepath.Join(tempDir, "file3.txt"), @@ -200,6 +187,7 @@ func Test_FileExtensionFilter(t *testing.T) { }() go func() { + time.Sleep(time.Second) select { case <-w.Event: assert.Fail(t, "handled event from filtered file")