From 2cfeb0bc0506273b76c47902986813b176261b6a Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Wed, 19 Nov 2025 20:27:32 -0700 Subject: [PATCH 1/8] github-style format --- services/pull/pull.go | 25 ++-- tests/integration/editor_test.go | 24 ++++ tests/integration/pull_merge_test.go | 179 +++++++++++++++++++++++++++ 3 files changed, 214 insertions(+), 14 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index a8468b5bf3446..825938e1ab3e0 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -837,7 +837,8 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ authors := make([]string, 0, len(commits)) stringBuilder := strings.Builder{} - if !setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages { + populateWithCommits := setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages + if !populateWithCommits { message := strings.TrimSpace(pr.Issue.Content) stringBuilder.WriteString(message) if stringBuilder.Len() > 0 { @@ -849,21 +850,18 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ } // commits list is in reverse chronological order - first := true for i := len(commits) - 1; i >= 0; i-- { commit := commits[i] - if setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages { + if populateWithCommits { maxSize := setting.Repository.PullRequest.DefaultMergeMessageSize if maxSize < 0 || stringBuilder.Len() < maxSize { - var toWrite []byte - if first { - first = false - toWrite = []byte(strings.TrimPrefix(commit.CommitMessage, pr.Issue.Title)) - } else { - toWrite = []byte(commit.CommitMessage) + if strings.TrimSpace(commit.CommitMessage) == "" { + continue } + toWrite := fmt.Appendf(nil, "* %s\n", commit.CommitMessage) + if len(toWrite) > maxSize-stringBuilder.Len() && maxSize > -1 { toWrite = append(toWrite[:maxSize-stringBuilder.Len()], "..."...) } @@ -871,11 +869,6 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ log.Error("Unable to write commit message Error: %v", err) return "" } - - if _, err := stringBuilder.WriteRune('\n'); err != nil { - log.Error("Unable to write commit message Error: %v", err) - return "" - } } } @@ -916,6 +909,10 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ } } + if populateWithCommits && stringBuilder.Len() > 0 && len(authors) > 0 { + stringBuilder.WriteString("---------\n\n") + } + for _, author := range authors { if _, err := stringBuilder.WriteString("Co-authored-by: "); err != nil { log.Error("Unable to write to string builder Error: %v", err) diff --git a/tests/integration/editor_test.go b/tests/integration/editor_test.go index 05c7b272abcd4..84f049012379b 100644 --- a/tests/integration/editor_test.go +++ b/tests/integration/editor_test.go @@ -83,6 +83,21 @@ func testCreateFile(t *testing.T, session *TestSession, user, repo, baseBranchNa }) } +func testCreateFileWithCommitMessage(t *testing.T, session *TestSession, user, repo, baseBranchName, newBranchName, filePath, content, commitSummary, commitMessage string) { + commitChoice := "direct" + if newBranchName != "" && newBranchName != baseBranchName { + commitChoice = "commit-to-new-branch" + } + testEditorActionEdit(t, session, user, repo, "_new", baseBranchName, "", map[string]string{ + "tree_path": filePath, + "content": content, + "commit_choice": commitChoice, + "new_branch_name": newBranchName, + "commit_summary": commitSummary, + "commit_message": commitMessage, + }) +} + func testEditorProtectedBranch(t *testing.T) { session := loginUser(t, "user2") // Change the "master" branch to "protected" @@ -139,6 +154,15 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa }) } +func testEditFileWithCommitMessage(t *testing.T, session *TestSession, user, repo, branch, filePath, newContent, commitSummary, commitMessage string) { + testEditorActionEdit(t, session, user, repo, "_edit", branch, filePath, map[string]string{ + "content": newContent, + "commit_choice": "direct", + "commit_summary": commitSummary, + "commit_message": commitMessage, + }) +} + func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath, newContent string) { testEditorActionEdit(t, session, user, repo, "_edit", branch, filePath, map[string]string{ "content": newContent, diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index f273d9fb3aacb..1bd734ce19c78 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -19,6 +19,7 @@ import ( auth_model "code.gitea.io/gitea/models/auth" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/perm" pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" @@ -1180,3 +1181,181 @@ func TestPullNonMergeForAdminWithBranchProtection(t *testing.T) { session.MakeRequest(t, mergeReq, http.StatusMethodNotAllowed) }) } + +func TestPullSquashMessage(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + user2Session := loginUser(t, user2.Name) + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + user4Session := loginUser(t, user4.Name) + + sessions := map[string]*TestSession{ + user2.Name: user2Session, + user4.Name: user4Session, + } + + // Enable POPULATE_SQUASH_COMMENT_WITH_COMMIT_MESSAGES + resetFunc1 := test.MockVariableValue(&setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages, true) + defer resetFunc1() + // Set DEFAULT_MERGE_MESSAGE_SIZE + resetFunc2 := test.MockVariableValue(&setting.Repository.PullRequest.DefaultMergeMessageSize, 512) + defer resetFunc2() + + repo, err := repo_service.CreateRepository(t.Context(), user2, user2, repo_service.CreateRepoOptions{ + Name: "squash-message-test", + Description: "Test squash message", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + doAPIAddCollaborator(NewAPITestContext(t, repo.OwnerName, repo.Name, auth_model.AccessTokenScopeWriteRepository), user4.Name, perm.AccessModeWrite)(t) + + type commitInfo struct { + userName string + commitSummary string + commitMessage string + } + + testCases := []struct { + name string + commitInfos []*commitInfo + expectedMessage string + }{ + { + name: "Only summaries", + commitInfos: []*commitInfo{ + { + userName: user2.Name, + commitSummary: "Implement the login endpoint", + }, + { + userName: user2.Name, + commitSummary: "Validate request body", + }, + }, + expectedMessage: `* Implement the login endpoint + +* Validate request body + +`, + }, + { + name: "Summaries and messages", + commitInfos: []*commitInfo{ + { + userName: user2.Name, + commitSummary: "Refactor user service", + commitMessage: `Implement the login endpoint. +Validate request body.`, + }, + { + userName: user2.Name, + commitSummary: "Add email notification service", + commitMessage: `Implements a new email notification module. + +- Supports templating +- Supports HTML and plain text modes +- Includes retry logic`, + }, + }, + expectedMessage: `* Refactor user service + +Implement the login endpoint. +Validate request body. + +* Add email notification service + +Implements a new email notification module. + +- Supports templating +- Supports HTML and plain text modes +- Includes retry logic + +`, + }, + { + name: "Long Message", + commitInfos: []*commitInfo{ + { + userName: user2.Name, + commitSummary: "Add advanced validation logic for user onboarding", + commitMessage: `This commit introduces a comprehensive validation layer for the user onboarding flow. +The primary goal is to ensure that all input data is strictly validated before being processed by downstream services. +This improves system reliability and significantly reduces runtime exceptions in the registration pipeline. + +The validation logic includes: + +1. Email format checking using RFC 5322-compliant patterns. +2. Username length and character limitation enforcement. +3. Password strength enforcement, including: + - Minimum length checks + - Mixed character type detection + - Optional entropy-based scoring +4. Optional phone number validation using region-specific rules. +`, + }, + }, + expectedMessage: `* Add advanced validation logic for user onboarding + +This commit introduces a comprehensive validation layer for the user onboarding flow. +The primary goal is to ensure that all input data is strictly validated before being processed by downstream services. +This improves system reliability and significantly reduces runtime exceptions in the registration pipeline. + +The validation logic includes: + +1. Email format checking using RFC 5322-compliant patterns. +2. Username length and character limitation enfor...`, + }, + { + name: "Test Co-authored-by", + commitInfos: []*commitInfo{ + { + userName: user2.Name, + commitSummary: "Implement the login endpoint", + }, + { + userName: user4.Name, + commitSummary: "Validate request body", + }, + }, + expectedMessage: `* Implement the login endpoint + +* Validate request body + +--------- + +Co-authored-by: user4 +`, + }, + } + + for tcNum, tc := range testCases { + branchName := "test-branch-" + strconv.Itoa(tcNum) + fileName := fmt.Sprintf("test-file-%d.txt", tcNum) + t.Run(tc.name, func(t *testing.T) { + for infoIdx, info := range tc.commitInfos { + content := "content-" + strconv.Itoa(infoIdx) + if infoIdx == 0 { + testCreateFileWithCommitMessage(t, sessions[info.userName], repo.OwnerName, repo.Name, repo.DefaultBranch, branchName, fileName, content, info.commitSummary, info.commitMessage) + } else { + testEditFileWithCommitMessage(t, sessions[info.userName], repo.OwnerName, repo.Name, branchName, fileName, content, info.commitSummary, info.commitMessage) + } + } + resp := testPullCreateDirectly(t, user2Session, createPullRequestOptions{ + BaseRepoOwner: user2.Name, + BaseRepoName: repo.Name, + BaseBranch: repo.DefaultBranch, + HeadBranch: branchName, + Title: "Pull for " + branchName, + }) + elems := strings.Split(test.RedirectURL(resp), "/") + pullIndex, err := strconv.Atoi(elems[4]) + assert.NoError(t, err) + pullRequest := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, Index: int64(pullIndex)}) + squashMergeCommitMessage := pull_service.GetSquashMergeCommitMessages(t.Context(), pullRequest) + assert.Equal(t, tc.expectedMessage, squashMergeCommitMessage) + }) + } + }) +} From b3f6b8cd9b2e2326de358a455a526993a492b2d5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 20 Nov 2025 14:57:05 +0800 Subject: [PATCH 2/8] fix tests --- tests/integration/api_repo_file_helpers.go | 15 ++++++- tests/integration/editor_test.go | 24 ----------- tests/integration/pull_merge_test.go | 49 +++++++++------------- 3 files changed, 33 insertions(+), 55 deletions(-) diff --git a/tests/integration/api_repo_file_helpers.go b/tests/integration/api_repo_file_helpers.go index 785416b0f580d..f8d6fc803f192 100644 --- a/tests/integration/api_repo_file_helpers.go +++ b/tests/integration/api_repo_file_helpers.go @@ -19,6 +19,9 @@ import ( type createFileInBranchOptions struct { OldBranch, NewBranch string + CommitMessage string + CommitterName string + CommitterEmail string } func testCreateFileInBranch(t *testing.T, user *user_model.User, repo *repo_model.Repository, createOpts createFileInBranchOptions, files map[string]string) *api.FilesResponse { @@ -29,7 +32,17 @@ func testCreateFileInBranch(t *testing.T, user *user_model.User, repo *repo_mode func createFileInBranch(user *user_model.User, repo *repo_model.Repository, createOpts createFileInBranchOptions, files map[string]string) (*api.FilesResponse, error) { ctx := context.TODO() - opts := &files_service.ChangeRepoFilesOptions{OldBranch: createOpts.OldBranch, NewBranch: createOpts.NewBranch} + opts := &files_service.ChangeRepoFilesOptions{ + OldBranch: createOpts.OldBranch, + NewBranch: createOpts.NewBranch, + Message: createOpts.CommitMessage, + } + if createOpts.CommitterName != "" || createOpts.CommitterEmail != "" { + opts.Committer = &files_service.IdentityOptions{ + GitUserName: createOpts.CommitterName, + GitUserEmail: createOpts.CommitterEmail, + } + } for path, content := range files { opts.Files = append(opts.Files, &files_service.ChangeRepoFile{ Operation: "create", diff --git a/tests/integration/editor_test.go b/tests/integration/editor_test.go index 84f049012379b..05c7b272abcd4 100644 --- a/tests/integration/editor_test.go +++ b/tests/integration/editor_test.go @@ -83,21 +83,6 @@ func testCreateFile(t *testing.T, session *TestSession, user, repo, baseBranchNa }) } -func testCreateFileWithCommitMessage(t *testing.T, session *TestSession, user, repo, baseBranchName, newBranchName, filePath, content, commitSummary, commitMessage string) { - commitChoice := "direct" - if newBranchName != "" && newBranchName != baseBranchName { - commitChoice = "commit-to-new-branch" - } - testEditorActionEdit(t, session, user, repo, "_new", baseBranchName, "", map[string]string{ - "tree_path": filePath, - "content": content, - "commit_choice": commitChoice, - "new_branch_name": newBranchName, - "commit_summary": commitSummary, - "commit_message": commitMessage, - }) -} - func testEditorProtectedBranch(t *testing.T) { session := loginUser(t, "user2") // Change the "master" branch to "protected" @@ -154,15 +139,6 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa }) } -func testEditFileWithCommitMessage(t *testing.T, session *TestSession, user, repo, branch, filePath, newContent, commitSummary, commitMessage string) { - testEditorActionEdit(t, session, user, repo, "_edit", branch, filePath, map[string]string{ - "content": newContent, - "commit_choice": "direct", - "commit_summary": commitSummary, - "commit_message": commitMessage, - }) -} - func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath, newContent string) { testEditorActionEdit(t, session, user, repo, "_edit", branch, filePath, map[string]string{ "content": newContent, diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 1bd734ce19c78..5820bae3dd017 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -19,7 +19,6 @@ import ( auth_model "code.gitea.io/gitea/models/auth" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" - "code.gitea.io/gitea/models/perm" pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" @@ -34,6 +33,7 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/translation" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/automerge" "code.gitea.io/gitea/services/automergequeue" pull_service "code.gitea.io/gitea/services/pull" @@ -1186,20 +1186,9 @@ func TestPullSquashMessage(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) user2Session := loginUser(t, user2.Name) - user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) - user4Session := loginUser(t, user4.Name) - sessions := map[string]*TestSession{ - user2.Name: user2Session, - user4.Name: user4Session, - } - - // Enable POPULATE_SQUASH_COMMENT_WITH_COMMIT_MESSAGES - resetFunc1 := test.MockVariableValue(&setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages, true) - defer resetFunc1() - // Set DEFAULT_MERGE_MESSAGE_SIZE - resetFunc2 := test.MockVariableValue(&setting.Repository.PullRequest.DefaultMergeMessageSize, 512) - defer resetFunc2() + defer test.MockVariableValue(&setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages, true)() + defer test.MockVariableValue(&setting.Repository.PullRequest.DefaultMergeMessageSize, 512)() repo, err := repo_service.CreateRepository(t.Context(), user2, user2, repo_service.CreateRepoOptions{ Name: "squash-message-test", @@ -1209,7 +1198,6 @@ func TestPullSquashMessage(t *testing.T) { DefaultBranch: "main", }) assert.NoError(t, err) - doAPIAddCollaborator(NewAPITestContext(t, repo.OwnerName, repo.Name, auth_model.AccessTokenScopeWriteRepository), user4.Name, perm.AccessModeWrite)(t) type commitInfo struct { userName string @@ -1280,8 +1268,8 @@ Implements a new email notification module. { userName: user2.Name, commitSummary: "Add advanced validation logic for user onboarding", - commitMessage: `This commit introduces a comprehensive validation layer for the user onboarding flow. -The primary goal is to ensure that all input data is strictly validated before being processed by downstream services. + commitMessage: `This commit introduces a comprehensive validation layer for the user onboarding flow. +The primary goal is to ensure that all input data is strictly validated before being processed by downstream services. This improves system reliability and significantly reduces runtime exceptions in the registration pipeline. The validation logic includes: @@ -1289,8 +1277,8 @@ The validation logic includes: 1. Email format checking using RFC 5322-compliant patterns. 2. Username length and character limitation enforcement. 3. Password strength enforcement, including: - - Minimum length checks - - Mixed character type detection + - Minimum length checks + - Mixed character type detection - Optional entropy-based scoring 4. Optional phone number validation using region-specific rules. `, @@ -1298,14 +1286,14 @@ The validation logic includes: }, expectedMessage: `* Add advanced validation logic for user onboarding -This commit introduces a comprehensive validation layer for the user onboarding flow. -The primary goal is to ensure that all input data is strictly validated before being processed by downstream services. +This commit introduces a comprehensive validation layer for the user onboarding flow. +The primary goal is to ensure that all input data is strictly validated before being processed by downstream services. This improves system reliability and significantly reduces runtime exceptions in the registration pipeline. The validation logic includes: 1. Email format checking using RFC 5322-compliant patterns. -2. Username length and character limitation enfor...`, +2. Username length and character limitation enforceme...`, }, { name: "Test Co-authored-by", @@ -1315,7 +1303,7 @@ The validation logic includes: commitSummary: "Implement the login endpoint", }, { - userName: user4.Name, + userName: "user4", commitSummary: "Validate request body", }, }, @@ -1331,16 +1319,17 @@ Co-authored-by: user4 } for tcNum, tc := range testCases { - branchName := "test-branch-" + strconv.Itoa(tcNum) - fileName := fmt.Sprintf("test-file-%d.txt", tcNum) t.Run(tc.name, func(t *testing.T) { + branchName := "test-branch-" + strconv.Itoa(tcNum) for infoIdx, info := range tc.commitInfos { - content := "content-" + strconv.Itoa(infoIdx) - if infoIdx == 0 { - testCreateFileWithCommitMessage(t, sessions[info.userName], repo.OwnerName, repo.Name, repo.DefaultBranch, branchName, fileName, content, info.commitSummary, info.commitMessage) - } else { - testEditFileWithCommitMessage(t, sessions[info.userName], repo.OwnerName, repo.Name, branchName, fileName, content, info.commitSummary, info.commitMessage) + createFileOpts := createFileInBranchOptions{ + CommitMessage: info.commitSummary + "\n\n" + info.commitMessage, + CommitterName: info.userName, + CommitterEmail: util.Iif(info.userName != "", info.userName+"@example.com", ""), + OldBranch: util.Iif(infoIdx == 0, "main", branchName), + NewBranch: branchName, } + testCreateFileInBranch(t, user2, repo, createFileOpts, map[string]string{"dummy-file-" + strconv.Itoa(infoIdx): "dummy content"}) } resp := testPullCreateDirectly(t, user2Session, createPullRequestOptions{ BaseRepoOwner: user2.Name, From 4b08bbdf0dd54da542cedcd9e549d1cf9d8b3f1d Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Thu, 20 Nov 2025 08:46:44 -0700 Subject: [PATCH 3/8] fix comment --- services/pull/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 825938e1ab3e0..116073fe3aa0b 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -909,7 +909,7 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ } } - if populateWithCommits && stringBuilder.Len() > 0 && len(authors) > 0 { + if stringBuilder.Len() > 0 && len(authors) > 0 { stringBuilder.WriteString("---------\n\n") } From 57ffa78e996c3de4456341061af7be990e912883 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Thu, 20 Nov 2025 09:26:29 -0700 Subject: [PATCH 4/8] fix tests --- tests/integration/pull_merge_test.go | 86 +++++++++------------------- 1 file changed, 28 insertions(+), 58 deletions(-) diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 5820bae3dd017..55df85410adac 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -1188,7 +1188,7 @@ func TestPullSquashMessage(t *testing.T) { user2Session := loginUser(t, user2.Name) defer test.MockVariableValue(&setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages, true)() - defer test.MockVariableValue(&setting.Repository.PullRequest.DefaultMergeMessageSize, 512)() + defer test.MockVariableValue(&setting.Repository.PullRequest.DefaultMergeMessageSize, 80)() repo, err := repo_service.CreateRepository(t.Context(), user2, user2, repo_service.CreateRepoOptions{ Name: "squash-message-test", @@ -1201,7 +1201,6 @@ func TestPullSquashMessage(t *testing.T) { type commitInfo struct { userName string - commitSummary string commitMessage string } @@ -1211,105 +1210,76 @@ func TestPullSquashMessage(t *testing.T) { expectedMessage string }{ { - name: "Only summaries", + name: "Single-line messages", commitInfos: []*commitInfo{ { userName: user2.Name, - commitSummary: "Implement the login endpoint", + commitMessage: "commit msg 1", }, { userName: user2.Name, - commitSummary: "Validate request body", + commitMessage: "commit msg 2", }, }, - expectedMessage: `* Implement the login endpoint + expectedMessage: `* commit msg 1 -* Validate request body +* commit msg 2 `, }, { - name: "Summaries and messages", + name: "Multiple-line messages", commitInfos: []*commitInfo{ { - userName: user2.Name, - commitSummary: "Refactor user service", - commitMessage: `Implement the login endpoint. -Validate request body.`, + userName: user2.Name, + commitMessage: `commit msg 1 + +Commit description.`, }, { - userName: user2.Name, - commitSummary: "Add email notification service", - commitMessage: `Implements a new email notification module. + userName: user2.Name, + commitMessage: `commit msg 2 -- Supports templating -- Supports HTML and plain text modes -- Includes retry logic`, +- Detail 1 +- Detail 2`, }, }, - expectedMessage: `* Refactor user service - -Implement the login endpoint. -Validate request body. + expectedMessage: `* commit msg 1 -* Add email notification service +Commit description. -Implements a new email notification module. +* commit msg 2 -- Supports templating -- Supports HTML and plain text modes -- Includes retry logic +- Detail 1 +- Detail 2 `, }, { - name: "Long Message", + name: "Too long message", commitInfos: []*commitInfo{ { userName: user2.Name, - commitSummary: "Add advanced validation logic for user onboarding", - commitMessage: `This commit introduces a comprehensive validation layer for the user onboarding flow. -The primary goal is to ensure that all input data is strictly validated before being processed by downstream services. -This improves system reliability and significantly reduces runtime exceptions in the registration pipeline. - -The validation logic includes: - -1. Email format checking using RFC 5322-compliant patterns. -2. Username length and character limitation enforcement. -3. Password strength enforcement, including: - - Minimum length checks - - Mixed character type detection - - Optional entropy-based scoring -4. Optional phone number validation using region-specific rules. -`, + commitMessage: `loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong message`, }, }, - expectedMessage: `* Add advanced validation logic for user onboarding - -This commit introduces a comprehensive validation layer for the user onboarding flow. -The primary goal is to ensure that all input data is strictly validated before being processed by downstream services. -This improves system reliability and significantly reduces runtime exceptions in the registration pipeline. - -The validation logic includes: - -1. Email format checking using RFC 5322-compliant patterns. -2. Username length and character limitation enforceme...`, + expectedMessage: `* looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo...`, }, { name: "Test Co-authored-by", commitInfos: []*commitInfo{ { userName: user2.Name, - commitSummary: "Implement the login endpoint", + commitMessage: "commit msg 1", }, { userName: "user4", - commitSummary: "Validate request body", + commitMessage: "commit msg 2", }, }, - expectedMessage: `* Implement the login endpoint + expectedMessage: `* commit msg 1 -* Validate request body +* commit msg 2 --------- @@ -1323,7 +1293,7 @@ Co-authored-by: user4 branchName := "test-branch-" + strconv.Itoa(tcNum) for infoIdx, info := range tc.commitInfos { createFileOpts := createFileInBranchOptions{ - CommitMessage: info.commitSummary + "\n\n" + info.commitMessage, + CommitMessage: info.commitMessage, CommitterName: info.userName, CommitterEmail: util.Iif(info.userName != "", info.userName+"@example.com", ""), OldBranch: util.Iif(infoIdx == 0, "main", branchName), From de6634fef84f5a9225bc96d8078b4caa5fc2a128 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 21 Nov 2025 00:50:29 +0800 Subject: [PATCH 5/8] fix corrupted utf8 --- services/pull/pull.go | 65 ++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 38 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 116073fe3aa0b..d3066c8a3703c 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -837,8 +837,8 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ authors := make([]string, 0, len(commits)) stringBuilder := strings.Builder{} - populateWithCommits := setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages - if !populateWithCommits { + if !setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages { + //use PR's title and description as squash commit message message := strings.TrimSpace(pr.Issue.Content) stringBuilder.WriteString(message) if stringBuilder.Len() > 0 { @@ -847,35 +847,33 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ stringBuilder.WriteRune('\n') } } - } - - // commits list is in reverse chronological order - for i := len(commits) - 1; i >= 0; i-- { - commit := commits[i] - - if populateWithCommits { - maxSize := setting.Repository.PullRequest.DefaultMergeMessageSize - if maxSize < 0 || stringBuilder.Len() < maxSize { - if strings.TrimSpace(commit.CommitMessage) == "" { - continue - } - - toWrite := fmt.Appendf(nil, "* %s\n", commit.CommitMessage) + } else { + // use PR's commit messages as squash commit message + // commits list is in reverse chronological order + maxSize := setting.Repository.PullRequest.DefaultMergeMessageSize + for i := len(commits) - 1; i >= 0; i-- { + commit := commits[i] + msg := strings.TrimSpace(commit.CommitMessage) + if msg == "" { + continue + } - if len(toWrite) > maxSize-stringBuilder.Len() && maxSize > -1 { - toWrite = append(toWrite[:maxSize-stringBuilder.Len()], "..."...) - } - if _, err := stringBuilder.Write(toWrite); err != nil { - log.Error("Unable to write commit message Error: %v", err) - return "" - } + _, _ = fmt.Fprintf(&stringBuilder, "* %s\n\n", msg) + if maxSize > 0 && stringBuilder.Len() >= maxSize { + tmp := strings.ToValidUTF8(stringBuilder.String()[:maxSize]+"...", "?") + stringBuilder.Reset() + stringBuilder.WriteString(tmp) + break } } + } + // collect co-authors + for _, commit := range commits { authorString := commit.Author.String() if uniqueAuthors.Add(authorString) && authorString != posterSig { // Compare use account as well to avoid adding the same author multiple times - // times when email addresses are private or multiple emails are used. + // when email addresses are private or multiple emails are used. commitUser, _ := user_model.GetUserByEmail(ctx, commit.Author.Email) if commitUser == nil || commitUser.ID != pr.Issue.Poster.ID { authors = append(authors, authorString) @@ -883,12 +881,12 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ } } - // Consider collecting the remaining authors + // collect the remaining authors if limit >= 0 && setting.Repository.PullRequest.DefaultMergeMessageAllAuthors { skip := limit limit = 30 for { - commits, err := gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, skip) + commits, err = gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, skip) if err != nil { log.Error("Unable to get commits between: %s %s Error: %v", pr.HeadBranch, pr.MergeBase, err) return "" @@ -914,18 +912,9 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ } for _, author := range authors { - if _, err := stringBuilder.WriteString("Co-authored-by: "); err != nil { - log.Error("Unable to write to string builder Error: %v", err) - return "" - } - if _, err := stringBuilder.WriteString(author); err != nil { - log.Error("Unable to write to string builder Error: %v", err) - return "" - } - if _, err := stringBuilder.WriteRune('\n'); err != nil { - log.Error("Unable to write to string builder Error: %v", err) - return "" - } + stringBuilder.WriteString("Co-authored-by: ") + stringBuilder.WriteString(author) + stringBuilder.WriteRune('\n') } return stringBuilder.String() From e5066a4a9f5a305e3883c88fd778b1b9ce8093ac Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 21 Nov 2025 01:00:33 +0800 Subject: [PATCH 6/8] fix comment --- services/pull/pull.go | 10 ++++++---- tests/integration/pull_merge_test.go | 7 ++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index d3066c8a3703c..35699f0eddd33 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -838,19 +838,20 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ stringBuilder := strings.Builder{} if !setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages { - //use PR's title and description as squash commit message + // use PR's title and description as squash commit message message := strings.TrimSpace(pr.Issue.Content) stringBuilder.WriteString(message) if stringBuilder.Len() > 0 { stringBuilder.WriteRune('\n') if !commitMessageTrailersPattern.MatchString(message) { + // TODO: this trailer check doesn't work with the separator line added below for the co-authors stringBuilder.WriteRune('\n') } } } else { // use PR's commit messages as squash commit message // commits list is in reverse chronological order - maxSize := setting.Repository.PullRequest.DefaultMergeMessageSize + maxMsgSize := setting.Repository.PullRequest.DefaultMergeMessageSize for i := len(commits) - 1; i >= 0; i-- { commit := commits[i] msg := strings.TrimSpace(commit.CommitMessage) @@ -859,8 +860,8 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ } _, _ = fmt.Fprintf(&stringBuilder, "* %s\n\n", msg) - if maxSize > 0 && stringBuilder.Len() >= maxSize { - tmp := strings.ToValidUTF8(stringBuilder.String()[:maxSize]+"...", "?") + if maxMsgSize > 0 && stringBuilder.Len() >= maxMsgSize { + tmp := strings.ToValidUTF8(stringBuilder.String()[:maxMsgSize]+"...", "?") stringBuilder.Reset() stringBuilder.WriteString(tmp) break @@ -908,6 +909,7 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ } if stringBuilder.Len() > 0 && len(authors) > 0 { + // TODO: this separator line doesn't work with the trailer check (commitMessageTrailersPattern) above stringBuilder.WriteString("---------\n\n") } diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 55df85410adac..bfb5254d7af9c 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -42,6 +42,7 @@ import ( files_service "code.gitea.io/gitea/services/repository/files" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type MergeOptions struct { @@ -1197,7 +1198,7 @@ func TestPullSquashMessage(t *testing.T) { Readme: "Default", DefaultBranch: "main", }) - assert.NoError(t, err) + require.NoError(t, err) type commitInfo struct { userName string @@ -1309,9 +1310,9 @@ Co-authored-by: user4 Title: "Pull for " + branchName, }) elems := strings.Split(test.RedirectURL(resp), "/") - pullIndex, err := strconv.Atoi(elems[4]) + pullIndex, err := strconv.ParseInt(elems[4], 10, 64) assert.NoError(t, err) - pullRequest := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, Index: int64(pullIndex)}) + pullRequest := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, Index: pullIndex}) squashMergeCommitMessage := pull_service.GetSquashMergeCommitMessages(t.Context(), pullRequest) assert.Equal(t, tc.expectedMessage, squashMergeCommitMessage) }) From 3bf639638c50952137d24c441805220b20fe5da3 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 21 Nov 2025 01:14:57 +0800 Subject: [PATCH 7/8] fix utf8 handling --- services/pull/pull.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 35699f0eddd33..21e89d16ec0d8 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -13,6 +13,7 @@ import ( "regexp" "strings" "time" + "unicode/utf8" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" @@ -861,7 +862,12 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ _, _ = fmt.Fprintf(&stringBuilder, "* %s\n\n", msg) if maxMsgSize > 0 && stringBuilder.Len() >= maxMsgSize { - tmp := strings.ToValidUTF8(stringBuilder.String()[:maxMsgSize]+"...", "?") + tmp := stringBuilder.String() + wasValidUtf8 := utf8.ValidString(tmp) + tmp = tmp[:maxMsgSize] + "..." + if wasValidUtf8 { + tmp = strings.ToValidUTF8(tmp, "?") + } stringBuilder.Reset() stringBuilder.WriteString(tmp) break From e362d1862e26e3bb07c481a4a4d00a305ff4c73c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 21 Nov 2025 01:24:21 +0800 Subject: [PATCH 8/8] comment --- services/pull/pull.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 21e89d16ec0d8..6f0318ea49c77 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -860,13 +860,18 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ continue } + // This format follows GitHub's squash commit message style, + // even if there are other "* " in the commit message body, they are written as-is. + // Maybe, ideally, we should indent those lines too. _, _ = fmt.Fprintf(&stringBuilder, "* %s\n\n", msg) if maxMsgSize > 0 && stringBuilder.Len() >= maxMsgSize { tmp := stringBuilder.String() wasValidUtf8 := utf8.ValidString(tmp) tmp = tmp[:maxMsgSize] + "..." if wasValidUtf8 { - tmp = strings.ToValidUTF8(tmp, "?") + // If the message was valid UTF-8 before truncation, ensure it remains valid after truncation + // For non-utf8 messages, we can't do much about it, end users should use utf-8 as much as possible + tmp = strings.ToValidUTF8(tmp, "") } stringBuilder.Reset() stringBuilder.WriteString(tmp)