Ensure git tag tests and others create test repos in tmpdir

modules/git/commit_info_test.go

@@ -17,21 +17,24 @@ import (
 )
 
 const (
-	testReposDir      = "tests/repos/"
-	benchmarkReposDir = "benchmark/repos/"
+	testReposDir = "tests/repos/"
 )
 
-func cloneRepo(url, dir, name string) (string, error) {
-	repoDir := filepath.Join(dir, name)
-	if _, err := os.Stat(repoDir); err == nil {
-		return repoDir, nil
+func cloneRepo(url, name string) (string, error) {
+	repoDir, err := os.MkdirTemp("", name)
+	if err != nil {
+		return "", err
 	}
-	return repoDir, Clone(DefaultContext, url, repoDir, CloneRepoOptions{
+	if err := Clone(DefaultContext, url, repoDir, CloneRepoOptions{
 		Mirror:  false,
 		Bare:    false,
 		Quiet:   true,
 		Timeout: 5 * time.Minute,
-	})
+	}); err != nil {
+		_ = util.RemoveAll(repoDir)
+		return "", err
+	}
+	return repoDir, nil
 }
 
 func testGetCommitsInfo(t *testing.T, repo1 *Repository) {
@@ -61,20 +64,35 @@ func testGetCommitsInfo(t *testing.T, repo1 *Repository) {
 	}
 	for _, testCase := range testCases {
 		commit, err := repo1.GetCommit(testCase.CommitID)
-		assert.NoError(t, err)
+		if err != nil {
+			assert.NoError(t, err, "Unable to get commit: %s from testcase due to error: %v", testCase.CommitID, err)
+			// no point trying to do anything else for this test.
+			continue
+		}
 		assert.NotNil(t, commit)
 		assert.NotNil(t, commit.Tree)
 		assert.NotNil(t, commit.Tree.repo)
 
 		tree, err := commit.Tree.SubTree(testCase.Path)
+		if err != nil {
+			assert.NoError(t, err, "Unable to get subtree: %s of commit: %s from testcase due to error: %v", testCase.Path, testCase.CommitID, err)
+			// no point trying to do anything else for this test.
+			continue
+		}
+
 		assert.NotNil(t, tree, "tree is nil for testCase CommitID %s in Path %s", testCase.CommitID, testCase.Path)
 		assert.NotNil(t, tree.repo, "repo is nil for testCase CommitID %s in Path %s", testCase.CommitID, testCase.Path)
 
-		assert.NoError(t, err)
 		entries, err := tree.ListEntries()
-		assert.NoError(t, err)
-		commitsInfo, treeCommit, err := entries.GetCommitsInfo(context.Background(), commit, testCase.Path, nil)
-		assert.NoError(t, err)
+		if err != nil {
+			assert.NoError(t, err, "Unable to get entries of subtree: %s in commit: %s from testcase due to error: %v", testCase.Path, testCase.CommitID, err)
+			// no point trying to do anything else for this test.
+			continue
+		}
+
+		// FIXME: Context.TODO() - if graceful has started we should use its Shutdown context otherwise use install signals in TestMain.
+		commitsInfo, treeCommit, err := entries.GetCommitsInfo(context.TODO(), commit, testCase.Path, nil)
+		assert.NoError(t, err, "Unable to get commit information for entries of subtree: %s in commit: %s from testcase due to error: %v", testCase.Path, testCase.CommitID, err)
 		if err != nil {
 			t.FailNow()
 		}
@@ -100,40 +118,52 @@ func TestEntries_GetCommitsInfo(t *testing.T) {
 
 	testGetCommitsInfo(t, bareRepo1)
 
-	clonedPath, err := cloneRepo(bareRepo1Path, testReposDir, "repo1_TestEntries_GetCommitsInfo")
-	assert.NoError(t, err)
+	clonedPath, err := cloneRepo(bareRepo1Path, "repo1_TestEntries_GetCommitsInfo")
+	if err != nil {
+		assert.NoError(t, err)
+	}
 	defer util.RemoveAll(clonedPath)
 	clonedRepo1, err := OpenRepository(clonedPath)
-	assert.NoError(t, err)
+	if err != nil {
+		assert.NoError(t, err)
+	}
 	defer clonedRepo1.Close()
 
 	testGetCommitsInfo(t, clonedRepo1)
 }
 
 func BenchmarkEntries_GetCommitsInfo(b *testing.B) {
-	benchmarks := []struct {
+	type benchmarkType struct {
 		url  string
 		name string
-	}{
+	}
+
+	benchmarks := []benchmarkType{
 		{url: "https://github.com/go-gitea/gitea.git", name: "gitea"},
 		{url: "https://github.com/ethantkoenig/manyfiles.git", name: "manyfiles"},
 		{url: "https://github.com/moby/moby.git", name: "moby"},
 		{url: "https://github.com/golang/go.git", name: "go"},
 		{url: "https://github.com/torvalds/linux.git", name: "linux"},
 	}
-	for _, benchmark := range benchmarks {
+
+	doBenchmark := func(benchmark benchmarkType) {
 		var commit *Commit
 		var entries Entries
 		var repo *Repository
-		if repoPath, err := cloneRepo(benchmark.url, benchmarkReposDir, benchmark.name); err != nil {
+		repoPath, err := cloneRepo(benchmark.url, benchmark.name)
+		if err != nil {
 			b.Fatal(err)
-		} else if repo, err = OpenRepository(repoPath); err != nil {
+		}
+		defer util.RemoveAll(repoPath)
+
+		if repo, err = OpenRepository(repoPath); err != nil {
 			b.Fatal(err)
-		} else if commit, err = repo.GetBranchCommit("master"); err != nil {
-			repo.Close()
+		}
+		defer repo.Close()
+
+		if commit, err = repo.GetBranchCommit("master"); err != nil {
 			b.Fatal(err)
 		} else if entries, err = commit.Tree.ListEntries(); err != nil {
-			repo.Close()
 			b.Fatal(err)
 		}
 		entries.Sort()
@@ -146,6 +176,9 @@ func BenchmarkEntries_GetCommitsInfo(b *testing.B) {
 				}
 			}
 		})
-		repo.Close()
+	}
+
+	for _, benchmark := range benchmarks {
+		doBenchmark(benchmark)
 	}
 }

modules/git/repo_compare_test.go

@@ -17,17 +17,33 @@ import (
 
 func TestGetFormatPatch(t *testing.T) {
 	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
-	clonedPath, err := cloneRepo(bareRepo1Path, testReposDir, "repo1_TestGetFormatPatch")
+	clonedPath, err := cloneRepo(bareRepo1Path, "repo1_TestGetFormatPatch")
+	if err != nil {
+		assert.NoError(t, err)
+		return
+	}
 	defer util.RemoveAll(clonedPath)
-	assert.NoError(t, err)
+
 	repo, err := OpenRepository(clonedPath)
+	if err != nil {
+		assert.NoError(t, err)
+		return
+	}
 	defer repo.Close()
-	assert.NoError(t, err)
+
 	rd := &bytes.Buffer{}
 	err = repo.GetPatch("8d92fc95^", "8d92fc95", rd)
-	assert.NoError(t, err)
+	if err != nil {
+		assert.NoError(t, err)
+		return
+	}
+
 	patchb, err := io.ReadAll(rd)
-	assert.NoError(t, err)
+	if err != nil {
+		assert.NoError(t, err)
+		return
+	}
+
 	patch := string(patchb)
 	assert.Regexp(t, "^From 8d92fc95", patch)
 	assert.Contains(t, patch, "Subject: [PATCH] Add file2.txt")
@@ -37,17 +53,25 @@ func TestReadPatch(t *testing.T) {
 	// Ensure we can read the patch files
 	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
 	repo, err := OpenRepository(bareRepo1Path)
+	if err != nil {
+		assert.NoError(t, err)
+		return
+	}
 	defer repo.Close()
-	assert.NoError(t, err)
 	// This patch doesn't exist
 	noFile, err := repo.ReadPatchCommit(0)
 	assert.Error(t, err)
+
 	// This patch is an empty one (sometimes it's a 404)
 	noCommit, err := repo.ReadPatchCommit(1)
 	assert.Error(t, err)
+
 	// This patch is legit and should return a commit
 	oldCommit, err := repo.ReadPatchCommit(2)
-	assert.NoError(t, err)
+	if err != nil {
+		assert.NoError(t, err)
+		return
+	}
 
 	assert.Empty(t, noFile)
 	assert.Empty(t, noCommit)
@@ -58,23 +82,45 @@ func TestReadPatch(t *testing.T) {
 func TestReadWritePullHead(t *testing.T) {
 	// Ensure we can write SHA1 head corresponding to PR and open them
 	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
-	repo, err := OpenRepository(bareRepo1Path)
-	assert.NoError(t, err)
+
+	// As we are writing we should clone the repository first
+	clonedPath, err := cloneRepo(bareRepo1Path, "TestReadWritePullHead")
+	if err != nil {
+		assert.NoError(t, err)
+		return
+	}
+	defer util.RemoveAll(clonedPath)
+
+	repo, err := OpenRepository(clonedPath)
+	if err != nil {
+		assert.NoError(t, err)
+		return
+	}
 	defer repo.Close()
+
 	// Try to open non-existing Pull
 	_, err = repo.GetRefCommitID(PullPrefix + "0/head")
 	assert.Error(t, err)
+
 	// Write a fake sha1 with only 40 zeros
 	newCommit := "feaf4ba6bc635fec442f46ddd4512416ec43c2c2"
 	err = repo.SetReference(PullPrefix+"1/head", newCommit)
-	assert.NoError(t, err)
-	// Remove file after the test
-	defer func() {
-		_ = repo.RemoveReference(PullPrefix + "1/head")
-	}()
+	if err != nil {
+		assert.NoError(t, err)
+		return
+	}
+
 	// Read the file created
 	headContents, err := repo.GetRefCommitID(PullPrefix + "1/head")
-	assert.NoError(t, err)
+	if err != nil {
+		assert.NoError(t, err)
+		return
+	}
+
 	assert.Len(t, string(headContents), 40)
 	assert.True(t, string(headContents) == newCommit)
+
+	// Remove file after the test
+	err = repo.RemoveReference(PullPrefix + "1/head")
+	assert.NoError(t, err)
 }