From 5c8de5e282961fcb68007b68e1a27bbdc6154cfe Mon Sep 17 00:00:00 2001 From: Ping Qiu Date: Wed, 28 Jan 2026 17:21:14 -0800 Subject: [PATCH] fix: close volumes and EC shards in tests for Windows compatibility (#8152) * fix: close volumes and EC shards in tests to prevent Windows cleanup failures On Windows, t.TempDir() cleanup fails when test files are still open because Windows enforces mandatory file locking. Add defer v.Close(), defer store.Close(), and EC volume cleanup to ensure all file handles are released before temp directory removal. Co-Authored-By: Claude Opus 4.5 * refactor: extract closeEcVolumes helper to reduce duplication Address code review feedback by extracting the repeated EC volume cleanup loop into a closeEcVolumes() helper function. Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: Claude Opus 4.5 --- weed/storage/disk_location_ec_test.go | 20 ++++++++++++++++++++ weed/storage/idx_binary_search_test.go | 1 + weed/storage/store_load_balancing_test.go | 5 ++++- weed/storage/volume_read_test.go | 2 ++ weed/storage/volume_vacuum_test.go | 1 + weed/storage/volume_write_test.go | 2 ++ 6 files changed, 30 insertions(+), 1 deletion(-) diff --git a/weed/storage/disk_location_ec_test.go b/weed/storage/disk_location_ec_test.go index 23b7d8c40..e40870b7f 100644 --- a/weed/storage/disk_location_ec_test.go +++ b/weed/storage/disk_location_ec_test.go @@ -11,6 +11,13 @@ import ( "github.com/seaweedfs/seaweedfs/weed/util" ) +// closeEcVolumes closes all EC volumes in the given DiskLocation to release file handles. +func closeEcVolumes(dl *DiskLocation) { + for _, ecVol := range dl.ecVolumes { + ecVol.Close() + } +} + // TestIncompleteEcEncodingCleanup tests the cleanup logic for incomplete EC encoding scenarios func TestIncompleteEcEncodingCleanup(t *testing.T) { tests := []struct { @@ -182,11 +189,18 @@ func TestIncompleteEcEncodingCleanup(t *testing.T) { t.Logf("loadAllEcShards returned error (expected in some cases): %v", loadErr) } + // Close EC volumes before idempotency test to avoid leaking file handles + closeEcVolumes(diskLocation) + diskLocation.ecVolumes = make(map[needle.VolumeId]*erasure_coding.EcVolume) + // Test idempotency - running again should not cause issues loadErr2 := diskLocation.loadAllEcShards(nil) if loadErr2 != nil { t.Logf("Second loadAllEcShards returned error: %v", loadErr2) } + t.Cleanup(func() { + closeEcVolumes(diskLocation) + }) // Verify cleanup expectations if tt.expectCleanup { @@ -554,6 +568,9 @@ func TestEcCleanupWithSeparateIdxDirectory(t *testing.T) { if loadErr != nil { t.Logf("loadAllEcShards error: %v", loadErr) } + t.Cleanup(func() { + closeEcVolumes(diskLocation) + }) // Verify cleanup occurred in data directory (shards) for i := 0; i < erasure_coding.TotalShardsCount; i++ { @@ -625,6 +642,9 @@ func TestDistributedEcVolumeNoFileDeletion(t *testing.T) { if loadErr != nil { t.Logf("loadAllEcShards returned error (expected): %v", loadErr) } + t.Cleanup(func() { + closeEcVolumes(diskLocation) + }) // CRITICAL CHECK: Verify shard files still exist (should NOT be deleted) for i := 0; i < 5; i++ { diff --git a/weed/storage/idx_binary_search_test.go b/weed/storage/idx_binary_search_test.go index e04185bcd..77d38e562 100644 --- a/weed/storage/idx_binary_search_test.go +++ b/weed/storage/idx_binary_search_test.go @@ -18,6 +18,7 @@ func TestFirstInvalidIndex(t *testing.T) { if err != nil { t.Fatalf("volume creation: %v", err) } + defer v.Close() type WriteInfo struct { offset int64 size int32 diff --git a/weed/storage/store_load_balancing_test.go b/weed/storage/store_load_balancing_test.go index 35475a6ae..a16d1474d 100644 --- a/weed/storage/store_load_balancing_test.go +++ b/weed/storage/store_load_balancing_test.go @@ -45,7 +45,10 @@ func newTestStore(t *testing.T, numDirs int) *Store { } } }() - t.Cleanup(func() { close(done) }) + t.Cleanup(func() { + store.Close() + close(done) + }) return store } diff --git a/weed/storage/volume_read_test.go b/weed/storage/volume_read_test.go index efedadc31..d3fb31c64 100644 --- a/weed/storage/volume_read_test.go +++ b/weed/storage/volume_read_test.go @@ -16,6 +16,7 @@ func TestReadNeedMetaWithWritesAndUpdates(t *testing.T) { if err != nil { t.Fatalf("volume creation: %v", err) } + defer v.Close() type WriteInfo struct { offset int64 size int32 @@ -55,6 +56,7 @@ func TestReadNeedMetaWithDeletesThenWrites(t *testing.T) { if err != nil { t.Fatalf("volume creation: %v", err) } + defer v.Close() type WriteInfo struct { offset int64 size int32 diff --git a/weed/storage/volume_vacuum_test.go b/weed/storage/volume_vacuum_test.go index 29b990f70..9022fc5c7 100644 --- a/weed/storage/volume_vacuum_test.go +++ b/weed/storage/volume_vacuum_test.go @@ -119,6 +119,7 @@ func testCompaction(t *testing.T, needleMapKind NeedleMapKind) { if err != nil { t.Fatalf("volume reloading: %v", err) } + defer v.Close() for i := 1; i <= beforeCommitFileCount+afterCommitFileCount; i++ { diff --git a/weed/storage/volume_write_test.go b/weed/storage/volume_write_test.go index ce7c184bd..f3e8ebe01 100644 --- a/weed/storage/volume_write_test.go +++ b/weed/storage/volume_write_test.go @@ -21,6 +21,7 @@ func TestSearchVolumesWithDeletedNeedles(t *testing.T) { if err != nil { t.Fatalf("volume creation: %v", err) } + defer v.Close() count := 20 @@ -119,6 +120,7 @@ func TestDestroyNonemptyVolumeWithOnlyEmpty(t *testing.T) { if err != nil { t.Fatalf("volume creation: %v", err) } + defer v.Close() path := v.DataBackend.Name() // should return "volume not empty" error and do not delete file when Destroy non-empty volume