GitCaddy/GitCaddy Server
Limited
2
0
Fork 0
Code Issues Releases 4

Fix merge panic (#35606)

Browse Source 
To prevent potential bugs, the logic in #35543 makes `gitcmd.Command`
panic when attempting to override stdout or stderr. Instead of using
`PrepareCmd`, this PR now uses the WithXXX methods directly to avoid the
panic.

Fix #35603
This commit is contained in:
Lunny Xiao
2025-10-11 19:24:00 -07:00
committed by GitHub
parent 070288d72e
commit 74e6118a2e
6 changed files with 114 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

100
tests/integration/pull_merge_test.go
View File

@@ -43,7 +43,13 @@ import (
"github.com/stretchr/testify/assert"
)
func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum string, mergeStyle repo_model.MergeStyle, deleteBranch bool) *httptest.ResponseRecorder {
type MergeOptions struct {
Style repo_model.MergeStyle
HeadCommitID string
DeleteBranch bool
}
func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum string, mergeOptions MergeOptions) *httptest.ResponseRecorder {
req := NewRequest(t, "GET", path.Join(user, repo, "pulls", pullnum))
resp := session.MakeRequest(t, req, http.StatusOK)
@@ -51,11 +57,12 @@ func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum strin
link := path.Join(user, repo, "pulls", pullnum, "merge")
options := map[string]string{
"_csrf": htmlDoc.GetCSRF(),
"do": string(mergeStyle),
"_csrf": htmlDoc.GetCSRF(),
"do": string(mergeOptions.Style),
"head_commit_id": mergeOptions.HeadCommitID,
}
if deleteBranch {
if mergeOptions.DeleteBranch {
options["delete_branch_after_merge"] = "on"
}
@@ -69,6 +76,14 @@ func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum strin
assert.Equal(t, fmt.Sprintf("/%s/%s/pulls/%s", user, repo, pullnum), respJSON.Redirect)
pullnumInt, err := strconv.ParseInt(pullnum, 10, 64)
assert.NoError(t, err)
repository, err := repo_model.GetRepositoryByOwnerAndName(t.Context(), user, repo)
assert.NoError(t, err)
pull, err := issues_model.GetPullRequestByIndex(t.Context(), repository.ID, pullnumInt)
assert.NoError(t, err)
assert.True(t, pull.HasMerged)
return resp
}
@@ -102,7 +117,10 @@ func TestPullMerge(t *testing.T) {
elem := strings.Split(test.RedirectURL(resp), "/")
assert.Equal(t, "pulls", elem[3])
testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, false)
testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{
Style: repo_model.MergeStyleMerge,
DeleteBranch: false,
})
hookTasks, err = webhook.HookTasks(t.Context(), 1, 1)
assert.NoError(t, err)
@@ -124,7 +142,10 @@ func TestPullRebase(t *testing.T) {
elem := strings.Split(test.RedirectURL(resp), "/")
assert.Equal(t, "pulls", elem[3])
testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleRebase, false)
testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{
Style: repo_model.MergeStyleRebase,
DeleteBranch: false,
})
hookTasks, err = webhook.HookTasks(t.Context(), 1, 1)
assert.NoError(t, err)
@@ -146,7 +167,10 @@ func TestPullRebaseMerge(t *testing.T) {
elem := strings.Split(test.RedirectURL(resp), "/")
assert.Equal(t, "pulls", elem[3])
testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleRebaseMerge, false)
testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{
Style: repo_model.MergeStyleRebaseMerge,
DeleteBranch: false,
})
hookTasks, err = webhook.HookTasks(t.Context(), 1, 1)
assert.NoError(t, err)
@@ -169,7 +193,42 @@ func TestPullSquash(t *testing.T) {
elem := strings.Split(test.RedirectURL(resp), "/")
assert.Equal(t, "pulls", elem[3])
testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleSquash, false)
testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{
Style: repo_model.MergeStyleSquash,
DeleteBranch: false,
})
hookTasks, err = webhook.HookTasks(t.Context(), 1, 1)
assert.NoError(t, err)
assert.Len(t, hookTasks, hookTasksLenBefore+1)
})
}
func TestPullSquashWithHeadCommitID(t *testing.T) {
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
hookTasks, err := webhook.HookTasks(t.Context(), 1, 1) // Retrieve previous hook number
assert.NoError(t, err)
hookTasksLenBefore := len(hookTasks)
session := loginUser(t, "user1")
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited!)\n")
resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title")
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"})
headBranch, err := git_model.GetBranch(t.Context(), repo1.ID, "master")
assert.NoError(t, err)
assert.NotNil(t, headBranch)
elem := strings.Split(test.RedirectURL(resp), "/")
assert.Equal(t, "pulls", elem[3])
testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{
Style: repo_model.MergeStyleSquash,
DeleteBranch: false,
HeadCommitID: headBranch.CommitID,
})
hookTasks, err = webhook.HookTasks(t.Context(), 1, 1)
assert.NoError(t, err)
@@ -187,7 +246,10 @@ func TestPullCleanUpAfterMerge(t *testing.T) {
elem := strings.Split(test.RedirectURL(resp), "/")
assert.Equal(t, "pulls", elem[3])
testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, false)
testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{
Style: repo_model.MergeStyleMerge,
DeleteBranch: false,
})
// Check PR branch deletion
resp = testPullCleanUp(t, session, elem[1], elem[2], elem[4])
@@ -556,7 +618,10 @@ func TestPullRetargetChildOnBranchDelete(t *testing.T) {
elemChildPR := strings.Split(test.RedirectURL(respChildPR), "/")
assert.Equal(t, "pulls", elemChildPR[3])
testPullMerge(t, session, elemBasePR[1], elemBasePR[2], elemBasePR[4], repo_model.MergeStyleMerge, true)
testPullMerge(t, session, elemBasePR[1], elemBasePR[2], elemBasePR[4], MergeOptions{
Style: repo_model.MergeStyleMerge,
DeleteBranch: true,
})
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"})
branchBasePR := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "base-pr"})
@@ -592,7 +657,10 @@ func TestPullDontRetargetChildOnWrongRepo(t *testing.T) {
defer test.MockVariableValue(&setting.Repository.PullRequest.RetargetChildrenOnMerge, false)()
testPullMerge(t, session, elemBasePR[1], elemBasePR[2], elemBasePR[4], repo_model.MergeStyleMerge, true)
testPullMerge(t, session, elemBasePR[1], elemBasePR[2], elemBasePR[4], MergeOptions{
Style: repo_model.MergeStyleMerge,
DeleteBranch: true,
})
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"})
branchBasePR := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "base-pr"})
@@ -624,7 +692,10 @@ func TestPullRequestMergedWithNoPermissionDeleteBranch(t *testing.T) {
// user2 has no permission to delete branch of repo user1/repo1
session2 := loginUser(t, "user2")
testPullMerge(t, session2, elemBasePR[1], elemBasePR[2], elemBasePR[4], repo_model.MergeStyleMerge, true)
testPullMerge(t, session2, elemBasePR[1], elemBasePR[2], elemBasePR[4], MergeOptions{
Style: repo_model.MergeStyleMerge,
DeleteBranch: true,
})
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user4", Name: "repo1"})
branchBasePR := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "base-pr"})
@@ -672,7 +743,10 @@ func TestPullMergeIndexerNotifier(t *testing.T) {
// merge the pull request
elem := strings.Split(test.RedirectURL(createPullResp), "/")
assert.Equal(t, "pulls", elem[3])
testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, false)
testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{
Style: repo_model.MergeStyleMerge,
DeleteBranch: false,
})
// check if the issue is closed
issue = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{

5
tests/integration/pull_review_test.go
View File

@@ -212,7 +212,10 @@ func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) {
resp := testPullCreate(t, user1Session, "user1", "repo1", false, "master", "master", "This is a pull title")
elem := strings.Split(test.RedirectURL(resp), "/")
assert.Equal(t, "pulls", elem[3])
testPullMerge(t, user1Session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, false)
testPullMerge(t, user1Session, elem[1], elem[2], elem[4], MergeOptions{
Style: repo_model.MergeStyleMerge,
DeleteBranch: false,
})
// Grab the CSRF token.
req := NewRequest(t, "GET", path.Join(elem[1], elem[2], "pulls", elem[4]))

5
tests/integration/repo_activity_test.go
View File

@@ -27,7 +27,10 @@ func TestRepoActivity(t *testing.T) {
resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title")
elem := strings.Split(test.RedirectURL(resp), "/")
assert.Equal(t, "pulls", elem[3])
testPullMerge(t, session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, false)
testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{
Style: repo_model.MergeStyleMerge,
DeleteBranch: false,
})
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feat/better_readme", "README.md", "Hello, World (Edited Again)\n")
testPullCreate(t, session, "user1", "repo1", false, "master", "feat/better_readme", "This is a pull title")

10
tests/integration/repo_branch_test.go
View File

@@ -218,13 +218,19 @@ func prepareRepoPR(t *testing.T, baseSession, headSession *TestSession, baseRepo
testCreateBranch(t, headSession, headRepo.OwnerName, headRepo.Name, "branch/new-commit", "merged-pr", http.StatusSeeOther)
prID = testCreatePullToDefaultBranch(t, baseSession, baseRepo, headRepo, "merged-pr", "merged pr")
testAPINewFile(t, headSession, headRepo.OwnerName, headRepo.Name, "merged-pr", fmt.Sprintf("new-commit-%s.txt", headRepo.Name), "new-commit")
testPullMerge(t, baseSession, baseRepo.OwnerName, baseRepo.Name, prID, repo_model.MergeStyleRebaseMerge, false)
testPullMerge(t, baseSession, baseRepo.OwnerName, baseRepo.Name, prID, MergeOptions{
Style: repo_model.MergeStyleRebaseMerge,
DeleteBranch: false,
})
// create merged PR with deleted branch
testCreateBranch(t, headSession, headRepo.OwnerName, headRepo.Name, "branch/new-commit", "merged-pr-deleted", http.StatusSeeOther)
prID = testCreatePullToDefaultBranch(t, baseSession, baseRepo, headRepo, "merged-pr-deleted", "merged pr with deleted branch")
testAPINewFile(t, headSession, headRepo.OwnerName, headRepo.Name, "merged-pr-deleted", fmt.Sprintf("new-commit-%s-2.txt", headRepo.Name), "new-commit")
testPullMerge(t, baseSession, baseRepo.OwnerName, baseRepo.Name, prID, repo_model.MergeStyleRebaseMerge, true)
testPullMerge(t, baseSession, baseRepo.OwnerName, baseRepo.Name, prID, MergeOptions{
Style: repo_model.MergeStyleRebaseMerge,
DeleteBranch: true,
})
}
func checkRecentlyPushedNewBranches(t *testing.T, session *TestSession, repoPath string, expected []string) {