From c491c22279835f529d6304c08c61136073f60fba Mon Sep 17 00:00:00 2001 From: Mura Li Date: Wed, 21 Aug 2019 10:14:09 +0800 Subject: [PATCH] Fix pull creation with empty changes (#7920) (#7926) * Logs the stderr of git-apply * Add an integration test * Skip testPatch when patch is empty --- integrations/pull_status_test.go | 26 ++++++++++++++++++++++++++ models/pull.go | 17 ++++++++++------- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/integrations/pull_status_test.go b/integrations/pull_status_test.go index 2a4d8e0b68..fde2d3cc9b 100644 --- a/integrations/pull_status_test.go +++ b/integrations/pull_status_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/url" "path" + "strings" "testing" "code.gitea.io/gitea/models" @@ -93,3 +94,28 @@ func TestPullCreate_CommitStatus(t *testing.T) { } }) } + +func TestPullCreate_EmptyChangesWithCommits(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + session := loginUser(t, "user1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testEditFileToNewBranch(t, session, "user1", "repo1", "master", "status1", "README.md", "status1") + testEditFileToNewBranch(t, session, "user1", "repo1", "status1", "status1", "README.md", "# repo1\n\nDescription for repo1") + + url := path.Join("user1", "repo1", "compare", "master...status1") + req := NewRequestWithValues(t, "POST", url, + map[string]string{ + "_csrf": GetCSRF(t, session, url), + "title": "pull request from status1", + }, + ) + session.MakeRequest(t, req, http.StatusFound) + + req = NewRequest(t, "GET", "/user1/repo1/pulls/1") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + text := strings.TrimSpace(doc.doc.Find(".item.text.green").Text()) + assert.EqualValues(t, "This pull request can be merged automatically.", text) + }) +} diff --git a/models/pull.go b/models/pull.go index 7dd6050c67..3f89088192 100644 --- a/models/pull.go +++ b/models/pull.go @@ -598,7 +598,7 @@ func (pr *PullRequest) testPatch(e Engine) (err error) { if err != nil { for i := range patchConflicts { if strings.Contains(stderr, patchConflicts[i]) { - log.Trace("PullRequest[%d].testPatch (apply): has conflict", pr.ID) + log.Trace("PullRequest[%d].testPatch (apply): has conflict: %s", pr.ID, stderr) const prefix = "error: patch failed:" pr.Status = PullRequestStatusConflict pr.ConflictedFiles = make([]string, 0, 5) @@ -661,13 +661,16 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str } pr.Index = pull.Index - if err = repo.savePatch(sess, pr.Index, patch); err != nil { - return fmt.Errorf("SavePatch: %v", err) - } - pr.BaseRepo = repo - if err = pr.testPatch(sess); err != nil { - return fmt.Errorf("testPatch: %v", err) + pr.Status = PullRequestStatusChecking + if len(patch) > 0 { + if err = repo.savePatch(sess, pr.Index, patch); err != nil { + return fmt.Errorf("SavePatch: %v", err) + } + + if err = pr.testPatch(sess); err != nil { + return fmt.Errorf("testPatch: %v", err) + } } // No conflict appears after test means mergeable. if pr.Status == PullRequestStatusChecking {