diff --git a/pkg/storage/stores/shipper/bloomshipper/cache_test.go b/pkg/storage/stores/shipper/bloomshipper/cache_test.go index 4fadfe1b35a33..c85f0382bafdd 100644 --- a/pkg/storage/stores/shipper/bloomshipper/cache_test.go +++ b/pkg/storage/stores/shipper/bloomshipper/cache_test.go @@ -1,7 +1,6 @@ package bloomshipper import ( - "os" "testing" "time" @@ -10,23 +9,25 @@ import ( "go.uber.org/atomic" ) -func Test_CachedBlock(t *testing.T) { +func TestBlockDirectory_Cleanup(t *testing.T) { + checkInterval := 50 * time.Millisecond + timeout := 200 * time.Millisecond + tests := map[string]struct { releaseQuerier bool expectDirectoryToBeDeletedWithin time.Duration }{ - "expected block directory to be removed once all queriers are released": { - releaseQuerier: true, - // four times grater than activeQueriersCheckInterval - expectDirectoryToBeDeletedWithin: 200 * time.Millisecond, + "expect directory to be removed once all queriers are released": { + releaseQuerier: true, + expectDirectoryToBeDeletedWithin: 2 * checkInterval, }, - "expected block directory to be force removed after timeout": { - releaseQuerier: false, - // four times grater than removeDirectoryTimeout - expectDirectoryToBeDeletedWithin: 2 * time.Second, + "expect directory to be force removed after timeout": { + releaseQuerier: false, + expectDirectoryToBeDeletedWithin: 2 * timeout, }, } - for name, testData := range tests { + for name, tc := range tests { + tc := tc t.Run(name, func(t *testing.T) { extractedBlockDirectory := t.TempDir() blockFilePath, _, _, _ := createBlockArchive(t) @@ -34,26 +35,27 @@ func Test_CachedBlock(t *testing.T) { require.NoError(t, err) require.DirExists(t, extractedBlockDirectory) - cached := BlockDirectory{ + blockDir := BlockDirectory{ Path: extractedBlockDirectory, - removeDirectoryTimeout: 500 * time.Millisecond, - activeQueriersCheckInterval: 50 * time.Millisecond, - logger: log.NewLogfmtLogger(os.Stderr), - refCount: atomic.NewInt32(1), + removeDirectoryTimeout: timeout, + activeQueriersCheckInterval: checkInterval, + logger: log.NewNopLogger(), + refCount: atomic.NewInt32(0), } - cached.removeDirectoryAsync() - //ensure directory exists - require.Never(t, func() bool { - return directoryDoesNotExist(extractedBlockDirectory) - }, 200*time.Millisecond, 50*time.Millisecond) + // acquire directory + blockDir.refCount.Inc() + // start cleanup goroutine + blockDir.removeDirectoryAsync() - if testData.releaseQuerier { - cached.refCount.Dec() + if tc.releaseQuerier { + // release directory + blockDir.refCount.Dec() } - //ensure directory does not exist + + // ensure directory does not exist any more require.Eventually(t, func() bool { return directoryDoesNotExist(extractedBlockDirectory) - }, testData.expectDirectoryToBeDeletedWithin, 50*time.Millisecond) + }, tc.expectDirectoryToBeDeletedWithin, 10*time.Millisecond) }) } } @@ -72,7 +74,7 @@ func Test_ClosableBlockQuerier(t *testing.T) { querier := blockDir.BlockQuerier() require.Equal(t, int32(1), blockDir.refCount.Load()) - querier.Close() + require.NoError(t, querier.Close()) require.Equal(t, int32(0), blockDir.refCount.Load()) }