From c62106fa056852f1e7f268412cf3a37a25b17f71 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 1 Nov 2024 12:23:45 +0800 Subject: [PATCH 1/4] fix --- modules/testlogger/testlogger.go | 6 +- modules/util/util.go | 14 ++++ .../integration/api_actions_artifact_test.go | 25 ++++--- .../api_actions_artifact_v4_test.go | 15 ++--- tests/test_utils.go | 66 +++++++++---------- 5 files changed, 71 insertions(+), 55 deletions(-) diff --git a/modules/testlogger/testlogger.go b/modules/testlogger/testlogger.go index 4215567c00488..af99903f44df9 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.DefArgZero(skip) + 1 _, filename, line, _ := runtime.Caller(actualSkip) if log.CanColorStdout { diff --git a/modules/util/util.go b/modules/util/util.go index 44b5a6ed81534..bcc00f374d4d5 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -230,6 +230,20 @@ func IfZero[T comparable](v, def T) T { return v } +// DefArg helps the "optional argument" in Golang: func(foo string, optionalArg ...int) +// it returns the first non-zero value from the given optional argument, +// or the default value if there is no optional argument. +func DefArg[T any](defArgs []T, def T) (ret T) { + if len(defArgs) == 1 { + return defArgs[0] + } + return def +} + +func DefArgZero[T any](defArgs []T) (ret T) { + return DefArg(defArgs, 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/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index aa2e2f2adfe06..de5797f289da3 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 e731542037f91..8821472801dab 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 b31491caf2781..012afd19264a9 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.DefArgZero(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.DefArgZero(skip)+1) } // Printf takes a format and args and prints the string to os.Stdout From ea9e0aebccc72ab0c44b76b3f9ade0a1e3a99b6d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 1 Nov 2024 22:32:23 +0800 Subject: [PATCH 2/4] improve DefaultArg --- modules/testlogger/testlogger.go | 2 +- modules/util/util.go | 27 ++++++++++++++++----------- modules/util/util_test.go | 13 +++++++++++++ tests/test_utils.go | 4 ++-- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/modules/testlogger/testlogger.go b/modules/testlogger/testlogger.go index af99903f44df9..10418d0589cb4 100644 --- a/modules/testlogger/testlogger.go +++ b/modules/testlogger/testlogger.go @@ -93,7 +93,7 @@ func (w *testLoggerWriterCloser) Reset() { func PrintCurrentTest(t testing.TB, skip ...int) func() { t.Helper() start := time.Now() - actualSkip := util.DefArgZero(skip) + 1 + actualSkip := util.DefaultArg(skip) + 1 _, filename, line, _ := runtime.Caller(actualSkip) if log.CanColorStdout { diff --git a/modules/util/util.go b/modules/util/util.go index bcc00f374d4d5..f9c5b23e0dc20 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -230,18 +230,23 @@ func IfZero[T comparable](v, def T) T { return v } -// DefArg helps the "optional argument" in Golang: func(foo string, optionalArg ...int) -// it returns the first non-zero value from the given optional argument, -// or the default value if there is no optional argument. -func DefArg[T any](defArgs []T, def T) (ret T) { - if len(defArgs) == 1 { - return defArgs[0] +// DefaultArg helps the "optional argument" in Golang: +// +// func foo(optionalArg ...int) { return DefaultArg(optionalArg) } +// calling `foo()` gets 0, calling `foo(100)` gets 100 +// func bar(optionalArg ...int) { return DefaultArg(optionalArg, 42) } +// calling `bar()` gets 42, calling `bar(100)` gets 100 +// +// Passing more than 1 item to `optionalArg` or `def` is undefined behavior. +// At the moment it only returns the first argument. +func DefaultArg[T any](optionalArg []T, def ...T) (ret T) { + if len(optionalArg) >= 1 { + return optionalArg[0] } - return def -} - -func DefArgZero[T any](defArgs []T) (ret T) { - return DefArg(defArgs, ret) + if len(def) >= 1 { + return def[0] + } + return ret } func ReserveLineBreakForTextarea(input string) string { diff --git a/modules/util/util_test.go b/modules/util/util_test.go index de8f065cadc73..99cc5cf002a1f 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 TestDefaultArg(t *testing.T) { + foo := func(other any, optionalArg ...int) int { + return DefaultArg(optionalArg) + } + bar := func(other any, optionalArg ...int) int { + return DefaultArg(optionalArg, 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/test_utils.go b/tests/test_utils.go index 012afd19264a9..ee598248e2a2d 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -262,7 +262,7 @@ func PrepareCleanPackageData(t testing.TB) { func PrepareTestEnv(t testing.TB, skip ...int) func() { t.Helper() - deferFn := PrintCurrentTest(t, util.DefArgZero(skip)+1) + deferFn := PrintCurrentTest(t, util.DefaultArg(skip)+1) // load database fixtures assert.NoError(t, unittest.LoadFixtures()) @@ -276,7 +276,7 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() { func PrintCurrentTest(t testing.TB, skip ...int) func() { t.Helper() - return testlogger.PrintCurrentTest(t, util.DefArgZero(skip)+1) + return testlogger.PrintCurrentTest(t, util.DefaultArg(skip)+1) } // Printf takes a format and args and prints the string to os.Stdout From e2ac3171ebeef14b16a360eb014b84be783a88ab Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 1 Nov 2024 22:39:31 +0800 Subject: [PATCH 3/4] use OptionalArg --- modules/testlogger/testlogger.go | 2 +- modules/util/util.go | 24 ++++++++++++------------ modules/util/util_test.go | 8 ++++---- tests/test_utils.go | 4 ++-- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/modules/testlogger/testlogger.go b/modules/testlogger/testlogger.go index 10418d0589cb4..9a54d63f20427 100644 --- a/modules/testlogger/testlogger.go +++ b/modules/testlogger/testlogger.go @@ -93,7 +93,7 @@ func (w *testLoggerWriterCloser) Reset() { func PrintCurrentTest(t testing.TB, skip ...int) func() { t.Helper() start := time.Now() - actualSkip := util.DefaultArg(skip) + 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 f9c5b23e0dc20..1fb4cb21cb884 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -230,21 +230,21 @@ func IfZero[T comparable](v, def T) T { return v } -// DefaultArg helps the "optional argument" in Golang: +// OptionalArg helps the "optional argument" in Golang: // -// func foo(optionalArg ...int) { return DefaultArg(optionalArg) } -// calling `foo()` gets 0, calling `foo(100)` gets 100 -// func bar(optionalArg ...int) { return DefaultArg(optionalArg, 42) } -// calling `bar()` gets 42, calling `bar(100)` gets 100 +// 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 `optionalArg` or `def` is undefined behavior. -// At the moment it only returns the first argument. -func DefaultArg[T any](optionalArg []T, def ...T) (ret T) { - if len(optionalArg) >= 1 { - return optionalArg[0] +// 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(def) >= 1 { - return def[0] + if len(defaultValue) >= 1 { + return defaultValue[0] } return ret } diff --git a/modules/util/util_test.go b/modules/util/util_test.go index 99cc5cf002a1f..ca27745b0bc83 100644 --- a/modules/util/util_test.go +++ b/modules/util/util_test.go @@ -242,11 +242,11 @@ func TestReserveLineBreakForTextarea(t *testing.T) { } func TestDefaultArg(t *testing.T) { - foo := func(other any, optionalArg ...int) int { - return DefaultArg(optionalArg) + foo := func(other any, optArg ...int) int { + return OptionalArg(optArg) } - bar := func(other any, optionalArg ...int) int { - return DefaultArg(optionalArg, 42) + bar := func(other any, optArg ...int) int { + return OptionalArg(optArg, 42) } assert.Equal(t, 0, foo(nil)) assert.Equal(t, 100, foo(nil, 100)) diff --git a/tests/test_utils.go b/tests/test_utils.go index ee598248e2a2d..e6ce3cce0e0e2 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -262,7 +262,7 @@ func PrepareCleanPackageData(t testing.TB) { func PrepareTestEnv(t testing.TB, skip ...int) func() { t.Helper() - deferFn := PrintCurrentTest(t, util.DefaultArg(skip)+1) + deferFn := PrintCurrentTest(t, util.OptionalArg(skip)+1) // load database fixtures assert.NoError(t, unittest.LoadFixtures()) @@ -276,7 +276,7 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() { func PrintCurrentTest(t testing.TB, skip ...int) func() { t.Helper() - return testlogger.PrintCurrentTest(t, util.DefaultArg(skip)+1) + return testlogger.PrintCurrentTest(t, util.OptionalArg(skip)+1) } // Printf takes a format and args and prints the string to os.Stdout From 1647dbae5a1eaf3d89a9f7f2134d6e19c9b91f76 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 1 Nov 2024 22:47:19 +0800 Subject: [PATCH 4/4] fix test name --- modules/util/util_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/util/util_test.go b/modules/util/util_test.go index ca27745b0bc83..9ce72fb86693f 100644 --- a/modules/util/util_test.go +++ b/modules/util/util_test.go @@ -241,7 +241,7 @@ func TestReserveLineBreakForTextarea(t *testing.T) { assert.Equal(t, ReserveLineBreakForTextarea("test\r\ndata\r\n"), "test\ndata\n") } -func TestDefaultArg(t *testing.T) { +func TestOptionalArg(t *testing.T) { foo := func(other any, optArg ...int) int { return OptionalArg(optArg) }