From 569b73e4952718d9673ff7f517af84be45b0e3d7 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Tue, 9 Apr 2024 15:08:01 +0200 Subject: [PATCH 1/5] [TEST] cancel all processes on PrepareTestEnv (cherry picked from commit aba99ab8fc3b8bc8f55fa0b7d6261eea7b28d7c0) --- tests/test_utils.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/test_utils.go b/tests/test_utils.go index 85d7462d73..6d3512ca10 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -11,7 +11,9 @@ import ( "os" "path" "path/filepath" + "runtime" "testing" + "time" "code.gitea.io/gitea/models/db" packages_model "code.gitea.io/gitea/models/packages" @@ -20,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/process" repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" @@ -193,6 +196,36 @@ func PrepareAttachmentsStorage(t testing.TB) { })) } +// cancelProcesses cancels all processes of the [process.Manager]. +// Returns immediately if delay is 0, otherwise wait until all processes are done +// and fails the test if it takes longer that the given delay. +func cancelProcesses(t testing.TB, delay time.Duration) { + processManager := process.GetManager() + processes, _ := processManager.Processes(true, true) + for _, p := range processes { + processManager.Cancel(p.PID) + t.Logf("PrepareTestEnv:Process %q cancelled", p.Description) + } + if delay == 0 || len(processes) == 0 { + return + } + + start := time.Now() + processes, _ = processManager.Processes(true, true) + for len(processes) > 0 { + if time.Since(start) > delay { + t.Errorf("ERROR PrepareTestEnv: could not cancel all processes within %s", delay) + for _, p := range processes { + t.Logf("PrepareTestEnv:Remaining Process: %q", p.Description) + } + return + } + runtime.Gosched() // let the context cancellation propagate + processes, _ = processManager.Processes(true, true) + } + t.Logf("PrepareTestEnv: all processes cancelled within %s", time.Since(start)) +} + func PrepareTestEnv(t testing.TB, skip ...int) func() { t.Helper() ourSkip := 1 @@ -201,6 +234,11 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() { } 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, time.Second) + t.Cleanup(func() { cancelProcesses(t, 0) }) // cancel remaining processes in a non-blocking way + // load database fixtures assert.NoError(t, unittest.LoadFixtures()) From dd474b72dfe47e6032dbce0ba68f3bb700e3da5c Mon Sep 17 00:00:00 2001 From: oliverpool Date: Tue, 9 Apr 2024 21:14:51 +0200 Subject: [PATCH 2/5] add missing defer (cherry picked from commit 8ffaa08b04f507c5133cef2b585777a469127563) --- tests/integration/git_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index 708f9a820d..f833366568 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -1076,6 +1076,7 @@ func TestDataAsync_Issue29101(t *testing.T) { gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, repo) assert.NoError(t, err) + defer gitRepo.Close() commit, err := gitRepo.GetCommit(sha) assert.NoError(t, err) From ec6b255c2c2968f9bb36f9f47f735385ae7bf00f Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Thu, 11 Apr 2024 07:36:59 +0200 Subject: [PATCH 3/5] [TESTS] disable test failure on log.Error for now (part 2) Fixes: https://codeberg.org/forgejo/forgejo/issues/3153 (cherry picked from commit fd62033b98ea40db0285309892ba61ae49abb85d) --- models/migrations/base/tests.go | 3 +-- tests/e2e/e2e_test.go | 3 +-- tests/integration/integration_test.go | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/models/migrations/base/tests.go b/models/migrations/base/tests.go index 0726211ea8..0989902a65 100644 --- a/models/migrations/base/tests.go +++ b/models/migrations/base/tests.go @@ -161,8 +161,7 @@ func MainTest(m *testing.M) { exitStatus := m.Run() if err := testlogger.WriterCloser.Reset(); err != nil && exitStatus == 0 { - fmt.Printf("testlogger.WriterCloser.Reset: %v\n", err) - os.Exit(1) + fmt.Printf("testlogger.WriterCloser.Reset: error ignored: %v\n", err) } if err := removeAllWithRetry(setting.RepoRootPath); err != nil { fmt.Fprintf(os.Stderr, "os.RemoveAll: %v\n", err) diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 304dac0f45..92c90f52e6 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -60,8 +60,7 @@ func TestMain(m *testing.M) { exitVal := m.Run() if err := testlogger.WriterCloser.Reset(); err != nil { - fmt.Printf("testlogger.WriterCloser.Reset: %v\n", err) - os.Exit(1) + fmt.Printf("testlogger.WriterCloser.Reset: error ignored: %v\n", err) } if err = util.RemoveAll(setting.Indexer.IssuePath); err != nil { fmt.Printf("util.RemoveAll: %v\n", err) diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index e147d6a21b..8fd55d7fa8 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -184,8 +184,7 @@ func TestMain(m *testing.M) { exitCode := m.Run() if err := testlogger.WriterCloser.Reset(); err != nil { - fmt.Printf("testlogger.WriterCloser.Reset: %v\n", err) - os.Exit(1) + fmt.Printf("testlogger.WriterCloser.Reset: error ignored: %v\n", err) } if err = util.RemoveAll(setting.Indexer.IssuePath); err != nil { From 8f06a99c2ce1add8e31082e1a7792b3aa15dfefe Mon Sep 17 00:00:00 2001 From: oliverpool Date: Wed, 10 Apr 2024 14:26:47 +0200 Subject: [PATCH 4/5] [TEST] make the indexer and pull tasks cancellable (without shutdown) See https://codeberg.org/forgejo/forgejo/pulls/3130#issuecomment-1763440 for the conflict resolution. (cherry picked from commit d79690ce1890e692add310532858a77076792965) --- modules/indexer/code/indexer.go | 5 ++++- services/pull/pull.go | 9 +++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/modules/indexer/code/indexer.go b/modules/indexer/code/indexer.go index c1ab26569c..31adce10e0 100644 --- a/modules/indexer/code/indexer.go +++ b/modules/indexer/code/indexer.go @@ -120,9 +120,12 @@ func Init() { case "bleve", "elasticsearch": handler := func(items ...*internal.IndexerData) (unhandled []*internal.IndexerData) { indexer := *globalIndexer.Load() + // make it a process to allow for cancellation (especially during integration tests where no global shutdown happens) + batchCtx, _, finished := process.GetManager().AddContext(ctx, "CodeIndexer batch") + defer finished() for _, indexerData := range items { log.Trace("IndexerData Process Repo: %d", indexerData.RepoID) - if err := index(ctx, indexer, indexerData.RepoID); err != nil { + if err := index(batchCtx, indexer, indexerData.RepoID); err != nil { unhandled = append(unhandled, indexerData) if !setting.IsInTesting { log.Error("Codes indexer handler: index error for repo %v: %v", indexerData.RepoID, err) diff --git a/services/pull/pull.go b/services/pull/pull.go index ae70a14a1e..720efdb0cb 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -26,6 +26,7 @@ import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/process" repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/sync" @@ -296,8 +297,12 @@ func checkForInvalidation(ctx context.Context, requests issues_model.PullRequest // AddTestPullRequestTask adds new test tasks by given head/base repository and head/base branch, // and generate new patch for testing as needed. func AddTestPullRequestTask(ctx context.Context, doer *user_model.User, repoID int64, branch string, isSync bool, oldCommitID, newCommitID string, timeNano int64) { - log.Trace("AddTestPullRequestTask [head_repo_id: %d, head_branch: %s]: only pull requests created before nano time %d will be considered", repoID, branch, timeNano) - go graceful.GetManager().RunWithShutdownContext(func(ctx context.Context) { + description := fmt.Sprintf("AddTestPullRequestTask [head_repo_id: %d, head_branch: %s]: only pull requests created before nano time %d will be considered", repoID, branch, timeNano) + log.Trace(description) + go graceful.GetManager().RunWithShutdownContext(func(shutdownCtx context.Context) { + // make it a process to allow for cancellation (especially during integration tests where no global shutdown happens) + ctx, _, finished := process.GetManager().AddContext(shutdownCtx, description) + defer finished() // There is no sensible way to shut this down ":-(" // If you don't let it run all the way then you will lose data // TODO: graceful: TestPullRequest needs to become a queue! From 2f54b76c5c155b6449d6d9cc4827c93b17a6b885 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Thu, 18 Apr 2024 18:45:40 +0200 Subject: [PATCH 5/5] fix(tests): 30s to cancel processes to avoid false negatives on slower machines it can take more than 1 second to cancel leftover tasks (cherry picked from commit 6316e21be285a968647a602be2441b3eb3c962bc) --- tests/test_utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_utils.go b/tests/test_utils.go index 6d3512ca10..5837e1263c 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -236,7 +236,7 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() { // kill all background processes to prevent them from interfering with the fixture loading // see https://codeberg.org/forgejo/forgejo/issues/2962 - cancelProcesses(t, time.Second) + cancelProcesses(t, 30*time.Second) t.Cleanup(func() { cancelProcesses(t, 0) }) // cancel remaining processes in a non-blocking way // load database fixtures