[PORT] Refactor tests to prevent from unnecessary preparations (gitea#32398)

Some preparations are only used by a few tests, so to make the tests fast, they should only be prepared when they are used.

By the way, this PR splits PrepareTestEnv into small functions to make it simple.

---

Conflict resolution: Mostly magical and just re-pasting the code into
the right places.
Done differently: use `require.NoError` instead of `assert.NoError`.

(cherry picked from commit ec2d1593c269e06655525deb96f74b8094221b6f)

.deadcode-out

@@ -247,6 +247,9 @@ code.gitea.io/gitea/modules/translation
 	MockLocale.TrSize
 	MockLocale.PrettyNumber
 
+code.gitea.io/gitea/modules/util
+	OptionalArg
+
 code.gitea.io/gitea/modules/util/filebuffer
 	CreateFromReader
 

modules/testlogger/testlogger.go

@@ -18,6 +18,7 @@ import (
 
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/queue"
+	"code.gitea.io/gitea/modules/util"
 )
 
 var (
@@ -445,10 +446,7 @@ func (w *testLoggerWriterCloser) Reset() error {
 func PrintCurrentTest(t testing.TB, skip ...int) func() {
 	t.Helper()
 	start := time.Now()
-	actualSkip := 1
+	actualSkip := util.OptionalArg(skip) + 1
-	if len(skip) > 0 {
-		actualSkip = skip[0] + 1
-	}
 	_, filename, line, _ := runtime.Caller(actualSkip)
 
 	if log.CanColorStdout {

modules/util/util.go

@@ -234,6 +234,25 @@ func IfZero[T comparable](v, def T) T {
 	return v
 }
 
+// OptionalArg helps the "optional argument" in Golang:
+//
+//	func foo(optArg ...int) { return OptionalArg(optArg) }
+//		calling `foo()` gets zero value 0, calling `foo(100)` gets 100
+//	func bar(optArg ...int) { return OptionalArg(optArg, 42) }
+//		calling `bar()` gets default value 42, calling `bar(100)` gets 100
+//
+// Passing more than 1 item to `optArg` or `defaultValue` is undefined behavior.
+// At the moment only the first item is used.
+func OptionalArg[T any](optArg []T, defaultValue ...T) (ret T) {
+	if len(optArg) >= 1 {
+		return optArg[0]
+	}
+	if len(defaultValue) >= 1 {
+		return defaultValue[0]
+	}
+	return ret
+}
+
 func ReserveLineBreakForTextarea(input string) string {
 	// Since the content is from a form which is a textarea, the line endings are \r\n.
 	// It's a standard behavior of HTML.

modules/util/util_test.go

@@ -275,3 +275,16 @@ func TestGeneratingEd25519Keypair(t *testing.T) {
 	assert.EqualValues(t, testPublicKey, string(publicKey))
 	assert.EqualValues(t, testPrivateKey, string(privateKey))
 }
+
+func TestOptionalArg(t *testing.T) {
+	foo := func(other any, optArg ...int) int {
+		return util.OptionalArg(optArg)
+	}
+	bar := func(other any, optArg ...int) int {
+		return util.OptionalArg(optArg, 42)
+	}
+	assert.Equal(t, 0, foo(nil))
+	assert.Equal(t, 100, foo(nil, 100))
+	assert.Equal(t, 42, bar(nil))
+	assert.Equal(t, 100, bar(nil, 100))
+}

tests/integration/api_actions_artifact_test.go

@@ -23,8 +23,15 @@ type getUploadArtifactRequest struct {
 	RetentionDays int64
 }
 
+func prepareTestEnvActionsArtifacts(t *testing.T) func() {
+	t.Helper()
+	f := tests.PrepareTestEnv(t, 1)
+	tests.PrepareArtifactsStorage(t)
+	return f
+}
+
 func TestActionsArtifactUploadSingleFile(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	// acquire artifact upload url
 	req := NewRequestWithJSON(t, "POST", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts", getUploadArtifactRequest{
@@ -58,7 +65,7 @@ func TestActionsArtifactUploadSingleFile(t *testing.T) {
 }
 
 func TestActionsArtifactUploadInvalidHash(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	// artifact id 54321 not exist
 	url := "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts/8e5b948a454515dbabfc7eb718ddddddd/upload?itemPath=artifact/abc.txt"
@@ -73,7 +80,7 @@ func TestActionsArtifactUploadInvalidHash(t *testing.T) {
 }
 
 func TestActionsArtifactConfirmUploadWithoutName(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	req := NewRequest(t, "PATCH", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts").
 		AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
@@ -82,7 +89,7 @@ func TestActionsArtifactConfirmUploadWithoutName(t *testing.T) {
 }
 
 func TestActionsArtifactUploadWithoutToken(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	req := NewRequestWithJSON(t, "POST", "/api/actions_pipeline/_apis/pipelines/workflows/1/artifacts", nil)
 	MakeRequest(t, req, http.StatusUnauthorized)
@@ -108,7 +115,7 @@ type (
 )
 
 func TestActionsArtifactDownload(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	req := NewRequest(t, "GET", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts").
 		AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
@@ -152,7 +159,7 @@ func TestActionsArtifactDownload(t *testing.T) {
 }
 
 func TestActionsArtifactUploadMultipleFile(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	const testArtifactName = "multi-files"
 
@@ -208,7 +215,7 @@ func TestActionsArtifactUploadMultipleFile(t *testing.T) {
 }
 
 func TestActionsArtifactDownloadMultiFiles(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	const testArtifactName = "multi-file-download"
 
@@ -263,7 +270,7 @@ func TestActionsArtifactDownloadMultiFiles(t *testing.T) {
 }
 
 func TestActionsArtifactUploadWithRetentionDays(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	// acquire artifact upload url
 	req := NewRequestWithJSON(t, "POST", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts", getUploadArtifactRequest{
@@ -299,7 +306,7 @@ func TestActionsArtifactUploadWithRetentionDays(t *testing.T) {
 }
 
 func TestActionsArtifactOverwrite(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	{
 		// download old artifact uploaded by tests above, it should 1024 A

tests/integration/api_actions_artifact_v4_test.go

@@ -80,13 +80,13 @@ func uploadArtifact(t *testing.T, body string) string {
 }
 
 func TestActionsArtifactV4UploadSingleFile(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 	body := strings.Repeat("A", 1024)
 	uploadArtifact(t, body)
 }
 
 func TestActionsArtifactV4UploadSingleFileWrongChecksum(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	token, err := actions_service.CreateAuthorizationToken(48, 792, 193)
 	require.NoError(t, err)
@@ -130,7 +130,7 @@ func TestActionsArtifactV4UploadSingleFileWrongChecksum(t *testing.T) {
 }
 
 func TestActionsArtifactV4UploadSingleFileWithRetentionDays(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	token, err := actions_service.CreateAuthorizationToken(48, 792, 193)
 	require.NoError(t, err)
@@ -178,7 +178,7 @@ func TestActionsArtifactV4UploadSingleFileWithRetentionDays(t *testing.T) {
 }
 
 func TestActionsArtifactV4UploadSingleFileWithPotentialHarmfulBlockID(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	token, err := actions_service.CreateAuthorizationToken(48, 792, 193)
 	require.NoError(t, err)
@@ -241,7 +241,7 @@ func TestActionsArtifactV4UploadSingleFileWithPotentialHarmfulBlockID(t *testing
 }
 
 func TestActionsArtifactV4UploadSingleFileWithChunksOutOfOrder(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	token, err := actions_service.CreateAuthorizationToken(48, 792, 193)
 	require.NoError(t, err)
@@ -306,7 +306,7 @@ func TestActionsArtifactV4UploadSingleFileWithChunksOutOfOrder(t *testing.T) {
 }
 
 func TestActionsArtifactV4DownloadSingle(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	token, err := actions_service.CreateAuthorizationToken(48, 792, 193)
 	require.NoError(t, err)
@@ -386,7 +386,7 @@ func TestActionsArtifactV4DownloadRange(t *testing.T) {
 }
 
 func TestActionsArtifactV4Delete(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	defer prepareTestEnvActionsArtifacts(t)()
 
 	token, err := actions_service.CreateAuthorizationToken(48, 792, 193)
 	require.NoError(t, err)

tests/test_utils.go

@@ -224,37 +224,7 @@ func cancelProcesses(t testing.TB, delay time.Duration) {
 	t.Logf("PrepareTestEnv: all processes cancelled within %s", time.Since(start))
 }
 
-func PrepareArtifactsStorage(t testing.TB) {
+func PrepareGitRepoDirectory(t testing.TB) {
-	// prepare actions artifacts directory and files
-	require.NoError(t, storage.Clean(storage.ActionsArtifacts))
-
-	s, err := storage.NewStorage(setting.LocalStorageType, &setting.Storage{
-		Path: filepath.Join(filepath.Dir(setting.AppPath), "tests", "testdata", "data", "artifacts"),
-	})
-	require.NoError(t, err)
-	require.NoError(t, s.IterateObjects("", func(p string, obj storage.Object) error {
-		_, err = storage.Copy(storage.ActionsArtifacts, p, s, p)
-		return err
-	}))
-}
-
-func PrepareTestEnv(t testing.TB, skip ...int) func() {
-	t.Helper()
-	ourSkip := 1
-	if len(skip) > 0 {
-		ourSkip += skip[0]
-	}
-	deferFn := PrintCurrentTest(t, ourSkip)
-
-	// kill all background processes to prevent them from interfering with the fixture loading
-	// see https://codeberg.org/forgejo/forgejo/issues/2962
-	cancelProcesses(t, 30*time.Second)
-	t.Cleanup(func() { cancelProcesses(t, 0) }) // cancel remaining processes in a non-blocking way
-
-	// load database fixtures
-	require.NoError(t, unittest.LoadFixtures())
-
-	// load git repo fixtures
 	require.NoError(t, util.RemoveAll(setting.RepoRootPath))
 	require.NoError(t, unittest.CopyDir(path.Join(filepath.Dir(setting.AppPath), "tests/gitea-repositories-meta"), setting.RepoRootPath))
 	ownerDirs, err := os.ReadDir(setting.RepoRootPath)
@@ -274,14 +244,28 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() {
 			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "info"), 0o755)
 			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "heads"), 0o755)
 			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "tag"), 0o755)
+			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "pull"), 0o755)
 		}
 	}
+}
 
-	// Initialize actions artifact data
+func PrepareArtifactsStorage(t testing.TB) {
-	PrepareArtifactsStorage(t)
+	// prepare actions artifacts directory and files
+	require.NoError(t, storage.Clean(storage.ActionsArtifacts))
 
+	s, err := storage.NewStorage(setting.LocalStorageType, &setting.Storage{
+		Path: filepath.Join(filepath.Dir(setting.AppPath), "tests", "testdata", "data", "artifacts"),
+	})
+	require.NoError(t, err)
+	require.NoError(t, s.IterateObjects("", func(p string, obj storage.Object) error {
+		_, err = storage.Copy(storage.ActionsArtifacts, p, s, p)
+		return err
+	}))
+}
+
+func PrepareLFSStorage(t testing.TB) {
 	// load LFS object fixtures
-	// (LFS storage can be on any of several backends, including remote servers, so we init it with the storage API)
+	// (LFS storage can be on any of several backends, including remote servers, so init it with the storage API)
 	lfsFixtures, err := storage.NewStorage(setting.LocalStorageType, &setting.Storage{
 		Path: filepath.Join(filepath.Dir(setting.AppPath), "tests/gitea-lfs-meta"),
 	})
@@ -291,7 +275,9 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() {
 		_, err := storage.Copy(storage.LFS, path, lfsFixtures, path)
 		return err
 	}))
+}
 
+func PrepareCleanPackageData(t testing.TB) {
 	// clear all package data
 	require.NoError(t, db.TruncateBeans(db.DefaultContext,
 		&packages_model.Package{},
@@ -303,17 +289,28 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() {
 		&packages_model.PackageCleanupRule{},
 	))
 	require.NoError(t, storage.Clean(storage.Packages))
+}
 
+func PrepareTestEnv(t testing.TB, skip ...int) func() {
+	t.Helper()
+	deferFn := PrintCurrentTest(t, util.OptionalArg(skip)+1)
+
+	cancelProcesses(t, 30*time.Second)
+	t.Cleanup(func() { cancelProcesses(t, 0) }) // cancel remaining processes in a non-blocking way
+
+	// load database fixtures
+	require.NoError(t, unittest.LoadFixtures())
+
+	// do not add more Prepare* functions here, only call necessary ones in the related test functions
+	PrepareGitRepoDirectory(t)
+	PrepareLFSStorage(t)
+	PrepareCleanPackageData(t)
 	return deferFn
 }
 
 func PrintCurrentTest(t testing.TB, skip ...int) func() {
 	t.Helper()
-	actualSkip := 1
+	return testlogger.PrintCurrentTest(t, util.OptionalArg(skip)+1)
-	if len(skip) > 0 {
-		actualSkip = skip[0] + 1
-	}
-	return testlogger.PrintCurrentTest(t, actualSkip)
 }
 
 // Printf takes a format and args and prints the string to os.Stdout