From c557540926826e82a118a085c3b510e072157cfe Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 17 Dec 2023 22:49:58 +0100 Subject: [PATCH] [GITEA] Remove redundant `syncBranchToDB` - The transaction in combination with Git push was causing deadlocks if you had the `push_update` queue set to `immediate`. This was the root cause of slow integration tests in CI. - Remove the sync branch code as this is already being done in the Git post-receive hook. - Add tests to proof the branch models are in sync even with this code removed. (cherry picked from commit 90110e1f44a40837a6ef5b3979a6ed96bfd614be) (cherry picked from commit a064065cb9a6e39597e38c37a405d066cfabf7f7) (cherry picked from commit 7713e558eb6419a3a7d3f2d1beaa8062899490c8) Conflicts: services/repository/branch.go https://codeberg.org/forgejo/forgejo/pulls/2068 (cherry picked from commit 3bb73e0813b46fd8b518a46d7499ee1c525bc434) --- tests/integration/branches_test.go | 99 +++++++++++---------------- tests/integration/repo_branch_test.go | 16 ++++- 2 files changed, 56 insertions(+), 59 deletions(-) diff --git a/tests/integration/branches_test.go b/tests/integration/branches_test.go index 99d7eef706..26bed8c90a 100644 --- a/tests/integration/branches_test.go +++ b/tests/integration/branches_test.go @@ -4,72 +4,55 @@ package integration import ( + "fmt" "net/http" "net/url" "testing" - "code.gitea.io/gitea/modules/translation" - "code.gitea.io/gitea/tests" + git_model "code.gitea.io/gitea/models/git" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + gitea_context "code.gitea.io/gitea/modules/context" "github.com/stretchr/testify/assert" ) -func TestViewBranches(t *testing.T) { - defer tests.PrepareTestEnv(t)() - - req := NewRequest(t, "GET", "/user2/repo1/branches") - resp := MakeRequest(t, req, http.StatusOK) - - htmlDoc := NewHTMLParser(t, resp.Body) - _, exists := htmlDoc.doc.Find(".delete-branch-button").Attr("data-url") - assert.False(t, exists, "The template has changed") -} - -func TestDeleteBranch(t *testing.T) { - defer tests.PrepareTestEnv(t)() - - deleteBranch(t) -} - -func TestUndoDeleteBranch(t *testing.T) { +func TestBranchActions(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { - deleteBranch(t) - htmlDoc, name := branchAction(t, ".restore-branch-button") - assert.Contains(t, - htmlDoc.doc.Find(".ui.positive.message").Text(), - translation.NewLocale("en-US").Tr("repo.branch.restore_success", name), - ) + session := loginUser(t, "user2") + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + branch3 := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{ID: 3, RepoID: repo1.ID}) + branchesLink := repo1.FullName() + "/branches" + + t.Run("View", func(t *testing.T) { + req := NewRequest(t, "GET", branchesLink) + MakeRequest(t, req, http.StatusOK) + }) + + t.Run("Delete branch", func(t *testing.T) { + link := fmt.Sprintf("/%s/branches/delete?name=%s", repo1.FullName(), branch3.Name) + req := NewRequestWithValues(t, "POST", link, map[string]string{ + "_csrf": GetCSRF(t, session, branchesLink), + }) + session.MakeRequest(t, req, http.StatusOK) + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.Contains(t, flashCookie.Value, "success%3DBranch%2B%2522branch2%2522%2Bhas%2Bbeen%2Bdeleted.") + + assert.True(t, unittest.AssertExistsAndLoadBean(t, &git_model.Branch{ID: 3, RepoID: repo1.ID}).IsDeleted) + }) + + t.Run("Restore branch", func(t *testing.T) { + link := fmt.Sprintf("/%s/branches/restore?branch_id=%d&name=%s", repo1.FullName(), branch3.ID, branch3.Name) + req := NewRequestWithValues(t, "POST", link, map[string]string{ + "_csrf": GetCSRF(t, session, branchesLink), + }) + session.MakeRequest(t, req, http.StatusOK) + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.Contains(t, flashCookie.Value, "success%3DBranch%2B%2522branch2%2522%2Bhas%2Bbeen%2Brestored") + + assert.False(t, unittest.AssertExistsAndLoadBean(t, &git_model.Branch{ID: 3, RepoID: repo1.ID}).IsDeleted) + }) }) } - -func deleteBranch(t *testing.T) { - htmlDoc, name := branchAction(t, ".delete-branch-button") - assert.Contains(t, - htmlDoc.doc.Find(".ui.positive.message").Text(), - translation.NewLocale("en-US").Tr("repo.branch.deletion_success", name), - ) -} - -func branchAction(t *testing.T, button string) (*HTMLDoc, string) { - session := loginUser(t, "user2") - req := NewRequest(t, "GET", "/user2/repo1/branches") - resp := session.MakeRequest(t, req, http.StatusOK) - - htmlDoc := NewHTMLParser(t, resp.Body) - link, exists := htmlDoc.doc.Find(button).Attr("data-url") - if !assert.True(t, exists, "The template has changed") { - t.Skip() - } - - req = NewRequestWithValues(t, "POST", link, map[string]string{ - "_csrf": htmlDoc.GetCSRF(), - }) - session.MakeRequest(t, req, http.StatusOK) - - url, err := url.Parse(link) - assert.NoError(t, err) - req = NewRequest(t, "GET", "/user2/repo1/branches") - resp = session.MakeRequest(t, req, http.StatusOK) - - return NewHTMLParser(t, resp.Body), url.Query().Get("name") -} diff --git a/tests/integration/repo_branch_test.go b/tests/integration/repo_branch_test.go index 91674ddc82..9eb17eda00 100644 --- a/tests/integration/repo_branch_test.go +++ b/tests/integration/repo_branch_test.go @@ -10,6 +10,8 @@ import ( "strings" "testing" + git_model "code.gitea.io/gitea/models/git" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/translation" @@ -47,12 +49,14 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) { CreateRelease string FlashMessage string ExpectedStatus int + CheckBranch bool }{ { OldRefSubURL: "branch/master", NewBranch: "feature/test1", ExpectedStatus: http.StatusSeeOther, FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature/test1"), + CheckBranch: true, }, { OldRefSubURL: "branch/master", @@ -65,6 +69,7 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) { NewBranch: "feature=test1", ExpectedStatus: http.StatusSeeOther, FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature=test1"), + CheckBranch: true, }, { OldRefSubURL: "branch/master", @@ -94,6 +99,7 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) { NewBranch: "feature/test3", ExpectedStatus: http.StatusSeeOther, FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature/test3"), + CheckBranch: true, }, { OldRefSubURL: "branch/master", @@ -108,10 +114,15 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) { CreateRelease: "v1.0.1", ExpectedStatus: http.StatusSeeOther, FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature/test4"), + CheckBranch: true, }, } + + session := loginUser(t, "user2") for _, test := range tests { - session := loginUser(t, "user2") + if test.CheckBranch { + unittest.AssertNotExistsBean(t, &git_model.Branch{RepoID: 1, Name: test.NewBranch}) + } if test.CreateRelease != "" { createNewRelease(t, session, "/user2/repo1", test.CreateRelease, test.CreateRelease, false, false) } @@ -125,6 +136,9 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) { test.FlashMessage, ) } + if test.CheckBranch { + unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: 1, Name: test.NewBranch}) + } } }