From 979df8ae88058ca0d4c02b7efe7a7b405fd23d42 Mon Sep 17 00:00:00 2001 From: Magnus Kokk Date: Tue, 18 Aug 2026 11:09:43 +0000 Subject: [PATCH] Close public API test coverage gaps, fix flaky tests cache.go and options.go had 0% direct coverage on their entire public surface (Len/Has/Load/Range/Evict/Store/Fetch/FetchTTL, WithPolicy, WithTTL) -- everything was only exercised one layer down against internal/backend.Backend directly. Add cache_test.go/options_test.go coverage for all of it, including Range's documented "f may modify the cache" contract and WithPolicy's panic on an invalid policy. Also fixes two flaky tests found by repeatedly running the suite under -race: - TestMapShrink: not actually a GC-timing issue as it first appeared. The map-shrink heuristic in delete() bounds the live map to ~2x the current live count, not a fixed fraction of the original size, so evicting exactly 75% of entries sat right on that heuristic's worst-case boundary and never converged below the assertion's threshold. Evicting 90% instead lets multiple compaction passes fire and converge reliably. - New()'s default 1s expiry debounce (no public option to disable it) means a short TTL can take just under 2x the debounce interval to actually expire -- TestCacheFetchTTL and TestWithTTL were relying on the default 1s EventuallyTrue timeout, which the debounce can exceed depending on clock alignment. Also splits TestFetchCallbackBlocks into independent tests, removing the shared, execution-order-dependent setup the author had already flagged with a TODO. Co-Authored-By: Claude Sonnet 5 --- cache_test.go | 141 ++++++++++++++++++++++++++++ internal/backend/backend_test.go | 152 ++++++++++++++++++++----------- list/list_test.go | 16 ++++ options_test.go | 73 +++++++++++++++ 4 files changed, 328 insertions(+), 54 deletions(-) create mode 100644 options_test.go diff --git a/cache_test.go b/cache_test.go index d560996..67edf5f 100644 --- a/cache_test.go +++ b/cache_test.go @@ -1,6 +1,7 @@ package evcache_test import ( + "errors" "runtime" "testing" "time" @@ -9,6 +10,146 @@ import ( . "github.com/mgnsk/evcache/v4/internal/testing" ) +func TestCacheStoreLoadHas(t *testing.T) { + c := evcache.New[string, string]() + + _, loaded := c.Load("key") + Equal(t, loaded, false) + Equal(t, c.Has("key"), false) + + c.Store("key", "value") + + value, loaded := c.Load("key") + Equal(t, loaded, true) + Equal(t, value, "value") + Equal(t, c.Has("key"), true) +} + +func TestCacheLen(t *testing.T) { + c := evcache.New[int, int]() + + Equal(t, c.Len(), 0) + + c.Store(1, 1) + c.Store(2, 2) + Equal(t, c.Len(), 2) +} + +func TestCacheEvict(t *testing.T) { + c := evcache.New[string, string]() + + _, evicted := c.Evict("key") + Equal(t, evicted, false) + + c.Store("key", "value") + + value, evicted := c.Evict("key") + Equal(t, evicted, true) + Equal(t, value, "value") + Equal(t, c.Has("key"), false) +} + +func TestCacheRange(t *testing.T) { + c := evcache.New[int, int]() + + c.Store(1, 10) + c.Store(2, 20) + c.Store(3, 30) + + seen := map[int]int{} + c.Range(func(key, value int) bool { + seen[key] = value + return true + }) + Equal(t, seen, map[int]int{1: 10, 2: 20, 3: 30}) +} + +func TestCacheRangeStopsEarly(t *testing.T) { + c := evcache.New[int, int]() + + c.Store(1, 10) + c.Store(2, 20) + + n := 0 + c.Range(func(key, value int) bool { + n++ + return false + }) + Equal(t, n, 1) +} + +func TestCacheRangeMayModifyCache(t *testing.T) { + c := evcache.New[int, int]() + + c.Store(1, 10) + c.Store(2, 20) + c.Store(3, 30) + + c.Range(func(key, value int) bool { + c.Evict(key) + c.Store(key+100, value) + return true + }) + + Equal(t, c.Len(), 3) + for _, key := range []int{1, 2, 3} { + Equal(t, c.Has(key), false) + } + for _, key := range []int{101, 102, 103} { + Equal(t, c.Has(key), true) + } +} + +func TestCacheFetchCachesValue(t *testing.T) { + c := evcache.New[string, int]() + + calls := 0 + fetch := func() (int, error) { + calls++ + return 1, nil + } + + v, err := c.Fetch("key", fetch) + Must(t, err) + Equal(t, v, 1) + + v, err = c.Fetch("key", fetch) + Must(t, err) + Equal(t, v, 1) + Equal(t, calls, 1) +} + +func TestCacheFetchPropagatesError(t *testing.T) { + c := evcache.New[string, int]() + + errFetch := errors.New("fetch failed") + + _, err := c.Fetch("key", func() (int, error) { + return 0, errFetch + }) + Equal(t, errors.Is(err, errFetch), true) + Equal(t, c.Has("key"), false) +} + +func TestCacheFetchTTL(t *testing.T) { + c := evcache.New[string, int]() + + _, err := c.FetchTTL("key", func() (int, time.Duration, error) { + return 1, time.Millisecond, nil + }) + Must(t, err) + Equal(t, c.Has("key"), true) + + // New() applies the default 1s expiry debounce (there is no public + // option to disable it), so a short TTL can take up to just under + // 2x the debounce interval to actually expire. Give this plenty of + // margin above that worst case rather than relying on the 1s + // default EventuallyTrue timeout. + EventuallyTrue(t, func() bool { + return c.Len() == 0 + }, 3*time.Second) +} + func TestCacheGoGC(t *testing.T) { capacity := 1_000_000 c := evcache.New[int, struct{}](evcache.WithCapacity(capacity)) diff --git a/internal/backend/backend_test.go b/internal/backend/backend_test.go index e58df68..fa4fe1e 100644 --- a/internal/backend/backend_test.go +++ b/internal/backend/backend_test.go @@ -12,10 +12,11 @@ import ( . "github.com/mgnsk/evcache/v4/internal/testing" ) -func TestFetchCallbackBlocks(t *testing.T) { - var b backend.Backend[string, string] - b.Init(0, "", 0, 0) - t.Cleanup(b.Close) +// startBlockedFetch starts a Fetch call for "key" that blocks until the +// returned unblock func is called, returning only after the fetch callback +// has started running. +func startBlockedFetch(t *testing.T, b *backend.Backend[string, string]) (unblock func()) { + t.Helper() wg := sync.WaitGroup{} done := make(chan struct{}) @@ -24,11 +25,6 @@ func TestFetchCallbackBlocks(t *testing.T) { }) fetchStarted := make(chan struct{}) - t.Cleanup(func() { - onceDone() - wg.Wait() - }) - wg.Add(1) go func() { defer wg.Done() @@ -45,64 +41,102 @@ func TestFetchCallbackBlocks(t *testing.T) { <-fetchStarted - t.Run("assert cache empty", func(t *testing.T) { - Equal(t, b.Len(), 0) + return func() { + onceDone() + wg.Wait() + } +} + +func TestFetchBlocksCacheEmpty(t *testing.T) { + var b backend.Backend[string, string] + b.Init(0, "", 0, 0) + t.Cleanup(b.Close) + + t.Cleanup(startBlockedFetch(t, &b)) + + Equal(t, b.Len(), 0) +} + +func TestFetchBlocksNonBlockingEvict(t *testing.T) { + var b backend.Backend[string, string] + b.Init(0, "", 0, 0) + t.Cleanup(b.Close) + + t.Cleanup(startBlockedFetch(t, &b)) + + _, ok := b.Evict("key") + Equal(t, ok, false) +} + +func TestFetchBlocksAutoexpiryOtherKeys(t *testing.T) { + var b backend.Backend[string, string] + b.Init(0, "", 0, 0) + t.Cleanup(b.Close) + + t.Cleanup(startBlockedFetch(t, &b)) + + b.FetchTTL("key1", func() (string, time.Duration, error) { + return "value1", time.Millisecond, nil }) - t.Run("non-blocking Evict", func(t *testing.T) { - _, ok := b.Evict("key") - Equal(t, ok, false) + EventuallyTrue(t, func() bool { + return b.Len() == 0 }) +} - t.Run("autoexpiry for other keys works", func(t *testing.T) { - b.FetchTTL("key1", func() (string, time.Duration, error) { - return "value1", time.Millisecond, nil - }) +func TestFetchBlocksNonBlockingHas(t *testing.T) { + var b backend.Backend[string, string] + b.Init(0, "", 0, 0) + t.Cleanup(b.Close) - EventuallyTrue(t, func() bool { - return b.Len() == 0 - }) + t.Cleanup(startBlockedFetch(t, &b)) + + b.Fetch("key1", func() (string, error) { + return "value1", nil }) - t.Run("non-blocking Has", func(t *testing.T) { - b.Fetch("key1", func() (string, error) { - return "value1", nil - }) + Equal(t, b.Has("key1"), true) +} - Equal(t, b.Has("key1"), true) - }) +func TestFetchBlocksNonBlockingRange(t *testing.T) { + var b backend.Backend[string, string] + b.Init(0, "", 0, 0) + t.Cleanup(b.Close) - t.Run("non-blocking Range", func(t *testing.T) { - b.Fetch("key1", func() (string, error) { - return "value1", nil - }) + t.Cleanup(startBlockedFetch(t, &b)) - var keys []string - b.Range(func(key string, _ string) bool { - keys = append(keys, key) - return true - }) - Equal(t, len(keys), 1) - Equal(t, keys[0], "key1") + b.Fetch("key1", func() (string, error) { + return "value1", nil }) - t.Run("Store discards the key", func(t *testing.T) { - b.Store("key", "value1") + var keys []string + b.Range(func(key string, _ string) bool { + keys = append(keys, key) + return true + }) + Equal(t, len(keys), 1) + Equal(t, keys[0], "key1") +} - value, loaded := b.Load("key") - Equal(t, loaded, true) - Equal(t, value, "value1") +func TestStoreDiscardsInFlightFetch(t *testing.T) { + var b backend.Backend[string, string] + b.Init(0, "", 0, 0) + t.Cleanup(b.Close) - t.Log("assert that overwritten value exists after Fetch returns") + unblock := startBlockedFetch(t, &b) - // TODO: test setup scope - onceDone() - wg.Wait() + b.Store("key", "value1") - value, loaded = b.Load("key") - Equal(t, loaded, true) - Equal(t, value, "value1") - }) + value, loaded := b.Load("key") + Equal(t, loaded, true) + Equal(t, value, "value1") + + t.Log("assert that overwritten value exists after the in-flight Fetch returns") + unblock() + + value, loaded = b.Load("key") + Equal(t, loaded, true) + Equal(t, value, "value1") } func TestFetchCallbackPanic(t *testing.T) { @@ -428,8 +462,18 @@ func TestMapShrink(t *testing.T) { t.Logf("alloc before:\t%d bytes", stats.Alloc) oldSize := stats.Alloc - // Delete more than half elements. - for i := range n * 3 / 4 { + // Delete all but 10% of elements. + // + // Note: the map-shrink heuristic in delete() is an amortized O(1) + // compaction that guarantees the live map stays within roughly 2x + // of the current live count, not within a fixed fraction of the + // original size. Deleting only e.g. 75% here would let the final + // map settle right at the 2x-of-live-count boundary (worst case + // 2 * 25% = 50% of the original size), making the assertion below + // flaky depending on incidental map/bucket overhead. Deleting 90% + // leaves enough deletions for multiple compaction passes to fire, + // so the final map converges much closer to the live count. + for i := range n * 9 / 10 { b.Evict(i) } @@ -439,7 +483,7 @@ func TestMapShrink(t *testing.T) { newSize := stats.Alloc return newSize < oldSize/2 - }) + }, 5*time.Second) t.Logf("alloc after:\t%d bytes", stats.Alloc) diff --git a/list/list_test.go b/list/list_test.go index 2f6267a..975b195 100644 --- a/list/list_test.go +++ b/list/list_test.go @@ -316,6 +316,22 @@ func TestMoveBackwards(t *testing.T) { }) } +func TestEmptyList(t *testing.T) { + var l list.List[int] + + assertEqual(t, l.Len(), 0) + assertEqual(t, l.Front(), (*list.Element[int])(nil)) + assertEqual(t, l.Back(), (*list.Element[int])(nil)) +} + +func TestNewElementDetached(t *testing.T) { + e := list.NewElement(1) + + assertEqual(t, e.Value, 1) + assertEqual(t, e.Next(), (*list.Element[int])(nil)) + assertEqual(t, e.Prev(), (*list.Element[int])(nil)) +} + func TestDo(t *testing.T) { var l list.List[string] diff --git a/options_test.go b/options_test.go new file mode 100644 index 0000000..5ed24fb --- /dev/null +++ b/options_test.go @@ -0,0 +1,73 @@ +package evcache_test + +import ( + "testing" + "time" + + "github.com/mgnsk/evcache/v4" + . "github.com/mgnsk/evcache/v4/internal/testing" +) + +func TestWithPolicyInvalidPanics(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Fatalf("expected New to panic on invalid policy") + } + }() + + evcache.New[int, int](evcache.WithPolicy("bogus")) +} + +func TestWithPolicyFIFO(t *testing.T) { + for _, policy := range []string{"", evcache.FIFO} { + c := evcache.New[int, int](evcache.WithCapacity(2), evcache.WithPolicy(policy)) + + c.Store(1, 1) + c.Store(2, 2) + c.Store(3, 3) // Overflows the cache, evicting the oldest key. + + Equal(t, c.Has(1), false) + Equal(t, c.Has(2), true) + Equal(t, c.Has(3), true) + } +} + +func TestWithPolicyLRU(t *testing.T) { + c := evcache.New[int, int](evcache.WithCapacity(2), evcache.WithPolicy(evcache.LRU)) + + c.Store(1, 1) + c.Store(2, 2) + c.Load(1) // Touch key 1, making key 2 the least recently used. + c.Store(3, 3) + + Equal(t, c.Has(1), true) + Equal(t, c.Has(2), false) + Equal(t, c.Has(3), true) +} + +func TestWithPolicyLFU(t *testing.T) { + c := evcache.New[int, int](evcache.WithCapacity(2), evcache.WithPolicy(evcache.LFU)) + + c.Store(1, 1) + c.Store(2, 2) + c.Load(1) // Increase key 1's hit count, making key 2 the least frequently used. + c.Store(3, 3) + + Equal(t, c.Has(1), true) + Equal(t, c.Has(2), false) + Equal(t, c.Has(3), true) +} + +func TestWithTTL(t *testing.T) { + c := evcache.New[int, int](evcache.WithTTL(time.Millisecond)) + + c.Store(1, 1) + Equal(t, c.Has(1), true) + + // New() applies the default 1s expiry debounce (there is no public + // option to disable it), so a short TTL can take up to just under + // 2x the debounce interval to actually expire. + EventuallyTrue(t, func() bool { + return c.Len() == 0 + }, 3*time.Second) +}