forgejo/services/pull/merge_test.go
Corentin 505ab87ae0
Some checks are pending
/ release (push) Waiting to run
testing-integration / test-unit (push) Waiting to run
testing-integration / test-sqlite (push) Waiting to run
testing-integration / test-mariadb (v10.6) (push) Waiting to run
testing-integration / test-mariadb (v11.8) (push) Waiting to run
testing / backend-checks (push) Waiting to run
testing / frontend-checks (push) Waiting to run
testing / test-unit (push) Blocked by required conditions
testing / test-e2e (push) Blocked by required conditions
testing / test-remote-cacher (redis) (push) Blocked by required conditions
testing / test-remote-cacher (valkey) (push) Blocked by required conditions
testing / test-remote-cacher (garnet) (push) Blocked by required conditions
testing / test-remote-cacher (redict) (push) Blocked by required conditions
testing / test-mysql (push) Blocked by required conditions
testing / test-pgsql (push) Blocked by required conditions
testing / test-sqlite (push) Blocked by required conditions
testing / security-check (push) Blocked by required conditions
fix: allow for merge message template with empty message (#9930)
Fixes #2691

This is changing the trimming of the merge message template to allow first empty line to be considered as empty message (also referred as title in frontend). 2 unit tests were added to check for proper behavior and possible edge case (simple template with only 2 lines, not trimming needed). Also a former unit test with 3 empty lines at the beginning of the template was changed.

The behavior with this patch is:
* splitting first `\n` to separate message from body
* trim remaining `\n` from body if `\n` was present or take template as is as message (no trim necessary as no `\n` is present in the template)

As this is an old issue the expected behavior might have changed from when the issue was triaged.

I additionally manually testes on the next.forgejo.org and locally built binary from this branch, I am attaching both screenshot of final result. The test PR can be seen at [https://v13.next.forgejo.org/kajika/merge_template_issue_2691/pulls/1](https://v13.next.forgejo.org/kajika/merge_template_issue_2691/pulls/1) and, in case the next repository is deletate the content of `.forgejo/default_merge_message/MERGE_TEMPLATE.md` is (first line is empty):
```

This is the description of the merge from "${HeadBranch}" to "${BaseBranch}"
```

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9930
Reviewed-by: oliverpool <oliverpool@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Corentin <corentin@ayo.tokyo>
Co-committed-by: Corentin <corentin@ayo.tokyo>
2025-12-10 19:57:19 +01:00

238 lines
8.1 KiB
Go

// Copyright 2022 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package pull
import (
"fmt"
"os"
"path"
"strings"
"testing"
"forgejo.org/models"
issues_model "forgejo.org/models/issues"
repo_model "forgejo.org/models/repo"
"forgejo.org/models/unittest"
user_model "forgejo.org/models/user"
"forgejo.org/modules/gitrepo"
"forgejo.org/modules/setting"
"forgejo.org/modules/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func Test_expandDefaultMergeMessage(t *testing.T) {
type args struct {
template string
vars map[string]string
}
title := "PullRequestTitle"
description := "Pull\nRequest\nDescription\n"
vars := map[string]string{
"PullRequestTitle": title,
"PullRequestDescription": description,
}
defaultTitle := "default_title"
defaultBody := "default_body"
expectedTitle := fmt.Sprintf("Merge %s", title)
expectedDescription := fmt.Sprintf("Description: %s", description)
expectedDescriptionMultiLine := fmt.Sprintf("Description:\n\n%s", description)
expectedDescriptionMerged := fmt.Sprintf("\n\nMerge %s\n\nDescription:\n\n%s", title, description)
emptyString := ""
tests := []struct {
name string
args args
want string
wantBody string
}{
{
name: "empty template",
args: args{template: "", vars: vars},
want: defaultTitle,
wantBody: defaultBody,
},
{
name: "single line",
args: args{template: "Merge ${PullRequestTitle}", vars: vars},
want: expectedTitle,
wantBody: defaultBody,
},
{
name: "empty message (space)",
args: args{template: " ", vars: vars},
want: emptyString,
wantBody: defaultBody,
},
{
name: "empty message (with newline)",
args: args{template: " \n", vars: vars},
want: emptyString,
wantBody: defaultBody,
},
{
name: "single newline",
args: args{template: "\n", vars: vars},
want: defaultTitle,
wantBody: defaultBody,
},
{
name: "empty description (newline)",
args: args{template: "\n\n", vars: vars},
want: defaultTitle,
wantBody: emptyString,
},
{
name: "empty description (space)",
args: args{template: "\n ", vars: vars},
want: defaultTitle,
wantBody: emptyString,
},
{
name: "empty title and description (spaces)",
args: args{template: " \n ", vars: vars},
want: emptyString,
wantBody: emptyString,
},
{
name: "empty title and description (space and newline)",
args: args{template: " \n\n", vars: vars},
want: emptyString,
wantBody: emptyString,
},
{
name: "simple message and description",
args: args{template: "Merge ${PullRequestTitle}\nDescription: ${PullRequestDescription}", vars: vars},
want: expectedTitle,
wantBody: expectedDescription,
},
{
name: "multiple lines",
args: args{template: "Merge ${PullRequestTitle}\nDescription:\n\n${PullRequestDescription}\n", vars: vars},
want: expectedTitle,
wantBody: expectedDescriptionMultiLine,
},
{
name: "description only",
args: args{template: "\nDescription: ${PullRequestDescription}\n", vars: vars},
want: defaultTitle,
wantBody: expectedDescription,
},
{
name: "leading newlines",
args: args{template: "\n\n\nMerge ${PullRequestTitle}\n\nDescription:\n\n${PullRequestDescription}\n", vars: vars},
want: defaultTitle,
wantBody: expectedDescriptionMerged,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
resultTitle, resultBody, err := expandDefaultMergeMessage(tt.args.template, tt.args.vars, defaultTitle, defaultBody)
require.NoError(t, err)
assert.Equalf(t, tt.want, resultTitle, "Wrong title for test '%s' -> expandDefaultMergeMessage(%q, %q)", tt.name, tt.args.template, tt.args.vars)
assert.Equalf(t, tt.wantBody, resultBody, "Wrong body for test '%s' -> expandDefaultMergeMessage(%q, %q)", tt.name, tt.args.template, tt.args.vars)
})
}
}
func TestAddCommitMessageTailer(t *testing.T) {
// add tailer for empty message
assert.Equal(t, "\n\nTest-tailer: TestValue", AddCommitMessageTrailer("", "Test-tailer", "TestValue"))
// add tailer for message without newlines
assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title", "Test-tailer", "TestValue"))
assert.Equal(t, "title\n\nNot tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nNot tailer: xxx", "Test-tailer", "TestValue"))
assert.Equal(t, "title\n\nNotTailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nNotTailer: xxx", "Test-tailer", "TestValue"))
assert.Equal(t, "title\n\nnot-tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nnot-tailer: xxx", "Test-tailer", "TestValue"))
// add tailer for message with one EOL
assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n", "Test-tailer", "TestValue"))
// add tailer for message with two EOLs
assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\n", "Test-tailer", "TestValue"))
// add tailer for message with existing tailer (won't duplicate)
assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nTest-tailer: TestValue", "Test-tailer", "TestValue"))
assert.Equal(t, "title\n\nTest-tailer: TestValue\n", AddCommitMessageTrailer("title\n\nTest-tailer: TestValue\n", "Test-tailer", "TestValue"))
// add tailer for message with existing tailer and different value (will append)
assert.Equal(t, "title\n\nTest-tailer: v1\nTest-tailer: v2", AddCommitMessageTrailer("title\n\nTest-tailer: v1", "Test-tailer", "v2"))
assert.Equal(t, "title\n\nTest-tailer: v1\nTest-tailer: v2", AddCommitMessageTrailer("title\n\nTest-tailer: v1\n", "Test-tailer", "v2"))
}
func prepareLoadMergeMessageTemplates(targetDir string) error {
for _, template := range []string{"MERGE", "REBASE", "REBASE-MERGE", "SQUASH", "MANUALLY-MERGED", "REBASE-UPDATE-ONLY"} {
file, err := os.Create(path.Join(targetDir, template+"_TEMPLATE.md"))
defer file.Close()
if err == nil {
_, err = file.WriteString("Contents for " + template)
}
if err != nil {
return err
}
}
return nil
}
func TestLoadMergeMessageTemplates(t *testing.T) {
defer test.MockVariableValue(&setting.CustomPath, t.TempDir())()
templateTemp := path.Join(setting.CustomPath, "default_merge_message")
require.NoError(t, os.MkdirAll(templateTemp, 0o755))
require.NoError(t, prepareLoadMergeMessageTemplates(templateTemp))
testStyles := []repo_model.MergeStyle{
repo_model.MergeStyleMerge,
repo_model.MergeStyleRebase,
repo_model.MergeStyleRebaseMerge,
repo_model.MergeStyleSquash,
repo_model.MergeStyleManuallyMerged,
repo_model.MergeStyleRebaseUpdate,
}
// Load all templates
require.NoError(t, LoadMergeMessageTemplates())
// Check their correctness
assert.Len(t, mergeMessageTemplates, len(testStyles))
for _, mergeStyle := range testStyles {
assert.Equal(t, "Contents for "+strings.ToUpper(string(mergeStyle)), mergeMessageTemplates[mergeStyle])
}
// Unload all templates
require.NoError(t, os.RemoveAll(templateTemp))
require.NoError(t, LoadMergeMessageTemplates())
assert.Empty(t, mergeMessageTemplates)
}
func TestMergeMergedPR(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1})
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
require.NoError(t, pr.LoadBaseRepo(t.Context()))
gitRepo, err := gitrepo.OpenRepository(t.Context(), pr.BaseRepo)
require.NoError(t, err)
defer gitRepo.Close()
assert.True(t, pr.HasMerged)
pr.HasMerged = false
err = Merge(t.Context(), pr, doer, gitRepo, repo_model.MergeStyleRebase, "", "I should not exist", false)
require.Error(t, err)
assert.True(t, models.IsErrPullRequestHasMerged(err))
if mergeErr, ok := err.(models.ErrPullRequestHasMerged); ok {
assert.Equal(t, pr.ID, mergeErr.ID)
assert.Equal(t, pr.IssueID, mergeErr.IssueID)
assert.Equal(t, pr.HeadBranch, mergeErr.HeadBranch)
assert.Equal(t, pr.BaseBranch, mergeErr.BaseBranch)
}
}