Use explicit checks in tests for removal notifications

Previously the number of notifications were checked for, but this
allowed for an incorrect publication to sneak through. The tests
now assert that the exact number of expected notifications are
observed.

.github/workflows/build.yml

@@ -67,6 +67,7 @@ jobs:
         if: always()
         run: ./gradlew --stop
       - uses: andymckay/[email protected]
+        continue-on-error: true
         if: failure()
 
   guava:
@@ -98,6 +99,7 @@ jobs:
         if: always()
         run: ./gradlew --stop
       - uses: andymckay/[email protected]
+        continue-on-error: true
         if: failure()
 
   event_file:

caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncCacheTest.java

@@ -24,7 +24,6 @@
 import static com.github.benmanes.caffeine.testing.FutureSubject.assertThat;
 import static com.github.benmanes.caffeine.testing.MapSubject.assertThat;
 import static com.google.common.truth.Truth.assertThat;
-import static java.util.Map.entry;
 import static java.util.function.Function.identity;
 import static java.util.stream.Collectors.toMap;
 import static org.mockito.ArgumentMatchers.any;
@@ -58,6 +57,7 @@
 import com.github.benmanes.caffeine.cache.testing.CacheSpec.Listener;
 import com.github.benmanes.caffeine.cache.testing.CacheSpec.Population;
 import com.github.benmanes.caffeine.cache.testing.CacheValidationListener;
+import com.github.benmanes.caffeine.cache.testing.CheckNoEvictions;
 import com.github.benmanes.caffeine.cache.testing.CheckNoStats;
 import com.github.benmanes.caffeine.testing.Int;
 import com.google.common.collect.ImmutableSet;
@@ -69,6 +69,7 @@
  *
  * @author [email protected] (Ben Manes)
  */
+@CheckNoEvictions
 @Listeners(CacheValidationListener.class)
 @Test(dataProviderClass = CacheProvider.class)
 @SuppressWarnings("FutureReturnValueIgnored")
