Skip to content

Add repository search unit and integration tests #2575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Oct 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 98 additions & 1 deletion integrations/api_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package integrations

import (
"fmt"
"net/http"
"testing"

Expand Down Expand Up @@ -32,7 +33,7 @@ func TestAPIUserReposNotLogin(t *testing.T) {
}
}

func TestAPISearchRepoNotLogin(t *testing.T) {
func TestAPISearchRepo(t *testing.T) {
prepareTestEnv(t)
const keyword = "test"

Expand All @@ -46,6 +47,102 @@ func TestAPISearchRepoNotLogin(t *testing.T) {
assert.Contains(t, repo.Name, keyword)
assert.False(t, repo.Private)
}

user := models.AssertExistsAndLoadBean(t, &models.User{ID: 15}).(*models.User)
user2 := models.AssertExistsAndLoadBean(t, &models.User{ID: 16}).(*models.User)
orgUser := models.AssertExistsAndLoadBean(t, &models.User{ID: 17}).(*models.User)

// Map of expected results, where key is user for login
type expectedResults map[*models.User]struct {
count int
repoOwnerID int64
repoName string
includesPrivate bool
}

testCases := []struct {
name, requestURL string
expectedResults
}{
{name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50", expectedResults: expectedResults{
nil: {count: 12},
user: {count: 12},
user2: {count: 12}},
},
{name: "RepositoriesMax10", requestURL: "/api/v1/repos/search?limit=10", expectedResults: expectedResults{
nil: {count: 10},
user: {count: 10},
user2: {count: 10}},
},
{name: "RepositoriesDefaultMax10", requestURL: "/api/v1/repos/search", expectedResults: expectedResults{
nil: {count: 10},
user: {count: 10},
user2: {count: 10}},
},
{name: "RepositoriesByName", requestURL: fmt.Sprintf("/api/v1/repos/search?q=%s", "big_test_"), expectedResults: expectedResults{
nil: {count: 4, repoName: "big_test_"},
user: {count: 4, repoName: "big_test_"},
user2: {count: 4, repoName: "big_test_"}},
},
{name: "RepositoriesAccessibleAndRelatedToUser", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", user.ID), expectedResults: expectedResults{
// FIXME: Should return 4 (all public repositories related to "another" user = owned + collaborative), now returns only public repositories directly owned by user
nil: {count: 2},
user: {count: 8, includesPrivate: true},
// FIXME: Should return 4 (all public repositories related to "another" user = owned + collaborative), now returns only public repositories directly owned by user
user2: {count: 2}},
},
{name: "RepositoriesAccessibleAndRelatedToUser2", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", user2.ID), expectedResults: expectedResults{
nil: {count: 1},
user: {count: 1},
user2: {count: 2, includesPrivate: true}},
},
{name: "RepositoriesOwnedByOrganization", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", orgUser.ID), expectedResults: expectedResults{
nil: {count: 1, repoOwnerID: orgUser.ID},
user: {count: 2, repoOwnerID: orgUser.ID, includesPrivate: true},
user2: {count: 1, repoOwnerID: orgUser.ID}},
},
}

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
for userToLogin, expected := range testCase.expectedResults {
var session *TestSession
var testName string
if userToLogin != nil && userToLogin.ID > 0 {
testName = fmt.Sprintf("LoggedUser%d", userToLogin.ID)
session = loginUser(t, userToLogin.Name)
} else {
testName = "AnonymousUser"
session = emptyTestSession(t)
}

t.Run(testName, func(t *testing.T) {
request := NewRequest(t, "GET", testCase.requestURL)
response := session.MakeRequest(t, request, http.StatusOK)

var body api.SearchResults
DecodeJSON(t, response, &body)

assert.Len(t, body.Data, expected.count)
for _, repo := range body.Data {
assert.NotEmpty(t, repo.Name)

if len(expected.repoName) > 0 {
assert.Contains(t, repo.Name, expected.repoName)
}

if expected.repoOwnerID > 0 {
assert.Equal(t, expected.repoOwnerID, repo.Owner.ID)
}

if !expected.includesPrivate {
assert.False(t, repo.Private)
}
}
})
}
})
}
}

func TestAPIViewRepo(t *testing.T) {
Expand Down
82 changes: 76 additions & 6 deletions models/repo_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ func TestSearchRepositoryByName(t *testing.T) {
Keyword: "repo_12",
Page: 1,
PageSize: 10,
Searcher: nil,
})

