From 748337537c13d953a86be259ea6304627975b0a7 Mon Sep 17 00:00:00 2001 From: Martin Tournoij Date: Thu, 19 Oct 2023 03:06:46 +0100 Subject: [PATCH] Fix tests --- .circleci/config.yml | 11 +- .cirrus.yml | 3 +- .github/workflows/test.yml | 132 ++++++++++--------- fsnotify_test.go | 259 +++++++++++++++++-------------------- helpers_test.go | 34 +++-- 5 files changed, 224 insertions(+), 215 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index fa8399ee..beb2cd46 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -22,10 +22,9 @@ jobs: name: test command: | uname -a - sysctl fs.inotify.max_user_watches fs.inotify.max_user_instances - ulimit -a go version - go test -parallel 1 -race ./... + FSNOTIFY_BUFFER=4096 go test -parallel 1 -race ./... + go test -parallel 1 -race ./... # iOS ios: @@ -50,7 +49,8 @@ jobs: export PATH=$PATH:/usr/local/Cellar/go/*/bin uname -a go version - go test -parallel 1 -race ./... + FSNOTIFY_BUFFER=4096 go test -parallel 1 -race ./... + go test -parallel 1 -race ./... # This is just Linux x86_64; also need to get a Go with GOOS=android, but # there aren't any pre-built versions of that on the Go site. Idk, disable for @@ -78,5 +78,6 @@ jobs: # uname -a # export PATH=/usr/local/go/bin:$PATH # go version - # go test -parallel 1 -race ./... + # FSNOTIFY_BUFFER=4096 go test -parallel 1 -race ./... + # go test -parallel 1 -race ./... # diff --git a/.cirrus.yml b/.cirrus.yml index d72f6e8b..ffc7b992 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -9,4 +9,5 @@ freebsd_task: # run tests as user "cirrus" instead of root - pw useradd cirrus -m - chown -R cirrus:cirrus . - - sudo -u cirrus go test -parallel 1 -race ./... + - FSNOTIFY_BUFFER=4096 sudo --preserve-env=FSNOTIFY_BUFFER -u cirrus go test -parallel 1 -race ./... + - sudo --preserve-env=FSNOTIFY_BUFFER -u cirrus go test -parallel 1 -race ./... diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 28ade379..0397e4e2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,71 +6,74 @@ on: branches: ['main', 'aix'] jobs: - # Test Windows and Linux with the latest Go version and the oldest we support. - test: + linux: strategy: fail-fast: false matrix: - os: - - ubuntu-latest - - windows-latest - go: - - '1.17' - - '1.21' + os: ['ubuntu-latest'] + go: ['1.17', '1.21'] runs-on: ${{ matrix.os }} steps: - - name: checkout - uses: actions/checkout@v3 - - - name: setup Go - uses: actions/setup-go@v4 + - uses: 'actions/checkout@v3' + - uses: 'actions/setup-go@v4' with: go-version: ${{ matrix.go }} + - name: test + run: | + FSNOTIFY_BUFFER=4096 go test -parallel 1 -race ./... + go test -parallel 1 -race ./... + windows: + strategy: + fail-fast: false + matrix: + os: ['windows-latest'] + go: ['1.17', '1.21'] + runs-on: ${{ matrix.os }} + steps: + - uses: 'actions/checkout@v3' + - uses: 'actions/setup-go@v4' + with: + go-version: ${{ matrix.go }} - name: test run: | go test -parallel 1 -race ./... + set "FSNOTIFY_BUFFER=4096" + go test -parallel 1 -race ./... # Test gccgo - testgcc: - runs-on: ubuntu-22.04 - name: test (ubuntu-22.04, gccgo 12.1) + gcc: + runs-on: 'ubuntu-22.04' + name: 'test (ubuntu-22.04, gccgo 12.1)' steps: - - name: checkout - uses: actions/checkout@v3 - + - uses: 'actions/checkout@v3' - name: test run: | sudo apt-get -y install gccgo-12 - go-12 test -parallel 1 ./... + FSNOTIFY_BUFFER=4096 go-12 test -parallel 1 ./... + go-12 test -parallel 1 ./... # Test only the latest Go version on macOS; we use the macOS builders for BSD # and illumos, and GitHub doesn't allow many of them to run concurrently. If # it works on Windows and Linux with Go 1.17, then it probably does on macOS # too. - testMacOS: + macos: name: test strategy: fail-fast: false matrix: - os: - - macos-11 - - macos-13 - go: - - '1.21' + os: ['macos-11', 'macos-13'] + go: ['1.21'] runs-on: ${{ matrix.os }} steps: - - name: checkout - uses: actions/checkout@v3 - - - name: setup Go - uses: actions/setup-go@v4 + - uses: 'actions/checkout@v3' + - uses: 'actions/setup-go@v4' with: go-version: ${{ matrix.go }} - - name: test run: | - go test -parallel 1 -race ./... + FSNOTIFY_BUFFER=4096 go test -parallel 1 -race ./... + go test -parallel 1 -race ./... # OpenBSD; no -race as the VM doesn't include the comp set. # @@ -79,45 +82,50 @@ jobs: # so should probably look into that first. Go 1.19 is supposed to have a # much faster race detector, so maybe waiting until we have that is # enough. - testOpenBSD: - runs-on: macos-12 - name: test (openbsd, 1.17) + openbsd: + runs-on: 'macos-12' + timeout-minutes: 30 + name: 'test (openbsd, 1.17)' steps: - - uses: actions/checkout@v3 - - name: test (openbsd, 1.17) - id: test - uses: vmactions/openbsd-vm@v0 + - uses: 'actions/checkout@v3' + - name: 'test (openbsd, 1.17)' + id: 'openbsd' + uses: 'vmactions/openbsd-vm@v0' with: prepare: pkg_add go run: | useradd -mG wheel action - su action -c 'go test -parallel 1 ./...' + FSNOTIFY_BUFFER=4096 su action -c 'go test -parallel 1 ./...' + su action -c 'go test -parallel 1 ./...' # NetBSD - testNetBSD: + netbsd: runs-on: macos-12 + timeout-minutes: 30 name: test (netbsd, 1.20) steps: - - uses: actions/checkout@v3 - - name: test (netbsd, 1.20) - id: test - uses: vmactions/netbsd-vm@v0 + - uses: 'actions/checkout@v3' + - name: 'test (netbsd, 1.20)' + id: 'netbsd' + uses: 'vmactions/netbsd-vm@v0' with: prepare: pkg_add go # TODO: no -race for the same reason as OpenBSD (the timing; it does run). run: | useradd -mG wheel action - su action -c 'go120 test -parallel 1 ./...' + FSNOTIFY_BUFFER=4096 su action -c 'go120 test -parallel 1 ./...' + su action -c 'go120 test -parallel 1 ./...' # illumos - testillumos: + illumos: runs-on: macos-12 + timeout-minutes: 30 name: test (illumos, 1.19) steps: - - uses: actions/checkout@v3 - - name: test (illumos, 1.19) - id: test - uses: papertigers/illumos-vm@r38 + - uses: 'actions/checkout@v3' + - name: 'test (illumos, 1.19)' + id: 'illumos' + uses: 'papertigers/illumos-vm@r38' with: prepare: | pkg install go-119 @@ -125,11 +133,13 @@ jobs: useradd action export GOCACHE=/tmp/go-cache export GOPATH=/tmp/go-path - su action -c '/opt/ooce/go-1.19/bin/go test -parallel 1 ./...' + FSNOTIFY_BUFFER=4096 su action -c '/opt/ooce/go-1.19/bin/go test -parallel 1 ./...' + su action -c '/opt/ooce/go-1.19/bin/go test -parallel 1 ./...' # Older Debian 6, for old Linux kernels. - testDebian6: + debian6: runs-on: macos-12 + timeout-minutes: 30 name: test (debian6, 1.19) strategy: fail-fast: false @@ -149,20 +159,18 @@ jobs: with: go-version: '1.19' - - name: test (debian6, 1.19) - id: test + - name: 'test (debian6, 1.19)' + id: 'debian6' run: | cp -f .github/workflows/Vagrantfile.debian6 Vagrantfile export GOOS=linux export GOARCH=amd64 for p in $(go list ./...); do - go test -c -o ${p//\//-}.test $p + FSNOTIFY_BUFFER=4096 go test -c -o ${p//\//-}.test $p + go test -c -o ${p//\//-}.test $p done vagrant up - - vagrant ssh -c 'uname -a' - vagrant ssh -c 'tail /proc/sys/fs/inotify/max_user_watches /proc/sys/fs/inotify/max_user_instances' - vagrant ssh -c 'ulimit -a' for t in *.test; do - vagrant ssh -c "/vagrant/$t -test.parallel 1" + FSNOTIFY_BUFFER=4096 vagrant ssh -c "/vagrant/$t -test.parallel 1" + vagrant ssh -c "/vagrant/$t -test.parallel 1" done diff --git a/fsnotify_test.go b/fsnotify_test.go index ce6e1d1b..f6e33d88 100644 --- a/fsnotify_test.go +++ b/fsnotify_test.go @@ -28,25 +28,6 @@ func init() { internal.SetRlimit() } -// Quick but somewhat ugly way to run tests against both the standard fsnotify -// and the buffered one. Should rewrite the tests to run more than once, but -// effort... -func TestMain(m *testing.M) { - c1 := m.Run() - os.Setenv("FSNOTIFY_BUFFER", "1") - c2 := m.Run() - os.Setenv("FSNOTIFY_BUFFER", "4") - c3 := m.Run() - - if c1 != 0 { - os.Exit(c1) - } - if c2 != 0 { - os.Exit(c2) - } - os.Exit(c3) -} - func TestWatch(t *testing.T) { tests := []testCase{ {"multiple creates", func(t *testing.T, w *Watcher, tmp string) { @@ -1089,21 +1070,40 @@ func TestClose(t *testing.T) { // a good reproducible test for this, but running it 150 times seems to // reproduce it in ~75% of cases and isn't too slow (~0.06s on my system). t.Run("double close", func(t *testing.T) { - t.Parallel() - - for i := 0; i < 150; i++ { - w, err := NewWatcher() - if err != nil { - if strings.Contains(err.Error(), "too many") { // syscall.EMFILE - time.Sleep(100 * time.Millisecond) - continue + t.Run("default", func(t *testing.T) { + t.Parallel() + + for i := 0; i < 150; i++ { + w, err := NewWatcher() + if err != nil { + if strings.Contains(err.Error(), "too many") { // syscall.EMFILE + time.Sleep(100 * time.Millisecond) + continue + } + t.Fatal(err) } - t.Fatal(err) + go w.Close() + go w.Close() + go w.Close() } - go w.Close() - go w.Close() - go w.Close() - } + }) + t.Run("buffered=4096", func(t *testing.T) { + t.Parallel() + + for i := 0; i < 150; i++ { + w, err := NewBufferedWatcher(4096) + if err != nil { + if strings.Contains(err.Error(), "too many") { // syscall.EMFILE + time.Sleep(100 * time.Millisecond) + continue + } + t.Fatal(err) + } + go w.Close() + go w.Close() + go w.Close() + } + }) }) t.Run("closes channels after read", func(t *testing.T) { @@ -1618,141 +1618,122 @@ func TestOpHas(t *testing.T) { } func BenchmarkWatch(b *testing.B) { - w, err := NewWatcher() - if err != nil { - b.Fatal(err) - } - - tmp := b.TempDir() - file := join(tmp, "file") - err = w.Add(tmp) - if err != nil { - b.Fatal(err) - } + do := func(b *testing.B, w *Watcher) { + tmp := b.TempDir() + file := join(tmp, "file") + err := w.Add(tmp) + if err != nil { + b.Fatal(err) + } - var wg sync.WaitGroup - wg.Add(1) - go func() { - for { - select { - case err, ok := <-w.Errors: - if !ok { - wg.Done() - return - } - b.Error(err) - case _, ok := <-w.Events: - if !ok { - wg.Done() - return + var wg sync.WaitGroup + wg.Add(1) + go func() { + for { + select { + case err, ok := <-w.Errors: + if !ok { + wg.Done() + return + } + b.Error(err) + case _, ok := <-w.Events: + if !ok { + wg.Done() + return + } } } - } - }() + }() - b.ResetTimer() - for n := 0; n < b.N; n++ { - fp, err := os.Create(file) - if err != nil { - b.Fatal(err) + b.ResetTimer() + for n := 0; n < b.N; n++ { + fp, err := os.Create(file) + if err != nil { + b.Fatal(err) + } + err = fp.Close() + if err != nil { + b.Fatal(err) + } } - err = fp.Close() + err = w.Close() if err != nil { b.Fatal(err) } - } - err = w.Close() - if err != nil { - b.Fatal(err) - } - wg.Wait() -} - -func BenchmarkBufferedWatch(b *testing.B) { - w, err := NewBufferedWatcher(4096) - if err != nil { - b.Fatal(err) - } - - tmp := b.TempDir() - file := join(tmp, "file") - err = w.Add(tmp) - if err != nil { - b.Fatal(err) + wg.Wait() } - var wg sync.WaitGroup - wg.Add(1) - go func() { - for { - select { - case err, ok := <-w.Errors: - if !ok { - wg.Done() - return - } - b.Error(err) - case _, ok := <-w.Events: - if !ok { - wg.Done() - return - } - } + b.Run("default", func(b *testing.B) { + w, err := NewWatcher() + if err != nil { + b.Fatal(err) } - }() - - b.ResetTimer() - for n := 0; n < b.N; n++ { - fp, err := os.Create(file) + do(b, w) + }) + b.Run("buffered=1", func(b *testing.B) { + w, err := NewBufferedWatcher(1) if err != nil { b.Fatal(err) } - err = fp.Close() + do(b, w) + }) + b.Run("buffered=1024", func(b *testing.B) { + w, err := NewBufferedWatcher(1024) if err != nil { b.Fatal(err) } - } - err = w.Close() - if err != nil { - b.Fatal(err) - } - wg.Wait() + do(b, w) + }) + b.Run("buffered=4096", func(b *testing.B) { + w, err := NewBufferedWatcher(4096) + if err != nil { + b.Fatal(err) + } + do(b, w) + }) } func BenchmarkAddRemove(b *testing.B) { - w, err := NewWatcher() - if err != nil { - b.Fatal(err) + do := func(b *testing.B, w *Watcher) { + tmp := b.TempDir() + b.ResetTimer() + for n := 0; n < b.N; n++ { + if err := w.Add(tmp); err != nil { + b.Fatal(err) + } + if err := w.Remove(tmp); err != nil { + b.Fatal(err) + } + } } - tmp := b.TempDir() - - b.ResetTimer() - for n := 0; n < b.N; n++ { - if err := w.Add(tmp); err != nil { + b.Run("default", func(b *testing.B) { + w, err := NewWatcher() + if err != nil { b.Fatal(err) } - if err := w.Remove(tmp); err != nil { + do(b, w) + }) + b.Run("buffered=1", func(b *testing.B) { + w, err := NewBufferedWatcher(1) + if err != nil { b.Fatal(err) } - } -} - -func BenchmarkBufferedAddRemove(b *testing.B) { - w, err := NewBufferedWatcher(4096) - if err != nil { - b.Fatal(err) - } - - tmp := b.TempDir() - - b.ResetTimer() - for n := 0; n < b.N; n++ { - if err := w.Add(tmp); err != nil { + do(b, w) + }) + b.Run("buffered=1024", func(b *testing.B) { + w, err := NewBufferedWatcher(1024) + if err != nil { b.Fatal(err) } - if err := w.Remove(tmp); err != nil { + do(b, w) + }) + b.Run("buffered=4096", func(b *testing.B) { + w, err := NewBufferedWatcher(4096) + if err != nil { b.Fatal(err) } - } + do(b, w) + }) } diff --git a/helpers_test.go b/helpers_test.go index 98acb239..c9a7dd78 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -43,17 +43,35 @@ func (tt testCase) run(t *testing.T) { func eventSeparator() { time.Sleep(50 * time.Millisecond) } func waitForEvents() { time.Sleep(500 * time.Millisecond) } +// To test the buffered watcher we run the tests twice in the CI: once as "go +// test" and once with FSNOTIFY_BUFFER set. This is a bit hacky, but saves +// having to refactor a lot of this code. Besides, running the tests in the CI +// more than once isn't a bad thing, since it helps catch flaky tests (should +// probably run it even more). +var testBuffered = func() uint { + s, ok := os.LookupEnv("FSNOTIFY_BUFFER") + if ok { + i, err := strconv.ParseUint(s, 0, 0) + if err != nil { + panic(fmt.Sprintf("FSNOTIFY_BUFFER: %s", err)) + } + return uint(i) + } + return 0 +}() + // newWatcher initializes an fsnotify Watcher instance. func newWatcher(t *testing.T, add ...string) *Watcher { t.Helper() - w, err := NewWatcher() - if e, ok := os.LookupEnv("FSNOTIFY_BUFFER"); ok { - t.Logf("using FSNOTIFY_BUFFER=%v", e) - n, err2 := strconv.Atoi(e) - if err2 != nil { - t.Fatalf("FSNOTIFY_BUFFER: %v", err2) - } - w, err = NewBufferedWatcher(uint(n)) + + var ( + w *Watcher + err error + ) + if testBuffered > 0 { + w, err = NewBufferedWatcher(testBuffered) + } else { + w, err = NewWatcher() } if err != nil { t.Fatalf("newWatcher: %s", err)