ensure reliable cleanup of test repository (#21880)

internal/checker/checker_test.go

@@ -75,8 +75,7 @@ func assertOnlyMixedPackHints(t *testing.T, hints []error) {
 }
 
 func TestCheckRepo(t *testing.T) {
-	repo, _, cleanup := repository.TestFromFixture(t, checkerTestData)
+	repo, _ := repository.TestFromFixture(t, checkerTestData)
-	defer cleanup()
 
 	chkr := checker.New(repo, false)
 	hints, errs := chkr.LoadIndex(context.TODO(), nil)
@@ -93,8 +92,7 @@ func TestCheckRepo(t *testing.T) {
 }
 
 func TestMissingPack(t *testing.T) {
-	repo, be, cleanup := repository.TestFromFixture(t, checkerTestData)
+	repo, be := repository.TestFromFixture(t, checkerTestData)
-	defer cleanup()
 
 	packID := restic.TestParseID("657f7fb64f6a854fff6fe9279998ee09034901eded4e6db9bcee0e59745bbce6")
 	test.OK(t, be.Remove(context.TODO(), backend.Handle{Type: restic.PackFile, Name: packID.String()}))
@@ -119,8 +117,7 @@ func TestMissingPack(t *testing.T) {
 }
 
 func TestUnreferencedPack(t *testing.T) {
-	repo, be, cleanup := repository.TestFromFixture(t, checkerTestData)
+	repo, be := repository.TestFromFixture(t, checkerTestData)
-	defer cleanup()
 
 	// index 3f1a only references pack 60e0
 	packID := "60e0438dcb978ec6860cc1f8c43da648170ee9129af8f650f876bad19f8f788e"
@@ -147,8 +144,7 @@ func TestUnreferencedPack(t *testing.T) {
 }
 
 func TestUnreferencedBlobs(t *testing.T) {
-	repo, be, cleanup := repository.TestFromFixture(t, checkerTestData)
+	repo, be := repository.TestFromFixture(t, checkerTestData)
-	defer cleanup()
 
 	snapshotID := restic.TestParseID("51d249d28815200d59e4be7b3f21a157b864dc343353df9d8e498220c2499b02")
 	test.OK(t, be.Remove(context.TODO(), backend.Handle{Type: restic.SnapshotFile, Name: snapshotID.String()}))
@@ -182,8 +178,7 @@ func TestUnreferencedBlobs(t *testing.T) {
 }
 
 func TestModifiedIndex(t *testing.T) {
-	repo, be, cleanup := repository.TestFromFixture(t, checkerTestData)
+	repo, be := repository.TestFromFixture(t, checkerTestData)
-	defer cleanup()
 
 	done := make(chan struct{})
 	defer close(done)
@@ -259,8 +254,7 @@ func TestModifiedIndex(t *testing.T) {
 var checkerDuplicateIndexTestData = filepath.Join("testdata", "duplicate-packs-in-index-test-repo.tar.gz")
 
 func TestDuplicatePacksInIndex(t *testing.T) {
-	repo, _, cleanup := repository.TestFromFixture(t, checkerDuplicateIndexTestData)
+	repo, _ := repository.TestFromFixture(t, checkerDuplicateIndexTestData)
-	defer cleanup()
 
 	chkr := checker.New(repo, false)
 	hints, errs := chkr.LoadIndex(context.TODO(), nil)
@@ -459,8 +453,7 @@ func (r *loadTreesOnceRepository) LoadBlob(ctx context.Context, t restic.BlobTyp
 }
 
 func TestCheckerNoDuplicateTreeDecodes(t *testing.T) {
-	repo, _, cleanup := repository.TestFromFixture(t, checkerTestData)
+	repo, _ := repository.TestFromFixture(t, checkerTestData)
-	defer cleanup()
 	checkRepo := &loadTreesOnceRepository{
 		Repository:  repo,
 		loadedTrees: restic.NewIDSet(),
@@ -608,13 +601,12 @@ func TestCheckerBlobTypeConfusion(t *testing.T) {
 	test.Assert(t, delayRepo.Triggered, "delay repository did not trigger")
 }
 
-func loadBenchRepository(t *testing.B) (*checker.Checker, restic.Repository, func()) {
+func loadBenchRepository(t *testing.B) (*checker.Checker, restic.Repository) {
-	repo, _, cleanup := repository.TestFromFixture(t, checkerTestData)
+	repo, _ := repository.TestFromFixture(t, checkerTestData)
 
 	chkr := checker.New(repo, false)
 	hints, errs := chkr.LoadIndex(context.TODO(), nil)
 	if len(errs) > 0 {
-		defer cleanup()
 		t.Fatalf("expected no errors, got %v: %v", len(errs), errs)
 	}
 
@@ -623,12 +615,11 @@ func loadBenchRepository(t *testing.B) (*checker.Checker, restic.Repository, fun
 			t.Fatalf("expected mixed pack hint, got %v", err)
 		}
 	}
-	return chkr, repo, cleanup
+	return chkr, repo
 }
 
 func BenchmarkChecker(t *testing.B) {
-	chkr, _, cleanup := loadBenchRepository(t)
+	chkr, _ := loadBenchRepository(t)
-	defer cleanup()
 
 	t.ResetTimer()
 
@@ -640,8 +631,7 @@ func BenchmarkChecker(t *testing.B) {
 }
 
 func benchmarkSnapshotScaling(t *testing.B, newSnapshots int) {
-	chkr, repo, cleanup := loadBenchRepository(t)
+	chkr, repo := loadBenchRepository(t)
-	defer cleanup()
 
 	snID := restic.TestParseID("51d249d28815200d59e4be7b3f21a157b864dc343353df9d8e498220c2499b02")
 	sn2, err := data.LoadSnapshot(context.TODO(), repo, snID)

internal/repository/index/index_parallel_test.go

@@ -15,8 +15,7 @@ import (
 var repoFixture = filepath.Join("..", "testdata", "test-repo.tar.gz")
 
 func TestRepositoryForAllIndexes(t *testing.T) {
-	repo, _, cleanup := repository.TestFromFixture(t, repoFixture)
+	repo, _ := repository.TestFromFixture(t, repoFixture)
-	defer cleanup()
 
 	expectedIndexIDs := restic.NewIDSet()
 	rtest.OK(t, repo.List(context.TODO(), restic.IndexFile, func(id restic.ID, size int64) error {

internal/repository/repository_test.go

@@ -319,8 +319,7 @@ func benchmarkLoadUnpacked(b *testing.B, version uint) {
 var repoFixture = filepath.Join("testdata", "test-repo.tar.gz")
 
 func TestRepositoryLoadIndex(t *testing.T) {
-	repo, _, cleanup := repository.TestFromFixture(t, repoFixture)
+	repo, _ := repository.TestFromFixture(t, repoFixture)
-	defer cleanup()
 
 	rtest.OK(t, repo.LoadIndex(context.TODO(), nil))
 }
@@ -373,8 +372,7 @@ func (be *damageOnceBackend) Load(ctx context.Context, h backend.Handle, length
 }
 
 func TestRepositoryLoadUnpackedRetryBroken(t *testing.T) {
-	repodir, cleanup := rtest.Env(t, repoFixture)
+	repodir := rtest.Env(t, repoFixture)
-	defer cleanup()
 
 	be, err := local.Open(context.TODO(), local.Config{Path: repodir, Connections: 2}, t.Logf)
 	rtest.OK(t, err)

internal/repository/testing.go

@@ -105,11 +105,11 @@ func TestRepositoryWithVersion(t testing.TB, version uint) (*Repository, restic.
 	return repo, &internalRepository{repo}, be
 }
 
-func TestFromFixture(t testing.TB, repoFixture string) (*Repository, backend.Backend, func()) {
+func TestFromFixture(t testing.TB, repoFixture string) (*Repository, backend.Backend) {
-	repodir, cleanup := test.Env(t, repoFixture)
+	repodir := test.Env(t, repoFixture)
 	repo, be := TestOpenLocal(t, repodir)
 
-	return repo, be, cleanup
+	return repo, be
 }
 
 // TestOpenLocal opens a local repository.

internal/test/helpers.go

@@ -137,10 +137,18 @@ func SetupTarTestFixture(t testing.TB, outputDir, tarFile string) {
 
 // Env creates a test environment and extracts the repository fixture.
 // Returned is the repo path and a cleanup function.
-func Env(t testing.TB, repoFixture string) (repodir string, cleanup func()) {
+func Env(t testing.TB, repoFixture string) string {
 	t.Helper()
-	tempdir, err := os.MkdirTemp(TestTempDir, "restic-test-env-")
+
-	OK(t, err)
+	var tempdir string
+	if TestCleanupTempDirs {
+		tempdir = t.TempDir()
+	} else {
+		var err error
+		tempdir, err = os.MkdirTemp(TestTempDir, "restic-test-env-")
+		OK(t, err)
+		t.Logf("leaving temporary directory %v used for test", tempdir)
+	}
 
 	fd, err := os.Open(repoFixture)
 	if err != nil {
@@ -150,14 +158,7 @@ func Env(t testing.TB, repoFixture string) (repodir string, cleanup func()) {
 
 	SetupTarTestFixture(t, tempdir, repoFixture)
 
-	return filepath.Join(tempdir, "repo"), func() {
+	return filepath.Join(tempdir, "repo")
-		if !TestCleanupTempDirs {
-			t.Logf("leaving temporary directory %v used for test", tempdir)
-			return
-		}
-
-		RemoveAll(t, tempdir)
-	}
 }
 
 func isFile(fi os.FileInfo) bool {