chore: QoL improvements to tests (#7917)

- Use mock helper functions, instead of home-brew solutions.
- Disable cron jobs that are not important to be run during integration tests and might even interfere.
- Avoid sleeping unnecessary, if there's some requirement then sleep or retry until that requirement is met.
- Avoid trying to deliver webhooks that will always result in a failure.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7917
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>

modules/auth/openid/discovery_cache_test.go

@@ -23,7 +23,7 @@ func (s *testDiscoveredInfo) OpLocalID() string {
 }
 
 func TestTimedDiscoveryCache(t *testing.T) {
-	dc := newTimedDiscoveryCache(1 * time.Second)
+	dc := newTimedDiscoveryCache(100 * time.Millisecond)
 
 	// Put some initial values
 	dc.Put("foo", &testDiscoveredInfo{}) // openid.opEndpoint: "a", openid.opLocalID: "b", openid.claimedID: "c"})
@@ -41,7 +41,7 @@ func TestTimedDiscoveryCache(t *testing.T) {
 	}
 
 	// Sleep one second and try retrieve again
-	time.Sleep(1 * time.Second)
+	time.Sleep(100 * time.Millisecond)
 
 	if di := dc.Get("foo"); di != nil {
 		t.Errorf("Expected a nil, got a result")

modules/auth/webauthn/webauthn_test.go

@@ -7,19 +7,19 @@ import (
 	"testing"
 
 	"forgejo.org/modules/setting"
+	"forgejo.org/modules/test"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestInit(t *testing.T) {
-	setting.Domain = "domain"
-	setting.AppName = "AppName"
-	setting.AppURL = "https://domain/"
-	rpOrigin := []string{"https://domain"}
+	defer test.MockVariableValue(&setting.Domain, "domain")()
+	defer test.MockVariableValue(&setting.AppName, "AppName")()
+	defer test.MockVariableValue(&setting.AppURL, "https://domain/")()
 
 	Init()
 
 	assert.Equal(t, setting.Domain, WebAuthn.Config.RPID)
 	assert.Equal(t, setting.AppName, WebAuthn.Config.RPDisplayName)
-	assert.Equal(t, rpOrigin, WebAuthn.Config.RPOrigins)
+	assert.Equal(t, []string{"https://domain"}, WebAuthn.Config.RPOrigins)
 }

modules/git/git_test.go

@@ -86,10 +86,7 @@ func TestGitConfig(t *testing.T) {
 }
 
 func TestSyncConfig(t *testing.T) {
-	oldGitConfig := setting.GitConfig
-	defer func() {
-		setting.GitConfig = oldGitConfig
-	}()
+	defer test.MockProtect(&setting.GitConfig)()
 
 	setting.GitConfig.Options["sync-test.cfg-key-a"] = "CfgValA"
 	require.NoError(t, syncGitConfig())

modules/indexer/issues/elasticsearch/elasticsearch_test.go

@@ -11,6 +11,8 @@ import (
 	"time"
 
 	"forgejo.org/modules/indexer/issues/internal/tests"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestElasticsearchIndexer(t *testing.T) {
@@ -26,20 +28,10 @@ func TestElasticsearchIndexer(t *testing.T) {
 		}
 	}
 
-	ok := false
-	for i := 0; i < 60; i++ {
+	require.Eventually(t, func() bool {
 		resp, err := http.Get(url)
-		if err == nil && resp.StatusCode == http.StatusOK {
-			ok = true
-			break
-		}
-		t.Logf("Waiting for elasticsearch to be up: %v", err)
-		time.Sleep(time.Second)
-	}
-	if !ok {
-		t.Fatalf("Failed to wait for elasticsearch to be up")
-		return
-	}
+		return err == nil && resp.StatusCode == http.StatusOK
+	}, time.Minute, time.Microsecond*100, "Failed to wait for elasticsearch to be up")
 
 	indexer := NewIndexer(url, fmt.Sprintf("test_elasticsearch_indexer_%d", time.Now().Unix()))
 	defer indexer.Close()

modules/indexer/issues/internal/tests/tests.go

@@ -8,7 +8,6 @@
 package tests
 
 import (
-	"context"
 	"fmt"
 	"slices"
 	"testing"
@@ -40,7 +39,7 @@ func TestIndexer(t *testing.T, indexer internal.Indexer) {
 			data[v.ID] = v
 		}
 		require.NoError(t, indexer.Index(t.Context(), d...))
-		require.NoError(t, waitData(indexer, int64(len(data))))
+		waitData(t, indexer, int64(len(data)))
 	}
 
 	defer func() {
@@ -54,13 +53,13 @@ func TestIndexer(t *testing.T, indexer internal.Indexer) {
 				for _, v := range c.ExtraData {
 					data[v.ID] = v
 				}
-				require.NoError(t, waitData(indexer, int64(len(data))))
+				waitData(t, indexer, int64(len(data)))
 				defer func() {
 					for _, v := range c.ExtraData {
 						require.NoError(t, indexer.Delete(t.Context(), v.ID))
 						delete(data, v.ID)
 					}
-					require.NoError(t, waitData(indexer, int64(len(data))))
+					waitData(t, indexer, int64(len(data)))
 				}()
 			}
 
@@ -783,22 +782,17 @@ func countIndexerData(data map[int64]*internal.IndexerData, f func(v *internal.I
 
 // waitData waits for the indexer to index all data.
 // Some engines like Elasticsearch index data asynchronously, so we need to wait for a while.
-func waitData(indexer internal.Indexer, total int64) error {
+func waitData(t testing.TB, indexer internal.Indexer, total int64) {
 	var actual int64
-	for i := 0; i < 100; i++ {
-		result, err := indexer.Search(context.Background(), &internal.SearchOptions{
+	assert.Eventually(t, func() bool {
+		result, err := indexer.Search(t.Context(), &internal.SearchOptions{
 			Paginator: &db.ListOptions{
 				PageSize: 0,
 			},
 		})
-		if err != nil {
-			return err
-		}
+		require.NoError(t, err)
+
 		actual = result.Total
-		if actual == total {
-			return nil
-		}
-		time.Sleep(100 * time.Millisecond)
-	}
-	return fmt.Errorf("waitData: expected %d, actual %d", total, actual)
+		return actual == total
+	}, time.Second*10, time.Millisecond*100, "expected %d but got %d", total, actual)
 }

modules/indexer/issues/meilisearch/meilisearch_test.go

@@ -34,20 +34,10 @@ func TestMeilisearchIndexer(t *testing.T) {
 		key = os.Getenv("TEST_MEILISEARCH_KEY")
 	}
 
-	ok := false
-	for i := 0; i < 60; i++ {
+	require.Eventually(t, func() bool {
 		resp, err := http.Get(url)
-		if err == nil && resp.StatusCode == http.StatusOK {
-			ok = true
-			break
-		}
-		t.Logf("Waiting for meilisearch to be up: %v", err)
-		time.Sleep(time.Second)
-	}
-	if !ok {
-		t.Fatalf("Failed to wait for meilisearch to be up")
-		return
-	}
+		return err == nil && resp.StatusCode == http.StatusOK
+	}, time.Minute, time.Microsecond*100, "Failed to wait for meilisearch to be up")
 
 	indexer := NewIndexer(url, key, fmt.Sprintf("test_meilisearch_indexer_%d", time.Now().Unix()))
 	defer indexer.Close()

modules/queue/manager_test.go

@@ -8,17 +8,14 @@ import (
 	"testing"
 
 	"forgejo.org/modules/setting"
+	"forgejo.org/modules/test"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestManager(t *testing.T) {
-	oldAppDataPath := setting.AppDataPath
-	setting.AppDataPath = t.TempDir()
-	defer func() {
-		setting.AppDataPath = oldAppDataPath
-	}()
+	defer test.MockVariableValue(&setting.AppDataPath, t.TempDir())()
 
 	newQueueFromConfig := func(name, cfg string) (*WorkerPoolQueue[int], error) {
 		cfgProvider, err := setting.NewConfigProviderFromData(cfg)

modules/setting/git_test.go

@@ -6,17 +6,15 @@ package setting
 import (
 	"testing"
 
+	"forgejo.org/modules/test"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestGitConfig(t *testing.T) {
-	oldGit := Git
-	oldGitConfig := GitConfig
-	defer func() {
-		Git = oldGit
-		GitConfig = oldGitConfig
-	}()
+	defer test.MockProtect(&Git)()
+	defer test.MockProtect(&GitConfig)()
 
 	cfg, err := NewConfigProviderFromData(`
 [git.config]
@@ -37,12 +35,8 @@ diff.algorithm = other
 }
 
 func TestGitReflog(t *testing.T) {
-	oldGit := Git
-	oldGitConfig := GitConfig
-	defer func() {
-		Git = oldGit
-		GitConfig = oldGitConfig
-	}()
+	defer test.MockProtect(&Git)()
+	defer test.MockProtect(&GitConfig)()
 
 	// default reflog config without legacy options
 	cfg, err := NewConfigProviderFromData(``)

modules/test/utils.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"strings"
+	"time"
 
 	"forgejo.org/modules/json"
 )
@@ -46,3 +47,8 @@ func MockProtect[T any](p *T) (reset func()) {
 	old := *p
 	return func() { *p = old }
 }
+
+// When this is called, sleep until the unix time was increased by one.
+func SleepTillNextSecond() {
+	time.Sleep(time.Second - time.Since(time.Now().Truncate(time.Second)))
+}

modules/validation/helpers_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"forgejo.org/modules/setting"
+	"forgejo.org/modules/test"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -47,7 +48,7 @@ func Test_IsValidURL(t *testing.T) {
 }
 
 func Test_IsValidExternalURL(t *testing.T) {
-	setting.AppURL = "https://try.gitea.io/"
+	defer test.MockVariableValue(&setting.AppURL, "https://code.forgejo.org/")()
 
 	cases := []struct {
 		description string
@@ -56,7 +57,7 @@ func Test_IsValidExternalURL(t *testing.T) {
 	}{
 		{
 			description: "Current instance URL",
-			url:         "https://try.gitea.io/test",
+			url:         "https://code.forgejo.org/test",
 			valid:       true,
 		},
 		{
@@ -66,7 +67,7 @@ func Test_IsValidExternalURL(t *testing.T) {
 		},
 		{
 			description: "Current instance API URL",
-			url:         "https://try.gitea.io/api/v1/user/follow",
+			url:         "https://code.forgejo.org/api/v1/user/follow",
 			valid:       false,
 		},
 		{
@@ -89,7 +90,7 @@ func Test_IsValidExternalURL(t *testing.T) {
 }
 
 func Test_IsValidExternalTrackerURLFormat(t *testing.T) {
-	setting.AppURL = "https://try.gitea.io/"
+	defer test.MockVariableValue(&setting.AppURL, "https://code.forgejo.org/")()
 
 	cases := []struct {
 		description string
@@ -156,7 +157,8 @@ func Test_IsValidExternalTrackerURLFormat(t *testing.T) {
 }
 
 func TestIsValidUsernameAllowDots(t *testing.T) {
-	setting.Service.AllowDotsInUsernames = true
+	defer test.MockVariableValue(&setting.Service.AllowDotsInUsernames, true)()
+
 	tests := []struct {
 		arg  string
 		want bool
@@ -188,10 +190,7 @@ func TestIsValidUsernameAllowDots(t *testing.T) {
 }
 
 func TestIsValidUsernameBanDots(t *testing.T) {
-	setting.Service.AllowDotsInUsernames = false
-	defer func() {
-		setting.Service.AllowDotsInUsernames = true
-	}()
+	defer test.MockVariableValue(&setting.Service.AllowDotsInUsernames, false)()
 
 	tests := []struct {
 		arg  string