From 51102afe08fe5f8df7c7690f3db7dbb010322e5e Mon Sep 17 00:00:00 2001 From: nielash Date: Tue, 29 Sep 2026 00:26:30 -0400 Subject: [PATCH] bisync: speed up integration tests on non-local remotes Before this change, the integration tests checked whether each remote could set modtimes on every `bisync` step (`checkPreReqs`, called from `runBisync`), not once per remote. On a remote that isn't `IsLocal`, each check sleeps for 1s twice (the Google Cloud Storage per-object rate limit workaround from be73a10a9), for each side. That added 2-4s to each of the ~110 `bisync` steps in each test, which on a fast remote was most of the run: on the integration test server, the `:memory:` tests took about 14 minutes. After this change, the result of the check is remembered for each remote, so it runs, and sleeps, once per remote per test run. The `:memory:` tests now take about 7s locally. --- cmd/bisync/bisync_test.go | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/cmd/bisync/bisync_test.go b/cmd/bisync/bisync_test.go index 4a0a3e657..9a96a5864 100644 --- a/cmd/bisync/bisync_test.go +++ b/cmd/bisync/bisync_test.go @@ -19,6 +19,7 @@ import ( "sort" "strconv" "strings" + mutex "sync" // renamed as "sync" already in use "testing" "time" "unicode/utf8" @@ -61,6 +62,12 @@ const ( var initDate = time.Date(2000, time.January, 1, 0, 0, 0, 0, bisync.TZ) +// modtimeProbes holds the SetModTime error for each remote already probed by checkPreReqs +var ( + modtimeProbesMu mutex.Mutex + modtimeProbes = map[string]error{} +) + /* Useful Command Shortcuts */ // go test ./cmd/bisync -remote local -race // go test ./cmd/bisync -remote local -golden @@ -1055,7 +1062,7 @@ func (b *bisyncTest) checkPreReqs(ctx context.Context, opt *bisync.Options) (con b.ignoreModtime = true } // test if modtimes are writeable - testSetModtime := func(f fs.Fs) { + probeSetModtime := func(f fs.Fs) error { ctx := accounting.WithStatsGroup(ctx, random.String(8)) // keep stats separate in := bytes.NewBufferString("modtime_write_test") objinfo := object.NewStaticObjectInfo("modtime_write_test", initDate, int64(len("modtime_write_test")), true, nil, nil) @@ -1066,7 +1073,7 @@ func (b *bisyncTest) checkPreReqs(ctx context.Context, opt *bisync.Options) (con } err = obj.SetModTime(ctx, initDate) if err == fs.ErrorCantSetModTime { - b.t.Skip("skipping test as at least one remote does not support setting modtime") + return err } if err == fs.ErrorCantSetModTimeWithoutDelete { // transfers stats expected to differ on this backend logReplacements = append(logReplacements, `^.*There was nothing to transfer.*$`, dropMe) @@ -1076,8 +1083,21 @@ func (b *bisyncTest) checkPreReqs(ctx context.Context, opt *bisync.Options) (con if !f.Features().IsLocal { time.Sleep(time.Second) // avoid GoogleCloudStorage Error 429 rateLimitExceeded } - err = obj.Remove(ctx) - require.NoError(b.t, err) + require.NoError(b.t, obj.Remove(ctx)) + return err + } + // probing on every bisync step would sleep for minutes on non-local remotes + testSetModtime := func(f fs.Fs) { + modtimeProbesMu.Lock() + defer modtimeProbesMu.Unlock() + err, probed := modtimeProbes[f.Name()] + if !probed { + err = probeSetModtime(f) + modtimeProbes[f.Name()] = err + } + if err == fs.ErrorCantSetModTime { + b.t.Skip("skipping test as at least one remote does not support setting modtime") + } } if b.testCase != "nomodtime" { testSetModtime(b.fs1)