From a3fa3eb182b3987bb80c1b7adbea246bd74579cd Mon Sep 17 00:00:00 2001 From: Michael Eischer <9106997+MichaelEischer@users.noreply.github.com> Date: Sun, 14 Jun 2026 14:25:00 +0200 Subject: [PATCH] ensure reliable cleanup of test repository (#21880) --- internal/checker/checker_test.go | 34 +++++++------------ .../repository/index/index_parallel_test.go | 3 +- internal/repository/repository_test.go | 6 ++-- internal/repository/testing.go | 6 ++-- internal/test/helpers.go | 23 +++++++------ 5 files changed, 30 insertions(+), 42 deletions(-) diff --git a/internal/checker/checker_test.go b/internal/checker/checker_test.go index eb53b14c2..3d7cd3c00 100644 --- a/internal/checker/checker_test.go +++ b/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) - defer cleanup() + repo, _ := repository.TestFromFixture(t, checkerTestData) 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) - defer cleanup() + repo, be := repository.TestFromFixture(t, checkerTestData) 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) - defer cleanup() + repo, be := repository.TestFromFixture(t, checkerTestData) // 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) - defer cleanup() + repo, be := repository.TestFromFixture(t, checkerTestData) 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) - defer cleanup() + repo, be := repository.TestFromFixture(t, checkerTestData) 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) - defer cleanup() + repo, _ := repository.TestFromFixture(t, checkerDuplicateIndexTestData) 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) - defer cleanup() + repo, _ := repository.TestFromFixture(t, checkerTestData) 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()) { - repo, _, cleanup := repository.TestFromFixture(t, checkerTestData) +func loadBenchRepository(t *testing.B) (*checker.Checker, restic.Repository) { + 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) - defer cleanup() + chkr, _ := loadBenchRepository(t) t.ResetTimer() @@ -640,8 +631,7 @@ func BenchmarkChecker(t *testing.B) { } func benchmarkSnapshotScaling(t *testing.B, newSnapshots int) { - chkr, repo, cleanup := loadBenchRepository(t) - defer cleanup() + chkr, repo := loadBenchRepository(t) snID := restic.TestParseID("51d249d28815200d59e4be7b3f21a157b864dc343353df9d8e498220c2499b02") sn2, err := data.LoadSnapshot(context.TODO(), repo, snID) diff --git a/internal/repository/index/index_parallel_test.go b/internal/repository/index/index_parallel_test.go index 96f1c2a6a..030fb9ea0 100644 --- a/internal/repository/index/index_parallel_test.go +++ b/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) - defer cleanup() + repo, _ := repository.TestFromFixture(t, repoFixture) expectedIndexIDs := restic.NewIDSet() rtest.OK(t, repo.List(context.TODO(), restic.IndexFile, func(id restic.ID, size int64) error { diff --git a/internal/repository/repository_test.go b/internal/repository/repository_test.go index 7bb89d385..5e0e04376 100644 --- a/internal/repository/repository_test.go +++ b/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) - defer cleanup() + repo, _ := repository.TestFromFixture(t, repoFixture) 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) - defer cleanup() + repodir := rtest.Env(t, repoFixture) be, err := local.Open(context.TODO(), local.Config{Path: repodir, Connections: 2}, t.Logf) rtest.OK(t, err) diff --git a/internal/repository/testing.go b/internal/repository/testing.go index 1cdca4110..3f8473f4a 100644 --- a/internal/repository/testing.go +++ b/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()) { - repodir, cleanup := test.Env(t, repoFixture) +func TestFromFixture(t testing.TB, repoFixture string) (*Repository, backend.Backend) { + repodir := test.Env(t, repoFixture) repo, be := TestOpenLocal(t, repodir) - return repo, be, cleanup + return repo, be } // TestOpenLocal opens a local repository. diff --git a/internal/test/helpers.go b/internal/test/helpers.go index e3fded66e..035afff0e 100644 --- a/internal/test/helpers.go +++ b/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() { - if !TestCleanupTempDirs { - t.Logf("leaving temporary directory %v used for test", tempdir) - return - } - - RemoveAll(t, tempdir) - } + return filepath.Join(tempdir, "repo") } func isFile(fi os.FileInfo) bool {