test: consolidate Go tests (#39962)

Consolidate all Go tests. Coverage is identical to before. Go tests are
around 5% faster while being 7% smaller. No non-test code is touched.

services/repository/adopt_test.go

@@ -76,17 +76,13 @@ func TestListUnadoptedRepositories_ListOptions(t *testing.T) {
 		_ = os.Mkdir(filepath.Join(setting.RepoRootPath, unadopted+".git"), 0o755)
 	}
 
-	opts := db.ListOptions{Page: 1, PageSize: 1}
-	repoNames, count, err := ListUnadoptedRepositories(t.Context(), "", &opts)
-	assert.NoError(t, err)
-	assert.EqualValues(t, 2, count)
-	assert.Equal(t, unadoptedList[0], repoNames[0])
-
-	opts = db.ListOptions{Page: 2, PageSize: 1}
-	repoNames, count, err = ListUnadoptedRepositories(t.Context(), "", &opts)
-	assert.NoError(t, err)
-	assert.EqualValues(t, 2, count)
-	assert.Equal(t, unadoptedList[1], repoNames[0])
+	for i, unadopted := range unadoptedList {
+		opts := db.ListOptions{Page: i + 1, PageSize: 1}
+		repoNames, count, err := ListUnadoptedRepositories(t.Context(), "", &opts)
+		assert.NoError(t, err)
+		assert.EqualValues(t, 2, count)
+		assert.Equal(t, unadopted, repoNames[0])
+	}
 }
 
 func TestAdoptRepository(t *testing.T) {

services/repository/avatar_test.go

@@ -17,48 +17,35 @@ import (
 )
 
 func TestUploadAvatar(t *testing.T) {
-	// Generate image
-	myImage := image.NewRGBA(image.Rect(0, 0, 1, 1))
-	var buff bytes.Buffer
-	png.Encode(&buff, myImage)
+	for _, testCase := range []struct {
+		width        int
+		deleteAvatar bool
+	}{
+		{1, false},
+		{5000, false},
+		{1, true},
+	} {
+		// Generate image
+		var buff bytes.Buffer
+		png.Encode(&buff, image.NewRGBA(image.Rect(0, 0, testCase.width, 1)))
 
-	assert.NoError(t, unittest.PrepareTestDatabase())
-	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
+		assert.NoError(t, unittest.PrepareTestDatabase())
+		repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
 
-	err := UploadAvatar(t.Context(), repo, buff.Bytes())
-	assert.NoError(t, err)
-	assert.Equal(t, avatar.HashAvatar(10, buff.Bytes()), repo.Avatar)
-}
-
-func TestUploadBigAvatar(t *testing.T) {
-	// Generate BIG image
-	myImage := image.NewRGBA(image.Rect(0, 0, 5000, 1))
-	var buff bytes.Buffer
-	png.Encode(&buff, myImage)
-
-	assert.NoError(t, unittest.PrepareTestDatabase())
-	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
-
-	err := UploadAvatar(t.Context(), repo, buff.Bytes())
-	assert.Error(t, err)
-}
-
-func TestDeleteAvatar(t *testing.T) {
-	// Generate image
-	myImage := image.NewRGBA(image.Rect(0, 0, 1, 1))
-	var buff bytes.Buffer
-	png.Encode(&buff, myImage)
-
-	assert.NoError(t, unittest.PrepareTestDatabase())
-	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
-
-	err := UploadAvatar(t.Context(), repo, buff.Bytes())
-	assert.NoError(t, err)
-
-	err = DeleteAvatar(t.Context(), repo)
-	assert.NoError(t, err)
-
-	assert.Empty(t, repo.Avatar)
+		err := UploadAvatar(t.Context(), repo, buff.Bytes())
+		if testCase.width == 5000 {
+			assert.Error(t, err)
+			continue
+		}
+		assert.NoError(t, err)
+		if testCase.deleteAvatar {
+			err = DeleteAvatar(t.Context(), repo)
+			assert.NoError(t, err)
+			assert.Empty(t, repo.Avatar)
+		} else {
+			assert.Equal(t, avatar.HashAvatar(10, buff.Bytes()), repo.Avatar)
+		}
+	}
 }
 
 func TestGenerateAvatar(t *testing.T) {

services/repository/collaboration_test.go

@@ -24,14 +24,11 @@ func TestRepository_AddCollaborator(t *testing.T) {
 	repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
 	repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
 	user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
-	testSuccess := func(repo *repo_model.Repository, user *user_model.User) {
+	for _, repo := range []*repo_model.Repository{repo1, repo1, repo3} {
 		assert.NoError(t, repo.LoadOwner(t.Context()))
-		assert.NoError(t, AddOrUpdateCollaborator(t.Context(), repo, user, perm.AccessModeWrite))
-		unittest.CheckConsistencyFor(t, repo, user)
+		assert.NoError(t, AddOrUpdateCollaborator(t.Context(), repo, user4, perm.AccessModeWrite))
+		unittest.CheckConsistencyFor(t, repo, user4)
 	}
-	testSuccess(repo1, user4)
-	testSuccess(repo1, user4)
-	testSuccess(repo3, user4)
 
 	assert.Error(t, AddOrUpdateCollaborator(t.Context(), repo1, user4, perm.AccessModeOwner))
 	assert.NoError(t, AddOrUpdateCollaborator(t.Context(), repo1, user4, perm.AccessModeAdmin))
@@ -44,11 +41,10 @@ func TestRepository_DeleteCollaboration(t *testing.T) {
 	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 22})
 
 	assert.NoError(t, repo.LoadOwner(t.Context()))
-	assert.NoError(t, DeleteCollaboration(t.Context(), repo, user))
-	unittest.AssertNotExistsBean(t, &repo_model.Collaboration{RepoID: repo.ID, UserID: user.ID})
-
-	assert.NoError(t, DeleteCollaboration(t.Context(), repo, user))
-	unittest.AssertNotExistsBean(t, &repo_model.Collaboration{RepoID: repo.ID, UserID: user.ID})
+	for range 2 {
+		assert.NoError(t, DeleteCollaboration(t.Context(), repo, user))
+		unittest.AssertNotExistsBean(t, &repo_model.Collaboration{RepoID: repo.ID, UserID: user.ID})
+	}
 
 	unittest.CheckConsistencyFor(t, &repo_model.Repository{ID: repo.ID})
 }

services/repository/delete_test.go

@@ -34,10 +34,6 @@ func TestTeam_HasRepository(t *testing.T) {
 
 	test(2, 3, true)
 	test(2, 5, false)
-}
-
-func TestTeam_RemoveRepository(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
 
 	testSuccess := func(teamID, repoID int64) {
 		team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: teamID})

services/repository/files/diff_test.go

@@ -31,24 +31,21 @@ func TestGetDiffPreview(t *testing.T) {
 	content := "# repo1\n\nDescription for repo1\nthis is a new line"
 
 	t.Run("Errors", func(t *testing.T) {
-		t.Run("empty repo", func(t *testing.T) {
-			diff, err := GetDiffPreview(ctx, &repo_model.Repository{}, branch, treePath, oldContent, content)
-			assert.Nil(t, diff)
-			assert.EqualError(t, err, "repository does not exist [id: 0, uid: 0, owner_name: , name: ]")
-		})
-
-		t.Run("bad branch", func(t *testing.T) {
-			badBranch := "bad_branch"
-			diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, badBranch, treePath, oldContent, content)
-			assert.Nil(t, diff)
-			assert.EqualError(t, err, "branch does not exist [name: "+badBranch+"]")
-		})
-
-		t.Run("empty treePath", func(t *testing.T) {
-			diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, branch, "", oldContent, content)
-			assert.Nil(t, diff)
-			assert.EqualError(t, err, "path is invalid [path: ]")
-		})
+		for _, test := range []struct {
+			name                            string
+			repo                            *repo_model.Repository
+			branch, treePath, expectedError string
+		}{
+			{"empty repo", &repo_model.Repository{}, branch, treePath, "repository does not exist [id: 0, uid: 0, owner_name: , name: ]"},
+			{"bad branch", ctx.Repo.Repository, "bad_branch", treePath, "branch does not exist [name: bad_branch]"},
+			{"empty treePath", ctx.Repo.Repository, branch, "", "path is invalid [path: ]"},
+		} {
+			t.Run(test.name, func(t *testing.T) {
+				diff, err := GetDiffPreview(ctx, test.repo, test.branch, test.treePath, oldContent, content)
+				assert.Nil(t, diff)
+				assert.EqualError(t, err, test.expectedError)
+			})
+		}
 	})
 
 	expectedDiff := &gitdiff.Diff{
@@ -133,23 +130,20 @@ func TestGetDiffPreview(t *testing.T) {
 		IsIncomplete: false,
 	}
 
-	t.Run("with given branch", func(t *testing.T) {
-		diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, branch, treePath, oldContent, content)
-		assert.NoError(t, err)
-		expectedBs, err := json.Marshal(expectedDiff)
-		assert.NoError(t, err)
-		bs, err := json.Marshal(diff)
-		assert.NoError(t, err)
-		assert.JSONEq(t, string(expectedBs), string(bs))
-	})
-
-	t.Run("empty branch, same results", func(t *testing.T) {
-		diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, "", treePath, oldContent, content)
-		assert.NoError(t, err)
-		expectedBs, err := json.Marshal(expectedDiff)
-		assert.NoError(t, err)
-		bs, err := json.Marshal(diff)
-		assert.NoError(t, err)
-		assert.JSONEq(t, string(expectedBs), string(bs))
-	})
+	for _, test := range []struct {
+		name, branch string
+	}{
+		{"with given branch", branch},
+		{"empty branch, same results", ""},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, test.branch, treePath, oldContent, content)
+			assert.NoError(t, err)
+			expectedBs, err := json.Marshal(expectedDiff)
+			assert.NoError(t, err)
+			bs, err := json.Marshal(diff)
+			assert.NoError(t, err)
+			assert.JSONEq(t, string(expectedBs), string(bs))
+		})
+	}
 }

