From a01a37c4ad9760b12641a5fb608f8f2814b707c3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 18 Sep 2017 22:01:23 +0800 Subject: [PATCH] add tests for pulls commit status and some fixes --- integrations/editor_test.go | 18 ++++ integrations/repo_pull_status_test.go | 130 ++++++++++++++++++++++++++ models/fixtures/repo_unit.yml | 40 ++++++++ models/status.go | 9 +- routers/repo/issue.go | 86 ++++++++++++----- routers/user/home.go | 70 ++++++++++---- templates/repo/commit_status.tmpl | 30 +++--- 7 files changed, 320 insertions(+), 63 deletions(-) create mode 100644 integrations/repo_pull_status_test.go diff --git a/integrations/editor_test.go b/integrations/editor_test.go index e2dd2e1dc..9c98402c1 100644 --- a/integrations/editor_test.go +++ b/integrations/editor_test.go @@ -150,6 +150,24 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra return resp } +func testEditFileToNewBranchAndSendPull(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath string) *TestResponse { + testEditFileToNewBranch(t, session, user, repo, branch, targetBranch, filePath) + + url := path.Join(user, repo, "compare", branch+"..."+targetBranch) + + req := NewRequest(t, "GET", url) + resp := session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + req = NewRequestWithValues(t, "POST", url, + map[string]string{ + "_csrf": htmlDoc.GetCSRF(), + "title": "pull request from " + targetBranch, + }, + ) + return session.MakeRequest(t, req, http.StatusFound) +} + func TestEditFile(t *testing.T) { prepareTestEnv(t) session := loginUser(t, "user2") diff --git a/integrations/repo_pull_status_test.go b/integrations/repo_pull_status_test.go new file mode 100644 index 000000000..b1cc8f2d8 --- /dev/null +++ b/integrations/repo_pull_status_test.go @@ -0,0 +1,130 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "fmt" + "net/http" + "path" + "strings" + "testing" + + "code.gitea.io/gitea/models" + api "code.gitea.io/sdk/gitea" + + "github.com/PuerkitoBio/goquery" + "github.com/stretchr/testify/assert" +) + +var ( + statesIcons = map[models.CommitStatusState]string{ + models.CommitStatusPending: "circle icon yellow", + models.CommitStatusSuccess: "check icon green", + models.CommitStatusError: "warning icon red", + models.CommitStatusFailure: "remove icon red", + models.CommitStatusWarning: "warning sign icon yellow", + } +) + +func TestRepoPullsWithStatus(t *testing.T) { + prepareTestEnv(t) + + session := loginUser(t, "user2") + + var size = 5 + // create some pulls + for i := 0; i < size; i++ { + testEditFileToNewBranchAndSendPull(t, session, "user2", "repo16", "master", fmt.Sprintf("test%d", i), "readme.md") + } + + // look for repo's pulls page + req := NewRequest(t, "GET", "/user2/repo16/pulls") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + var indexes = make([]string, 0, size) + doc.doc.Find("li.item").Each(func(idx int, s *goquery.Selection) { + indexes = append(indexes, strings.TrimLeft(s.Find("div").Eq(1).Text(), "#")) + }) + + indexes = indexes[:5] + + var status = make([]models.CommitStatusState, len(indexes)) + for i := 0; i < len(indexes); i++ { + switch i { + case 0: + status[i] = models.CommitStatusPending + case 1: + status[i] = models.CommitStatusSuccess + case 2: + status[i] = models.CommitStatusError + case 3: + status[i] = models.CommitStatusFailure + case 4: + status[i] = models.CommitStatusWarning + default: + status[i] = models.CommitStatusSuccess + } + } + + for i, index := range indexes { + // Request repository commits page + req = NewRequestf(t, "GET", "/user2/repo16/pulls/%s/commits", index) + resp = session.MakeRequest(t, req, http.StatusOK) + doc = NewHTMLParser(t, resp.Body) + + // Get first commit URL + commitURL, exists := doc.doc.Find("#commits-table tbody tr td.sha a").Last().Attr("href") + assert.True(t, exists) + assert.NotEmpty(t, commitURL) + + commitID := path.Base(commitURL) + // Call API to add status for commit + req = NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo16/statuses/"+commitID, + api.CreateStatusOption{ + State: api.StatusState(status[i]), + TargetURL: "http://test.ci/", + Description: "", + Context: "testci", + }, + ) + session.MakeRequest(t, req, http.StatusCreated) + + req = NewRequestf(t, "GET", "/user2/repo16/pulls/%s/commits", index) + resp = session.MakeRequest(t, req, http.StatusOK) + doc = NewHTMLParser(t, resp.Body) + + commitURL, exists = doc.doc.Find("#commits-table tbody tr td.sha a").Last().Attr("href") + assert.True(t, exists) + assert.NotEmpty(t, commitURL) + assert.EqualValues(t, commitID, path.Base(commitURL)) + + cls, ok := doc.doc.Find("#commits-table tbody tr td.message i.commit-status").Last().Attr("class") + assert.True(t, ok) + assert.EqualValues(t, "commit-status "+statesIcons[status[i]], cls) + } + + req = NewRequest(t, "GET", "/user2/repo16/pulls") + resp = session.MakeRequest(t, req, http.StatusOK) + doc = NewHTMLParser(t, resp.Body) + + doc.doc.Find("li.item").Each(func(i int, s *goquery.Selection) { + cls, ok := s.Find("i.commit-status").Attr("class") + assert.True(t, ok) + assert.EqualValues(t, "commit-status "+statesIcons[status[i]], cls) + }) + + req = NewRequest(t, "GET", "/pulls?type=all&repo=16&sort=&state=open") + resp = session.MakeRequest(t, req, http.StatusOK) + doc = NewHTMLParser(t, resp.Body) + + fmt.Println(string(resp.Body)) + + doc.doc.Find("li.item").Each(func(i int, s *goquery.Selection) { + cls, ok := s.Find("i.commit-status").Attr("class") + assert.True(t, ok) + assert.EqualValues(t, "commit-status "+statesIcons[status[i]], cls) + }) +} diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index 6780de753..a1e251c53 100644 --- a/models/fixtures/repo_unit.yml +++ b/models/fixtures/repo_unit.yml @@ -74,3 +74,43 @@ type: 1 config: "{}" created_unix: 1524304355 + +- + id: 12 + repo_id: 16 + type: 1 + index: 0 + config: "{}" + created_unix: 946684810 + +- + id: 13 + repo_id: 16 + type: 2 + index: 1 + config: "{\"EnableTimetracker\":false,\"AllowOnlyContributorsToTrackTime\":false}" + created_unix: 946684810 + +- + id: 14 + repo_id: 16 + type: 3 + index: 2 + config: "{}" + created_unix: 946684810 + +- + id: 15 + repo_id: 16 + type: 4 + index: 3 + config: "{}" + created_unix: 946684810 + +- + id: 16 + repo_id: 16 + type: 5 + index: 4 + config: "{}" + created_unix: 946684810 diff --git a/models/status.go b/models/status.go index 2a3277219..57ff9783d 100644 --- a/models/status.go +++ b/models/status.go @@ -168,6 +168,7 @@ func GetLatestCommitStatuses(repoIDs []int64, shas []string) ([][]*CommitStatus, var results = make([]struct { ID int64 RepoID int64 + SHA string }, 0, 10*len(repoIDs)) var cond = builder.NewCond() @@ -180,7 +181,7 @@ func GetLatestCommitStatuses(repoIDs []int64, shas []string) ([][]*CommitStatus, err := x.Table(&CommitStatus{}). Where(cond). - Select("max( id ) as id, repo_id"). + Select("max( id ) as id, repo_id, sha"). GroupBy("repo_id, sha, context").OrderBy("max( id ) desc").Find(&results) if err != nil { return nil, err @@ -192,10 +193,10 @@ func GetLatestCommitStatuses(repoIDs []int64, shas []string) ([][]*CommitStatus, } var ids = make([]int64, 0, len(results)) - var repoIDsMap = make(map[int64][]int64, len(repoIDs)) + var repoIDsMap = make(map[string][]int64, len(repoIDs)) for _, res := range results { ids = append(ids, res.ID) - repoIDsMap[res.RepoID] = append(repoIDsMap[res.RepoID], res.ID) + repoIDsMap[fmt.Sprintf("%d-%s", res.RepoID, res.SHA)] = append(repoIDsMap[fmt.Sprintf("%d-%s", res.RepoID, res.SHA)], res.ID) } statuses := make(map[int64]*CommitStatus, len(ids)) @@ -205,7 +206,7 @@ func GetLatestCommitStatuses(repoIDs []int64, shas []string) ([][]*CommitStatus, } for i := 0; i < len(repoIDs); i++ { - for _, id := range repoIDsMap[repoIDs[i]] { + for _, id := range repoIDsMap[fmt.Sprintf("%d-%s", repoIDs[i], shas[i])] { returns[i] = append(returns[i], statuses[id]) } } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 52f5ca0ec..5b2a02698 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -209,44 +209,80 @@ func Issues(ctx *context.Context) { } } - var repoIDs = make([]int64, 0, len(issues)) - var shas = make([]string, 0, len(issues)) - var pullIDs = make([]int64, 0, len(issues)) - // Get posters. - for i, issue := range issues { - // Check read status - if !ctx.IsSigned { - issues[i].IsRead = true - } else if err = issues[i].GetIsRead(ctx.User.ID); err != nil { - ctx.ServerError("GetIsRead", err) - return + if !isPullList { + // Get posters. + for i := 0; i < len(issues); i++ { + // Check read status + if !ctx.IsSigned { + issues[i].IsRead = true + } else if err = issues[i].GetIsRead(ctx.User.ID); err != nil { + ctx.ServerError("GetIsRead", err) + return + } } + } else { + var repoIDs = make([]int64, 0, len(issues)) + var shas = make([]string, 0, len(issues)) + var pullIDs = make([]int64, 0, len(issues)) + var repoCache = make(map[int64]*git.Repository) + + // Get posters. + for i, issue := range issues { + // Check read status + if !ctx.IsSigned { + issues[i].IsRead = true + } else if err = issues[i].GetIsRead(ctx.User.ID); err != nil { + ctx.ServerError("GetIsRead", err) + return + } - if issue.IsPull { if err := issue.LoadAttributes(); err != nil { ctx.ServerError("LoadAttributes", err) return } - repoIDs = append(repoIDs, ctx.Repo.Repository.ID) - shas = append(shas, issue.PullRequest.MergeBase) - pullIDs = append(pullIDs, issue.ID) + var rep *git.Repository + var ok bool + if rep, ok = repoCache[issue.PullRequest.HeadRepoID]; !ok { + if err := issue.PullRequest.GetHeadRepo(); err != nil { + ctx.ServerError("GetHeadRepo", err) + return + } + + rep, err = git.OpenRepository(issue.PullRequest.HeadRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return + } + repoCache[issue.PullRequest.HeadRepoID] = rep + } + + sha, err := rep.GetBranchCommitID(issue.PullRequest.HeadBranch) + if err != nil { + log.Error(4, "GetBranchCommitID: %v", err) + } else { + repoIDs = append(repoIDs, issue.RepoID) + shas = append(shas, sha) + pullIDs = append(pullIDs, issue.ID) + } } - } - commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) - if err != nil { - ctx.ServerError("GetLatestCommitStatuses", err) - return - } + var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) + if len(repoIDs) > 0 { + commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) + if err != nil { + ctx.ServerError("GetLatestCommitStatuses", err) + return + } - var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) - for i, statuses := range commitStatuses { - issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) + for i, statuses := range commitStatuses { + issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) + } + } + ctx.Data["IssuesStates"] = issuesStates } ctx.Data["Issues"] = issues - ctx.Data["IssuesStates"] = issuesStates // Get milestones. ctx.Data["Milestones"], err = models.GetMilestonesByRepoID(repo.ID) diff --git a/routers/user/home.go b/routers/user/home.go index 6694509d0..48b827caa 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -9,9 +9,11 @@ import ( "fmt" "sort" + "code.gitea.io/git" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -291,37 +293,65 @@ func Issues(ctx *context.Context) { return } - var repoIDs = make([]int64, 0, len(issues)) - var shas = make([]string, 0, len(issues)) - var pullIDs = make([]int64, 0, len(issues)) - for _, issue := range issues { - issue.Repo = showReposMap[issue.RepoID] + if !isPullList { + for _, issue := range issues { + issue.Repo = showReposMap[issue.RepoID] + } + } else { + var repoIDs = make([]int64, 0, len(issues)) + var shas = make([]string, 0, len(issues)) + var pullIDs = make([]int64, 0, len(issues)) + var repoCache = make(map[int64]*git.Repository) + + for _, issue := range issues { + issue.Repo = showReposMap[issue.RepoID] - if issue.IsPull { if err := issue.LoadAttributes(); err != nil { ctx.ServerError("LoadAttributes", fmt.Errorf("%v", err)) return } - repoIDs = append(repoIDs, issue.Repo.ID) - shas = append(shas, issue.PullRequest.MergeBase) - pullIDs = append(pullIDs, issue.ID) + var rep *git.Repository + var ok bool + if rep, ok = repoCache[issue.PullRequest.HeadRepoID]; !ok { + if err := issue.PullRequest.GetHeadRepo(); err != nil { + ctx.ServerError("GetHeadRepo", err) + return + } + + rep, err = git.OpenRepository(issue.PullRequest.HeadRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return + } + repoCache[issue.PullRequest.HeadRepoID] = rep + } + + sha, err := rep.GetBranchCommitID(issue.PullRequest.HeadBranch) + if err != nil { + log.Error(4, "GetBranchCommitID: %v", err) + } else { + repoIDs = append(repoIDs, issue.RepoID) + shas = append(shas, sha) + pullIDs = append(pullIDs, issue.ID) + } } - } - commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) - if err != nil { - ctx.ServerError("GetLatestCommitStatuses", err) - return - } + var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) + if len(repoIDs) > 0 { + commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) + if err != nil { + ctx.ServerError("GetLatestCommitStatuses", err) + return + } - var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) - for i, statuses := range commitStatuses { - issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) + for i, statuses := range commitStatuses { + issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) + } + } + ctx.Data["IssuesStates"] = issuesStates } - ctx.Data["IssuesStates"] = issuesStates - issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{ UserID: ctxUser.ID, RepoID: repoID, diff --git a/templates/repo/commit_status.tmpl b/templates/repo/commit_status.tmpl index d5f75c132..6b80baaa6 100644 --- a/templates/repo/commit_status.tmpl +++ b/templates/repo/commit_status.tmpl @@ -1,15 +1,17 @@ -{{if eq .State "pending"}} - -{{end}} -{{if eq .State "success"}} - -{{end}} -{{if eq .State "error"}} - -{{end}} -{{if eq .State "failure"}} - -{{end}} -{{if eq .State "warning"}} - +{{if .}} + {{if eq .State "pending"}} + + {{end}} + {{if eq .State "success"}} + + {{end}} + {{if eq .State "error"}} + + {{end}} + {{if eq .State "failure"}} + + {{end}} + {{if eq .State "warning"}} + + {{end}} {{end}} \ No newline at end of file