From ec2d1593c269e06655525deb96f74b8094221b6f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 1 Nov 2024 23:18:29 +0800 Subject: [PATCH] Refactor tests to prevent from unnecessary preparations (#32398) --- modules/testlogger/testlogger.go | 6 +- modules/util/util.go | 19 ++++++ modules/util/util_test.go | 13 ++++ .../integration/api_actions_artifact_test.go | 25 ++++--- .../api_actions_artifact_v4_test.go | 15 ++--- tests/test_utils.go | 66 +++++++++---------- 6 files changed, 89 insertions(+), 55 deletions(-) diff --git a/modules/testlogger/testlogger.go b/modules/testlogger/testlogger.go index 4215567c00..9a54d63f20 100644 --- 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 44b5a6ed81..1fb4cb21cb 100644 --- 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 de8f065cad..9ce72fb866 100644 --- 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 aa2e2f2adf..de5797f289 100644 --- 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 e731542037..8821472801 100644 --- 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 b31491caf2..e6ce3cce0e 100644 --- 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