services/repository/files/tree_test.go

@@ -78,7 +78,7 @@ func TestGetTreeViewNodes(t *testing.T) {
 	}
 	treeNodes, err := GetTreeViewNodes(ctx, curRepoLink, renderedIconPool, ctx.Repo.GitRepo, ctx.Repo.Commit, "", "")
 	assert.NoError(t, err)
-	assert.Equal(t, []*TreeViewNode{
+	expectedNodes := []*TreeViewNode{
 		{
 			EntryName:     "docs",
 			EntryMode:     "tree",
@@ -86,36 +86,22 @@ func TestGetTreeViewNodes(t *testing.T) {
 			EntryIcon:     mockIconForFolder(),
 			EntryIconOpen: mockOpenIconForFolder(),
 		},
-	}, treeNodes)
+	}
+	assert.Equal(t, expectedNodes, treeNodes)
 
 	treeNodes, err = GetTreeViewNodes(ctx, curRepoLink, renderedIconPool, ctx.Repo.GitRepo, ctx.Repo.Commit, "", "docs/README.md")
 	assert.NoError(t, err)
-	assert.Equal(t, []*TreeViewNode{
-		{
-			EntryName:     "docs",
-			EntryMode:     "tree",
-			FullPath:      "docs",
-			EntryIcon:     mockIconForFolder(),
-			EntryIconOpen: mockOpenIconForFolder(),
-			Children: []*TreeViewNode{
-				{
-					EntryName: "README.md",
-					EntryMode: "blob",
-					FullPath:  "docs/README.md",
-					EntryIcon: mockIconForFile(`svg-mfi-readme`),
-				},
-			},
-		},
-	}, treeNodes)
-
-	treeNodes, err = GetTreeViewNodes(ctx, curRepoLink, renderedIconPool, ctx.Repo.GitRepo, ctx.Repo.Commit, "docs", "README.md")
-	assert.NoError(t, err)
-	assert.Equal(t, []*TreeViewNode{
+	expectedNodes[0].Children = []*TreeViewNode{
 		{
 			EntryName: "README.md",
 			EntryMode: "blob",
 			FullPath:  "docs/README.md",
 			EntryIcon: mockIconForFile(`svg-mfi-readme`),
 		},
-	}, treeNodes)
+	}
+	assert.Equal(t, expectedNodes, treeNodes)
+
+	treeNodes, err = GetTreeViewNodes(ctx, curRepoLink, renderedIconPool, ctx.Repo.GitRepo, ctx.Repo.Commit, "docs", "README.md")
+	assert.NoError(t, err)
+	assert.Equal(t, expectedNodes[0].Children, treeNodes)
 }

