mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-12 22:10:25 +00:00
[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/<pull request number>.md` to be be used for the release notes instead of the title. Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10717 Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org> Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org> Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
This commit is contained in:
parent
342a54a142
commit
8e083c9f3e
2 changed files with 20 additions and 3 deletions
|
|
@ -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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue