mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
test(images): stop the reconciler's cadence tests racing the clock and eviction
Two flakes in ImageDiskCacheReconcilerTest, both mine, both green locally and red on CI. aStartAfterTheIntervalIsDueAgain measured the interval from the clock it sampled, but isDue() compares against the marker's file system mtime. A file system that keeps mtime at whole-second resolution reads the marker back up to a second before the write that made it, so `now + interval - 1` was already past the interval and the pass ran when the test expected it skipped. Reproduced exactly by truncating the marker's mtime to its whole second: same result object CI reported, ceilingBytes and all. It now pins the recorded pass to a whole second and measures from that, so the boundary holds at any mtime resolution — and asserts the file system kept the value, so an environment that cannot would fail loudly instead of flaking. Renamed to theIntervalIsMeasuredFromTheRecordedPass, which is the property. aSecondStartWithinTheIntervalSkipsTheWalk asserted the directory's byte total was unchanged across the skipped call. Coil evicts asynchronously on its own scope and the drainer unlinks behind it, so the two measurements raced both: CI saw 24103 where the test had recorded 25127. What the test needs to rule out is a wipe, and clear() takes DiskCache.size to zero — so it asserts on that instead, which no amount of eviction churn can move. Verified by running the class ten times, and by running CI's own task list locally (both lintBenchmark variants and both unit-test variants — the pre-push hook covers only testPlayDebugUnitTest, which is how these reached CI). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017RYCgbvhtBCBNVLMxSWoCJ
This commit is contained in:
+28
-7
@@ -266,33 +266,54 @@ class ImageDiskCacheReconcilerTest {
|
||||
// Even a directory well over budget is left alone until the next pass is due — the
|
||||
// point of the gate is that the common start does no work at all.
|
||||
repeat(40) { i -> writeEntry(diskCache, "key$i", 1024) }
|
||||
val overBudget = bytesOnDisk()
|
||||
|
||||
val skipped = ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now + 1000, minSlackBytes = 0)
|
||||
|
||||
assertNull("within the interval the pass must not run", skipped)
|
||||
assertEquals(overBudget, bytesOnDisk())
|
||||
// Coil's size, not the directory's bytes: eviction runs asynchronously on its own scope
|
||||
// and the drainer unlinks behind it, so comparing byte totals across the call races
|
||||
// both. A wipe is what this needs to rule out, and clear() takes the size to zero.
|
||||
assertTrue("a skipped pass must not have cleared the cache", diskCache.size > 0)
|
||||
} finally {
|
||||
diskCache.shutdown()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aStartAfterTheIntervalIsDueAgain() {
|
||||
fun theIntervalIsMeasuredFromTheRecordedPass() {
|
||||
val fs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
|
||||
val diskCache = newCache(fs, 1024L * 1024)
|
||||
try {
|
||||
val now = System.currentTimeMillis()
|
||||
val interval = 24L * 60 * 60 * 1000
|
||||
ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now, intervalMs = interval)
|
||||
ImageDiskCacheReconciler.reconcileIfDue(diskCache, intervalMs = interval)
|
||||
|
||||
assertNull(ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now + interval - 1, intervalMs = interval))
|
||||
assertNotNull(ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now + interval, intervalMs = interval))
|
||||
// Pin the recorded pass to a whole second before measuring from it. isDue() compares
|
||||
// against the marker's mtime, and a file system that keeps mtime at whole-second
|
||||
// resolution reads it back up to a second before the write that made it — so a boundary
|
||||
// measured from our own clock instead lands a second early there. That is what made the
|
||||
// first version of this test pass on a dev box and fail on CI.
|
||||
val lastPass = (System.currentTimeMillis() / 1000) * 1000
|
||||
assertTrue("could not set the marker's mtime", File(markerPath.toString()).setLastModified(lastPass))
|
||||
assertEquals("the file system must keep the timestamp we set", lastPass, recordedPassAt())
|
||||
|
||||
assertNull(
|
||||
"a millisecond before the interval is up, the pass must not run",
|
||||
ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = lastPass + interval - 1, intervalMs = interval),
|
||||
)
|
||||
assertNotNull(
|
||||
"one interval after the recorded pass, it is due again",
|
||||
ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = lastPass + interval, intervalMs = interval),
|
||||
)
|
||||
} finally {
|
||||
diskCache.shutdown()
|
||||
}
|
||||
}
|
||||
|
||||
private fun recordedPassAt(): Long =
|
||||
requireNotNull(FileSystem.SYSTEM.metadataOrNull(markerPath)?.lastModifiedAtMillis) {
|
||||
"a pass must record a timestamp"
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aMarkerDatedInTheFutureDoesNotParkTheCheck() {
|
||||
// A clock that jumped back, or a restored backup, would otherwise strand the check until
|
||||
|
||||
Reference in New Issue
Block a user