services/repository/gitgraph/graph_test.go

@@ -114,22 +114,25 @@ func TestReleaseUnusedColors(t *testing.T) {
 		parser.firstAvailable = testcase.firstAvailable
 		parser.firstInUse = testcase.firstInUse
 		parser.releaseUnusedColors()
+		reportColorError := func(format string, color int) {
+			t.Errorf(format,
+				testcase.availableColors,
+				testcase.oldColors,
+				testcase.firstAvailable,
+				testcase.firstInUse,
+				parser.availableColors,
+				parser.oldColors,
+				parser.firstAvailable,
+				parser.firstInUse,
+				color)
+		}
 
 		if parser.firstAvailable == -1 {
 			// All in use
 			for _, color := range parser.availableColors {
 				found := slices.Contains(parser.oldColors, color)
 				if !found {
-					t.Errorf("In testcase:\n%d\t%d\t%d %d =>\n%d\t%d\t%d %d: %d should be available but is not",
-						testcase.availableColors,
-						testcase.oldColors,
-						testcase.firstAvailable,
-						testcase.firstInUse,
-						parser.availableColors,
-						parser.oldColors,
-						parser.firstAvailable,
-						parser.firstInUse,
-						color)
+					reportColorError("In testcase:\n%d\t%d\t%d %d =>\n%d\t%d\t%d %d: %d should be available but is not", color)
 				}
 			}
 		} else if parser.firstInUse != -1 {
@@ -138,48 +141,21 @@ func TestReleaseUnusedColors(t *testing.T) {
 				color := parser.availableColors[i]
 				found := slices.Contains(parser.oldColors, color)
 				if !found {
-					t.Errorf("In testcase:\n%d\t%d\t%d %d =>\n%d\t%d\t%d %d: %d should be available but is not",
-						testcase.availableColors,
-						testcase.oldColors,
-						testcase.firstAvailable,
-						testcase.firstInUse,
-						parser.availableColors,
-						parser.oldColors,
-						parser.firstAvailable,
-						parser.firstInUse,
-						color)
+					reportColorError("In testcase:\n%d\t%d\t%d %d =>\n%d\t%d\t%d %d: %d should be available but is not", color)
 				}
 			}
 			for i := parser.firstAvailable; i != parser.firstInUse; i = (i + 1) % len(parser.availableColors) {
 				color := parser.availableColors[i]
 				found := slices.Contains(parser.oldColors, color)
 				if found {
-					t.Errorf("In testcase:\n%d\t%d\t%d %d =>\n%d\t%d\t%d %d: %d should not be available but is",
-						testcase.availableColors,
-						testcase.oldColors,
-						testcase.firstAvailable,
-						testcase.firstInUse,
-						parser.availableColors,
-						parser.oldColors,
-						parser.firstAvailable,
-						parser.firstInUse,
-						color)
+					reportColorError("In testcase:\n%d\t%d\t%d %d =>\n%d\t%d\t%d %d: %d should not be available but is", color)
 				}
 			}
 		} else {
 			// None in use
 			for _, color := range parser.oldColors {
 				if color != 0 {
-					t.Errorf("In testcase:\n%d\t%d\t%d %d =>\n%d\t%d\t%d %d: %d should not be available but is",
-						testcase.availableColors,
-						testcase.oldColors,
-						testcase.firstAvailable,
-						testcase.firstInUse,
-						parser.availableColors,
-						parser.oldColors,
-						parser.firstAvailable,
-						parser.firstInUse,
-						color)
+					reportColorError("In testcase:\n%d\t%d\t%d %d =>\n%d\t%d\t%d %d: %d should not be available but is", color)
 				}
 			}
 		}

services/repository/license_test.go

@@ -107,49 +107,37 @@ func Test_resolveLicenses(t *testing.T) {
 	require.NoError(t, err)
 	defer gitRepo.Close()
 
-	// 1. repo with no license at all
-	require.NoError(t, git.ForceFastImport(t.Context(), gitRepo, []git.FastImportCommit{{
-		Ref:     "refs/heads/master",
-		Message: "empty",
-	}}))
-	commit, err := gitRepo.GetBranchCommit(t.Context(), "master")
-	require.NoError(t, err)
+	resolve := func(message string, files ...git.FastImportFile) ([]repo_model.DetectedLicense, error) {
+		require.NoError(t, git.ForceFastImport(t.Context(), gitRepo, []git.FastImportCommit{{
+			Ref:     "refs/heads/master",
+			Message: message,
+			Files:   files,
+		}}))
+		commit, err := gitRepo.GetBranchCommit(t.Context(), "master")
+		require.NoError(t, err)
+		return resolveLicenses(t.Context(), gitRepo, commit)
+	}
 
-	licenses, err := resolveLicenses(t.Context(), gitRepo, commit)
+	// 1. repo with no license at all
+	licenses, err := resolve("empty")
 	assert.Empty(t, licenses)
 	assert.NoError(t, err)
 
 	// 2. repo with a plain LICENSE file — classifier should detect MIT
-	require.NoError(t, git.ForceFastImport(t.Context(), gitRepo, []git.FastImportCommit{{
-		Ref:     "refs/heads/master",
-		Message: "add LICENSE",
-		Files: []git.FastImportFile{
-			{Mode: git.EntryModeBlob, Path: "LICENSE", Content: string(mitLicense)},
-		},
-	}}))
-	commit, err = gitRepo.GetBranchCommit(t.Context(), "master")
-	require.NoError(t, err)
-
-	licenses, err = resolveLicenses(t.Context(), gitRepo, commit)
+	licenses, err = resolve("add LICENSE",
+		git.FastImportFile{Mode: git.EntryModeBlob, Path: "LICENSE", Content: string(mitLicense)},
+	)
 	assert.NoError(t, err)
 	assert.Len(t, licenses, 1)
 	assert.Equal(t, "MIT", licenses[0].SPDXID)
 	assert.Equal(t, "LICENSE", licenses[0].LicensePath)
 
 	// 3. repo with REUSE LICENSES/ dir — should take priority over root LICENSE
-	require.NoError(t, git.ForceFastImport(t.Context(), gitRepo, []git.FastImportCommit{{
-		Ref:     "refs/heads/master",
-		Message: "add LICENSES dir",
-		Files: []git.FastImportFile{
-			{Mode: git.EntryModeBlob, Path: "LICENSE", Content: string(mitLicense)},
-			{Mode: git.EntryModeBlob, Path: "LICENSES/MIT.txt", Content: "MIT license text"},
-			{Mode: git.EntryModeBlob, Path: "LICENSES/Apache-2.0.txt", Content: "Apache license text"},
-		},
-	}}))
-	commit, err = gitRepo.GetBranchCommit(t.Context(), "master")
-	require.NoError(t, err)
-
-	licenses, err = resolveLicenses(t.Context(), gitRepo, commit)
+	licenses, err = resolve("add LICENSES dir",
+		git.FastImportFile{Mode: git.EntryModeBlob, Path: "LICENSE", Content: string(mitLicense)},
+		git.FastImportFile{Mode: git.EntryModeBlob, Path: "LICENSES/MIT.txt", Content: "MIT license text"},
+		git.FastImportFile{Mode: git.EntryModeBlob, Path: "LICENSES/Apache-2.0.txt", Content: "Apache license text"},
+	)
 	assert.NoError(t, err)
 	assert.Len(t, licenses, 2)
 
@@ -161,14 +149,7 @@ func Test_resolveLicenses(t *testing.T) {
 	}
 
 	// 4. remove all licenses — should return not exist
-	require.NoError(t, git.ForceFastImport(t.Context(), gitRepo, []git.FastImportCommit{{
-		Ref:     "refs/heads/master",
-		Message: "remove licenses",
-	}}))
-	commit, err = gitRepo.GetBranchCommit(t.Context(), "master")
-	require.NoError(t, err)
-
-	licenses, err = resolveLicenses(t.Context(), gitRepo, commit)
+	licenses, err = resolve("remove licenses")
 	assert.Empty(t, licenses)
 	assert.NoError(t, err)
 }

services/repository/repository_test.go

@@ -71,45 +71,33 @@ func TestRepository_HasWiki(t *testing.T) {
 }
 
 func TestMakeRepoPrivateClearsWatches(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
+	for _, name := range []string{"MakeRepoPrivate", "updateRepository"} {
+		t.Run(name, func(t *testing.T) {
+			assert.NoError(t, unittest.PrepareTestDatabase())
 
-	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
-	assert.False(t, repo.IsPrivate)
+			repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+			assert.False(t, repo.IsPrivate)
 
-	watchers, err := repo_model.GetRepoWatchers(t.Context(), repo.ID, db.ListOptions{Page: 1})
-	require.NoError(t, err)
-	require.NotEmpty(t, watchers)
+			watchers, err := repo_model.GetRepoWatchers(t.Context(), repo.ID, db.ListOptions{Page: 1})
+			require.NoError(t, err)
+			require.NotEmpty(t, watchers)
 
-	assert.NoError(t, MakeRepoPrivate(t.Context(), repo, true))
+			if name == "MakeRepoPrivate" {
+				assert.NoError(t, MakeRepoPrivate(t.Context(), repo, true))
+			} else {
+				repo.IsPrivate = true
+				require.NoError(t, updateRepository(t.Context(), repo, true))
+			}
 
-	watchers, err = repo_model.GetRepoWatchers(t.Context(), repo.ID, db.ListOptions{Page: 1})
-	assert.NoError(t, err)
-	assert.Empty(t, watchers)
+			watchers, err = repo_model.GetRepoWatchers(t.Context(), repo.ID, db.ListOptions{Page: 1})
+			assert.NoError(t, err)
+			assert.Empty(t, watchers)
 
-	updatedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID})
-	assert.True(t, updatedRepo.IsPrivate)
-	assert.Zero(t, updatedRepo.NumWatches)
-}
-
-// TestUpdateRepositoryClearsWatchesOnVisibilityChange ensures the shared updateRepository
-// helper (used by the API EditRepo path) also clears watches when a repo goes private.
-func TestUpdateRepositoryClearsWatchesOnVisibilityChange(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
-	assert.False(t, repo.IsPrivate)
-
-	watchers, err := repo_model.GetRepoWatchers(t.Context(), repo.ID, db.ListOptions{Page: 1})
-	require.NoError(t, err)
-	require.NotEmpty(t, watchers)
-
-	repo.IsPrivate = true
-	require.NoError(t, updateRepository(t.Context(), repo, true))
-
-	watchers, err = repo_model.GetRepoWatchers(t.Context(), repo.ID, db.ListOptions{Page: 1})
-	assert.NoError(t, err)
-	assert.Empty(t, watchers)
-
-	updatedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID})
-	assert.Zero(t, updatedRepo.NumWatches)
+			updatedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID})
+			if name == "MakeRepoPrivate" {
+				assert.True(t, updatedRepo.IsPrivate)
+			}
+			assert.Zero(t, updatedRepo.NumWatches)
+		})
+	}
 }

