Skip to content

Commit ebf9b4d

Browse files
lunnylunny
authored
Use a migration test instead of a wrong test which populated the meta test repositories and fix a migration bug (#36160)
The test `TestGiteaUploadUpdateGitForPullRequest` modified the shared meta test repositories directly, so this PR removes that test and replaces it with an integration test that migrates a real repository from gitea.com into a local test instance. This PR also fixes a bug where pull-request migrations were not correctly syncing head branches to the database.
1 parent ad49b7b commit ebf9b4d

File tree

6 files changed

+200
-302
lines changed

6 files changed

+200
-302
lines changed

‎modules/migration/uploader.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ type Uploader interface {
1414
CreateMilestones(ctx context.Context, milestones ...*Milestone) error
1515
CreateReleases(ctx context.Context, releases ...*Release) error
1616
SyncTags(ctx context.Context) error
17+
SyncBranches(ctx context.Context) error
1718
CreateLabels(ctx context.Context, labels ...*Label) error
1819
CreateIssues(ctx context.Context, issues ...*Issue) error
1920
CreateComments(ctx context.Context, comments ...*Comment) error

‎services/migrations/dump.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,11 @@ func (g *RepositoryDumper) SyncTags(ctx context.Context) error {
358358
return nil
359359
}
360360

361+
// SyncBranches syncs branches in the database
362+
func (g *RepositoryDumper) SyncBranches(ctx context.Context) error {
363+
return nil
364+
}
365+
361366
// CreateIssues creates issues
362367
func (g *RepositoryDumper) CreateIssues(_ context.Context, issues ...*base.Issue) error {
363368
var err error

‎services/migrations/gitea_uploader.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,11 @@ func (g *GiteaLocalUploader) SyncTags(ctx context.Context) error {
368368
return repo_module.SyncReleasesWithTags(ctx, g.repo, g.gitRepo)
369369
}
370370

371+
func (g *GiteaLocalUploader) SyncBranches(ctx context.Context) error {
372+
_, err := repo_module.SyncRepoBranchesWithRepo(ctx, g.repo, g.gitRepo, g.doer.ID)
373+
return err
374+
}
375+
371376
// CreateIssues creates issues
372377
func (g *GiteaLocalUploader) CreateIssues(ctx context.Context, issues ...*base.Issue) error {
373378
iss := make([]*issues_model.Issue, 0, len(issues))

‎services/migrations/gitea_uploader_test.go‎

Lines changed: 0 additions & 302 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,6 @@
55
package migrations
66

77
import (
8-
"fmt"
9-
"os"
10-
"path/filepath"
118
"strconv"
129
"testing"
1310
"time"
@@ -17,15 +14,10 @@ import (
1714
repo_model "code.gitea.io/gitea/models/repo"
1815
"code.gitea.io/gitea/models/unittest"
1916
user_model "code.gitea.io/gitea/models/user"
20-
"code.gitea.io/gitea/modules/git"
21-
"code.gitea.io/gitea/modules/git/gitcmd"
22-
"code.gitea.io/gitea/modules/gitrepo"
2317
"code.gitea.io/gitea/modules/graceful"
24-
"code.gitea.io/gitea/modules/log"
2518
base "code.gitea.io/gitea/modules/migration"
2619
"code.gitea.io/gitea/modules/optional"
2720
"code.gitea.io/gitea/modules/structs"
28-
"code.gitea.io/gitea/modules/test"
2921
repo_service "code.gitea.io/gitea/services/repository"
3022

3123
"github.com/stretchr/testify/assert"
@@ -228,297 +220,3 @@ func TestGiteaUploadRemapExternalUser(t *testing.T) {
228220
assert.NoError(t, err)
229221
assert.Equal(t, linkedUser.ID, target.GetUserID())
230222
}
231-
232-
func TestGiteaUploadUpdateGitForPullRequest(t *testing.T) {
233-
unittest.PrepareTestEnv(t)
234-
235-
//
236-
// fromRepo master
237-
//
238-
fromRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
239-
baseRef := "master"
240-
// this is very different from the real situation. It should be a bare repository for all the Gitea managed repositories
241-
assert.NoError(t, git.InitRepository(t.Context(), fromRepo.RepoPath(), false, fromRepo.ObjectFormatName))
242-
err := gitrepo.RunCmd(t.Context(), fromRepo, gitcmd.NewCommand("symbolic-ref").AddDynamicArguments("HEAD", git.BranchPrefix+baseRef))
243-
assert.NoError(t, err)
244-
assert.NoError(t, os.WriteFile(filepath.Join(fromRepo.RepoPath(), "README.md"), []byte("# Testing Repository\n\nOriginally created in: "+fromRepo.RepoPath()), 0o644))
245-
assert.NoError(t, git.AddChanges(t.Context(), fromRepo.RepoPath(), true))
246-
signature := git.Signature{
247-
Email: "test@example.com",
248-
Name: "test",
249-
When: time.Now(),
250-
}
251-
assert.NoError(t, git.CommitChanges(t.Context(), fromRepo.RepoPath(), git.CommitChangesOptions{
252-
Committer: &signature,
253-
Author: &signature,
254-
Message: "Initial Commit",
255-
}))
256-
fromGitRepo, err := gitrepo.OpenRepository(t.Context(), fromRepo)
257-
assert.NoError(t, err)
258-
defer fromGitRepo.Close()
259-
baseSHA, err := fromGitRepo.GetBranchCommitID(baseRef)
260-
assert.NoError(t, err)
261-
262-
//
263-
// fromRepo branch1
264-
//
265-
headRef := "branch1"
266-
_, err = gitrepo.RunCmdString(t.Context(), fromRepo, gitcmd.NewCommand("checkout", "-b").AddDynamicArguments(headRef))
267-
assert.NoError(t, err)
268-
assert.NoError(t, os.WriteFile(filepath.Join(fromRepo.RepoPath(), "README.md"), []byte("SOMETHING"), 0o644))
269-
assert.NoError(t, git.AddChanges(t.Context(), fromRepo.RepoPath(), true))
270-
signature.When = time.Now()
271-
assert.NoError(t, git.CommitChanges(t.Context(), fromRepo.RepoPath(), git.CommitChangesOptions{
272-
Committer: &signature,
273-
Author: &signature,
274-
Message: "Pull request",
275-
}))
276-
assert.NoError(t, err)
277-
headSHA, err := fromGitRepo.GetBranchCommitID(headRef)
278-
assert.NoError(t, err)
279-
280-
fromRepoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: fromRepo.OwnerID})
281-
282-
//
283-
// forkRepo branch2
284-
//
285-
forkHeadRef := "branch2"
286-
forkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 8})
287-
assert.NoError(t, git.Clone(t.Context(), fromRepo.RepoPath(), forkRepo.RepoPath(), git.CloneRepoOptions{
288-
Branch: headRef,
289-
}))
290-
_, err = gitrepo.RunCmdString(t.Context(), forkRepo, gitcmd.NewCommand("checkout", "-b").AddDynamicArguments(forkHeadRef))
291-
assert.NoError(t, err)
292-
assert.NoError(t, os.WriteFile(filepath.Join(forkRepo.RepoPath(), "README.md"), []byte("# branch2 "+forkRepo.RepoPath()), 0o644))
293-
assert.NoError(t, git.AddChanges(t.Context(), forkRepo.RepoPath(), true))
294-
assert.NoError(t, git.CommitChanges(t.Context(), forkRepo.RepoPath(), git.CommitChangesOptions{
295-
Committer: &signature,
296-
Author: &signature,
297-
Message: "branch2 commit",
298-
}))
299-
forkGitRepo, err := gitrepo.OpenRepository(t.Context(), forkRepo)
300-
assert.NoError(t, err)
301-
defer forkGitRepo.Close()
302-
forkHeadSHA, err := forkGitRepo.GetBranchCommitID(forkHeadRef)
303-
assert.NoError(t, err)
304-
305-
toRepoName := "migrated"
306-
ctx := t.Context()
307-
uploader := NewGiteaLocalUploader(ctx, fromRepoOwner, fromRepoOwner.Name, toRepoName)
308-
uploader.gitServiceType = structs.GiteaService
309-
310-
assert.NoError(t, repo_service.Init(t.Context()))
311-
assert.NoError(t, uploader.CreateRepo(ctx, &base.Repository{
312-
Description: "description",
313-
OriginalURL: fromRepo.RepoPath(),
314-
CloneURL: fromRepo.RepoPath(),
315-
IsPrivate: false,
316-
IsMirror: true,
317-
}, base.MigrateOptions{
318-
GitServiceType: structs.GiteaService,
319-
Private: false,
320-
Mirror: true,
321-
}))
322-
323-
for _, testCase := range []struct {
324-
name string
325-
head string
326-
logFilter []string
327-
logFiltered []bool
328-
pr base.PullRequest
329-
}{
330-
{
331-
name: "fork, good Head.SHA",
332-
head: fmt.Sprintf("%s/%s", forkRepo.OwnerName, forkHeadRef),
333-
pr: base.PullRequest{
334-
PatchURL: "",
335-
Number: 1,
336-
State: "open",
337-
Base: base.PullRequestBranch{
338-
CloneURL: fromRepo.RepoPath(),
339-
Ref: baseRef,
340-
SHA: baseSHA,
341-
RepoName: fromRepo.Name,
342-
OwnerName: fromRepo.OwnerName,
343-
},
344-
Head: base.PullRequestBranch{
345-
CloneURL: forkRepo.RepoPath(),
346-
Ref: forkHeadRef,
347-
SHA: forkHeadSHA,
348-
RepoName: forkRepo.Name,
349-
OwnerName: forkRepo.OwnerName,
350-
},
351-
},
352-
},
353-
{
354-
name: "fork, invalid Head.Ref",
355-
head: "unknown repository",
356-
pr: base.PullRequest{
357-
PatchURL: "",
358-
Number: 1,
359-
State: "open",
360-
Base: base.PullRequestBranch{
361-
CloneURL: fromRepo.RepoPath(),
362-
Ref: baseRef,
363-
SHA: baseSHA,
364-
RepoName: fromRepo.Name,
365-
OwnerName: fromRepo.OwnerName,
366-
},
367-
Head: base.PullRequestBranch{
368-
CloneURL: forkRepo.RepoPath(),
369-
Ref: "INVALID",
370-
SHA: forkHeadSHA,
371-
RepoName: forkRepo.Name,
372-
OwnerName: forkRepo.OwnerName,
373-
},
374-
},
375-
logFilter: []string{"Fetch branch from"},
376-
logFiltered: []bool{true},
377-
},
378-
{
379-
name: "invalid fork CloneURL",
380-
head: "unknown repository",
381-
pr: base.PullRequest{
382-
PatchURL: "",
383-
Number: 1,
384-
State: "open",
385-
Base: base.PullRequestBranch{
386-
CloneURL: fromRepo.RepoPath(),
387-
Ref: baseRef,
388-
SHA: baseSHA,
389-
RepoName: fromRepo.Name,
390-
OwnerName: fromRepo.OwnerName,
391-
},
392-
Head: base.PullRequestBranch{
393-
CloneURL: "UNLIKELY",
394-
Ref: forkHeadRef,
395-
SHA: forkHeadSHA,
396-
RepoName: forkRepo.Name,
397-
OwnerName: "WRONG",
398-
},
399-
},
400-
logFilter: []string{"AddRemote"},
401-
logFiltered: []bool{true},
402-
},
403-
{
404-
name: "no fork, good Head.SHA",
405-
head: headRef,
406-
pr: base.PullRequest{
407-
PatchURL: "",
408-
Number: 1,
409-
State: "open",
410-
Base: base.PullRequestBranch{
411-
CloneURL: fromRepo.RepoPath(),
412-
Ref: baseRef,
413-
SHA: baseSHA,
414-
RepoName: fromRepo.Name,
415-
OwnerName: fromRepo.OwnerName,
416-
},
417-
Head: base.PullRequestBranch{
418-
CloneURL: fromRepo.RepoPath(),
419-
Ref: headRef,
420-
SHA: headSHA,
421-
RepoName: fromRepo.Name,
422-
OwnerName: fromRepo.OwnerName,
423-
},
424-
},
425-
},
426-
{
427-
name: "no fork, empty Head.SHA",
428-
head: headRef,
429-
pr: base.PullRequest{
430-
PatchURL: "",
431-
Number: 1,
432-
State: "open",
433-
Base: base.PullRequestBranch{
434-
CloneURL: fromRepo.RepoPath(),
435-
Ref: baseRef,
436-
SHA: baseSHA,
437-
RepoName: fromRepo.Name,
438-
OwnerName: fromRepo.OwnerName,
439-
},
440-
Head: base.PullRequestBranch{
441-
CloneURL: fromRepo.RepoPath(),
442-
Ref: headRef,
443-
SHA: "",
444-
RepoName: fromRepo.Name,
445-
OwnerName: fromRepo.OwnerName,
446-
},
447-
},
448-
logFilter: []string{"Empty reference", "Cannot remove local head"},
449-
logFiltered: []bool{true, false},
450-
},
451-
{
452-
name: "no fork, invalid Head.SHA",
453-
head: headRef,
454-
pr: base.PullRequest{
455-
PatchURL: "",
456-
Number: 1,
457-
State: "open",
458-
Base: base.PullRequestBranch{
459-
CloneURL: fromRepo.RepoPath(),
460-
Ref: baseRef,
461-
SHA: baseSHA,
462-
RepoName: fromRepo.Name,
463-
OwnerName: fromRepo.OwnerName,
464-
},
465-
Head: base.PullRequestBranch{
466-
CloneURL: fromRepo.RepoPath(),
467-
Ref: headRef,
468-
SHA: "brokenSHA",
469-
RepoName: fromRepo.Name,
470-
OwnerName: fromRepo.OwnerName,
471-
},
472-
},
473-
logFilter: []string{"Deprecated local head"},
474-
logFiltered: []bool{true},
475-
},
476-
{
477-
name: "no fork, not found Head.SHA",
478-
head: headRef,
479-
pr: base.PullRequest{
480-
PatchURL: "",
481-
Number: 1,
482-
State: "open",
483-
Base: base.PullRequestBranch{
484-
CloneURL: fromRepo.RepoPath(),
485-
Ref: baseRef,
486-
SHA: baseSHA,
487-
RepoName: fromRepo.Name,
488-
OwnerName: fromRepo.OwnerName,
489-
},
490-
Head: base.PullRequestBranch{
491-
CloneURL: fromRepo.RepoPath(),
492-
Ref: headRef,
493-
SHA: "2697b352310fcd01cbd1f3dbd43b894080027f68",
494-
RepoName: fromRepo.Name,
495-
OwnerName: fromRepo.OwnerName,
496-
},
497-
},
498-
logFilter: []string{"Deprecated local head", "Cannot remove local head"},
499-
logFiltered: []bool{true, false},
500-
},
501-
} {
502-
t.Run(testCase.name, func(t *testing.T) {
503-
stopMark := fmt.Sprintf(">>>>>>>>>>>>>STOP: %s<<<<<<<<<<<<<<<", testCase.name)
504-
505-
logChecker, cleanup := test.NewLogChecker(log.DEFAULT)
506-
logChecker.Filter(testCase.logFilter...).StopMark(stopMark)
507-
defer cleanup()
508-
509-
testCase.pr.EnsuredSafe = true
510-
511-
head, err := uploader.updateGitForPullRequest(ctx, &testCase.pr)
512-
assert.NoError(t, err)
513-
assert.Equal(t, testCase.head, head)
514-
515-
log.Info(stopMark)
516-
517-
logFiltered, logStopped := logChecker.Check(5 * time.Second)
518-
assert.True(t, logStopped)
519-
if len(testCase.logFilter) > 0 {
520-
assert.Equal(t, testCase.logFiltered, logFiltered, "for log message filters: %v", testCase.logFilter)
521-
}
522-
})
523-
}
524-
}

‎services/migrations/migrate.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,15 @@ func migrateRepository(ctx context.Context, doer *user_model.User, downloader ba
478478
break
479479
}
480480
}
481+
if len(mapInsertedPRIndexes) > 0 {
482+
// The pull requests migrating process may created head branches in the base repository
483+
// because head repository maybe a fork one which will not be migrated. So that we need
484+
// to sync branches again.
485+
log.Trace("syncing branches after migrating pull requests")
486+
if err = uploader.SyncBranches(ctx); err != nil {
487+
return err
488+
}
489+
}
481490
}
482491

483492
if opts.Comments && supportAllComments {

0 commit comments

Comments
 (0)