assert.NotNil(t, repos)
assert.NoError(t, err)
if assert.Len(t, repos, 1) {
assert.Equal(t, "test_repo_12", repos[0].Name)
Expand All @@ -32,12 +30,11 @@ func TestSearchRepositoryByName(t *testing.T) {
Keyword: "test_repo",
Page: 1,
PageSize: 10,
Searcher: nil,
})

assert.NotNil(t, repos)
assert.NoError(t, err)
assert.Equal(t, int64(2), count)
assert.Len(t, repos, 2)

// test search private repository on explore page
repos, count, err = SearchRepositoryByName(&SearchRepoOptions{
Expand All @@ -48,7 +45,6 @@ func TestSearchRepositoryByName(t *testing.T) {
Searcher: &User{ID: 14},
})

assert.NotNil(t, repos)
assert.NoError(t, err)
if assert.Len(t, repos, 1) {
assert.Equal(t, "test_repo_13", repos[0].Name)
Expand All @@ -63,7 +59,81 @@ func TestSearchRepositoryByName(t *testing.T) {
Searcher: &User{ID: 14},
})

assert.NotNil(t, repos)
assert.NoError(t, err)
assert.Equal(t, int64(3), count)
assert.Len(t, repos, 3)

testCases := []struct {
name string
opts *SearchRepoOptions
count int
}{
{name: "PublicRepositoriesByName",
opts: &SearchRepoOptions{Keyword: "big_test_", PageSize: 10},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add similar checks as in api_repo_tests for all test cases. E.g. repo name, private/public repositories

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

count: 4},
{name: "PublicAndPrivateRepositoriesByName",
opts: &SearchRepoOptions{Keyword: "big_test_", Page: 1, PageSize: 10, Private: true},
count: 8},
{name: "PublicAndPrivateRepositoriesByNameWithPagesizeLimitFirstPage",
opts: &SearchRepoOptions{Keyword: "big_test_", Page: 1, PageSize: 5, Private: true},
count: 8},
{name: "PublicAndPrivateRepositoriesByNameWithPagesizeLimitSecondPage",
opts: &SearchRepoOptions{Keyword: "big_test_", Page: 2, PageSize: 5, Private: true},
count: 8},
{name: "PublicRepositoriesOfUser",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15},
count: 3}, // FIXME: Should return 2 (only directly owned repositories), now includes 1 public repository from owned organization
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment from above. Transitive owning.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Organization and other repositories related to user should be included only if collaborate option is supplied.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. Should be 2 here.

{name: "PublicAndPrivateRepositoriesOfUser",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Private: true},
count: 6}, // FIXME: Should return 4 (only directly owned repositories), now includes 2 repositories from owned organization
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. Should be 4 here.

{name: "PublicRepositoriesOfUserIncludingCollaborative",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Collaborate: true},
count: 4},
{name: "PublicAndPrivateRepositoriesOfUserIncludingCollaborative",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Private: true, Collaborate: true},
count: 8},
{name: "PublicRepositoriesOfOrganization",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 17},
count: 1},
{name: "PublicAndPrivateRepositoriesOfOrganization",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 17, Private: true},
count: 2},
}

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
if testCase.opts.OwnerID > 0 {
testCase.opts.Searcher = &User{ID: testCase.opts.OwnerID}
}
repos, count, err := SearchRepositoryByName(testCase.opts)

assert.NoError(t, err)
assert.Equal(t, int64(testCase.count), count)

var expectedLen int
if testCase.opts.PageSize*testCase.opts.Page > testCase.count {
expectedLen = testCase.count % testCase.opts.PageSize
} else {
expectedLen = testCase.opts.PageSize
}
assert.Len(t, repos, expectedLen)

for _, repo := range repos {
assert.NotEmpty(t, repo.Name)

if len(testCase.opts.Keyword) > 0 {
assert.Contains(t, repo.Name, testCase.opts.Keyword)
}

// FIXME: Can't check, need to fix current behaviour (see previous FIXME comments in test cases)
/*if testCase.opts.OwnerID > 0 && !testCase.opts.Collaborate {
assert.Equal(t, testCase.opts.OwnerID, repo.Owner.ID)
}*/

if !testCase.opts.Private {
assert.False(t, repo.IsPrivate)
}
}
})
}
}