From 831fb5641477424eb9423197f1dd2dc0537b6e3d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 23 Dec 2024 10:43:49 -0800 Subject: [PATCH] Add more test --- routers/api/v1/repo/pull.go | 6 ++ routers/common/compare.go | 52 +++++---- routers/common/compare_test.go | 190 +++++++++++++++++++++++++++++++++ 3 files changed, 225 insertions(+), 23 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 9971e4512f..d7394c18b7 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -451,6 +451,12 @@ func CreatePullRequest(ctx *context.APIContext) { } } + ci.CompareInfo, err = ci.HeadGitRepo.GetCompareInfo(repo_model.RepoPath(baseRepo.Owner.Name, baseRepo.Name), ci.BaseOriRef, ci.HeadOriRef, false, false) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetCompareInfo", err) + return + } + // Check if another PR exists with the same targets existingPr, err := issues_model.GetUnmergedPullRequest(ctx, ci.HeadRepo.ID, baseRepo.ID, ci.HeadOriRef, ci.BaseOriRef, issues_model.PullRequestFlowGithub) if err != nil { diff --git a/routers/common/compare.go b/routers/common/compare.go index 1bbd9f35fd..71ace773b3 100644 --- a/routers/common/compare.go +++ b/routers/common/compare.go @@ -26,14 +26,6 @@ type CompareRouter struct { DotTimes int // 2(..) or 3(...) } -func (cr *CompareRouter) IsSameRepo() bool { - return cr.HeadOwnerName == "" && cr.HeadRepoName == "" -} - -func (cr *CompareRouter) IsSameRef() bool { - return cr.IsSameRepo() && cr.BaseOriRef == cr.HeadOriRef -} - func (cr *CompareRouter) DirectComparison() bool { return cr.DotTimes == 2 } @@ -98,6 +90,7 @@ func parseCompareRouter(router string) (*CompareRouter, error) { // CompareInfo represents the collected results from ParseCompareInfo type CompareInfo struct { *CompareRouter + BaseRepo *repo_model.Repository HeadUser *user_model.User HeadRepo *repo_model.Repository HeadGitRepo *git.Repository @@ -107,10 +100,18 @@ type CompareInfo struct { IsHeadCommit bool } +func (cr *CompareInfo) IsSameRepo() bool { + return cr.HeadRepo.ID == cr.BaseRepo.ID +} + +func (cr *CompareInfo) IsSameRef() bool { + return cr.IsSameRepo() && cr.BaseOriRef == cr.HeadOriRef +} + // display pull related information or not func (ci *CompareInfo) IsPull() bool { return ci.CaretTimes == 0 && !ci.DirectComparison() && - ci.BaseFullRef.IsBranch() && ci.HeadFullRef.IsBranch() + ci.BaseFullRef.IsBranch() && (ci.HeadRepo == nil || ci.HeadFullRef.IsBranch()) } func (ci *CompareInfo) Close() { @@ -232,7 +233,7 @@ func getRootRepo(ctx context.Context, repo *repo_model.Repository) (*repo_model. // base<-head: master...head:feature // same repo: master...feature func ParseComparePathParams(ctx context.Context, pathParam string, baseRepo *repo_model.Repository, baseGitRepo *git.Repository) (*CompareInfo, error) { - ci := &CompareInfo{} + ci := &CompareInfo{BaseRepo: baseRepo} var err error if pathParam == "" { @@ -247,7 +248,8 @@ func ParseComparePathParams(ctx context.Context, pathParam string, baseRepo *rep ci.BaseOriRef = baseRepo.DefaultBranch } - if ci.IsSameRepo() { + if (ci.HeadOwnerName == "" && ci.HeadRepoName == "") || + (ci.HeadOwnerName == baseRepo.Owner.Name && ci.HeadRepoName == baseRepo.Name) { ci.HeadOwnerName = baseRepo.Owner.Name ci.HeadRepoName = baseRepo.Name ci.HeadUser = baseRepo.Owner @@ -277,14 +279,16 @@ func ParseComparePathParams(ctx context.Context, pathParam string, baseRepo *rep return nil, err } } - ci.HeadRepo.Owner = ci.HeadUser - ci.HeadGitRepo, err = gitrepo.OpenRepository(ctx, ci.HeadRepo) - if err != nil { - return nil, err - } - ci.close = func() { - if ci.HeadGitRepo != nil { - ci.HeadGitRepo.Close() + if ci.HeadRepo != nil { + ci.HeadRepo.Owner = ci.HeadUser + ci.HeadGitRepo, err = gitrepo.OpenRepository(ctx, ci.HeadRepo) + if err != nil { + return nil, err + } + ci.close = func() { + if ci.HeadGitRepo != nil { + ci.HeadGitRepo.Close() + } } } } @@ -295,10 +299,12 @@ func ParseComparePathParams(ctx context.Context, pathParam string, baseRepo *rep return nil, err } - ci.HeadFullRef, ci.IsHeadCommit, err = detectFullRef(ctx, ci.HeadRepo.ID, ci.HeadGitRepo, ci.HeadOriRef) - if err != nil { - ci.Close() - return nil, err + if ci.HeadRepo != nil { + ci.HeadFullRef, ci.IsHeadCommit, err = detectFullRef(ctx, ci.HeadRepo.ID, ci.HeadGitRepo, ci.HeadOriRef) + if err != nil { + ci.Close() + return nil, err + } } return ci, nil } diff --git a/routers/common/compare_test.go b/routers/common/compare_test.go index aa30389401..aa1b7a5949 100644 --- a/routers/common/compare_test.go +++ b/routers/common/compare_test.go @@ -4,8 +4,13 @@ package common import ( + "context" "testing" + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/gitrepo" "github.com/stretchr/testify/assert" ) @@ -136,3 +141,188 @@ func TestCompareRouters(t *testing.T) { }) } } + +func Test_ParseComparePathParams(t *testing.T) { + baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + assert.NotNil(t, baseRepo) + assert.NoError(t, baseRepo.LoadOwner(db.DefaultContext)) + baseGitRepo, err := gitrepo.OpenRepository(context.Background(), baseRepo) + assert.NoError(t, err) + defer baseGitRepo.Close() + + kases := []struct { + router string + compareInfo *CompareInfo + }{ + { + router: "main...develop", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: "main", + HeadOriRef: "develop", + DotTimes: 3, + }, + BaseRepo: baseRepo, + HeadUser: baseRepo.Owner, + HeadRepo: baseRepo, + HeadGitRepo: baseGitRepo, + }, + }, + { + router: "main..develop", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: "main", + HeadOriRef: "develop", + DotTimes: 2, + }, + BaseRepo: baseRepo, + HeadUser: baseRepo.Owner, + HeadRepo: baseRepo, + HeadGitRepo: baseGitRepo, + }, + }, + { + router: "main^...develop", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: "main", + HeadOriRef: "develop", + CaretTimes: 1, + DotTimes: 3, + }, + BaseRepo: baseRepo, + HeadUser: baseRepo.Owner, + HeadRepo: baseRepo, + HeadGitRepo: baseGitRepo, + }, + }, + { + router: "main^^^^^...develop", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: "main", + HeadOriRef: "develop", + CaretTimes: 5, + DotTimes: 3, + }, + BaseRepo: baseRepo, + HeadUser: baseRepo.Owner, + HeadRepo: baseRepo, + HeadGitRepo: baseGitRepo, + }, + }, + { + router: "develop", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: baseRepo.DefaultBranch, + HeadOriRef: "develop", + DotTimes: 3, + }, + BaseRepo: baseRepo, + HeadUser: baseRepo.Owner, + HeadRepo: baseRepo, + HeadGitRepo: baseGitRepo, + }, + }, + { + router: "lunny/forked_repo:develop", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: baseRepo.DefaultBranch, + HeadOwnerName: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + }, + BaseRepo: baseRepo, + }, + }, + { + router: "main...lunny/forked_repo:develop", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: "main", + HeadOwnerName: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + }, + { + router: "main...lunny/forked_repo:develop", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: "main", + HeadOwnerName: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + }, + { + router: "main^...lunny/forked_repo:develop", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: "main", + HeadOwnerName: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + CaretTimes: 1, + }, + }, + }, + { + router: "v1.0...v1.1", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: "v1.0", + HeadOriRef: "v1.1", + DotTimes: 3, + }, + }, + }, + { + router: "teabot-patch-1...v0.0.1", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: "teabot-patch-1", + HeadOriRef: "v0.0.1", + DotTimes: 3, + }, + }, + }, + { + router: "teabot:feature1", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + HeadOwnerName: "teabot", + HeadOriRef: "feature1", + DotTimes: 3, + }, + }, + }, + { + router: "8eb19a5ae19abae15c0666d4ab98906139a7f439...283c030497b455ecfa759d4649f9f8b45158742e", + compareInfo: &CompareInfo{ + CompareRouter: &CompareRouter{ + BaseOriRef: "8eb19a5ae19abae15c0666d4ab98906139a7f439", + HeadOriRef: "283c030497b455ecfa759d4649f9f8b45158742e", + DotTimes: 3, + }, + }, + }, + } + + for _, kase := range kases { + t.Run(kase.router, func(t *testing.T) { + r, err := ParseComparePathParams(context.Background(), kase.router, baseRepo, baseGitRepo) + assert.NoError(t, err) + assert.EqualValues(t, kase.compareInfo, r) + }) + } +}