Skip to content

Commit

Permalink
Add a little tolerance due to async scheduling
Browse files Browse the repository at this point in the history
When shutting down the executor, other async tasks may be rejected
that are benign to the test. For example the failed future is
removed, a no-op removal notification is published (and ignored),
and a cleanup cycle is triggered (and falls back to the calling
thread if rejected). In these cases the executor can fail without
harming the test.
  • Loading branch information
ben-manes committed Dec 2, 2015
1 parent 023674e commit 5016c14
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import com.github.benmanes.caffeine.cache.testing.CacheProvider;
import com.github.benmanes.caffeine.cache.testing.CacheSpec;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.CacheExecutor;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.ExecutorFailure;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.Listener;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.Loader;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.Population;
Expand Down Expand Up @@ -161,7 +162,8 @@ public void getFunc_absent_null(AsyncLoadingCache<Integer, Integer> cache,

@CheckNoWriter
@Test(dataProvider = "caches")
@CacheSpec(loader = Loader.NULL, executor = CacheExecutor.SINGLE)
@CacheSpec(loader = Loader.NULL,
executor = CacheExecutor.SINGLE, executorFailure = ExecutorFailure.IGNORED)
public void getFunc_absent_null_async(AsyncLoadingCache<Integer, Integer> cache,
CacheContext context) {
Integer key = context.absentKey();
Expand All @@ -175,9 +177,10 @@ public void getFunc_absent_null_async(AsyncLoadingCache<Integer, Integer> cache,

ready.set(true);
Awaits.await().untilTrue(done);
Awaits.await().until(() -> cache.getIfPresent(key) == null);
MoreExecutors.shutdownAndAwaitTermination(context.executor(), 1, TimeUnit.MINUTES);

assertThat(context, both(hasMissCount(1)).and(hasHitCount(0)));
assertThat(context, both(hasMissCount(2)).and(hasHitCount(0)));
assertThat(context, both(hasLoadSuccessCount(0)).and(hasLoadFailureCount(1)));

assertThat(valueFuture.isDone(), is(true));
Expand All @@ -201,7 +204,7 @@ public void getFunc_absent_failure(AsyncLoadingCache<Integer, Integer> cache,

@CheckNoWriter
@Test(dataProvider = "caches")
@CacheSpec(executor = CacheExecutor.SINGLE)
@CacheSpec(executor = CacheExecutor.SINGLE, executorFailure = ExecutorFailure.IGNORED)
public void getFunc_absent_failure_async(AsyncLoadingCache<Integer, Integer> cache,
CacheContext context) {
AtomicBoolean ready = new AtomicBoolean();
Expand All @@ -214,9 +217,10 @@ public void getFunc_absent_failure_async(AsyncLoadingCache<Integer, Integer> cac

ready.set(true);
Awaits.await().untilTrue(done);
Awaits.await().until(() -> cache.getIfPresent(context.absentKey()) == null);
MoreExecutors.shutdownAndAwaitTermination(context.executor(), 1, TimeUnit.MINUTES);

assertThat(context, both(hasMissCount(1)).and(hasHitCount(0)));
assertThat(context, both(hasMissCount(2)).and(hasHitCount(0)));
assertThat(context, both(hasLoadSuccessCount(0)).and(hasLoadFailureCount(1)));

assertThat(valueFuture.isCompletedExceptionally(), is(true));
Expand Down Expand Up @@ -432,7 +436,8 @@ public void get_absent_failure(AsyncLoadingCache<Integer, Integer> cache, CacheC

@CheckNoWriter
@Test(dataProvider = "caches")
@CacheSpec(executor = CacheExecutor.SINGLE, loader = Loader.EXCEPTIONAL)
@CacheSpec(loader = Loader.EXCEPTIONAL,
executor = CacheExecutor.SINGLE, executorFailure = ExecutorFailure.IGNORED)
public void get_absent_failure_async(AsyncLoadingCache<Integer, Integer> cache,
CacheContext context) throws InterruptedException {
AtomicBoolean done = new AtomicBoolean();
Expand All @@ -441,9 +446,10 @@ public void get_absent_failure_async(AsyncLoadingCache<Integer, Integer> cache,
valueFuture.whenComplete((r, e) -> done.set(true));

Awaits.await().untilTrue(done);
Awaits.await().until(() -> cache.getIfPresent(context.absentKey()) == null);
MoreExecutors.shutdownAndAwaitTermination(context.executor(), 1, TimeUnit.MINUTES);

assertThat(context, both(hasMissCount(1)).and(hasHitCount(0)));
assertThat(context, both(hasMissCount(2)).and(hasHitCount(0)));
assertThat(context, both(hasLoadSuccessCount(0)).and(hasLoadFailureCount(1)));

assertThat(valueFuture.isCompletedExceptionally(), is(true));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import com.github.benmanes.caffeine.cache.testing.CacheSpec.CacheExecutor;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.CacheWeigher;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.Compute;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.ExecutorFailure;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.Expire;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.Implementation;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.InitialCapacity;
Expand Down Expand Up @@ -94,8 +95,9 @@ public void putWeighted_noOverflow() {

@Test(dataProvider = "caches")
@CacheSpec(compute = Compute.SYNC, implementation = Implementation.Caffeine,
population = Population.FULL, maximumSize = MaximumSize.FULL, executorMayFail = true,
executor = CacheExecutor.REJECTING, removalListener = Listener.CONSUMING)
population = Population.FULL, maximumSize = MaximumSize.FULL,
executorFailure = ExecutorFailure.EXPECTED, executor = CacheExecutor.REJECTING,
removalListener = Listener.CONSUMING)
public void evict_rejected(Cache<Integer, Integer> cache, CacheContext context) {
cache.put(context.absentKey(), context.absentValue());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.github.benmanes.caffeine.cache.testing.CacheSpec;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.CacheWeigher;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.Compute;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.ExecutorFailure;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.Expire;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.Implementation;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.Listener;
Expand Down Expand Up @@ -87,9 +88,9 @@ public void expire_zero(Cache<Integer, Integer> cache, CacheContext context) {
@Test(dataProvider = "caches", expectedExceptions = DeleteException.class)
@CacheSpec(implementation = Implementation.Caffeine, keys = ReferenceType.STRONG,
population = Population.FULL, writer = Writer.EXCEPTIONAL, requiresExpiration = true,
expireAfterAccess = {Expire.DISABLED, Expire.ONE_MINUTE}, executorMayFail = true,
expireAfterWrite = {Expire.DISABLED, Expire.ONE_MINUTE},
compute = Compute.SYNC, removalListener = Listener.REJECTING)
expireAfterAccess = {Expire.DISABLED, Expire.ONE_MINUTE},
expireAfterWrite = {Expire.DISABLED, Expire.ONE_MINUTE}, compute = Compute.SYNC,
executorFailure = ExecutorFailure.EXPECTED, removalListener = Listener.REJECTING)
public void getIfPresent_writerFails(Cache<Integer, Integer> cache, CacheContext context) {
try {
context.ticker().advance(1, TimeUnit.HOURS);
Expand Down Expand Up @@ -834,10 +835,11 @@ public void computeIfPresent(Map<Integer, Integer> map, CacheContext context) {

@Test(dataProvider = "caches", expectedExceptions = DeleteException.class)
@CacheSpec(implementation = Implementation.Caffeine, keys = ReferenceType.STRONG,
population = Population.FULL, requiresExpiration = true, executorMayFail = true,
population = Population.FULL, requiresExpiration = true,
expireAfterAccess = {Expire.DISABLED, Expire.ONE_MINUTE},
expireAfterWrite = {Expire.DISABLED, Expire.ONE_MINUTE},
compute = Compute.SYNC, writer = Writer.EXCEPTIONAL, removalListener = Listener.REJECTING)
compute = Compute.SYNC, writer = Writer.EXCEPTIONAL,
executorFailure = ExecutorFailure.EXPECTED, removalListener = Listener.REJECTING)
public void computeIfPresent_writerFails(Map<Integer, Integer> map, CacheContext context) {
try {
Integer key = context.firstKey();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,11 @@ CacheExecutor[] executor() default {
};

/** If the executor is allowed to have failures. */
boolean executorMayFail() default false;
ExecutorFailure executorFailure() default ExecutorFailure.DISALLOWED;

enum ExecutorFailure {
EXPECTED, DISALLOWED, IGNORED
}

/** The executors that the cache can be configured with. */
enum CacheExecutor implements Supplier<TrackingExecutor> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@

import com.github.benmanes.caffeine.cache.AsyncLoadingCache;
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.testing.CacheSpec.ExecutorFailure;

/**
* A listener that validates the internal structure after a successful test execution.
Expand Down Expand Up @@ -88,9 +89,9 @@ private static void checkExecutor(ITestResult testResult, CacheContext context)

assertThat("CacheContext required", context, is(not(nullValue())));
TrackingExecutor executor = context.executor();
if (cacheSpec.executorMayFail()) {
if (cacheSpec.executorFailure() == ExecutorFailure.EXPECTED) {
assertThat(executor.failureCount(), is(greaterThan(0)));
} else {
} else if (cacheSpec.executorFailure() == ExecutorFailure.DISALLOWED) {
assertThat(executor.failureCount(), is(0));
}
}
Expand Down

0 comments on commit 5016c14

Please sign in to comment.