Skip to content

Commit 9b575f8

Browse files
committed
Amend code owner test to include branch protection rule
1 parent 625cd36 commit 9b575f8

File tree

2 files changed

+64
-6
lines changed

2 files changed

+64
-6
lines changed

services/issue/pull.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ func HasAllRequiredCodeownerReviews(ctx context.Context, pb *git_model.Protected
6161
return false
6262
}
6363

64+
if err := pr.LoadIssue(ctx); err != nil {
65+
return false
66+
}
67+
6468
pr.Issue.Repo = pr.BaseRepo
6569

6670
if pr.BaseRepo.IsFork {
@@ -114,9 +118,8 @@ func HasAllRequiredCodeownerReviews(ctx context.Context, pb *git_model.Protected
114118
}
115119

116120
reviews, err := issues_model.FindLatestReviews(ctx, issues_model.FindReviewOptions{
117-
Types: []issues_model.ReviewType{issues_model.ReviewTypeApprove},
118-
IssueID: pr.IssueID,
119-
OfficialOnly: setting.Repository.PullRequest.DefaultMergeMessageOfficialApproversOnly,
121+
Types: []issues_model.ReviewType{issues_model.ReviewTypeApprove},
122+
IssueID: pr.IssueID,
120123
})
121124
if err != nil {
122125
return false

tests/integration/pull_review_test.go

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@ import (
1212
"testing"
1313

1414
"code.gitea.io/gitea/models/db"
15+
git_model "code.gitea.io/gitea/models/git"
1516
issues_model "code.gitea.io/gitea/models/issues"
1617
repo_model "code.gitea.io/gitea/models/repo"
1718
"code.gitea.io/gitea/models/unittest"
1819
user_model "code.gitea.io/gitea/models/user"
1920
"code.gitea.io/gitea/modules/git"
2021
"code.gitea.io/gitea/modules/test"
2122
issue_service "code.gitea.io/gitea/services/issue"
23+
pull_service "code.gitea.io/gitea/services/pull"
2224
repo_service "code.gitea.io/gitea/services/repository"
2325
files_service "code.gitea.io/gitea/services/repository/files"
2426
"code.gitea.io/gitea/tests"
@@ -49,6 +51,8 @@ func TestPullView_ReviewerMissed(t *testing.T) {
4951
func TestPullView_CodeOwner(t *testing.T) {
5052
onGiteaRun(t, func(t *testing.T, u *url.URL) {
5153
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
54+
user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
55+
user8 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 8})
5256

5357
// Create the repo.
5458
repo, err := repo_service.CreateRepositoryDirectly(db.DefaultContext, user2, user2, repo_service.CreateRepoOptions{
@@ -60,6 +64,17 @@ func TestPullView_CodeOwner(t *testing.T) {
6064
}, true)
6165
assert.NoError(t, err)
6266

67+
// create code owner branch protection
68+
protectBranch := git_model.ProtectedBranch{
69+
BlockOnCodeownerReviews: true,
70+
RepoID: repo.ID,
71+
RuleName: "master",
72+
CanPush: true,
73+
}
74+
75+
err = pull_service.CreateOrUpdateProtectedBranch(db.DefaultContext, repo, &protectBranch, git_model.WhitelistOptions{})
76+
assert.NoError(t, err)
77+
6378
// add CODEOWNERS to default branch
6479
_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
6580
OldBranch: repo.DefaultBranch,
@@ -96,7 +111,7 @@ func TestPullView_CodeOwner(t *testing.T) {
96111
assert.NoError(t, pr.LoadIssue(db.DefaultContext))
97112

98113
// update the file on the pr branch
99-
_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
114+
resp, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
100115
OldBranch: "codeowner-basebranch",
101116
Files: []*files_service.ChangeRepoFile{
102117
{
@@ -124,6 +139,22 @@ func TestPullView_CodeOwner(t *testing.T) {
124139
prUpdated2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID})
125140
assert.NoError(t, prUpdated2.LoadIssue(db.DefaultContext))
126141
assert.Equal(t, "Test Pull Request2", prUpdated2.Issue.Title)
142+
143+
// ensure it cannot be merged
144+
hasCodeownerReviews := issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
145+
assert.False(t, hasCodeownerReviews)
146+
147+
issues_model.SubmitReview(db.DefaultContext, user5, pr.Issue, issues_model.ReviewTypeApprove, "Very good", resp.Commit.SHA, false, make([]string, 0))
148+
149+
// should still fail (we also need user8)
150+
hasCodeownerReviews = issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
151+
assert.False(t, hasCodeownerReviews)
152+
153+
issues_model.SubmitReview(db.DefaultContext, user8, pr.Issue, issues_model.ReviewTypeApprove, "Very good", resp.Commit.SHA, false, make([]string, 0))
154+
155+
// now we should be able to merge
156+
hasCodeownerReviews = issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
157+
assert.True(t, hasCodeownerReviews)
127158
})
128159

129160
// change the default branch CODEOWNERS file to change README.md's codeowner
@@ -140,7 +171,7 @@ func TestPullView_CodeOwner(t *testing.T) {
140171

141172
t.Run("Second Pull Request", func(t *testing.T) {
142173
// create a new branch to prepare for pull request
143-
_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
174+
resp, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
144175
NewBranch: "codeowner-basebranch2",
145176
Files: []*files_service.ChangeRepoFile{
146177
{
@@ -158,6 +189,15 @@ func TestPullView_CodeOwner(t *testing.T) {
158189

159190
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "codeowner-basebranch2"})
160191
unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
192+
193+
// should need user8 approval only now
194+
hasCodeownerReviews := issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
195+
assert.False(t, hasCodeownerReviews)
196+
197+
issues_model.SubmitReview(db.DefaultContext, user8, pr.Issue, issues_model.ReviewTypeApprove, "Very good", resp.Commit.SHA, false, make([]string, 0))
198+
199+
hasCodeownerReviews = issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
200+
assert.True(t, hasCodeownerReviews)
161201
})
162202

163203
t.Run("Forked Repo Pull Request", func(t *testing.T) {
@@ -169,7 +209,7 @@ func TestPullView_CodeOwner(t *testing.T) {
169209
assert.NoError(t, err)
170210

171211
// create a new branch to prepare for pull request
172-
_, err = files_service.ChangeRepoFiles(db.DefaultContext, forkedRepo, user5, &files_service.ChangeRepoFilesOptions{
212+
resp, err := files_service.ChangeRepoFiles(db.DefaultContext, forkedRepo, user5, &files_service.ChangeRepoFilesOptions{
173213
NewBranch: "codeowner-basebranch-forked",
174214
Files: []*files_service.ChangeRepoFile{
175215
{
@@ -194,6 +234,21 @@ func TestPullView_CodeOwner(t *testing.T) {
194234

195235
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadRepoID: forkedRepo.ID, HeadBranch: "codeowner-basebranch-forked"})
196236
unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
237+
238+
// will also need user8 for this
239+
hasCodeownerReviews := issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
240+
assert.False(t, hasCodeownerReviews)
241+
242+
issues_model.SubmitReview(db.DefaultContext, user5, pr.Issue, issues_model.ReviewTypeApprove, "Very good", resp.Commit.SHA, false, make([]string, 0))
243+
244+
// should still fail (user5 is not a code owner for this PR)
245+
hasCodeownerReviews = issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
246+
assert.False(t, hasCodeownerReviews)
247+
248+
issues_model.SubmitReview(db.DefaultContext, user8, pr.Issue, issues_model.ReviewTypeApprove, "Very good", resp.Commit.SHA, false, make([]string, 0))
249+
250+
hasCodeownerReviews = issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
251+
assert.True(t, hasCodeownerReviews)
197252
})
198253
})
199254
}

0 commit comments

Comments
 (0)