Skip to content

Commit

Permalink
Use explicit checks in tests for removal notifications
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@ben-manes
ben-manes committed Apr 27, 2022
1 parent 8538376 commit 970c178
Show file tree
Hide file tree
Showing 21 changed files with 1,020 additions and 590 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/build.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ jobs:
if: always()
run: ./gradlew --stop
- uses: andymckay/cancel-action@0.2
continue-on-error: true
if: failure()

guava:
Expand Down Expand Up @@ -98,6 +99,7 @@ jobs:
if: always()
run: ./gradlew --stop
- uses: andymckay/cancel-action@0.2
continue-on-error: true
if: failure()

event_file:
Expand Down
210 changes: 109 additions & 101 deletions caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsMapTest.java
View file

Large diffs are not rendered by default.

156 changes: 75 additions & 81 deletions caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncAsMapTest.java
View file

Large diffs are not rendered by default.

15 changes: 9 additions & 6 deletions caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -69,6 +69,7 @@
*
* @author ben.manes@gmail.com (Ben Manes)
*/
@CheckNoEvictions
@Listeners(CacheValidationListener.class)
@Test(dataProviderClass = CacheProvider.class)
@SuppressWarnings("FutureReturnValueIgnored")
Expand Down Expand Up @@ -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 --------------- */
Expand Down
9 changes: 6 additions & 3 deletions caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncLoadingCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -67,6 +68,7 @@
*
* @author ben.manes@gmail.com (Ben Manes)
*/
@CheckNoEvictions
@Listeners(CacheValidationListener.class)
@Test(dataProviderClass = CacheProvider.class)
@SuppressWarnings({"FutureReturnValueIgnored", "PreferJavaTimeOverload"})
Expand Down Expand Up @@ -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 --------------- */
Expand Down
46 changes: 33 additions & 13 deletions caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down
Loading

0 comments on commit 970c178

Please sign in to comment.