diff --git a/go/viperutil/internal/sync/sync_darwin_test.go b/go/viperutil/internal/sync/sync_darwin_test.go new file mode 100644 index 00000000000..3c27ed97616 --- /dev/null +++ b/go/viperutil/internal/sync/sync_darwin_test.go @@ -0,0 +1,34 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sync_test + +import "os" + +// atomicWrite overwrites a file in such a way as to produce exactly one +// filesystem event of the type CREATE or WRITE (which are tracked by viper) +// without producing any REMOVE events. +// +// At time of writing, this produces the following on darwin: +// CHMOD => WRITE => CHMOD. +func atomicWrite(path string, data []byte) error { + stat, err := os.Stat(path) + if err != nil { + return err + } + + return os.WriteFile(path, data, stat.Mode()) +} diff --git a/go/viperutil/internal/sync/sync_linux_test.go b/go/viperutil/internal/sync/sync_linux_test.go new file mode 100644 index 00000000000..83ccfad66cc --- /dev/null +++ b/go/viperutil/internal/sync/sync_linux_test.go @@ -0,0 +1,39 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sync_test + +import "os" + +// atomicWrite overwrites a file in such a way as to produce exactly one +// filesystem event of the type CREATE or WRITE (which are tracked by viper) +// without producing any REMOVE events. +// +// At time of writing, this produces the following on x86_64 linux: +// CREATE. +func atomicWrite(path string, data []byte) error { + stat, err := os.Stat(path) + if err != nil { + return err + } + + tmp := path + ".tmp" + if err := os.WriteFile(tmp, data, stat.Mode()); err != nil { + return err + } + + return os.Rename(tmp, path) +} diff --git a/go/viperutil/internal/sync/sync_test.go b/go/viperutil/internal/sync/sync_test.go index c8e6da26424..5b1c5541896 100644 --- a/go/viperutil/internal/sync/sync_test.go +++ b/go/viperutil/internal/sync/sync_test.go @@ -19,16 +19,12 @@ package sync_test import ( "context" "encoding/json" - "fmt" - "math" "math/rand" "os" "sync" "testing" "time" - "vitess.io/vitess/go/vt/log" - "github.com/fsnotify/fsnotify" "github.com/spf13/viper" "github.com/stretchr/testify/require" @@ -44,36 +40,17 @@ func TestWatchConfig(t *testing.T) { } writeConfig := func(tmp *os.File, a, b int) error { - stat, err := os.Stat(tmp.Name()) - if err != nil { - return err - } - data, err := json.Marshal(&config{A: a, B: b}) if err != nil { return err } - err = os.WriteFile(tmp.Name(), data, stat.Mode()) - if err != nil { - return err - } - - data, err = os.ReadFile(tmp.Name()) - if err != nil { - return err - } - - var cfg config - if err := json.Unmarshal(data, &cfg); err != nil { - return err - } - - if cfg.A != a || cfg.B != b { - return fmt.Errorf("config did not persist; want %+v got %+v", config{A: a, B: b}, cfg) - } - - return nil + // In order to guarantee viper's watcher detects exactly one config + // change, we perform a write specific to the platform we're executing + // on. + // + // Consequently, this test only supports linux and macos for now. + return atomicWrite(tmp.Name(), data) } writeRandomConfig := func(tmp *os.File) error { a, b := rand.Intn(100), rand.Intn(100) @@ -109,19 +86,8 @@ func TestWatchConfig(t *testing.T) { require.NoError(t, writeConfig(tmp, a+1, b+1)) <-wCh // wait for the update to finish - // temporary hack to fix flakiness where we seem to miss one update. - const permittedVariance = 1 - closeEnoughTo := func(want, got int) bool { - if math.Abs(float64(want-got)) <= permittedVariance { - return true - } - log.Infof("TestWatchConfig: count not close enough: want %d, got %d, permitted variance %d", - want, got, permittedVariance) - return false - } - - require.True(t, closeEnoughTo(a+1, v.GetInt("a"))) - require.True(t, closeEnoughTo(b+1, v.GetInt("b"))) + require.Equal(t, a+1, v.GetInt("a")) + require.Equal(t, b+1, v.GetInt("b")) rCh <- struct{}{}