From 8e083c9f3eff79c45d4de87e131eee5a9d7dba17 Mon Sep 17 00:00:00 2001 From: forgejo-backport-action Date: Tue, 6 Jan 2026 17:06:22 +0100 Subject: [PATCH] [v14.0/forgejo] fix: prevent intermittent test failures caused by uncancellable tasks (#10717) **Backport:** https://codeberg.org/forgejo/forgejo/pulls/10713 Attempt to fix intermittent test failure noted in #10633, detailed technical notes in https://codeberg.org/forgejo/forgejo/issues/10633#issuecomment-9571199. - Failure to cancel the previous processes is now a test error that aborts immediately, preventing 2hr long test runs that won't succeed. - When the process cancellation fails, the stack trace of all goroutines is printed to help diagnose the cause of any failure to cancel tasks. - `context.Background()` referenced in the actions notifier is corrected when opening git repos, which seems to be a cause of failure to cancel the tasks -- git subprocesses are spawned from the repo context, which is the background context, and that prevents the context registered in the process manager from cancelling them. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] I do not want this change to show in the release notes. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. Co-authored-by: Mathieu Fenniak Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10717 Reviewed-by: Mathieu Fenniak Co-authored-by: forgejo-backport-action Co-committed-by: forgejo-backport-action --- services/actions/notifier_helper.go | 6 +++--- tests/test_utils.go | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index cb27914ae0..1b0bd67f12 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -187,8 +187,8 @@ func notify(ctx context.Context, input *notifyInput) error { return handleWorkflows(ctx, detectedWorkflows, commit, input, ref.String()) } -func getGitRepoAndCommit(_ context.Context, input *notifyInput) (*git.Repository, *git.Commit, git.RefName, error) { - gitRepo, err := gitrepo.OpenRepository(context.Background(), input.Repo) +func getGitRepoAndCommit(ctx context.Context, input *notifyInput) (*git.Repository, *git.Commit, git.RefName, error) { + gitRepo, err := gitrepo.OpenRepository(ctx, input.Repo) if err != nil { return nil, nil, "", fmt.Errorf("git.OpenRepository: %w", err) } @@ -598,7 +598,7 @@ func DetectAndHandleSchedules(ctx context.Context, repo *repo_model.Repository) return nil } - gitRepo, err := gitrepo.OpenRepository(context.Background(), repo) + gitRepo, err := gitrepo.OpenRepository(ctx, repo) if err != nil { return fmt.Errorf("git.OpenRepository: %w", err) } diff --git a/tests/test_utils.go b/tests/test_utils.go index 6432dfc271..0fc9329b77 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -263,6 +263,11 @@ func cancelProcesses(t testing.TB, delay time.Duration) { for _, p := range processes { t.Logf("PrepareTestEnv:Remaining Process: %q", p.Description) } + stacks := allGoroutineStacks() + t.Errorf("All goroutine stacks during process cancellation failure:\n%s", string(stacks)) + // exit so that we don't spin in a loop executing `delay` wait over and over again when we won't be able to + // complete tests correctly due to the environmental issue present. + exitf("terminating test run due to unrecoverable failure") return } runtime.Gosched() // let the context cancellation propagate @@ -271,6 +276,18 @@ func cancelProcesses(t testing.TB, delay time.Duration) { t.Logf("PrepareTestEnv: all processes cancelled within %s", time.Since(start)) } +// allGoroutineStacks is the same as runtime/debug.Stack(), but it captures the stack of all goroutines. +func allGoroutineStacks() []byte { + buf := make([]byte, 1024) + for { + n := runtime.Stack(buf, true) + if n < len(buf) { + return buf[:n] + } + buf = make([]byte, 2*len(buf)) + } +} + func PrepareGitRepoDirectory(t testing.TB) { setting.RepoRootPath = t.TempDir() require.NoError(t, unittest.CopyDir(preparedDir, setting.RepoRootPath))