fix sonarlint warnings for tests, remove duplicative, add missing

ben-manes · Jun 9, 2024 · 6cef9eb · 6cef9eb
1 parent 7e3fe4e
commit 6cef9eb
Show file tree

Hide file tree

Showing 34 changed files with 255 additions and 152 deletions.
diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalCache.java
@@ -165,9 +165,9 @@ default void notifyOnReplace(K key, V oldValue, V newValue) {
       long startTime = statsTicker().read();
       try {
         value = mappingFunction.apply(key);
-      } catch (RuntimeException | Error e) {
+      } catch (Throwable t) {
         statsCounter().recordLoadFailure(statsTicker().read() - startTime);
-        throw e;
+        throw t;
       }
       long loadTime = statsTicker().read() - startTime;
       if (recordLoad) {

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsMapTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsMapTest.java
@@ -2752,7 +2752,7 @@ public void entrySet_removeAll_bySet(Map<Int, Int> map, CacheContext context) {
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void entrySet_remove_null(Map<Int, Int> map, CacheContext context) {
-    assertThat(map.values().remove(null)).isFalse();
+    assertThat(map.entrySet().remove(null)).isFalse();
     assertThat(map).isEqualTo(context.original());
   }
 

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncAsMapTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncAsMapTest.java
@@ -2507,7 +2507,7 @@ public void entrySet_removeAll_bySet(AsyncCache<Int, Int> cache, CacheContext co
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void entrySet_remove_null(AsyncCache<Int, Int> cache, CacheContext context) {
-    assertThat(cache.asMap().values().remove(null)).isFalse();
+    assertThat(cache.asMap().entrySet().remove(null)).isFalse();
     assertThat(cache.synchronous().asMap()).isEqualTo(context.original());
   }
 
@@ -2586,7 +2586,6 @@ public void entrySet_removeIf_partial(AsyncCache<Int, Int> cache, CacheContext c
     Predicate<Map.Entry<Int, CompletableFuture<Int>>> isEven =
         entry -> (entry.getValue().join().intValue() % 2) == 0;
     boolean hasEven = cache.asMap().entrySet().stream().anyMatch(isEven);
-
     boolean removedIfEven = cache.asMap().entrySet().removeIf(isEven);
     assertThat(cache.asMap().entrySet().stream().anyMatch(isEven)).isFalse();
     assertThat(removedIfEven).isEqualTo(hasEven);
@@ -2610,21 +2609,6 @@ public void entrySet_removeIf_all(AsyncCache<Int, Int> cache, CacheContext conte
     }
   }
 
-  @CacheSpec
-  @CheckNoStats
-  @Test(dataProvider = "caches")
-  public void entrySet_removeIf(AsyncCache<Int, Int> cache, CacheContext context) {
-    Predicate<Map.Entry<Int, CompletableFuture<Int>>> isEven =
-        entry -> (entry.getValue().join().intValue() % 2) == 0;
-    boolean hasEven = cache.asMap().entrySet().stream().anyMatch(isEven);
-    boolean removedIfEven = cache.asMap().entrySet().removeIf(isEven);
-    assertThat(cache.asMap().entrySet().stream().anyMatch(isEven)).isFalse();
-    assertThat(removedIfEven).isEqualTo(hasEven);
-    if (removedIfEven) {
-      assertThat(cache).hasSizeLessThan(context.initialSize());
-    }
-  }
-
   @CacheSpec
   @CheckNoStats
   @Test(dataProvider = "caches")

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncCacheTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncCacheTest.java
@@ -36,6 +36,7 @@
 import static org.mockito.Mockito.verify;
 import static org.slf4j.event.Level.WARN;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -262,7 +263,16 @@ public void getBiFunc_nullKeyAndLoader(AsyncCache<Int, Int> cache, CacheContext
 
   @CacheSpec
   @Test(dataProvider = "caches")
-  public void getBiFunc_throwsException(AsyncCache<Int, Int> cache, CacheContext context) {
+  public void getBiFunc_throwsCheckedException(AsyncCache<Int, Int> cache, CacheContext context) {
+    assertThrows(IOException.class, () -> cache.get(context.absentKey(),
+        (key, executor) -> { throw uncheckedThrow(new IOException()); }));
+    assertThat(context).stats().hits(0).misses(1).success(0).failures(1);
+    assertThat(cache).doesNotContainKey(context.absentKey());
+  }
+
+  @CacheSpec
+  @Test(dataProvider = "caches")
+  public void getBiFunc_throwsRuntimeException(AsyncCache<Int, Int> cache, CacheContext context) {
     assertThrows(IllegalStateException.class, () ->
         cache.get(context.absentKey(), (key, executor) -> { throw new IllegalStateException(); }));
     assertThat(context).stats().hits(0).misses(1).success(0).failures(1);
@@ -751,7 +761,22 @@ public void getAllBifunction_absent_failure(AsyncCache<Int, Int> cache, CacheCon
 
   @CacheSpec
   @Test(dataProvider = "caches")
-  public void getAllBifunction_absent_throwsException(
+  public void getAllBifunction_absent_throwsCheckedException(
+      AsyncCache<Int, Int> cache, CacheContext context) {
+    var error = assertThrows(CompletionException.class, () -> {
+      cache.getAll(context.absentKeys(),
+          (keys, executor) -> { throw uncheckedThrow(new IOException()); });
+    });
+    assertThat(error).hasCauseThat().isInstanceOf(IOException.class);
+
+    int misses = context.absentKeys().size();
+    assertThat(context).stats().hits(0).misses(misses).success(0).failures(1);
+    assertThat(cache).doesNotContainKey(context.absentKey());
+  }
+
+  @CacheSpec
+  @Test(dataProvider = "caches")
+  public void getAllBifunction_absent_throwsRuntimeException(
       AsyncCache<Int, Int> cache, CacheContext context) {
     assertThrows(IllegalStateException.class, () -> {
       cache.getAll(context.absentKeys(),
@@ -1151,6 +1176,11 @@ public void serialize(AsyncCache<Int, Int> cache, CacheContext context) {
     assertThat(cache).isReserialize();
   }
 
+  @SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"})
+  static <E extends Throwable> E uncheckedThrow(Throwable throwable) throws E {
+    throw (E) throwable;
+  }
+
   private static final class LoadAllException extends RuntimeException {
     private static final long serialVersionUID = 1L;
   }

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CacheTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CacheTest.java
@@ -36,6 +36,7 @@
 import static org.slf4j.event.Level.ERROR;
 import static org.slf4j.event.Level.WARN;
 
+import java.io.IOException;
 import java.io.InvalidObjectException;
 import java.io.ObjectInputStream;
 import java.lang.reflect.InvocationTargetException;
@@ -174,7 +175,20 @@ public void get_absent_null(Cache<Int, Int> cache, CacheContext context) {
 
   @CacheSpec
   @Test(dataProvider = "caches")
-  public void get_absent_throwsException(Cache<Int, Int> cache, CacheContext context) {
+  public void get_absent_throwsCheckedException(Cache<Int, Int> cache, CacheContext context) {
+    var error = assertThrows(Exception.class, () ->
+        cache.get(context.absentKey(), key -> { throw uncheckedThrow(new IOException()); }));
+    if (context.isSync() && context.isCaffeine()) {
+      assertThat(error).isInstanceOf(IOException.class);
+    } else {
+      assertThat(error).hasCauseThat().isInstanceOf(IOException.class);
+    }
+    assertThat(context).stats().hits(0).misses(1).success(0).failures(1);
+  }
+
+  @CacheSpec
+  @Test(dataProvider = "caches")
+  public void get_absent_throwsRuntimeException(Cache<Int, Int> cache, CacheContext context) {
     assertThrows(IllegalStateException.class, () ->
         cache.get(context.absentKey(), key -> { throw new IllegalStateException(); }));
     assertThat(context).stats().hits(0).misses(1).success(0).failures(1);
@@ -404,7 +418,22 @@ public void getAll_nullLookup(Cache<Int, Int> cache, CacheContext context) {
 
   @CacheSpec
   @Test(dataProvider = "caches")
-  public void getAll_absent_throwsException(Cache<Int, Int> cache, CacheContext context) {
+  public void getAll_absent_throwsCheckedException(Cache<Int, Int> cache, CacheContext context) {
+    var error = assertThrows(Exception.class, () ->
+        cache.getAll(context.absentKeys(), keys -> { throw uncheckedThrow(new IOException()); }));
+    if (context.isSync()) {
+      assertThat(error).isInstanceOf(IOException.class);
+    } else {
+      assertThat(error).hasCauseThat().isInstanceOf(IOException.class);
+    }
+
+    int misses = context.absentKeys().size();
+    assertThat(context).stats().hits(0).misses(misses).success(0).failures(1);
+  }
+
+  @CacheSpec
+  @Test(dataProvider = "caches")
+  public void getAll_absent_throwsRuntimeException(Cache<Int, Int> cache, CacheContext context) {
     assertThrows(IllegalStateException.class, () ->
         cache.getAll(context.absentKeys(), keys -> { throw new IllegalStateException(); }));
     int misses = context.absentKeys().size();
@@ -413,7 +442,7 @@ public void getAll_absent_throwsException(Cache<Int, Int> cache, CacheContext co
 
   @CacheSpec
   @Test(dataProvider = "caches")
-  public void getAll_function_throwsError(Cache<Int, Int> cache, CacheContext context) {
+  public void getAll_absent_throwsError(Cache<Int, Int> cache, CacheContext context) {
     assertThrows(UnknownError.class, () ->
         cache.getAll(context.absentKeys(), keys -> { throw new UnknownError(); }));
     int misses = context.absentKeys().size();
@@ -1114,4 +1143,9 @@ public void cacheEntry_equals_hashCode_toString() {
     }
     tester.testEquals();
   }
+
+  @SuppressWarnings({"TypeParameterUnusedInFormals", "unchecked"})
+  static <E extends Throwable> E uncheckedThrow(Throwable throwable) throws E {
+    throw (E) throwable;
+  }
 }
diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CaffeineSpecTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CaffeineSpecTest.java
@@ -118,7 +118,7 @@ private void runScenarios(CacheContext context, Epoch epoch) {
   }
 
   private void runTest(CacheContext context, Epoch epoch, Function<Duration, String> formatter) {
-    CaffeineSpec spec = toSpec(context, epoch, formatter);
+    CaffeineSpec spec = toSpec(context, formatter);
     Caffeine<Object, Object> builder = Caffeine.from(spec);
 
     checkInitialCapacity(spec, context, builder);
@@ -134,8 +134,7 @@ private void runTest(CacheContext context, Epoch epoch, Function<Duration, Strin
     assertThat(spec).isEqualTo(CaffeineSpec.parse(spec.toParsableString().replaceAll(",", ",,")));
   }
 
-  static CaffeineSpec toSpec(CacheContext context,
-      Epoch epoch, Function<Duration, String> formatter) {
+  static CaffeineSpec toSpec(CacheContext context, Function<Duration, String> formatter) {
     var options = new ArrayList<String>();
     if (context.initialCapacity() != InitialCapacity.DEFAULT) {
       options.add("initialCapacity=" + context.initialCapacity().size());

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CaffeineTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CaffeineTest.java
@@ -205,13 +205,13 @@ public void loading_nullLoader() {
   @Test
   public void async_weakValues() {
     var builder = Caffeine.newBuilder().weakValues();
-    assertThrows(IllegalStateException.class, () -> builder.buildAsync(loader));
+    assertThrows(IllegalStateException.class, () -> builder.buildAsync());
   }
 
   @Test
   public void async_softValues() {
     var builder = Caffeine.newBuilder().softValues();
-    assertThrows(IllegalStateException.class, () -> builder.buildAsync(loader));
+    assertThrows(IllegalStateException.class, () -> builder.buildAsync());
   }
 
   @Test

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/EvictionTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/EvictionTest.java
@@ -69,6 +69,7 @@
 import com.github.benmanes.caffeine.cache.testing.CheckNoStats;
 import com.github.benmanes.caffeine.cache.testing.RemovalListeners.RejectingRemovalListener;
 import com.github.benmanes.caffeine.testing.Int;
+import com.google.common.collect.HashBiMap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Range;
@@ -325,6 +326,46 @@ public void getAll_weigherFails_async(AsyncCache<Int, Int> cache, CacheContext c
         .hasSize(context.absentKeys().size());
   }
 
+  @Test(dataProvider = "caches")
+  @CacheSpec(population = Population.FULL,
+      maximumSize = Maximum.FULL, weigher = CacheWeigher.MOCKITO)
+  public void getAll_weigherFails_newEntries(Cache<Int, Int> cache, CacheContext context) {
+    when(context.weigher().weigh(any(), any())).thenThrow(IllegalStateException.class);
+    assertThrows(IllegalStateException.class, () ->
+        cache.getAll(context.absentKeys(), keys -> HashBiMap.create(context.original()).inverse()));
+    assertThat(cache).containsExactlyEntriesIn(context.original());
+    if (context.isCaffeine()) {
+      assertThat(logEvents()
+          .withMessage("Exception thrown during asynchronous load")
+          .withThrowable(IllegalStateException.class)
+          .withLevel(WARN)
+          .exclusively())
+          .hasSize(context.isAsync() ? context.original().size() : 0);
+    }
+  }
+
+  @Test(dataProvider = "caches")
+  @CacheSpec(population = Population.FULL,
+      maximumSize = Maximum.FULL, weigher = CacheWeigher.MOCKITO)
+  public void getAll_weigherFails_newEntries_async(
+      AsyncCache<Int, Int> cache, CacheContext context) {
+    when(context.weigher().weigh(any(), any())).thenThrow(IllegalStateException.class);
+    var future = new CompletableFuture<Map<Int, Int>>();
+    var result = cache.getAll(context.absentKeys(), (keys, executor) -> future);
+    future.complete(HashBiMap.create(context.original()).inverse());
+
+    assertThat(context).stats().failures(1);
+    assertThat(result).failsWith(CompletionException.class)
+        .hasCauseThat().isInstanceOf(IllegalStateException.class);
+    assertThat(cache).containsExactlyEntriesIn(context.original());
+    assertThat(logEvents()
+        .withMessage("Exception thrown during asynchronous load")
+        .withThrowable(IllegalStateException.class)
+        .withLevel(WARN)
+        .exclusively())
+        .hasSize(context.original().size());
+  }
+
   @Test(dataProvider = "caches")
   @CacheSpec(population = Population.EMPTY,
       maximumSize = Maximum.FULL, weigher = CacheWeigher.NEGATIVE)
@@ -563,49 +604,17 @@ public void replaceConditionally_weigherFails_absent(
   public void replaceConditionally_weigherFails_presentKey(
       Cache<Int, Int> cache, CacheContext context, Eviction<Int, Int> eviction) {
     when(context.weigher().weigh(any(), any())).thenThrow(IllegalStateException.class);
-    assertThrows(IllegalStateException.class, () -> cache.asMap().replace(
-        context.firstKey(), context.original().get(context.firstKey()), context.absentValue()));
-    assertThat(cache).containsExactlyEntriesIn(context.original());
-    assertThat(eviction.weightOf(context.firstKey())).hasValue(1);
-  }
-
-  @Test(dataProvider = "caches")
-  @CacheSpec(population = Population.FULL,
-      maximumSize = Maximum.FULL, weigher = CacheWeigher.MOCKITO)
-  public void replaceConditionally_weigherFails_presentKey_async(
-      AsyncCache<Int, Int> cache, CacheContext context) {
-    when(context.weigher().weigh(any(), any())).thenThrow(IllegalStateException.class);
-    var future = new CompletableFuture<Int>();
-    cache.asMap().replace(context.firstKey(), cache.getIfPresent(context.firstKey()), future);
-    future.complete(context.absentValue());
-    assertThat(context).stats().failures(1);
-    assertThat(cache).doesNotContainKey(context.absentKey());
-    assertThat(logEvents()
-        .withMessage("Exception thrown during asynchronous load")
-        .withThrowable(IllegalStateException.class)
-        .withLevel(WARN)
-        .exclusively())
-        .hasSize(1);
-  }
-
-  @Test(dataProvider = "caches")
-  @CacheSpec(population = Population.FULL,
-      maximumSize = Maximum.FULL, weigher = CacheWeigher.MOCKITO)
-  public void replaceConditionally_weigherFails_presentKeyAndValue(
-      Cache<Int, Int> cache, CacheContext context, Eviction<Int, Int> eviction) {
-    when(context.weigher().weigh(any(), any())).thenThrow(IllegalStateException.class);
-    assertThrows(IllegalStateException.class, () -> {
+    assertThrows(IllegalStateException.class, () ->
       cache.asMap().replace(context.firstKey(),
-          context.original().get(context.firstKey()), context.absentValue());
-    });
+          context.original().get(context.firstKey()), context.absentValue()));
     assertThat(cache).containsExactlyEntriesIn(context.original());
     assertThat(eviction.weightOf(context.firstKey())).hasValue(1);
   }
 
   @Test(dataProvider = "caches")
   @CacheSpec(population = Population.FULL,
       maximumSize = Maximum.FULL, weigher = CacheWeigher.MOCKITO)
-  public void replaceConditionally_weigherFails_presentKeyAndValue_async(
+  public void replaceConditionally_weigherFails_presentKey_async(
       AsyncCache<Int, Int> cache, CacheContext context) {
     when(context.weigher().weigh(any(), any())).thenThrow(IllegalStateException.class);
     var future = new CompletableFuture<Int>();

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpirationTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpirationTest.java
@@ -1557,7 +1557,7 @@ public void entrySet_toArray(Map<Int, Int> map, CacheContext context) {
       expireAfterWrite = {Expire.DISABLED, Expire.ONE_MINUTE}, expiryTime = Expire.ONE_MINUTE)
   public void entrySet_iterator(Map<Int, Int> map, CacheContext context) {
     context.ticker().advance(Duration.ofMinutes(10));
-    assertThat(map.keySet().iterator().hasNext()).isFalse();
+    assertThat(map.entrySet().iterator().hasNext()).isFalse();
     assertThat(map).isExhaustivelyEmpty();
     assertThat(context).notifications().withCause(EXPIRED)
         .contains(context.original()).exclusively();
@@ -1630,10 +1630,10 @@ public void entrySet_hashCode(Map<Int, Int> map, CacheContext context) {
     map.putAll(context.absent());
 
     context.ticker().advance(Duration.ofSeconds(45));
-    assertThat(map.hashCode()).isEqualTo(context.absent().hashCode());
+    assertThat(map.entrySet().hashCode()).isEqualTo(context.absent().entrySet().hashCode());
 
     context.cleanUp();
-    assertThat(map.hashCode()).isEqualTo(context.absent().hashCode());
+    assertThat(map.entrySet().hashCode()).isEqualTo(context.absent().entrySet().hashCode());
   }
 
   @Test(dataProvider = "caches")