mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2025-08-02 00:15:21 +02:00
fix: make API /repos/{owner}/{repo}/compare/{basehead} work with forks (#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.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8326
Reviewed-by: Otto <otto@codeberg.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
This commit is contained in:
Earl Warren
Earl Warren
parent
225a0f7026
commit
d6e4342353
14 changed files with 311 additions and 138 deletions
|
|
@ -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)()
|
||||
|
||||
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
||||
// Login as User2.
|
||||
session := loginUser(t, user.Name)
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
|
||||
|
||||
req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/c8e31bc...8babce9").
|
||||
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)
|
||||
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)
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue