[v11.0/forgejo] fix: make API /repos/{owner}/{repo}/compare/{basehead} work with forks (#8333)

**Backport:** https://codeberg.org/forgejo/forgejo/pulls/8326

- fix: make API /repos/{owner}/{repo}/compare/{basehead} work with forks
- add test coverage for both fixes and the underlying function `parseCompareInfo`
- refactor and improve part of the helpers from `tests/integration/api_helper_for_declarative_test.go`
- remove a few wrong or misleading comments

Refs forgejo/forgejo#7978

---

Note that this is a partial backport for reasons explained at https://codeberg.org/forgejo/forgejo/pulls/8326

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### Tests

- I added test coverage for Go changes...
  - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.

### Documentation

- [ ] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [x] I want the title to show in the release notes with a link to this pull request.

<!--start release-notes-assistant-->

## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Bug fixes
  - [PR](https://codeberg.org/forgejo/forgejo/pulls/8333): <!--number 8333 --><!--line 0 --><!--description Zml4OiBtYWtlIEFQSSAvcmVwb3Mve293bmVyfS97cmVwb30vY29tcGFyZS97YmFzZWhlYWR9IHdvcmsgd2l0aCBmb3Jrcw==-->fix: make API /repos/{owner}/{repo}/compare/{basehead} work with forks<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8333
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>

routers/api/v1/repo/compare.go

@@ -64,7 +64,7 @@ func CompareDiff(ctx *context.APIContext) {
 		}
 	}
 
-	_, headGitRepo, ci, _, _ := parseCompareInfo(ctx, api.CreatePullRequestOption{
+	headRepository, headGitRepo, ci, _, _ := parseCompareInfo(ctx, api.CreatePullRequestOption{
 		Base: infos[0],
 		Head: infos[1],
 	})
@@ -80,7 +80,7 @@ func CompareDiff(ctx *context.APIContext) {
 	apiFiles := []*api.CommitAffectedFiles{}
 	userCache := make(map[string]*user_model.User)
 	for i := 0; i < len(ci.Commits); i++ {
-		apiCommit, err := convert.ToCommit(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, ci.Commits[i], userCache,
+		apiCommit, err := convert.ToCommit(ctx, headRepository, headGitRepo, ci.Commits[i], userCache,
 			convert.ToCommitOptions{
 				Stat:         true,
 				Verification: verification,

routers/api/v1/repo/pull.go

@@ -1125,7 +1125,6 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
 		}
 	}
 
-	// Check if current user has fork of repository or in the same repository.
 	headRepo := repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID)
 	if headRepo == nil && !isSameRepo {
 		err := baseRepo.GetBaseRepo(ctx)
@@ -1134,13 +1133,11 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
 			return nil, nil, nil, "", ""
 		}
 
-		// Check if baseRepo's base repository is the same as headUser's repository.
 		if baseRepo.BaseRepo == nil || baseRepo.BaseRepo.OwnerID != headUser.ID {
 			log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID)
 			ctx.NotFound("GetBaseRepo")
 			return nil, nil, nil, "", ""
 		}
-		// Assign headRepo so it can be used below.
 		headRepo = baseRepo.BaseRepo
 	}
 

tests/integration/api_branch_test.go

@@ -116,7 +116,7 @@ func testAPICreateBranches(t *testing.T, giteaURL *url.URL) {
 		ctx := NewAPITestContext(t, "user2", "my-noo-repo-"+objectFormat.Name(), auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
 		giteaURL.Path = ctx.GitPath()
 
-		t.Run("CreateRepo", doAPICreateRepository(ctx, false, objectFormat))
+		t.Run("CreateRepo", doAPICreateRepository(ctx, nil, objectFormat))
 		testCases := []struct {
 			OldBranch          string
 			NewBranch          string

tests/integration/api_helper_for_declarative_test.go

@@ -48,20 +48,22 @@ func (ctx APITestContext) GitPath() string {
 	return fmt.Sprintf("%s/%s.git", ctx.Username, ctx.Reponame)
 }
 
-func doAPICreateRepository(ctx APITestContext, empty bool, objectFormat git.ObjectFormat, callback ...func(*testing.T, api.Repository)) func(*testing.T) {
+func doAPICreateRepository(ctx APITestContext, opts *api.CreateRepoOption, objectFormat git.ObjectFormat, callback ...func(*testing.T, api.Repository)) func(*testing.T) {
 	return func(t *testing.T) {
-		createRepoOption := &api.CreateRepoOption{
+		if opts == nil {
-			AutoInit:         !empty,
+			opts = &api.CreateRepoOption{
-			Description:      "Temporary repo",
+				AutoInit:    true,
-			Name:             ctx.Reponame,
+				Description: "Temporary repo",
-			Private:          true,
+				Name:        ctx.Reponame,
-			Template:         true,
+				Private:     true,
-			Gitignores:       "",
+				Template:    true,
-			License:          "WTFPL",
+				Gitignores:  "",
-			Readme:           "Default",
+				License:     "WTFPL",
-			ObjectFormatName: objectFormat.Name(),
+				Readme:      "Default",
+			}
 		}
-		req := NewRequestWithJSON(t, "POST", "/api/v1/user/repos", createRepoOption).
+		opts.ObjectFormatName = objectFormat.Name()
+		req := NewRequestWithJSON(t, "POST", "/api/v1/user/repos", opts).
 			AddTokenAuth(ctx.Token)
 		if ctx.ExpectedCode != 0 {
 			ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
@@ -237,8 +239,8 @@ func doAPICreatePullRequest(ctx APITestContext, owner, repo, baseBranch, headBra
 	}
 }
 
-func doAPIGetPullRequest(ctx APITestContext, owner, repo string, index int64) func(*testing.T) (api.PullRequest, error) {
+func doAPIGetPullRequest(ctx APITestContext, owner, repo string, index int64) func(*testing.T) api.PullRequest {
-	return func(t *testing.T) (api.PullRequest, error) {
+	return func(t *testing.T) api.PullRequest {
 		req := NewRequest(t, http.MethodGet, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d", owner, repo, index)).
 			AddTokenAuth(ctx.Token)
 
@@ -248,10 +250,9 @@ func doAPIGetPullRequest(ctx APITestContext, owner, repo string, index int64) fu
 		}
 		resp := ctx.Session.MakeRequest(t, req, expected)
 
-		decoder := json.NewDecoder(resp.Body)
 		pr := api.PullRequest{}
-		err := decoder.Decode(&pr)
+		DecodeJSON(t, resp, &pr)
-		return pr, err
+		return pr
 	}
 }
 
@@ -347,20 +348,40 @@ func doAPICancelAutoMergePullRequest(ctx APITestContext, owner, repo string, ind
 	}
 }
 
-func doAPIGetBranch(ctx APITestContext, branch string, callback ...func(*testing.T, api.Branch)) func(*testing.T) {
+func doAPIGetBranch(ctx APITestContext, branch string) func(*testing.T) api.Branch {
-	return func(t *testing.T) {
+	return func(t *testing.T) api.Branch {
 		req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/branches/%s", ctx.Username, ctx.Reponame, branch).
 			AddTokenAuth(ctx.Token)
+		expected := http.StatusOK
+		if ctx.ExpectedCode != 0 {
+			expected = ctx.ExpectedCode
+		}
+		resp := ctx.Session.MakeRequest(t, req, expected)
+
+		branch := api.Branch{}
+		DecodeJSON(t, resp, &branch)
+		return branch
+	}
+}
+
+func doAPICreateTag(ctx APITestContext, tag, target, message string, callback ...func(*testing.T, api.Tag)) func(*testing.T) {
+	return func(t *testing.T) {
+		req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/tags", ctx.Username, ctx.Reponame), &api.CreateTagOption{
+			TagName: tag,
+			Message: message,
+			Target:  target,
+		}).
+			AddTokenAuth(ctx.Token)
 		if ctx.ExpectedCode != 0 {
 			ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
 			return
 		}
-		resp := ctx.Session.MakeRequest(t, req, http.StatusOK)
+		resp := ctx.Session.MakeRequest(t, req, http.StatusCreated)
 
-		var branch api.Branch
+		var tag api.Tag
-		DecodeJSON(t, resp, &branch)
+		DecodeJSON(t, resp, &tag)
 		if len(callback) > 0 {
-			callback[0](t, branch)
+			callback[0](t, tag)
 		}
 	}
 }

tests/integration/api_repo_compare_test.go

@@ -4,56 +4,200 @@
 package integration
 
 import (
+	"encoding/base64"
+	"fmt"
 	"net/http"
+	"net/url"
 	"testing"
+	"time"
 
 	auth_model "forgejo.org/models/auth"
 	"forgejo.org/models/unittest"
 	user_model "forgejo.org/models/user"
+	"forgejo.org/modules/git"
 	api "forgejo.org/modules/structs"
-	"forgejo.org/tests"
 
 	"github.com/stretchr/testify/assert"
 )
 
-func TestAPICompareBranches(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
-	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
-	// Login as User2.
-	session := loginUser(t, user.Name)
-	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
-
-	repoName := "repo20"
-
-	req := NewRequestf(t, "GET", "/api/v1/repos/user2/%s/compare/add-csv...remove-files-b", repoName).
-		AddTokenAuth(token)
-	resp := MakeRequest(t, req, http.StatusOK)
-
-	var apiResp *api.Compare
-	DecodeJSON(t, resp, &apiResp)
-
-	assert.Equal(t, 2, apiResp.TotalCommits)
-	assert.Len(t, apiResp.Commits, 2)
-	assert.Len(t, apiResp.Files, 3)
-}
-
 func TestAPICompareCommits(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+	forEachObjectFormat(t, testAPICompareCommits)
-
+}
-	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+
-	// Login as User2.
+func testAPICompareCommits(t *testing.T, objectFormat git.ObjectFormat) {
-	session := loginUser(t, user.Name)
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
-	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+		newBranchAndFile := func(ctx APITestContext, user *user_model.User, branch, filename string) func(*testing.T) {
-
+			return func(t *testing.T) {
-	req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/c8e31bc...8babce9").
+				doAPICreateFile(ctx, filename, &api.CreateFileOptions{
-		AddTokenAuth(token)
+					FileOptions: api.FileOptions{
-	resp := MakeRequest(t, req, http.StatusOK)
+						NewBranchName: branch,
-
+						Message:       "create " + filename,
-	var apiResp *api.Compare
+						Author: api.Identity{
-	DecodeJSON(t, resp, &apiResp)
+							Name:  user.Name,
-
+							Email: user.Email,
-	assert.Equal(t, 2, apiResp.TotalCommits)
+						},
-	assert.Len(t, apiResp.Commits, 2)
+						Committer: api.Identity{
-	assert.Len(t, apiResp.Files, 3)
+							Name:  user.Name,
+							Email: user.Email,
+						},
+						Dates: api.CommitDateOptions{
+							Author:    time.Now(),
+							Committer: time.Now(),
+						},
+					},
+					ContentBase64: base64.StdEncoding.EncodeToString([]byte("content " + filename)),
+				})(t)
+			}
+		}
+
+		user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+		user2repo := "repoA"
+		user2Ctx := NewAPITestContext(t, user2.Name, user2repo, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
+		t.Run("CreateUser2Repository", doAPICreateRepository(user2Ctx, &api.CreateRepoOption{
+			AutoInit:    true,
+			Description: "Temporary repo",
+			Name:        user2Ctx.Reponame,
+		}, objectFormat))
+		user2branchName := "user2branch"
+		t.Run("CreateUser2RepositoryBranch", newBranchAndFile(user2Ctx, user2, user2branchName, "user2branchfilename.txt"))
+		user2branch := doAPIGetBranch(user2Ctx, user2branchName)(t)
+		user2master := doAPIGetBranch(user2Ctx, "master")(t)
+		user2tag1 := "tag1"
+		t.Run("CreateUser2RepositoryTag1", doAPICreateTag(user2Ctx, user2tag1, "master", "user2branchtag1"))
+		user2tag2 := "tag2"
+		t.Run("CreateUser2RepositoryTag1", doAPICreateTag(user2Ctx, user2tag2, user2branchName, "user2branchtag2"))
+
+		shortCommitLength := 7
+
+		for _, testCase := range []struct {
+			name string
+			a    string
+			b    string
+		}{
+			{
+				name: "Commits",
+				a:    user2master.Commit.ID,
+				b:    user2branch.Commit.ID,
+			},
+			{
+				name: "ShortCommits",
+				a:    user2master.Commit.ID[:shortCommitLength],
+				b:    user2branch.Commit.ID[:shortCommitLength],
+			},
+			{
+				name: "Branches",
+				a:    "master",
+				b:    user2branchName,
+			},
+			{
+				name: "Tags",
+				a:    user2tag1,
+				b:    user2tag2,
+			},
+		} {
+			t.Run("SameRepo"+testCase.name, func(t *testing.T) {
+				// a...b
+				req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/compare/%s...%s", user2.Name, user2repo, testCase.a, testCase.b).
+					AddTokenAuth(user2Ctx.Token)
+				resp := MakeRequest(t, req, http.StatusOK)
+
+				var apiResp *api.Compare
+				DecodeJSON(t, resp, &apiResp)
+
+				assert.Equal(t, 1, apiResp.TotalCommits)
+				assert.Len(t, apiResp.Commits, 1)
+				assert.Len(t, apiResp.Files, 1)
+
+				// b...a
+				req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/compare/%s...%s", user2.Name, user2repo, testCase.b, testCase.a).
+					AddTokenAuth(user2Ctx.Token)
+				resp = MakeRequest(t, req, http.StatusOK)
+
+				DecodeJSON(t, resp, &apiResp)
+
+				assert.Equal(t, 0, apiResp.TotalCommits)
+				assert.Empty(t, apiResp.Commits)
+				assert.Empty(t, apiResp.Files)
+			})
+		}
+
+		user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
+		user4Ctx := NewAPITestContext(t, user4.Name, user2repo, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
+		t.Run("User4ForksUser2Repository", doAPIForkRepository(user4Ctx, user2.Name))
+		user4branchName := "user4branch"
+		t.Run("CreateUser4RepositoryBranch", newBranchAndFile(user4Ctx, user4, user4branchName, "user4branchfilename.txt"))
+		user4branch := doAPIGetBranch(user4Ctx, user4branchName)(t)
+		user4tag4 := "tag4"
+		t.Run("CreateUser4RepositoryTag4", doAPICreateTag(user4Ctx, user4tag4, user4branchName, "user4branchtag4"))
+
+		t.Run("FromTheForkedRepo", func(t *testing.T) {
+			// user4/repoA is a fork of user2/repoA and when evaluating
+			//
+			// user4/repoA/compare/master...user2:user2branch
+			//
+			// user2/repoA is not explicitly specified, it is implicitly the repository
+			// from which user4/repoA was forked
+			req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/compare/%s...%s:%s", user4.Name, user2repo, "master", user2.Name, user2branchName).
+				AddTokenAuth(user4Ctx.Token)
+			resp := MakeRequest(t, req, http.StatusOK)
+
+			var apiResp *api.Compare
+			DecodeJSON(t, resp, &apiResp)
+
+			assert.Equal(t, 1, apiResp.TotalCommits)
+			assert.Len(t, apiResp.Commits, 1)
+			assert.Len(t, apiResp.Files, 1)
+		})
+
+		for _, testCase := range []struct {
+			name string
+			a    string
+			b    string
+		}{
+			{
+				name: "Commits",
+				a:    user2master.Commit.ID,
+				b:    fmt.Sprintf("%s:%s", user4.Name, user4branch.Commit.ID),
+			},
+			{
+				name: "ShortCommits",
+				a:    user2master.Commit.ID[:shortCommitLength],
+				b:    fmt.Sprintf("%s:%s", user4.Name, user4branch.Commit.ID[:shortCommitLength]),
+			},
+			{
+				name: "Branches",
+				a:    "master",
+				b:    fmt.Sprintf("%s:%s", user4.Name, user4branchName),
+			},
+			{
+				name: "Tags",
+				a:    user2tag1,
+				b:    fmt.Sprintf("%s:%s", user4.Name, user4tag4),
+			},
+			{
+				name: "SameRepo",
+				a:    "master",
+				b:    fmt.Sprintf("%s:%s", user2.Name, user2branchName),
+			},
+		} {
+			t.Run("ForkedRepo"+testCase.name, func(t *testing.T) {
+				// user2/repoA is forked into user4/repoA and when evaluating
+				//
+				// user2/repoA/compare/a...user4:b
+				//
+				// user4/repoA is not explicitly specified, it is implicitly the repository
+				// owned by user4 which is a fork of repoA
+				req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/compare/%s...%s", user2.Name, user2repo, testCase.a, testCase.b).
+					AddTokenAuth(user2Ctx.Token)
+				resp := MakeRequest(t, req, http.StatusOK)
+
+				var apiResp *api.Compare
+				DecodeJSON(t, resp, &apiResp)
+
+				assert.Equal(t, 1, apiResp.TotalCommits)
+				assert.Len(t, apiResp.Commits, 1)
+				assert.Len(t, apiResp.Files, 1)
+			})
+		}
+	})
 }

tests/integration/api_repo_file_create_test.go

@@ -283,7 +283,14 @@ func TestAPICreateFile(t *testing.T) {
 		// Test creating a file in an empty repository
 		forEachObjectFormat(t, func(t *testing.T, objectFormat git.ObjectFormat) {
 			reponame := "empty-repo-" + objectFormat.Name()
-			doAPICreateRepository(NewAPITestContext(t, "user2", reponame, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser), true, objectFormat)(t)
+			ctx := NewAPITestContext(t, "user2", reponame, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
+			opts := &api.CreateRepoOption{
+				Description: "Temporary repo",
+				Name:        ctx.Reponame,
+				Private:     true,
+				Template:    true,
+			}
+			doAPICreateRepository(ctx, opts, objectFormat)(t)
 			createFileOptions = getCreateFileOptions()
 			fileID++
 			treePath = fmt.Sprintf("new/file%d.txt", fileID)

tests/integration/api_repo_file_get_test.go

@@ -27,7 +27,7 @@ func TestAPIGetRawFileOrLFS(t *testing.T) {
 	// Test with LFS
 	onGiteaRun(t, func(t *testing.T, u *url.URL) {
 		httpContext := NewAPITestContext(t, "user2", "repo-lfs-test", auth_model.AccessTokenScopeWriteRepository)
-		doAPICreateRepository(httpContext, false, git.Sha1ObjectFormat, func(t *testing.T, repository api.Repository) { // FIXME: use forEachObjectFormat
+		doAPICreateRepository(httpContext, nil, git.Sha1ObjectFormat, func(t *testing.T, repository api.Repository) { // FIXME: use forEachObjectFormat
 			u.Path = httpContext.GitPath()
 			dstPath := t.TempDir()
 

tests/integration/api_repo_lfs_test.go

@@ -63,7 +63,7 @@ func TestAPILFSMediaType(t *testing.T) {
 
 func createLFSTestRepository(t *testing.T, name string) *repo_model.Repository {
 	ctx := NewAPITestContext(t, "user2", "lfs-"+name+"-repo", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
-	t.Run("CreateRepo", doAPICreateRepository(ctx, false, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
+	t.Run("CreateRepo", doAPICreateRepository(ctx, nil, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
 
 	repo, err := repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, "user2", "lfs-"+name+"-repo")
 	require.NoError(t, err)

tests/integration/api_repo_test.go

@@ -430,7 +430,7 @@ func testAPIRepoMigrateConflict(t *testing.T, u *url.URL) {
 		httpContext := baseAPITestContext
 
 		httpContext.Reponame = "repo-tmp-17"
-		t.Run("CreateRepo", doAPICreateRepository(httpContext, false, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
+		t.Run("CreateRepo", doAPICreateRepository(httpContext, nil, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
 
 		user, err := user_model.GetUserByName(db.DefaultContext, httpContext.Username)
 		require.NoError(t, err)
@@ -513,7 +513,7 @@ func testAPIRepoCreateConflict(t *testing.T, u *url.URL) {
 		httpContext := baseAPITestContext
 
 		httpContext.Reponame = "repo-tmp-17"
-		t.Run("CreateRepo", doAPICreateRepository(httpContext, false, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
+		t.Run("CreateRepo", doAPICreateRepository(httpContext, nil, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
 
 		req := NewRequestWithJSON(t, "POST", "/api/v1/user/repos",
 			&api.CreateRepoOption{

tests/integration/git_test.go

@@ -69,7 +69,7 @@ func testGit(t *testing.T, u *url.URL) {
 
 			dstPath := t.TempDir()
 
-			t.Run("CreateRepoInDifferentUser", doAPICreateRepository(forkedUserCtx, false, objectFormat))
+			t.Run("CreateRepoInDifferentUser", doAPICreateRepository(forkedUserCtx, nil, objectFormat))
 			t.Run("AddUserAsCollaborator", doAPIAddCollaborator(forkedUserCtx, httpContext.Username, perm.AccessModeRead))
 
 			t.Run("ForkFromDifferentUser", doAPIForkRepository(httpContext, forkedUserCtx.Username))
@@ -110,7 +110,7 @@ func testGit(t *testing.T, u *url.URL) {
 			sshContext.Reponame = "repo-tmp-18-" + objectFormat.Name()
 			keyname := "my-testing-key"
 			forkedUserCtx.Reponame = sshContext.Reponame
-			t.Run("CreateRepoInDifferentUser", doAPICreateRepository(forkedUserCtx, false, objectFormat))
+			t.Run("CreateRepoInDifferentUser", doAPICreateRepository(forkedUserCtx, nil, objectFormat))
 			t.Run("AddUserAsCollaborator", doAPIAddCollaborator(forkedUserCtx, sshContext.Username, perm.AccessModeRead))
 			t.Run("ForkFromDifferentUser", doAPIForkRepository(sshContext, forkedUserCtx.Username))
 
@@ -529,8 +529,7 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun
 		t.Run("EnsureCanSeePull", doEnsureCanSeePull(headCtx, pr, false))
 		t.Run("CheckPR", func(t *testing.T) {
 			oldMergeBase := pr.MergeBase
-			pr2, err := doAPIGetPullRequest(baseCtx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
+			pr2 := doAPIGetPullRequest(baseCtx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
-			require.NoError(t, err)
 			assert.Equal(t, oldMergeBase, pr2.MergeBase)
 		})
 		t.Run("EnsurDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffHash, diffLength))
@@ -730,16 +729,14 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) {
 
 		// Check pr status
 		ctx.ExpectedCode = 0
-		pr, err = doAPIGetPullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
+		pr = doAPIGetPullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
-		require.NoError(t, err)
 		assert.False(t, pr.HasMerged)
 
 		// Call API to add Failure status for commit
 		t.Run("CreateStatus", addCommitStatus(api.CommitStatusFailure))
 
 		// Check pr status
-		pr, err = doAPIGetPullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
+		pr = doAPIGetPullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
-		require.NoError(t, err)
 		assert.False(t, pr.HasMerged)
 
 		// Call API to add Success status for commit
@@ -749,8 +746,7 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) {
 		time.Sleep(time.Second)
 
 		// test pr status
-		pr, err = doAPIGetPullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
+		pr = doAPIGetPullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
-		require.NoError(t, err)
 		assert.True(t, pr.HasMerged)
 	}
 }
@@ -839,8 +835,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
 			assert.Equal(t, 1, pr1.CommitsAhead)
 			assert.Equal(t, 0, pr1.CommitsBehind)
 
-			prMsg, err := doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr1.Index)(t)
+			prMsg := doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr1.Index)(t)
-			require.NoError(t, err)
 
 			assert.Equal(t, "user2/"+headBranch, pr1.HeadBranch)
 			assert.False(t, prMsg.HasMerged)
@@ -861,8 +856,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
 			}
 			assert.Equal(t, 1, pr2.CommitsAhead)
 			assert.Equal(t, 0, pr2.CommitsBehind)
-			prMsg, err = doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr2.Index)(t)
+			prMsg = doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr2.Index)(t)
-			require.NoError(t, err)
 
 			assert.Equal(t, "user2/test/"+headBranch, pr2.HeadBranch)
 			assert.False(t, prMsg.HasMerged)
@@ -913,8 +907,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
 			require.NoError(t, err)
 
 			unittest.AssertCount(t, &issues_model.PullRequest{}, pullNum+2)
-			prMsg, err := doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr1.Index)(t)
+			prMsg := doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr1.Index)(t)
-			require.NoError(t, err)
 
 			assert.False(t, prMsg.HasMerged)
 			assert.Equal(t, commit, prMsg.Head.Sha)
@@ -931,8 +924,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
 			require.NoError(t, err)
 
 			unittest.AssertCount(t, &issues_model.PullRequest{}, pullNum+2)
-			prMsg, err = doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr2.Index)(t)
+			prMsg = doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr2.Index)(t)
-			require.NoError(t, err)
 
 			assert.False(t, prMsg.HasMerged)
 			assert.Equal(t, commit, prMsg.Head.Sha)
@@ -956,8 +948,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
 				err := pr3.LoadIssue(db.DefaultContext)
 				require.NoError(t, err)
 
-				_, err2 := doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr3.Index)(t)
+				doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr3.Index)(t)
-				require.NoError(t, err2)
 
 				assert.Equal(t, "Testing commit 2", pr3.Issue.Title)
 				assert.Contains(t, pr3.Issue.Content, "Longer description.")
@@ -978,8 +969,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
 				err := pr.LoadIssue(db.DefaultContext)
 				require.NoError(t, err)
 
-				_, err = doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr.Index)(t)
+				doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr.Index)(t)
-				require.NoError(t, err)
 
 				assert.Equal(t, "my-shiny-title", pr.Issue.Title)
 				assert.Contains(t, pr.Issue.Content, "Longer description.")
@@ -1001,8 +991,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
 				err := pr.LoadIssue(db.DefaultContext)
 				require.NoError(t, err)
 
-				_, err = doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr.Index)(t)
+				doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr.Index)(t)
-				require.NoError(t, err)
 
 				assert.Equal(t, "Testing commit 2", pr.Issue.Title)
 				assert.Contains(t, pr.Issue.Content, "custom")

tests/integration/gpg_git_test.go

@@ -54,13 +54,14 @@ func TestGPGGit(t *testing.T) {
 		t.Run("Unsigned-Initial", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()
 			testCtx := NewAPITestContext(t, username, "initial-unsigned", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
-			t.Run("CreateRepository", doAPICreateRepository(testCtx, false, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
+			t.Run("CreateRepository", doAPICreateRepository(testCtx, nil, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
-			t.Run("CheckMasterBranchUnsigned", doAPIGetBranch(testCtx, "master", func(t *testing.T, branch api.Branch) {
+			t.Run("CheckMasterBranchUnsigned", func(t *testing.T) {
+				branch := doAPIGetBranch(testCtx, "master")(t)
 				assert.NotNil(t, branch.Commit)
 				assert.NotNil(t, branch.Commit.Verification)
 				assert.False(t, branch.Commit.Verification.Verified)
 				assert.Empty(t, branch.Commit.Verification.Signature)
-			}))
+			})
 			t.Run("CreateCRUDFile-Never", crudActionCreateFile(
 				t, testCtx, user, "master", "never", "unsigned-never.txt", func(t *testing.T, response api.FileResponse) {
 					assert.False(t, response.Verification.Verified)
@@ -147,8 +148,9 @@ func TestGPGGit(t *testing.T) {
 		t.Run("AlwaysSign-Initial", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()
 			testCtx := NewAPITestContext(t, username, "initial-always", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
-			t.Run("CreateRepository", doAPICreateRepository(testCtx, false, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
+			t.Run("CreateRepository", doAPICreateRepository(testCtx, nil, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
-			t.Run("CheckMasterBranchSigned", doAPIGetBranch(testCtx, "master", func(t *testing.T, branch api.Branch) {
+			t.Run("CheckMasterBranchSigned", func(t *testing.T) {
+				branch := doAPIGetBranch(testCtx, "master")(t)
 				assert.NotNil(t, branch.Commit)
 				if branch.Commit == nil {
 					assert.FailNow(t, "no commit provided with branch! %v", branch)
@@ -162,14 +164,14 @@ func TestGPGGit(t *testing.T) {
 					t.FailNow()
 				}
 				assert.Equal(t, "gitea@fake.local", branch.Commit.Verification.Signer.Email)
-			}))
+			})
 		})
 
 		setting.Repository.Signing.CRUDActions = []string{"never"}
 		t.Run("AlwaysSign-Initial-CRUD-Never", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()
 			testCtx := NewAPITestContext(t, username, "initial-always-never", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
-			t.Run("CreateRepository", doAPICreateRepository(testCtx, false, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
+			t.Run("CreateRepository", doAPICreateRepository(testCtx, nil, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
 			t.Run("CreateCRUDFile-Never", crudActionCreateFile(
 				t, testCtx, user, "master", "never", "unsigned-never.txt", func(t *testing.T, response api.FileResponse) {
 					assert.False(t, response.Verification.Verified)
@@ -180,7 +182,7 @@ func TestGPGGit(t *testing.T) {
 		t.Run("AlwaysSign-Initial-CRUD-ParentSigned-On-Always", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()
 			testCtx := NewAPITestContext(t, username, "initial-always-parent", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
-			t.Run("CreateRepository", doAPICreateRepository(testCtx, false, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
+			t.Run("CreateRepository", doAPICreateRepository(testCtx, nil, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
 			t.Run("CreateCRUDFile-ParentSigned", crudActionCreateFile(
 				t, testCtx, user, "master", "parentsigned", "signed-parent.txt", func(t *testing.T, response api.FileResponse) {
 					assert.True(t, response.Verification.Verified)
@@ -196,7 +198,7 @@ func TestGPGGit(t *testing.T) {
 		t.Run("AlwaysSign-Initial-CRUD-Always", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()
 			testCtx := NewAPITestContext(t, username, "initial-always-always", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
-			t.Run("CreateRepository", doAPICreateRepository(testCtx, false, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
+			t.Run("CreateRepository", doAPICreateRepository(testCtx, nil, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
 			t.Run("CreateCRUDFile-Always", crudActionCreateFile(
 				t, testCtx, user, "master", "always", "signed-always.txt", func(t *testing.T, response api.FileResponse) {
 					assert.True(t, response.Verification.Verified)
@@ -217,12 +219,13 @@ func TestGPGGit(t *testing.T) {
 				require.NoError(t, err)
 				t.Run("MergePR", doAPIMergePullRequest(testCtx, testCtx.Username, testCtx.Reponame, pr.Index))
 			})
-			t.Run("CheckMasterBranchUnsigned", doAPIGetBranch(testCtx, "master", func(t *testing.T, branch api.Branch) {
+			t.Run("CheckMasterBranchUnsigned", func(t *testing.T) {
+				branch := doAPIGetBranch(testCtx, "master")(t)
 				assert.NotNil(t, branch.Commit)
 				assert.NotNil(t, branch.Commit.Verification)
 				assert.False(t, branch.Commit.Verification.Verified)
 				assert.Empty(t, branch.Commit.Verification.Signature)
-			}))
+			})
 		})
 
 		setting.Repository.Signing.Merges = []string{"basesigned"}
@@ -234,12 +237,13 @@ func TestGPGGit(t *testing.T) {
 				require.NoError(t, err)
 				t.Run("MergePR", doAPIMergePullRequest(testCtx, testCtx.Username, testCtx.Reponame, pr.Index))
 			})
-			t.Run("CheckMasterBranchUnsigned", doAPIGetBranch(testCtx, "master", func(t *testing.T, branch api.Branch) {
+			t.Run("CheckMasterBranchUnsigned", func(t *testing.T) {
+				branch := doAPIGetBranch(testCtx, "master")(t)
 				assert.NotNil(t, branch.Commit)
 				assert.NotNil(t, branch.Commit.Verification)
 				assert.False(t, branch.Commit.Verification.Verified)
 				assert.Empty(t, branch.Commit.Verification.Signature)
-			}))
+			})
 		})
 
 		setting.Repository.Signing.Merges = []string{"commitssigned"}
@@ -251,11 +255,12 @@ func TestGPGGit(t *testing.T) {
 				require.NoError(t, err)
 				t.Run("MergePR", doAPIMergePullRequest(testCtx, testCtx.Username, testCtx.Reponame, pr.Index))
 			})
-			t.Run("CheckMasterBranchUnsigned", doAPIGetBranch(testCtx, "master", func(t *testing.T, branch api.Branch) {
+			t.Run("CheckMasterBranchUnsigned", func(t *testing.T) {
+				branch := doAPIGetBranch(testCtx, "master")(t)
 				assert.NotNil(t, branch.Commit)
 				assert.NotNil(t, branch.Commit.Verification)
 				assert.True(t, branch.Commit.Verification.Verified)
-			}))
+			})
 		})
 	})
 }

tests/integration/ssh_key_test.go

@@ -55,7 +55,13 @@ func testPushDeployKeyOnEmptyRepo(t *testing.T, u *url.URL) {
 		keyname := fmt.Sprintf("%s-push", ctx.Reponame)
 		u.Path = ctx.GitPath()
 
-		t.Run("CreateEmptyRepository", doAPICreateRepository(ctx, true, objectFormat))
+		opts := &api.CreateRepoOption{
+			Description: "Temporary repo",
+			Name:        ctx.Reponame,
+			Private:     true,
+			Template:    true,
+		}
+		t.Run("CreateEmptyRepository", doAPICreateRepository(ctx, opts, objectFormat))
 
 		t.Run("CheckIsEmpty", doCheckRepositoryEmptyStatus(ctx, true))
 
@@ -105,8 +111,8 @@ func testKeyOnlyOneType(t *testing.T, u *url.URL) {
 	failCtx := ctx
 	failCtx.ExpectedCode = http.StatusUnprocessableEntity
 
-	t.Run("CreateRepository", doAPICreateRepository(ctx, false, git.Sha1ObjectFormat))           // FIXME: use forEachObjectFormat
+	t.Run("CreateRepository", doAPICreateRepository(ctx, nil, git.Sha1ObjectFormat))           // FIXME: use forEachObjectFormat
-	t.Run("CreateOtherRepository", doAPICreateRepository(otherCtx, false, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
+	t.Run("CreateOtherRepository", doAPICreateRepository(otherCtx, nil, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
 
 	withKeyFile(t, keyname, func(keyFile string) {
 		var userKeyPublicKeyID int64
@@ -180,7 +186,7 @@ func testKeyOnlyOneType(t *testing.T, u *url.URL) {
 
 			t.Run("DeleteOtherRepository", doAPIDeleteRepository(otherCtxWithDeleteRepo))
 
-			t.Run("RecreateRepository", doAPICreateRepository(ctxWithDeleteRepo, false, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
+			t.Run("RecreateRepository", doAPICreateRepository(ctxWithDeleteRepo, nil, git.Sha1ObjectFormat)) // FIXME: use forEachObjectFormat
 
 			t.Run("CreateUserKey", doAPICreateUserKey(ctx, keyname, keyFile, func(t *testing.T, publicKey api.PublicKey) {
 				userKeyPublicKeyID = publicKey.ID