services/repository/transfer_test.go

@@ -78,32 +78,26 @@ func TestStartRepositoryTransferSetPermission(t *testing.T) {
 	assert.NoError(t, err)
 	assert.False(t, hasAccess)
 
-	t.Run("RevokeAccessAfterRejection", func(t *testing.T) {
-		repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
-		assert.NoError(t, repo.LoadOwner(t.Context()))
-		assert.NoError(t, StartRepositoryTransfer(t.Context(), doer, recipient, repo, nil))
-		hasAccess, err = access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
-		assert.NoError(t, err)
-		assert.True(t, hasAccess)
-		assert.NoError(t, RejectRepositoryTransfer(t.Context(), repo, recipient))
-		hasAccess, err = access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
-		assert.NoError(t, err)
-		assert.False(t, hasAccess)
-	})
-	t.Run("RevokeAccessAfterCancel", func(t *testing.T) {
-		repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
-		assert.NoError(t, repo.LoadOwner(t.Context()))
-		assert.NoError(t, StartRepositoryTransfer(t.Context(), doer, recipient, repo, nil))
-		hasAccess, err = access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
-		assert.NoError(t, err)
-		assert.True(t, hasAccess)
-		transfer, err := repo_model.GetPendingRepositoryTransfer(t.Context(), repo)
-		assert.NoError(t, err)
-		assert.NoError(t, CancelRepositoryTransfer(t.Context(), transfer, doer))
-		hasAccess, err = access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
-		assert.NoError(t, err)
-		assert.False(t, hasAccess)
-	})
+	for _, name := range []string{"RevokeAccessAfterRejection", "RevokeAccessAfterCancel"} {
+		t.Run(name, func(t *testing.T) {
+			repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
+			assert.NoError(t, repo.LoadOwner(t.Context()))
+			assert.NoError(t, StartRepositoryTransfer(t.Context(), doer, recipient, repo, nil))
+			hasAccess, err = access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
+			assert.NoError(t, err)
+			assert.True(t, hasAccess)
+			if name == "RevokeAccessAfterRejection" {
+				assert.NoError(t, RejectRepositoryTransfer(t.Context(), repo, recipient))
+			} else {
+				transfer, err := repo_model.GetPendingRepositoryTransfer(t.Context(), repo)
+				assert.NoError(t, err)
+				assert.NoError(t, CancelRepositoryTransfer(t.Context(), transfer, doer))
+			}
+			hasAccess, err = access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
+			assert.NoError(t, err)
+			assert.False(t, hasAccess)
+		})
+	}
 	t.Run("KeepOriginAccessOnStop", func(t *testing.T) {
 		repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
 		assert.NoError(t, repo.LoadOwner(t.Context()))