@@ -934,31 +935,33 @@ public void put_replace_failure_after(AsyncCache<Int, Int> cache, CacheContext c
   @Test(dataProvider = "caches")
   @CacheSpec(population = { Population.SINGLETON, Population.PARTIAL, Population.FULL })
   public void put_replace_nullValue(AsyncCache<Int, Int> cache, CacheContext context) {
+    var removed = new HashMap<Int, Int>();
     var value = CompletableFuture.completedFuture((Int) null);
     for (Int key : context.firstMiddleLastKeys()) {
       cache.put(key, value);
       assertThat(cache).doesNotContainKey(key);
+      removed.put(key, context.original().get(key));
     }
     int count = context.firstMiddleLastKeys().size();
     assertThat(cache).hasSize(context.initialSize() - count);
-    assertThat(context).removalNotifications().withCause(EXPLICIT).hasSize(count).exclusively();
+    assertThat(context).removalNotifications().withCause(EXPLICIT)
+        .contains(removed).exclusively();
   }
 
   @Test(dataProvider = "caches")
   @CacheSpec(population = { Population.SINGLETON, Population.PARTIAL, Population.FULL })
   public void put_replace_differentValue(AsyncCache<Int, Int> cache, CacheContext context) {
+    var replaced = new HashMap<Int, Int>();
     for (Int key : context.firstMiddleLastKeys()) {
       var newValue = context.absentValue().asFuture();
       cache.put(key, newValue);
       assertThat(cache).containsEntry(key, newValue);
+      replaced.put(key, context.original().get(key));
     }
 
     assertThat(cache).hasSize(context.initialSize());
     assertThat(context).removalNotifications().withCause(REPLACED)
-        .contains(entry(context.firstKey(), context.original().get(context.firstKey())),
-            entry(context.middleKey(), context.original().get(context.middleKey())),
-            entry(context.lastKey(), context.original().get(context.lastKey())))
-        .exclusively();
+        .contains(replaced).exclusively();
   }
 
   /* --------------- misc --------------- */

caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncLoadingCacheTest.java

@@ -56,6 +56,7 @@
 import com.github.benmanes.caffeine.cache.testing.CacheSpec.Loader;
 import com.github.benmanes.caffeine.cache.testing.CacheSpec.Population;
 import com.github.benmanes.caffeine.cache.testing.CacheValidationListener;
+import com.github.benmanes.caffeine.cache.testing.CheckNoEvictions;
 import com.github.benmanes.caffeine.cache.testing.CheckNoStats;
 import com.github.benmanes.caffeine.testing.Int;
 import com.google.common.collect.ImmutableSet;
@@ -67,6 +68,7 @@
  *
  * @author [email protected] (Ben Manes)
  */
+@CheckNoEvictions
 @Listeners(CacheValidationListener.class)
 @Test(dataProviderClass = CacheProvider.class)
 @SuppressWarnings({"FutureReturnValueIgnored", "PreferJavaTimeOverload"})
@@ -408,15 +410,16 @@ private static final class LoadAllException extends RuntimeException {}
   @Test(dataProvider = "caches")
   @CacheSpec(population = { Population.SINGLETON, Population.PARTIAL, Population.FULL })
   public void put_replace(AsyncLoadingCache<Int, Int> cache, CacheContext context) {
+    var replaced = new HashMap<Int, Int>();
     var value = context.absentValue().asFuture();
     for (Int key : context.firstMiddleLastKeys()) {
       cache.put(key, value);
       assertThat(cache.get(key)).succeedsWith(context.absentValue());
+      replaced.put(key, context.original().get(key));
     }
     assertThat(cache).hasSize(context.initialSize());
-
-    int count = context.firstMiddleLastKeys().size();
-    assertThat(context).removalNotifications().withCause(REPLACED).hasSize(count).exclusively();
+    assertThat(context).removalNotifications().withCause(REPLACED)
+        .contains(replaced).exclusively();
   }
 
   /* --------------- refresh --------------- */

caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java

@@ -26,6 +26,7 @@
 import static com.github.benmanes.caffeine.cache.RemovalCause.COLLECTED;
 import static com.github.benmanes.caffeine.cache.RemovalCause.EXPIRED;
 import static com.github.benmanes.caffeine.cache.RemovalCause.EXPLICIT;
+import static com.github.benmanes.caffeine.cache.RemovalCause.REPLACED;
 import static com.github.benmanes.caffeine.cache.RemovalCause.SIZE;
 import static com.github.benmanes.caffeine.cache.testing.CacheContextSubject.assertThat;
 import static com.github.benmanes.caffeine.cache.testing.CacheSpec.Expiration.AFTER_ACCESS;
@@ -36,6 +37,7 @@
 import static com.github.benmanes.caffeine.testing.MapSubject.assertThat;
 import static com.google.common.truth.Truth.assertThat;
 import static java.lang.Thread.State.BLOCKED;
+import static java.util.Map.entry;
 import static org.hamcrest.Matchers.is;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
@@ -182,8 +184,7 @@ public void rescheduleDrainBuffers() {
   }
 
   @Test(dataProvider = "caches")
-  @CacheSpec(compute = Compute.SYNC, implementation = Implementation.Caffeine,
-      population = Population.FULL, maximumSize = Maximum.FULL,
+  @CacheSpec(compute = Compute.SYNC, population = Population.FULL, maximumSize = Maximum.FULL,
       executor = CacheExecutor.REJECTING, executorFailure = ExecutorFailure.EXPECTED,
       removalListener = Listener.CONSUMING)
   public void scheduleDrainBuffers_rejected(
@@ -296,7 +297,8 @@ public void evict_alreadyRemoved(BoundedLocalCache<Int, Int> cache, CacheContext
 
       checkStatus(node, Status.DEAD);
       assertThat(cache).containsKey(newEntry.getKey());
-      assertThat(context).removalNotifications().withCause(EXPLICIT).hasSize(1).exclusively();
+      assertThat(context).removalNotifications().withCause(EXPLICIT)
+          .contains(oldEntry).exclusively();
     } finally {
       cache.evictionLock.unlock();
     }
@@ -425,8 +427,11 @@ public void evict_update_entryTooBig_window(
     cache.put(Int.valueOf(1), Int.valueOf(20));
 
     assertThat(cache.weightedSize()).isAtMost(context.maximumSize());
+    assertThat(context).removalNotifications().withCause(REPLACED)
+        .contains(Int.valueOf(1), Int.valueOf(1));
     assertThat(context).removalNotifications().withCause(SIZE)
         .contains(Int.valueOf(1), Int.valueOf(20));
+    assertThat(context).removalNotifications().hasSize(2);
   }
 
   @Test(dataProvider = "caches")
@@ -444,8 +449,11 @@ public void evict_update_entryTooBig_probation(
     cache.put(Int.valueOf(1), Int.valueOf(20));
 
     assertThat(cache.weightedSize()).isAtMost(context.maximumSize());
+    assertThat(context).removalNotifications().withCause(REPLACED)
+        .contains(Int.valueOf(1), Int.valueOf(1));
     assertThat(context).removalNotifications().withCause(SIZE)
         .contains(Int.valueOf(1), Int.valueOf(20));
+    assertThat(context).removalNotifications().hasSize(2);
   }
 
   @Test(dataProvider = "caches")
@@ -465,8 +473,11 @@ public void evict_update_entryTooBig_protected(
     cache.put(Int.valueOf(1), Int.valueOf(20));
 
     assertThat(cache.weightedSize()).isAtMost(context.maximumSize());
+    assertThat(context).removalNotifications().withCause(REPLACED)
+        .contains(Int.valueOf(1), Int.valueOf(1));
     assertThat(context).removalNotifications().withCause(SIZE)
         .contains(Int.valueOf(1), Int.valueOf(20));
+    assertThat(context).removalNotifications().hasSize(2);
   }
 
   @Test(dataProvider = "caches")
@@ -535,7 +546,8 @@ public void evict_resurrect_weight(Cache<Int, List<Int>> cache, CacheContext con
     await().untilTrue(done);
 
     assertThat(cache).containsEntry(key, List.of());
-    assertThat(context).removalNotifications().withCause(SIZE).isEmpty();
+    assertThat(context).removalNotifications().withCause(REPLACED)
+        .contains(entry(key, List.of(key))).exclusively();
   }
 
   @Test(dataProvider = "caches")
@@ -567,7 +579,8 @@ public void evict_resurrect_expireAfter(Cache<Int, Int> cache, CacheContext cont
     await().untilTrue(done);
 
     assertThat(cache).containsEntry(key, key.negate());
-    assertThat(context).removalNotifications().withCause(EXPIRED).hasSize(1).exclusively();
+    assertThat(context).removalNotifications().withCause(EXPIRED)
+        .contains(key, key).exclusively();
   }
 
   @Test(dataProvider = "caches")
@@ -659,7 +672,8 @@ public void evict_resurrect_expireAfterWrite_entry(Cache<Int, Int> cache, CacheC
     await().untilTrue(done);
 
     assertThat(cache).containsEntry(key, key.negate());
-    assertThat(context).removalNotifications().withCause(EXPIRED).hasSize(1).exclusively();
+    assertThat(context).removalNotifications().withCause(EXPIRED)
+        .contains(key, key).exclusively();
   }
 
   @Test(dataProvider = "caches")
@@ -700,12 +714,15 @@ public void evict_resurrect_expireAfterVar(
       keys = ReferenceType.WEAK, removalListener = Listener.CONSUMING)
   public void evict_collected_candidate(BoundedLocalCache<Int, Int> cache, CacheContext context) {
     var candidate = cache.accessOrderWindowDeque().getFirst();
+    var value = candidate.getValue();
+
     @SuppressWarnings("unchecked")
     var keyReference = (WeakKeyReference<Int>) candidate.getKeyReference();
     keyReference.clear();
 
     cache.put(context.absentKey(), context.absentValue());
-    assertThat(context).removalNotifications().withCause(COLLECTED).hasSize(1);
+    assertThat(context).removalNotifications().withCause(COLLECTED)
+        .contains(null, value).exclusively();
   }
 
   @Test(dataProvider = "caches")
@@ -714,14 +731,17 @@ public void evict_collected_candidate(BoundedLocalCache<Int, Int> cache, CacheCo
       keys = ReferenceType.WEAK, removalListener = Listener.CONSUMING)
   public void evict_collected_victim(BoundedLocalCache<Int, Int> cache, CacheContext context) {
     var victim = cache.accessOrderProbationDeque().getFirst();
+    var value = victim.getValue();
+
     @SuppressWarnings("unchecked")
     var keyReference = (WeakKeyReference<Int>) victim.getKeyReference();
     keyReference.clear();
 
     cache.setMaximumSize(cache.size() - 1);
     cache.cleanUp();
 
-    assertThat(context).removalNotifications().withCause(COLLECTED).hasSize(1);
+    assertThat(context).removalNotifications().withCause(COLLECTED)
+        .contains(null, value).exclusively();
   }
 
   @Test(dataProvider = "caches")
@@ -1065,9 +1085,9 @@ public void put_expireTolerance_expiry(BoundedLocalCache<Int, Int> cache, CacheC
   }
 
   @Test(dataProvider = "caches", groups = "isolated")
-  @CacheSpec(implementation = Implementation.Caffeine, population = Population.EMPTY,
-      refreshAfterWrite = Expire.DISABLED, expireAfterAccess = Expire.DISABLED,
-      expireAfterWrite = Expire.DISABLED, expiry = CacheExpiry.DISABLED,
+  @CacheSpec(population = Population.EMPTY,
+      expireAfterAccess = Expire.DISABLED, expireAfterWrite = Expire.DISABLED,
+      refreshAfterWrite = Expire.DISABLED, expiry = CacheExpiry.DISABLED,
       maximumSize = Maximum.UNREACHABLE, weigher = CacheWeigher.DEFAULT,
       compute = Compute.SYNC, loader = Loader.DISABLED, stats = Stats.DISABLED,
       removalListener = Listener.DEFAULT, evictionListener = Listener.DEFAULT,
@@ -1342,8 +1362,8 @@ public CompletableFuture<Int> asyncReload(Int key, Int oldValue, Executor execut
   /* --------------- Node --------------- */
 
   @Test(dataProviderClass = CacheProvider.class, dataProvider = "caches")
-  @CacheSpec(implementation = Implementation.Caffeine, population = Population.SINGLETON,
-      initialCapacity = {InitialCapacity.DEFAULT, InitialCapacity.FULL}, compute = Compute.SYNC)
+  @CacheSpec(population = Population.SINGLETON, compute = Compute.SYNC,
+      initialCapacity = {InitialCapacity.DEFAULT, InitialCapacity.FULL})
   public void string(BoundedLocalCache<Int, Int> cache, CacheContext context) {
     var node = cache.data.values().iterator().next();
     var description = node.toString();