mirror of
https://github.com/navidrome/navidrome.git
synced 2026-06-19 07:37:15 +00:00
test: fix flaky tests in utils/cache (#5567)
* test: fix flaky tests in utils/cache Two tests in the utils/cache suite were timing- and ordering-dependent and failed intermittently on CI (notably on the Windows runner). The FileHaunter tests raced the asynchronous cache-cleanup goroutine with a fixed 400ms sleep, then asserted the directory state once. On slow runners the haunter had not finished scrubbing, so the assertion saw the original files and failed. Replace the fixed sleep with Eventually polling so the assertions wait for the haunter to converge. While doing so, the exact set and count of reaped files proved nondeterministic (the empty file is double-counted in the size loop and LRU survivors depend on OS access-time ordering), so the assertions now check the haunter's actual guarantees: the empty file is always scrubbed and the cache stays within the configured maxSize/maxItems bound. This also lets the previously-disabled maxItems context and its commented-out assertions be re-enabled. The HTTPClient 'caches repeated requests' test relied on a shared requestsReceived counter that was never reset in BeforeEach. Under randomized spec order another spec could run first and leave the counter non-zero, breaking the first assertion. Reset the counter and header in BeforeEach to make the spec independent of execution order. Verified with: ginkgo -race -repeat=80 --randomize-all ./utils/cache/ * test: surface errors in dirSize and align Eventually with house style Address code review feedback on the cache flaky-test fix: - dirSize now returns (uint64, error) and the maxSize spec asserts the error is nil. Previously a ReadDir/Info failure silently returned 0, which always satisfies '<= maxSize' and would mask a real filesystem error as a passing test. - dirSize skips non-regular entries (info.Mode().IsRegular()) to match its doc comment and avoid counting directories or symlinks. - The Eventually blocks now use .WithTimeout()/.WithPolling() with time.Duration values instead of string-literal durations, matching the prevailing pattern in the test suite.
This commit is contained in:
+2
@@ -20,6 +20,8 @@ var _ = Describe("HTTPClient", func() {
|
||||
var header string
|
||||
|
||||
BeforeEach(func() {
|
||||
requestsReceived = 0
|
||||
header = ""
|
||||
ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
requestsReceived++
|
||||
header = r.Header.Get("head")
|
||||
|
||||
Vendored
+48
-13
@@ -29,15 +29,15 @@ var _ = Describe("FileHaunter", func() {
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
DeferCleanup(func() { _ = os.RemoveAll(tempDir) })
|
||||
|
||||
// Use a short haunter period so cleanup runs promptly; the assertions
|
||||
// below poll with Eventually instead of racing a fixed sleep.
|
||||
fsCache, err = fscache.NewCacheWithHaunter(fs, fscache.NewLRUHaunterStrategy(
|
||||
cache.NewFileHaunter("", maxItems, maxSize, 300*time.Millisecond),
|
||||
cache.NewFileHaunter("", maxItems, maxSize, 100*time.Millisecond),
|
||||
))
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
DeferCleanup(fsCache.Clean)
|
||||
|
||||
Expect(createTestFiles(fsCache)).To(Succeed())
|
||||
|
||||
<-time.After(400 * time.Millisecond)
|
||||
})
|
||||
|
||||
Context("When maxSize is defined", func() {
|
||||
@@ -46,24 +46,39 @@ var _ = Describe("FileHaunter", func() {
|
||||
})
|
||||
|
||||
It("removes files", func() {
|
||||
Expect(os.ReadDir(cacheDir)).To(HaveLen(4))
|
||||
Expect(fsCache.Exists("stream-5")).To(BeFalse(), "stream-5 (empty file) should have been scrubbed")
|
||||
// TODO Fix flaky tests
|
||||
//Expect(fsCache.Exists("stream-0")).To(BeFalse(), "stream-0 should have been scrubbed")
|
||||
// stream-0..4 hold "hello" (5 bytes each) and stream-5 is empty.
|
||||
// With maxSize=20, the haunter scrubs the empty file plus enough of
|
||||
// the oldest files to bring the total size down to <= 20 bytes.
|
||||
// Which files survive (and therefore the exact count) depends on
|
||||
// access-time ordering, so we only assert the haunter's guarantees:
|
||||
// the empty file is always scrubbed and the total size stays within
|
||||
// the configured limit.
|
||||
Eventually(func(g Gomega) {
|
||||
g.Expect(fsCache.Exists("stream-5")).To(BeFalse(), "stream-5 (empty file) should have been scrubbed")
|
||||
size, err := dirSize(cacheDir)
|
||||
g.Expect(err).ToNot(HaveOccurred())
|
||||
g.Expect(size).To(BeNumerically("<=", maxSize))
|
||||
}).WithTimeout(5 * time.Second).WithPolling(50 * time.Millisecond).Should(Succeed())
|
||||
})
|
||||
})
|
||||
|
||||
XContext("When maxItems is defined", func() {
|
||||
Context("When maxItems is defined", func() {
|
||||
BeforeEach(func() {
|
||||
maxItems = 3
|
||||
})
|
||||
|
||||
It("removes files", func() {
|
||||
Expect(os.ReadDir(cacheDir)).To(HaveLen(maxItems))
|
||||
Expect(fsCache.Exists("stream-5")).To(BeFalse(), "stream-5 (empty file) should have been scrubbed")
|
||||
// TODO Fix flaky tests
|
||||
//Expect(fsCache.Exists("stream-0")).To(BeFalse(), "stream-0 should have been scrubbed")
|
||||
//Expect(fsCache.Exists("stream-1")).To(BeFalse(), "stream-1 should have been scrubbed")
|
||||
// With maxItems=3, the haunter scrubs the empty file plus enough of
|
||||
// the oldest files to bring the count within the limit. As above, the
|
||||
// exact survivors depend on access-time ordering, so we assert the
|
||||
// guaranteed invariants: the empty file is gone and the item count
|
||||
// stays within the configured limit.
|
||||
Eventually(func(g Gomega) {
|
||||
g.Expect(fsCache.Exists("stream-5")).To(BeFalse(), "stream-5 (empty file) should have been scrubbed")
|
||||
entries, readErr := os.ReadDir(cacheDir)
|
||||
g.Expect(readErr).ToNot(HaveOccurred())
|
||||
g.Expect(len(entries)).To(BeNumerically("<=", maxItems))
|
||||
}).WithTimeout(5 * time.Second).WithPolling(50 * time.Millisecond).Should(Succeed())
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -93,6 +108,26 @@ func createTestFiles(c *fscache.FSCache) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// dirSize returns the total size in bytes of all regular files in dir.
|
||||
func dirSize(dir string) (uint64, error) {
|
||||
entries, err := os.ReadDir(dir)
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
var total uint64
|
||||
for _, e := range entries {
|
||||
info, err := e.Info()
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
if !info.Mode().IsRegular() {
|
||||
continue
|
||||
}
|
||||
total += uint64(info.Size())
|
||||
}
|
||||
return total, nil
|
||||
}
|
||||
|
||||
func createCachedStream(c *fscache.FSCache, name string, contents string) fscache.ReadAtCloser {
|
||||
r, w, _ := c.Get(name)
|
||||
_, _ = w.Write([]byte(contents))
|
||||
|
||||
Reference in New Issue
Block a user