Refactor tests to prevent from unnecessary preparations (#32398)

diff --git a/modules/testlogger/testlogger.go b/modules/testlogger/testlogger.go
index 4215567c00488074ae4609940b08aa3708f3cfad..9a54d63f204278fa3b93347e75c6c6880190df42 100644 (file)
--- a/modules/testlogger/testlogger.go
+++ b/modules/testlogger/testlogger.go
@@ -15,6 +15,7 @@ import (
 
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/queue"
+       "code.gitea.io/gitea/modules/util"
 )
 
 var (
@@ -92,10 +93,7 @@ func (w *testLoggerWriterCloser) Reset() {
 func PrintCurrentTest(t testing.TB, skip ...int) func() {
        t.Helper()
        start := time.Now()
-       actualSkip := 1
-       if len(skip) > 0 {
-               actualSkip = skip[0] + 1
-       }
+       actualSkip := util.OptionalArg(skip) + 1
        _, filename, line, _ := runtime.Caller(actualSkip)
 
        if log.CanColorStdout {

diff --git a/modules/util/util.go b/modules/util/util.go
index 44b5a6ed81534512131b3021a2ae785146241485..1fb4cb21cb884458ecb08d4ce4a9ae958d5134e1 100644 (file)
--- a/modules/util/util.go
+++ b/modules/util/util.go
@@ -230,6 +230,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.

diff --git a/modules/util/util_test.go b/modules/util/util_test.go
index de8f065cadc73fb946bfbeb2eaf2a93d32fc2a79..9ce72fb86693fd24b2537f5227cfec68cd619212 100644 (file)
--- a/modules/util/util_test.go
+++ b/modules/util/util_test.go
@@ -240,3 +240,16 @@ func TestReserveLineBreakForTextarea(t *testing.T) {
        assert.Equal(t, ReserveLineBreakForTextarea("test\r\ndata"), "test\ndata")
        assert.Equal(t, ReserveLineBreakForTextarea("test\r\ndata\r\n"), "test\ndata\n")
 }
+
+func TestOptionalArg(t *testing.T) {
+       foo := func(other any, optArg ...int) int {
+               return OptionalArg(optArg)
+       }
+       bar := func(other any, optArg ...int) int {
+               return 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))
+}

diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go
index aa2e2f2adfe06e020ac31b58adf9b8fecc1e4618..de5797f289da329a0963adab9202a5ca9fc0fdf9 100644 (file)
--- a/tests/integration/api_actions_artifact_test.go
+++ b/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

diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go
index e731542037f91b50f05ff62f97b920505ad33b15..8821472801dab2b6f18e64e7a4872a27abc65d69 100644 (file)
--- a/tests/integration/api_actions_artifact_v4_test.go
+++ b/tests/integration/api_actions_artifact_v4_test.go
@@ -17,7 +17,6 @@ import (
        "code.gitea.io/gitea/modules/storage"
        "code.gitea.io/gitea/routers/api/actions"
        actions_service "code.gitea.io/gitea/services/actions"
-       "code.gitea.io/gitea/tests"
 
        "github.com/stretchr/testify/assert"
        "google.golang.org/protobuf/encoding/protojson"
@@ -34,7 +33,7 @@ func toProtoJSON(m protoreflect.ProtoMessage) io.Reader {
 }
 
 func TestActionsArtifactV4UploadSingleFile(t *testing.T) {
-       defer tests.PrepareTestEnv(t)()
+       defer prepareTestEnvActionsArtifacts(t)()
 
        token, err := actions_service.CreateAuthorizationToken(48, 792, 193)
        assert.NoError(t, err)
@@ -81,7 +80,7 @@ func TestActionsArtifactV4UploadSingleFile(t *testing.T) {
 }
 
 func TestActionsArtifactV4UploadSingleFileWrongChecksum(t *testing.T) {
-       defer tests.PrepareTestEnv(t)()
+       defer prepareTestEnvActionsArtifacts(t)()
 
        token, err := actions_service.CreateAuthorizationToken(48, 792, 193)
        assert.NoError(t, err)
@@ -125,7 +124,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)
        assert.NoError(t, err)
@@ -173,7 +172,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)
        assert.NoError(t, err)
@@ -236,7 +235,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)
        assert.NoError(t, err)
@@ -301,7 +300,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)
        assert.NoError(t, err)
@@ -336,7 +335,7 @@ func TestActionsArtifactV4DownloadSingle(t *testing.T) {
 }
 
 func TestActionsArtifactV4Delete(t *testing.T) {
-       defer tests.PrepareTestEnv(t)()
+       defer prepareTestEnvActionsArtifacts(t)()
 
        token, err := actions_service.CreateAuthorizationToken(48, 792, 193)
        assert.NoError(t, err)

diff --git a/tests/test_utils.go b/tests/test_utils.go
index b31491caf278115d82d0c7d75cfc527f8fdba963..e6ce3cce0e0e210a7358d3883057d087abb84a52 100644 (file)
--- a/tests/test_utils.go
+++ b/tests/test_utils.go
@@ -192,32 +192,7 @@ func PrepareAttachmentsStorage(t testing.TB) {
        }))
 }
 
-func PrepareArtifactsStorage(t testing.TB) {
-       // prepare actions artifacts directory and files
-       assert.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"),
-       })
-       assert.NoError(t, err)
-       assert.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)
-
-       // load database fixtures
-       assert.NoError(t, unittest.LoadFixtures())
-
-       // load git repo fixtures
+func PrepareGitRepoDirectory(t testing.TB) {
        assert.NoError(t, util.RemoveAll(setting.RepoRootPath))
        assert.NoError(t, unittest.CopyDir(path.Join(filepath.Dir(setting.AppPath), "tests/gitea-repositories-meta"), setting.RepoRootPath))
 
@@ -241,12 +216,25 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() {
                        _ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "pull"), 0o755)
                }
        }
+}
 
-       // Initialize actions artifact data
-       PrepareArtifactsStorage(t)
+func PrepareArtifactsStorage(t testing.TB) {
+       // prepare actions artifacts directory and files
+       assert.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"),
+       })
+       assert.NoError(t, err)
+       assert.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"),
        })
@@ -256,7 +244,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
        assert.NoError(t, db.TruncateBeans(db.DefaultContext,
                &packages_model.Package{},
@@ -268,17 +258,25 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() {
                &packages_model.PackageCleanupRule{},
        ))
        assert.NoError(t, storage.Clean(storage.Packages))
+}
 
+func PrepareTestEnv(t testing.TB, skip ...int) func() {
+       t.Helper()
+       deferFn := PrintCurrentTest(t, util.OptionalArg(skip)+1)
+
+       // load database fixtures
+       assert.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
-       if len(skip) > 0 {
-               actualSkip = skip[0] + 1
-       }
-       return testlogger.PrintCurrentTest(t, actualSkip)
+       return testlogger.PrintCurrentTest(t, util.OptionalArg(skip)+1)
 }
 
 // Printf takes a format and args and prints the string to os.Stdout