From 85211296ddcf2640d7e4110cb0d034cd92cf4c04 Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 26 Oct 2026 12:22:44 +0100 Subject: [PATCH] test: speed up go tests (#39914) - Skip race instrumentation of the transpiled SQLite engine `modernc.org/sqlite/lib` when `GOTEST_FLAGS` contains `-race`, Gitea code and the driver boundary stay instrumented. `models/issues` drops from ~10s to ~5s. - Hash OAuth2 client secrets with `bcrypt.MinCost` in tests, like user passwords already use the `dummy` hasher. `TestOAuth2` drops from ~22s to ~2s on the PostgreSQL job. - Stop the Redis locker's extension loop directly on `Close` instead of waiting for its next tick, and run the memory locker tests under `testing/synctest`. `modules/globallock` drops from ~12s to ~4s. Signed-off-by: wxiaoguang Co-authored-by: Lunny Xiao Co-authored-by: wxiaoguang --- Makefile | 2 ++ models/auth/oauth2.go | 2 +- models/auth/oauth2_test.go | 11 ++++++++ modules/globallock/globallock_test.go | 11 ++++---- modules/globallock/locker_test.go | 19 +++++++++++--- modules/globallock/redis_locker.go | 38 ++++++++++++++++----------- 6 files changed, 57 insertions(+), 26 deletions(-) diff --git a/Makefile b/Makefile index 43ef91fa4b6..a5fa24b1d76 100644 --- a/Makefile +++ b/Makefile @@ -68,6 +68,8 @@ endif # GOTEST_FLAGS is for unit test and integration test GOTEST_FLAGS ?= -timeout 40m +# disable race detection for the packages that slow down the tests +GOTEST_FLAGS := -gcflags=modernc.org/sqlite/lib=-race=false $(GOTEST_FLAGS) STORED_VERSION_FILE := VERSION diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 1a39bf034e5..0abd69bf19f 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -217,7 +217,7 @@ func (app *OAuth2Application) GenerateClientSecret(ctx context.Context) (string, // for code scanners to grab sensitive tokens. clientSecret := "gto_" + base32Lower.EncodeToString(rBytes) - hashedSecret, err := bcrypt.GenerateFromPassword([]byte(clientSecret), bcrypt.DefaultCost) + hashedSecret, err := bcrypt.GenerateFromPassword([]byte(clientSecret), util.Iif(setting.IsInTesting, bcrypt.MinCost, bcrypt.DefaultCost)) if err != nil { return "", err } diff --git a/models/auth/oauth2_test.go b/models/auth/oauth2_test.go index dee37d14067..21ae8fc060a 100644 --- a/models/auth/oauth2_test.go +++ b/models/auth/oauth2_test.go @@ -9,10 +9,13 @@ import ( auth_model "gitea.dev/models/auth" "gitea.dev/models/unittest" + "gitea.dev/modules/setting" + "gitea.dev/modules/test" "gitea.dev/modules/timeutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" "golang.org/x/oauth2" ) @@ -147,6 +150,13 @@ func TestOAuth2ApplicationAndGrant(t *testing.T) { unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Application{ID: 1, ClientSecret: app.ClientSecret}) assert.True(t, app.ValidateClientSecret([]byte(secret))) assert.False(t, app.ValidateClientSecret([]byte("fewijfowejgfiowjeoifew"))) + + defer test.MockVariableValue(&setting.IsInTesting, false)() + _, err = app.GenerateClientSecret(t.Context()) + require.NoError(t, err) + cost, err := bcrypt.Cost([]byte(app.ClientSecret)) + require.NoError(t, err) + assert.Equal(t, bcrypt.DefaultCost, cost) }) t.Run("CreateOAuth2Application", func(t *testing.T) { @@ -161,6 +171,7 @@ func TestOAuth2ApplicationAndGrant(t *testing.T) { func BenchmarkOAuth2Application_GenerateClientSecret(b *testing.B) { assert.NoError(b, unittest.PrepareTestDatabase()) app := unittest.AssertExistsAndLoadBean(b, &auth_model.OAuth2Application{ID: 1}) + defer test.MockVariableValue(&setting.IsInTesting, false)() for b.Loop() { _, _ = app.GenerateClientSecret(b.Context()) } diff --git a/modules/globallock/globallock_test.go b/modules/globallock/globallock_test.go index 39f58848a46..fd114918c7a 100644 --- a/modules/globallock/globallock_test.go +++ b/modules/globallock/globallock_test.go @@ -7,9 +7,7 @@ import ( "context" "sync" "testing" - "time" - - "gitea.dev/modules/test" + "testing/synctest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -17,7 +15,6 @@ import ( func TestLockAndDo(t *testing.T) { t.Run("redis", func(t *testing.T) { - defer test.MockVariableValue(&redisLockExpiry, 5*time.Second)() // Close waits for the extend goroutine's next tick locker := newTestRedisLocker(t) defaultLocker.Store(new(locker)) testLockAndDo(t) @@ -26,8 +23,10 @@ func TestLockAndDo(t *testing.T) { require.NoError(t, rl.Close()) }) t.Run("memory", func(t *testing.T) { - defaultLocker.Store(new(NewMemoryLocker())) - testLockAndDo(t) + synctest.Test(t, func(t *testing.T) { + defaultLocker.Store(new(NewMemoryLocker())) + testLockAndDo(t) + }) }) } diff --git a/modules/globallock/locker_test.go b/modules/globallock/locker_test.go index 6f186fae9d5..f195d21681b 100644 --- a/modules/globallock/locker_test.go +++ b/modules/globallock/locker_test.go @@ -7,6 +7,7 @@ import ( "context" "sync" "testing" + "testing/synctest" "time" "gitea.dev/modules/test" @@ -41,7 +42,17 @@ func TestLocker(t *testing.T) { } func testLocker(t *testing.T, locker Locker) { - t.Run("lock", func(t *testing.T) { + runTest := func(name string, run func(*testing.T)) { + t.Run(name, func(t *testing.T) { + if _, memory := locker.(*memoryLocker); memory { + synctest.Test(t, run) + } else { + run(t) + } + }) + } + + runTest("lock", func(t *testing.T) { parentCtx := t.Context() release, err := locker.Lock(parentCtx, "test") defer release() @@ -67,7 +78,7 @@ func testLocker(t *testing.T, locker Locker) { }() }) - t.Run("try lock", func(t *testing.T) { + runTest("try lock", func(t *testing.T) { parentCtx := t.Context() ok, release, err := locker.TryLock(parentCtx, "test") defer release() @@ -95,7 +106,7 @@ func testLocker(t *testing.T, locker Locker) { }() }) - t.Run("wait and acquired", func(t *testing.T) { + runTest("wait and acquired", func(t *testing.T) { ctx := t.Context() release, err := locker.Lock(ctx, "test") require.NoError(t, err) @@ -115,7 +126,7 @@ func testLocker(t *testing.T, locker Locker) { wg.Wait() }) - t.Run("multiple release", func(t *testing.T) { + runTest("multiple release", func(t *testing.T) { ctx := t.Context() release1, err := locker.Lock(ctx, "test") diff --git a/modules/globallock/redis_locker.go b/modules/globallock/redis_locker.go index 1883b3bb52b..1e0e0e896df 100644 --- a/modules/globallock/redis_locker.go +++ b/modules/globallock/redis_locker.go @@ -7,7 +7,6 @@ import ( "context" "errors" "sync" - "sync/atomic" "time" "gitea.dev/modules/nosql" @@ -27,9 +26,10 @@ type redisLocker struct { conn redis.UniversalClient rs *redsync.Redsync - mutexM sync.Map - closed atomic.Bool - extendWg sync.WaitGroup + mutexM sync.Map + closeCtx context.Context + closeCancel context.CancelFunc + extendWg sync.WaitGroup } var _ Locker = &redisLocker{} @@ -40,7 +40,7 @@ func NewRedisLocker(connection string) Locker { conn: conn, rs: redsync.New(goredis.NewPool(conn)), } - l.extendWg.Add(1) + l.closeCtx, l.closeCancel = context.WithCancel(context.Background()) l.startExtend() return l } @@ -65,15 +65,14 @@ func (l *redisLocker) TryLock(ctx context.Context, key string) (bool, ReleaseFun // It will stop extending the locks and refuse to acquire new locks. // In actual use, it is not necessary to call this function. // But it's useful in tests to release resources. -// It could take some time since it waits for the extending goroutine to finish. func (l *redisLocker) Close() error { - l.closed.Store(true) + l.closeCancel() l.extendWg.Wait() return nil } func (l *redisLocker) lock(ctx context.Context, key string, tries int) (ReleaseFunc, error) { - if l.closed.Load() { + if l.closeCtx.Err() != nil { return func() {}, errors.New("locker is closed") } @@ -104,11 +103,22 @@ func (l *redisLocker) lock(ctx context.Context, key string, tries int) (ReleaseF } func (l *redisLocker) startExtend() { - if l.closed.Load() { - l.extendWg.Done() - return - } + timer := time.NewTimer(redisLockExpiry / 2) + l.extendWg.Go(func() { + defer timer.Stop() + for { + select { + case <-l.closeCtx.Done(): + return + case <-timer.C: + l.extendLocks() + timer.Reset(redisLockExpiry / 2) + } + } + }) +} +func (l *redisLocker) extendLocks() { toExtend := make([]*redsync.Mutex, 0) l.mutexM.Range(func(_, value any) bool { m := value.(*redsync.Mutex) //nolint:forcetypeassert // mutexM only ever holds *redsync.Mutex @@ -126,8 +136,6 @@ func (l *redisLocker) startExtend() { }) for _, v := range toExtend { // If it failed to extend, it will be released automatically after the lock expires. - _, _ = v.Extend() + _, _ = v.ExtendContext(l.closeCtx) } - - time.AfterFunc(redisLockExpiry/2, l.startExtend) }