Skip to content

Commit 0142df4

Browse files
lunnylunny
committed
Refactor to make test easier
1 parent 2322172 commit 0142df4

File tree

8 files changed

+69
-69
lines changed

8 files changed

+69
-69
lines changed

‎services/repository/adopt.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateR
6565

6666
// 1 - create the repository database operations first
6767
err := db.WithTx(ctx, func(ctx context.Context) error {
68-
return CreateRepositoryInDB(ctx, doer, u, repo, true, false)
68+
return createRepositoryInDB(ctx, doer, u, repo, false)
6969
})
7070
if err != nil {
7171
return nil, err

‎services/repository/adopt_test.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,10 @@
44
package repository
55

66
import (
7-
"context"
87
"os"
98
"path"
109
"path/filepath"
1110
"testing"
12-
"time"
1311

1412
"code.gitea.io/gitea/models/db"
1513
repo_model "code.gitea.io/gitea/models/repo"
@@ -94,7 +92,8 @@ func TestAdoptRepository(t *testing.T) {
9492
assert.NoError(t, unittest.PrepareTestDatabase())
9593

9694
// a successful adopt
97-
assert.NoError(t, unittest.SyncDirs(filepath.Join(setting.RepoRootPath, "user2", "repo1.git"), filepath.Join(setting.RepoRootPath, "user2", "test-adopt.git")))
95+
destDir := filepath.Join(setting.RepoRootPath, "user2", "test-adopt.git")
96+
assert.NoError(t, unittest.SyncDirs(filepath.Join(setting.RepoRootPath, "user2", "repo1.git"), destDir))
9897
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
9998
adoptedRepo, err := AdoptRepository(db.DefaultContext, user2, user2, CreateRepoOptions{Name: "test-adopt"})
10099
assert.NoError(t, err)
@@ -107,10 +106,12 @@ func TestAdoptRepository(t *testing.T) {
107106

108107
unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerName: "user2", Name: "test-adopt"})
109108

110-
// a failed adopt
111-
ctx, cancel := context.WithTimeout(db.DefaultContext, 1*time.Microsecond)
112-
defer cancel()
113-
adoptedRepo, err = AdoptRepository(ctx, user2, user2, CreateRepoOptions{Name: "test-adopt"})
109+
// a failed adopt because some mock data
110+
// remove the hooks directory and create a file so that we cannot create the hooks successfully
111+
_ = os.RemoveAll(filepath.Join(destDir, "hooks", "update.d"))
112+
assert.NoError(t, os.WriteFile(filepath.Join(destDir, "hooks", "update.d"), []byte("tests"), os.ModePerm))
113+
114+
adoptedRepo, err = AdoptRepository(db.DefaultContext, user2, user2, CreateRepoOptions{Name: "test-adopt"})
114115
assert.Error(t, err)
115116
assert.Nil(t, adoptedRepo)
116117

‎services/repository/create.go‎

Lines changed: 7 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt
249249

250250
// 1 - create the repository database operations first
251251
err := db.WithTx(ctx, func(ctx context.Context) error {
252-
return CreateRepositoryInDB(ctx, doer, u, repo, false, false)
252+
return createRepositoryInDB(ctx, doer, u, repo, false)
253253
})
254254
if err != nil {
255255
return nil, err
@@ -284,10 +284,12 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt
284284
// Previously Gitea would just delete and start afresh - this was naughty.
285285
// So we will now fail and delegate to other functionality to adopt or delete
286286
log.Error("Files already exist in %s and we are not going to adopt or delete.", repo.FullName())
287-
return nil, repo_model.ErrRepoFilesAlreadyExist{
288-
Uname: u.Name,
287+
// we need err in defer to cleanupRepository
288+
err = repo_model.ErrRepoFilesAlreadyExist{
289+
Uname: repo.OwnerName,
289290
Name: repo.Name,
290291
}
292+
return nil, err
291293
}
292294

293295
if err = initRepository(ctx, doer, repo, opts); err != nil {
@@ -331,8 +333,8 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt
331333
return repo, nil
332334
}
333335

334-
// CreateRepositoryInDB creates a repository for the user/organization.
335-
func CreateRepositoryInDB(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository, overwriteOrAdopt, isFork bool) (err error) {
336+
// createRepositoryInDB creates a repository for the user/organization.
337+
func createRepositoryInDB(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository, isFork bool) (err error) {
336338
if err = repo_model.IsUsableRepoName(repo.Name); err != nil {
337339
return err
338340
}
@@ -347,29 +349,6 @@ func CreateRepositoryInDB(ctx context.Context, doer, u *user_model.User, repo *r
347349
}
348350
}
349351

350-
isExist, err := gitrepo.IsRepositoryExist(ctx, repo)
351-
if err != nil {
352-
log.Error("Unable to check if %s exists. Error: %v", repo.FullName(), err)
353-
return err
354-
}
355-
if overwriteOrAdopt != isExist {
356-
if overwriteOrAdopt {
357-
// repo should exist but doesn't - We have two or three options.
358-
log.Error("Files do not exist in %s and we are not going to adopt or delete.", repo.FullName())
359-
return repo_model.ErrRepoNotExist{
360-
OwnerName: u.Name,
361-
Name: repo.Name,
362-
}
363-
}
364-
365-
// repo already exists - We have two or three options.
366-
log.Error("Files already exist in %s and we are not going to adopt or delete.", repo.FullName())
367-
return repo_model.ErrRepoFilesAlreadyExist{
368-
Uname: u.Name,
369-
Name: repo.Name,
370-
}
371-
}
372-
373352
if err = db.Insert(ctx, repo); err != nil {
374353
return err
375354
}

‎services/repository/create_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@
44
package repository
55

66
import (
7-
"context"
7+
"os"
88
"testing"
9-
"time"
109

1110
"code.gitea.io/gitea/models/db"
1211
repo_model "code.gitea.io/gitea/models/repo"
@@ -39,10 +38,11 @@ func TestCreateRepositoryDirectly(t *testing.T) {
3938
err = DeleteRepositoryDirectly(db.DefaultContext, user2, createdRepo.ID)
4039
assert.NoError(t, err)
4140

42-
// a failed creating because a timeout
43-
ctx, cancel := context.WithTimeout(db.DefaultContext, 1*time.Millisecond)
44-
defer cancel()
45-
createdRepo2, err := CreateRepositoryDirectly(ctx, user2, user2, CreateRepoOptions{
41+
// a failed creating because some mock data
42+
// create the repository directory so that the creation will fail after database record created.
43+
assert.NoError(t, os.MkdirAll(repo_model.RepoPath(user2.Name, createdRepo.Name), os.ModePerm))
44+
45+
createdRepo2, err := CreateRepositoryDirectly(db.DefaultContext, user2, user2, CreateRepoOptions{
4646
Name: "created-repo",
4747
})
4848
assert.Nil(t, createdRepo2)

‎services/repository/fork.go‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork
105105

106106
// 1 - Create the repository in the database
107107
err = db.WithTx(ctx, func(ctx context.Context) error {
108-
if err = CreateRepositoryInDB(ctx, doer, owner, repo, false, true); err != nil {
108+
if err = createRepositoryInDB(ctx, doer, owner, repo, true); err != nil {
109109
return err
110110
}
111111
if err = repo_model.IncrementRepoForkNum(ctx, opts.BaseRepo.ID); err != nil {
@@ -128,6 +128,22 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork
128128
}
129129
}()
130130

131+
var isExist bool
132+
isExist, err = gitrepo.IsRepositoryExist(ctx, repo)
133+
if err != nil {
134+
log.Error("Unable to check if %s exists. Error: %v", repo.FullName(), err)
135+
return nil, err
136+
}
137+
if isExist {
138+
log.Error("Files already exist in %s and we are not going to adopt or delete.", repo.FullName())
139+
// we need err in defer to cleanupRepository
140+
err = repo_model.ErrRepoFilesAlreadyExist{
141+
Uname: repo.OwnerName,
142+
Name: repo.Name,
143+
}
144+
return nil, err
145+
}
146+
131147
// 2 - Clone the repository
132148
cloneCmd := git.NewCommand("clone", "--bare")
133149
if opts.SingleBranch != "" {

‎services/repository/fork_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@
44
package repository
55

66
import (
7-
"context"
7+
"os"
88
"testing"
9-
"time"
109

1110
"code.gitea.io/gitea/models/db"
1211
repo_model "code.gitea.io/gitea/models/repo"
@@ -72,10 +71,11 @@ func TestForkRepositoryCleanup(t *testing.T) {
7271
err = DeleteRepositoryDirectly(db.DefaultContext, user2, fork.ID)
7372
assert.NoError(t, err)
7473

75-
// a failed fork because a timeout
76-
ctx, cancel := context.WithTimeout(db.DefaultContext, 1*time.Millisecond)
77-
defer cancel()
78-
fork2, err := ForkRepository(ctx, user2, user2, ForkRepoOptions{
74+
// a failed creating because some mock data
75+
// create the repository directory so that the creation will fail after database record created.
76+
assert.NoError(t, os.MkdirAll(repo_model.RepoPath(user2.Name, "test"), os.ModePerm))
77+
78+
fork2, err := ForkRepository(db.DefaultContext, user2, user2, ForkRepoOptions{
7979
BaseRepo: repo10,
8080
Name: "test",
8181
})

‎services/repository/template.go‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -91,22 +91,9 @@ func GenerateRepository(ctx context.Context, doer, owner *user_model.User, templ
9191
Status: repo_model.RepositoryBeingMigrated,
9292
}
9393

94-
// 1 - check whether the repository with the same storage exists
95-
isExist, err := gitrepo.IsRepositoryExist(ctx, generateRepo)
96-
if err != nil {
97-
log.Error("Unable to check if %s exists. Error: %v", generateRepo.FullName(), err)
98-
return nil, err
99-
}
100-
if isExist {
101-
return nil, repo_model.ErrRepoFilesAlreadyExist{
102-
Uname: generateRepo.OwnerName,
103-
Name: generateRepo.Name,
104-
}
105-
}
106-
107-
// 2 - Create the repository in the database
94+
// 1 - Create the repository in the database
10895
if err := db.WithTx(ctx, func(ctx context.Context) error {
109-
return CreateRepositoryInDB(ctx, doer, owner, generateRepo, false, false)
96+
return createRepositoryInDB(ctx, doer, owner, generateRepo, false)
11097
}); err != nil {
11198
return nil, err
11299
}
@@ -119,6 +106,21 @@ func GenerateRepository(ctx context.Context, doer, owner *user_model.User, templ
119106
}
120107
}()
121108

109+
// 2 - check whether the repository with the same storage exists
110+
isExist, err := gitrepo.IsRepositoryExist(ctx, generateRepo)
111+
if err != nil {
112+
log.Error("Unable to check if %s exists. Error: %v", generateRepo.FullName(), err)
113+
return nil, err
114+
}
115+
if isExist {
116+
// we need err in defer to cleanupRepository
117+
err = repo_model.ErrRepoFilesAlreadyExist{
118+
Uname: generateRepo.OwnerName,
119+
Name: generateRepo.Name,
120+
}
121+
return nil, err
122+
}
123+
122124
// 3 - Generate the git repository in storage
123125
if err = repo_module.CheckInitRepository(ctx, generateRepo); err != nil {
124126
return nil, err

‎tests/integration/repo_test.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
package integration
55

66
import (
7-
"context"
87
"fmt"
98
"net/http"
9+
"os"
1010
"path"
1111
"strconv"
1212
"strings"
@@ -502,6 +502,7 @@ func testViewCommit(t *testing.T) {
502502
assert.True(t, test.IsNormalPageCompleted(resp.Body.String()), "non-existing commit should render 404 page")
503503
}
504504

505+
// TestGenerateRepository the test cannot succeed when moved as a unit test
505506
func TestGenerateRepository(t *testing.T) {
506507
defer tests.PrepareTestEnv(t)()
507508

@@ -525,10 +526,11 @@ func TestGenerateRepository(t *testing.T) {
525526
err = repo_service.DeleteRepositoryDirectly(db.DefaultContext, user2, generatedRepo.ID)
526527
assert.NoError(t, err)
527528

528-
// a failed generate because a timeout
529-
ctx, cancel := context.WithTimeout(db.DefaultContext, 1*time.Millisecond)
530-
defer cancel()
531-
generatedRepo2, err := repo_service.GenerateRepository(ctx, user2, user2, repo44, repo_service.GenerateRepoOptions{
529+
// a failed creating because some mock data
530+
// create the repository directory so that the creation will fail after database record created.
531+
assert.NoError(t, os.MkdirAll(repo_model.RepoPath(user2.Name, "generated-from-template-44"), os.ModePerm))
532+
533+
generatedRepo2, err := repo_service.GenerateRepository(db.DefaultContext, user2, user2, repo44, repo_service.GenerateRepoOptions{
532534
Name: "generated-from-template-44",
533535
GitContent: true,
534536
})

0 commit comments

Comments
 (0)