mirror of
				https://codeberg.org/forgejo/forgejo.git
				synced 2025-11-03 16:01:11 +00:00 
			
		
		
		
	**Backport:** https://codeberg.org/forgejo/forgejo/pulls/8326 - fix: API must use headGitRepo instead of ctx.Repo.GitRepo for comparing - 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 on the focus of the PR It was initially created to address a regression introduced in v12. But the tests that verify it is fixed discovered a v11.0 bug. They cannot conveniently be separated because they both relate to the same area of code that was previously not covered by any test. ## Note on v11.0 backport It must be manually done by cherry-picking all commits up to and not including `fix: API must use headGitRepo instead of ctx.Repo.GitRepo for comparing` because it is v12 specific. ## 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... - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. ### Documentation - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] I do not want this change to show in the release notes. Co-authored-by: Earl Warren <contact@earl-warren.org> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8331 Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org> Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
		
			
				
	
	
		
			203 lines
		
	
	
	
		
			6.3 KiB
		
	
	
	
		
			Go
		
	
	
	
	
	
			
		
		
	
	
			203 lines
		
	
	
	
		
			6.3 KiB
		
	
	
	
		
			Go
		
	
	
	
	
	
// Copyright 2024 The Gitea Authors. All rights reserved.
 | 
						|
// SPDX-License-Identifier: MIT
 | 
						|
 | 
						|
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"
 | 
						|
 | 
						|
	"github.com/stretchr/testify/assert"
 | 
						|
)
 | 
						|
 | 
						|
func TestAPICompareCommits(t *testing.T) {
 | 
						|
	forEachObjectFormat(t, testAPICompareCommits)
 | 
						|
}
 | 
						|
 | 
						|
func testAPICompareCommits(t *testing.T, objectFormat git.ObjectFormat) {
 | 
						|
	onGiteaRun(t, func(t *testing.T, u *url.URL) {
 | 
						|
		newBranchAndFile := func(ctx APITestContext, user *user_model.User, branch, filename string) func(*testing.T) {
 | 
						|
			return func(t *testing.T) {
 | 
						|
				doAPICreateFile(ctx, filename, &api.CreateFileOptions{
 | 
						|
					FileOptions: api.FileOptions{
 | 
						|
						NewBranchName: branch,
 | 
						|
						Message:       "create " + filename,
 | 
						|
						Author: api.Identity{
 | 
						|
							Name:  user.Name,
 | 
						|
							Email: user.Email,
 | 
						|
						},
 | 
						|
						Committer: api.Identity{
 | 
						|
							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)
 | 
						|
			})
 | 
						|
		}
 | 
						|
	})
 | 
						|
}
 |