fix sonarlint warnings for tests, remove duplicative, add missing

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

Hide file tree

Showing 30 changed files with 183 additions and 144 deletions.
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/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")

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterVarTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterVarTest.java
@@ -78,6 +78,7 @@
 import com.github.benmanes.caffeine.cache.testing.CheckNoStats;
 import com.github.benmanes.caffeine.testing.ConcurrentTestHarness;
 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.testing.SerializableTester;
@@ -412,6 +413,14 @@ public void getAll_expiryFails(Cache<Int, Int> cache, CacheContext context) {
         cache.getAll(context.absentKeys(), keys -> Maps.toMap(keys, Int::negate)));
     context.ticker().advance(Duration.ofHours(-1));
     assertThat(cache).containsExactlyEntriesIn(context.original());
+    if (context.isCaffeine()) {
+      assertThat(logEvents()
+          .withMessage("Exception thrown during asynchronous load")
+          .withThrowable(ExpirationException.class)
+          .withLevel(WARN)
+          .exclusively())
+          .hasSize(context.isAsync() ? context.absentKeys().size() : 0);
+    }
   }
 
   @Test(dataProvider = "caches")
@@ -434,6 +443,47 @@ public void getAll_expiryFails_async(AsyncCache<Int, Int> cache, CacheContext co
         .hasSize(context.absentKeys().size());
   }
 
+  @Test(dataProvider = "caches")
+  @CacheSpec(population = Population.FULL, expiry = CacheExpiry.MOCKITO)
+  public void getAll_expiryFails_newEntries(Cache<Int, Int> cache, CacheContext context) {
+    context.ticker().advance(Duration.ofHours(1));
+    when(context.expiry().expireAfterCreate(any(), any(), anyLong()))
+        .thenThrow(ExpirationException.class);
+    assertThrows(ExpirationException.class, () ->
+        cache.getAll(context.absentKeys(), keys -> HashBiMap.create(context.original()).inverse()));
+    context.ticker().advance(Duration.ofHours(-1));
+    assertThat(cache).containsExactlyEntriesIn(context.original());
+    if (context.isCaffeine()) {
+      assertThat(logEvents()
+          .withMessage("Exception thrown during asynchronous load")
+          .withThrowable(ExpirationException.class)
+          .withLevel(WARN)
+          .exclusively())
+          .hasSize(context.isAsync() ? context.original().size() : 0);
+    }
+  }
+
+  @Test(dataProvider = "caches")
+  @CacheSpec(population = Population.FULL, expiry = CacheExpiry.MOCKITO)
+  public void getAll_expiryFails_newEntries_async(
+      AsyncCache<Int, Int> cache, CacheContext context) {
+    context.ticker().advance(Duration.ofHours(1));
+    when(context.expiry().expireAfterCreate(any(), any(), anyLong()))
+        .thenThrow(ExpirationException.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(result).failsWith(CompletionException.class)
+        .hasCauseThat().isInstanceOf(ExpirationException.class);
+    assertThat(cache).containsExactlyEntriesIn(context.original());
+    assertThat(logEvents()
+        .withMessage("Exception thrown during asynchronous load")
+        .withThrowable(ExpirationException.class)
+        .withLevel(WARN)
+        .exclusively())
+        .hasSize(context.original().size());
+  }
+
   @Test(dataProvider = "caches")
   @CacheSpec(population = Population.FULL, expiry = CacheExpiry.MOCKITO)
   public void getAllPresent_expiryFails(Cache<Int, Int> cache, CacheContext context) {

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/FrequencySketchTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/FrequencySketchTest.java
@@ -125,13 +125,13 @@ public void full() {
     for (int i = 0; i < 100_000; i++) {
       sketch.increment(i);
     }
-    for (long item : sketch.table) {
-      assertThat(Long.bitCount(item)).isEqualTo(64);
+    for (long slot : sketch.table) {
+      assertThat(Long.bitCount(slot)).isEqualTo(64);
     }
 
     sketch.reset();
-    for (long item : sketch.table) {
-      assertThat(item).isEqualTo(FrequencySketch.RESET_MASK);
+    for (long slot : sketch.table) {
+      assertThat(slot).isEqualTo(FrequencySketch.RESET_MASK);
     }
   }
 

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/LoadingCacheTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/LoadingCacheTest.java
@@ -1110,7 +1110,7 @@ public void refreshAll_nullFuture_reload(CacheContext context) {
   /* --------------- CacheLoader --------------- */
 
   @Test
-  public void loadAll() throws Exception {
+  public void loadAll() {
     CacheLoader<Object, ?> loader = key -> key;
     assertThrows(UnsupportedOperationException.class, () -> loader.loadAll(Set.of()));
   }

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/QueueSanityTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/QueueSanityTest.java
@@ -87,7 +87,6 @@ public void sanity() {
     } else {
       // expect sum of elements is (size - 1) * size / 2 = 0 + 1 + .... + (size - 1)
       int sum = (size - 1) * size / 2;
-      i = 0;
       Integer e;
       while ((e = queue.poll()) != null) {
         assertEquals(--size, queue.size());

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java
@@ -964,9 +964,10 @@ public void setRefreshAfter_excessive(Cache<Int, Int> cache,
   }
 
   @Test(dataProvider = "caches")
+  @SuppressWarnings("PreferJavaTimeOverload")
   @CacheSpec(refreshAfterWrite = Expire.ONE_MINUTE)
   public void setRefreshesAfter(CacheContext context, FixedRefresh<Int, Int> refreshAfterWrite) {
-    refreshAfterWrite.setRefreshesAfter(Duration.ofMinutes(2));
+    refreshAfterWrite.setRefreshesAfter(2, TimeUnit.MINUTES);
     assertThat(refreshAfterWrite.getRefreshesAfter().toMinutes()).isEqualTo(2);
     assertThat(refreshAfterWrite.getRefreshesAfter(TimeUnit.MINUTES)).isEqualTo(2);
   }

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/TimerWheelTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/TimerWheelTest.java
@@ -82,7 +82,7 @@ public void beforeMethod() throws Exception {
   }
 
   @AfterMethod
-  public void afterMethod(ITestResult testResult) throws Exception {
+  public void afterMethod(ITestResult testResult) {
     if (!testResult.isSuccess()) {
       printTimerWheel();
     }

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/buffer/BufferTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/buffer/BufferTest.java
@@ -42,7 +42,7 @@ public Iterator<Object[]> buffers() {
 
   @Test(dataProvider = "buffers")
   @SuppressWarnings("ThreadPriorityCheck")
-  public void record(ReadBuffer<Boolean> buffer) {
+  public void offer(ReadBuffer<Boolean> buffer) {
     ConcurrentTestHarness.timeTasks(100, () -> {
       for (int i = 0; i < 1000; i++) {
         int added = buffer.offer(Boolean.TRUE);