Skip to content

Commit

Permalink
run spotbugs static analyzer agaisnt the test suite
Browse files Browse the repository at this point in the history
  • Loading branch information
ben-manes committed Sep 22, 2024
1 parent eafcb4a commit 477976f
Show file tree
Hide file tree
Showing 20 changed files with 290 additions and 80 deletions.
2 changes: 1 addition & 1 deletion caffeine/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ data class Scenario(val implementation: Implementation,
}

fun testName(): String = buildString {
append(keys.name.lowercase() + "Keys")
append(keys.name.lowercase()).append("Keys")
append("And").append(values).append("Values")
if (stats == Stats.Enabled) {
append("Stats")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
import static org.slf4j.event.Level.WARN;

import java.util.AbstractMap;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
Expand Down Expand Up @@ -1810,7 +1810,7 @@ public void keySet_removeAll_null(Map<Int, Int> map, CacheContext context) {
@CheckNoStats
@Test(dataProvider = "caches")
public void keySet_removeAll_nullKey(Map<Int, Int> map, CacheContext context) {
map.keySet().removeAll(Arrays.asList((Object) null));
map.keySet().removeAll(Collections.singletonList((Object) null));
assertThat(map).isEqualTo(context.original());
}

Expand Down Expand Up @@ -1978,7 +1978,7 @@ public void keySet_retainAll_null(Map<Int, Int> map, CacheContext context) {
@CheckNoStats
@Test(dataProvider = "caches")
public void keySet_retainAll_nullKey(Map<Int, Int> map, CacheContext context) {
map.keySet().retainAll(Arrays.asList((Object) null));
map.keySet().retainAll(Collections.singletonList((Object) null));
assertThat(map).isExhaustivelyEmpty();
}

Expand Down Expand Up @@ -2221,7 +2221,7 @@ public void values_removeAll_null(Map<Int, Int> map, CacheContext context) {
@CheckNoStats
@Test(dataProvider = "caches")
public void values_removeAll_nullValue(Map<Int, Int> map, CacheContext context) {
map.values().removeAll(Arrays.asList((Object) null));
map.values().removeAll(Collections.singletonList((Object) null));
assertThat(map).isEqualTo(context.original());
}

Expand Down Expand Up @@ -2382,7 +2382,7 @@ public void values_retainAll_null(Map<Int, Int> map, CacheContext context) {
@CheckNoStats
@Test(dataProvider = "caches")
public void values_retainAll_nullValue(Map<Int, Int> map, CacheContext context) {
map.values().retainAll(Arrays.asList((Object) null));
map.values().retainAll(Collections.singletonList((Object) null));
assertThat(map).isExhaustivelyEmpty();
}

Expand Down Expand Up @@ -2651,7 +2651,7 @@ public void entrySet_removeAll_null(Map<Int, Int> map, CacheContext context) {
@CheckNoStats
@Test(dataProvider = "caches")
public void entrySet_removeAll_nullEntry(Map<Int, Int> map, CacheContext context) {
map.entrySet().removeAll(Arrays.asList((Object) null));
map.entrySet().removeAll(Collections.singletonList((Object) null));
assertThat(map).isEqualTo(context.original());
}

Expand Down Expand Up @@ -2853,7 +2853,7 @@ public void entrySet_retainAll_null(Map<Int, Int> map, CacheContext context) {
@CheckNoStats
@Test(dataProvider = "caches")
public void entrySet_retainAll_nullEntry(Map<Int, Int> map, CacheContext context) {
map.entrySet().retainAll(Arrays.asList((Object) null));
map.entrySet().retainAll(Collections.singletonList((Object) null));
assertThat(map).isExhaustivelyEmpty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.AbstractMap;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -1534,7 +1535,7 @@ public void keySet_removeAll_null(AsyncCache<Int, Int> cache, CacheContext conte
@CheckNoStats
@Test(dataProvider = "caches")
public void keySet_removeAll_nullKey(AsyncCache<Int, Int> cache, CacheContext context) {
cache.asMap().keySet().removeAll(Arrays.asList((Object) null));
cache.asMap().keySet().removeAll(Collections.singletonList((Object) null));
assertThat(cache.synchronous().asMap()).isEqualTo(context.original());
}

Expand Down Expand Up @@ -1698,7 +1699,7 @@ public void keySet_retainAll_null(AsyncCache<Int, Int> cache, CacheContext conte
@CheckNoStats
@Test(dataProvider = "caches")
public void keySet_retainAll_nullKey(AsyncCache<Int, Int> cache, CacheContext context) {
cache.asMap().keySet().retainAll(Arrays.asList((Object) null));
cache.asMap().keySet().retainAll(Collections.singletonList((Object) null));
assertThat(cache).isEmpty();
}

Expand Down Expand Up @@ -1948,7 +1949,7 @@ public void values_removeAll_null(AsyncCache<Int, Int> cache, CacheContext conte
@CheckNoStats
@Test(dataProvider = "caches")
public void values_removeAll_nullValue(AsyncCache<Int, Int> cache, CacheContext context) {
cache.asMap().values().removeAll(Arrays.asList((Object) null));
cache.asMap().values().removeAll(Collections.singletonList((Object) null));
assertThat(cache.synchronous().asMap()).isEqualTo(context.original());
}

Expand Down Expand Up @@ -2126,7 +2127,7 @@ public void values_retainAll_null(AsyncCache<Int, Int> cache, CacheContext conte
@CheckNoStats
@Test(dataProvider = "caches")
public void values_retainAll_nullValue(AsyncCache<Int, Int> cache, CacheContext context) {
cache.asMap().values().retainAll(Arrays.asList((Object) null));
cache.asMap().values().retainAll(Collections.singletonList((Object) null));
assertThat(cache).isEmpty();
}

Expand Down Expand Up @@ -2410,7 +2411,7 @@ public void entrySet_removeAll_null(AsyncCache<Int, Int> cache, CacheContext con
@CheckNoStats
@Test(dataProvider = "caches")
public void entrySet_removeAll_nullEntry(AsyncCache<Int, Int> cache, CacheContext context) {
cache.asMap().entrySet().removeAll(Arrays.asList((Object) null));
cache.asMap().entrySet().removeAll(Collections.singletonList((Object) null));
assertThat(cache.synchronous().asMap()).isEqualTo(context.original());
}

Expand Down Expand Up @@ -2614,7 +2615,7 @@ public void entrySet_retainAll_null(AsyncCache<Int, Int> cache, CacheContext con
@CheckNoStats
@Test(dataProvider = "caches")
public void entrySet_retainAll_nullEntry(AsyncCache<Int, Int> cache, CacheContext context) {
cache.asMap().entrySet().retainAll(Arrays.asList((Object) null));
cache.asMap().entrySet().retainAll(Collections.singletonList((Object) null));
assertThat(cache).isEmpty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;

import org.mockito.Mockito;
import org.testng.annotations.DataProvider;
Expand Down Expand Up @@ -80,7 +81,7 @@ public void getWhenSuccessful_success(CompletableFuture<Integer> future) {
@Test
public void getWhenSuccessful_success_async() {
var future = new CompletableFuture<Integer>();
var result = new AtomicInteger();
var result = new AtomicReference<Integer>();
ConcurrentTestHarness.execute(() -> {
result.set(1);
result.set(Async.getWhenSuccessful(future));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -769,20 +769,21 @@ public void evict_wtinylfu(Cache<Int, Int> cache, CacheContext context) {
assertThat(context).stats().evictions(6);
}

private static void checkReorder(Cache<Int, Int> cache, List<Int> keys, List<Int> expect) {
private static void checkReorder(Cache<Int, Int> cache,
Iterable<Int> keys, Iterable<Int> expect) {
for (var key : keys) {
var value = cache.getIfPresent(key);
assertThat(value).isNotNull();
}
checkContainsInOrder(cache, expect);
}

private static void checkEvict(Cache<Int, Int> cache, List<Int> keys, List<Int> expect) {
private static void checkEvict(Cache<Int, Int> cache, Iterable<Int> keys, Iterable<Int> expect) {
keys.forEach(i -> cache.put(i, i));
checkContainsInOrder(cache, expect);
}

private static void checkContainsInOrder(Cache<Int, Int> cache, List<Int> expect) {
private static void checkContainsInOrder(Cache<Int, Int> cache, Iterable<Int> expect) {
var evictionOrder = cache.policy().eviction().orElseThrow().coldest(Integer.MAX_VALUE).keySet();
assertThat(cache).containsExactlyKeys(expect);
assertThat(evictionOrder).containsExactlyElementsIn(expect).inOrder();
Expand Down Expand Up @@ -2839,7 +2840,7 @@ static final class BadBoundedLocalCache<K, V> extends BoundedLocalCache<K, V> {
}
static final class BadLocalCacheFactory implements LocalCacheFactory {
@Override public <K, V> BoundedLocalCache<K, V> newInstance(Caffeine<K, V> builder,
AsyncCacheLoader<? super K, V> cacheLoader, boolean async) throws Throwable {
AsyncCacheLoader<? super K, V> cacheLoader, boolean async) {
throw new IllegalStateException();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,7 @@ public void invalidateAll_removalListener_writeback(Cache<Int, Int> cache, Cache
@CacheSpec
@CheckNoStats
@Test(dataProvider = "caches")
public void cleanup(Cache<Int, Int> cache, CacheContext context) {
public void cleanUp(Cache<Int, Int> cache, CacheContext context) {
cache.cleanUp();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import static org.slf4j.event.Level.WARN;

import java.time.Duration;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -73,7 +72,9 @@
import com.github.benmanes.caffeine.cache.testing.CheckNoStats;
import com.github.benmanes.caffeine.testing.Int;
import com.google.common.base.Splitter;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimaps;
import com.google.common.collect.Range;
import com.google.common.util.concurrent.Futures;

Expand Down Expand Up @@ -1217,15 +1218,15 @@ public void compute(Map<Int, Int> map, CacheContext context) {
return value;
})).isEqualTo(value);

var evicted = new ArrayList<Map.Entry<Int, Int>>(context.original().size());
var evicted = ArrayListMultimap.<Int, Int>create();
var difference = Maps.difference(context.original(), map);
evicted.addAll(difference.entriesOnlyOnRight().entrySet());
evicted.addAll(difference.entriesOnlyOnLeft().entrySet());
evicted.add(entry(key, context.original().get(key)));
evicted.putAll(Multimaps.forMap(difference.entriesOnlyOnRight()));
evicted.putAll(Multimaps.forMap(difference.entriesOnlyOnLeft()));
evicted.put(key, context.original().get(key));

assertThat(evicted).hasSize(context.original().size() - map.size() + 1);
assertThat(context).notifications().withCause(EXPIRED)
.contains(evicted.toArray(Map.Entry[]::new))
.contains(evicted.entries())
.exclusively();

if (context.expiryType() == CacheExpiry.MOCKITO) {
Expand Down Expand Up @@ -1357,15 +1358,15 @@ public void merge(Map<Int, Int> map, CacheContext context) {
throw new AssertionError("Should never be called");
})).isEqualTo(value);

var evicted = new ArrayList<Map.Entry<Int, Int>>(context.original().size());
var evicted = ArrayListMultimap.<Int, Int>create();
var difference = Maps.difference(context.original(), map);
evicted.addAll(difference.entriesOnlyOnRight().entrySet());
evicted.addAll(difference.entriesOnlyOnLeft().entrySet());
evicted.add(entry(key, context.original().get(key)));
evicted.putAll(Multimaps.forMap(difference.entriesOnlyOnRight()));
evicted.putAll(Multimaps.forMap(difference.entriesOnlyOnLeft()));
evicted.put(key, context.original().get(key));

assertThat(evicted).hasSize(context.original().size() - map.size() + 1);
assertThat(context).notifications().withCause(EXPIRED)
.contains(evicted.toArray(Map.Entry[]::new))
.contains(evicted.entries())
.exclusively();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@

import java.util.AbstractMap;
import java.util.AbstractMap.SimpleEntry;
import java.util.AbstractMap.SimpleImmutableEntry;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -1300,13 +1301,14 @@ public Object[][] providesReferences() {
};
}

@SuppressWarnings("MapEntry")
private static List<Map.Entry<Int, Int>> getExpectedAfterGc(
CacheContext context, Map<Int, Int> original) {
var expected = new ArrayList<Map.Entry<Int, Int>>();
original.forEach((key, value) -> {
key = context.isStrongKeys() ? new Int(key) : null;
value = context.isStrongValues() ? new Int(value) : null;
expected.add(new SimpleEntry<>(key, value));
expected.add(new SimpleImmutableEntry<>(key, value));
});
return expected;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,16 @@ private void execute() {
}

private void status() {
int drainStatus;
var evictionLock = local.evictionLock;
int pendingWrites;
local.evictionLock.lock();
int drainStatus;

evictionLock.lock();
try {
pendingWrites = local.writeBuffer.size();
drainStatus = local.drainStatusAcquire();
} finally {
local.evictionLock.unlock();
evictionLock.unlock();
}

var elapsedTime = LocalTime.ofSecondOfDay(stopwatch.elapsed(SECONDS));
Expand All @@ -134,8 +136,7 @@ private void status() {
System.out.printf(US, "Evictions = %,d%n", cache.stats().evictionCount());
System.out.printf(US, "Size = %,d (max: %,d)%n",
local.data.mappingCount(), workload.maxEntries);
System.out.printf(US, "Lock = [%s%n", StringUtils.substringAfter(
local.evictionLock.toString(), "["));
System.out.printf(US, "Lock = [%s%n", StringUtils.substringAfter(evictionLock.toString(), "["));
System.out.printf(US, "Pending reloads = %,d%n", local.refreshes.size());
System.out.printf(US, "Pending tasks = %,d%n",
ForkJoinPool.commonPool().getQueuedSubmissionCount());
Expand Down Expand Up @@ -163,8 +164,8 @@ private enum Workload {
WRITE(MAX_THREADS, WRITE_MAX_SIZE),
REFRESH(MAX_THREADS, TOTAL_KEYS / 4);

private final int maxThreads;
private final int maxEntries;
final int maxThreads;
final int maxEntries;

Workload(int maxThreads, int maxEntries) {
this.maxThreads = maxThreads;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

import java.util.AbstractMap.SimpleEntry;
import java.util.Arrays;
import java.util.List;
import java.util.Collection;
import java.util.Map;
import java.util.Map.Entry;
import java.util.function.BiConsumer;
Expand Down Expand Up @@ -366,7 +366,7 @@ public Exclusive contains(Map<Int, Int> map) {
}

@CanIgnoreReturnValue
public Exclusive contains(List<Entry<Int, Int>> entries) {
public Exclusive contains(Collection<Entry<Int, Int>> entries) {
return contains(entries.toArray(Map.Entry[]::new));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.lang.annotation.Target;
import java.time.Duration;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
Expand All @@ -57,6 +56,7 @@
import com.github.benmanes.caffeine.cache.testing.RemovalListeners.ConsumingRemovalListener;
import com.github.benmanes.caffeine.testing.ConcurrentTestHarness;
import com.github.benmanes.caffeine.testing.Int;
import com.google.common.collect.Maps;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.testing.TestingExecutors;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
Expand Down Expand Up @@ -471,7 +471,7 @@ enum Loader implements CacheLoader<Int, Int> {
throw new UnsupportedOperationException();
}
@Override public Map<Int, Int> loadAll(Set<? extends Int> keys) {
var result = new HashMap<Int, Int>(keys.size());
Map<Int, Int> result = Maps.newHashMapWithExpectedSize(keys.size());
for (Int key : keys) {
result.put(key, key);
intern(key);
Expand All @@ -484,7 +484,7 @@ enum Loader implements CacheLoader<Int, Int> {
throw new UnsupportedOperationException();
}
@Override public Map<Int, Int> loadAll(Set<? extends Int> keys) throws Exception {
var result = new HashMap<Int, Int>(keys.size());
Map<Int, Int> result = Maps.newHashMapWithExpectedSize(keys.size());
for (Int key : keys) {
result.put(key, NEGATIVE.load(key));
intern(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,11 @@ private static void resetCache(ITestResult testResult) {
}

private static void clearTestResults(ITestResult testResult) {
var result = (TestResult) testResult;
result.setParameters(EMPTY_PARAMS);
result.setContext(testngContext);
testResult.setParameters(EMPTY_PARAMS);
if (testResult instanceof TestResult) {
var result = (TestResult) testResult;
result.setContext(testngContext);
}
}

private static void stringifyParams(ITestResult testResult, boolean briefParams) {
Expand Down
Loading

0 comments on commit 477976f

Please sign in to comment.