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 <wxiaoguang@gmail.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

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
 

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
 	}

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())
 	}

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)
+		})
 	})
 }
 

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")

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)
 }