Address [CatchFail](https://errorprone.info/bugpattern/CatchFail), [StaticQualifiedUsingExpression](https://errorprone.info/bugpattern/StaticQualifiedUsingExpression), [UnusedMethod](https://errorprone.info/bugpattern/UnusedMethod), and [CacheLoaderNull](https://errorprone.info/bugpattern/CacheLoaderNull) warnings.

And use `assertThrows` in some more places where it is now obvious that that is possible.

PiperOrigin-RevId: 686721569

Author: cpovirk

android/guava-testlib/test/com/google/common/collect/testing/features/FeatureEnumTest.java

@@ -52,8 +52,7 @@ private static void assertGoodTesterAnnotation(Class<? extends Annotation> annot
       try {
         method = annotationClass.getMethod(propertyName);
       } catch (NoSuchMethodException e) {
-        fail(
-            rootLocaleFormat("%s must have a property named '%s'.", annotationClass, propertyName));
+        throw new AssertionError("Annotation is missing required method", e);
       }
       final Class<?> returnType = method.getReturnType();
       assertTrue(

android/guava-tests/test/com/google/common/cache/CacheLoadingTest.java

@@ -21,7 +21,6 @@
 import static com.google.common.cache.TestingCacheLoaders.identityLoader;
 import static com.google.common.cache.TestingRemovalListeners.countingRemovalListener;
 import static com.google.common.truth.Truth.assertThat;
-import static java.lang.Thread.currentThread;
 import static java.util.Arrays.asList;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.junit.Assert.assertThrows;
@@ -75,7 +74,7 @@ public void setUp() throws Exception {
   public void tearDown() throws Exception {
     super.tearDown();
     // TODO(cpovirk): run tests in other thread instead of messing with main thread interrupt status
-    currentThread().interrupted();
+    Thread.interrupted();
     LocalCache.logger.removeHandler(logHandler);
   }
 
@@ -1148,11 +1147,11 @@ public void testLoadInterruptedException() {
     assertEquals(0, stats.hitCount());
 
     // Sanity check:
-    assertFalse(currentThread().interrupted());
+    assertFalse(Thread.interrupted());
 
     Exception expected = assertThrows(ExecutionException.class, () -> cache.get(new Object()));
     assertThat(expected).hasCauseThat().isSameInstanceAs(e);
-    assertTrue(currentThread().interrupted());
+    assertTrue(Thread.interrupted());
     stats = cache.stats();
     assertEquals(1, stats.missCount());
     assertEquals(0, stats.loadSuccessCount());
@@ -1162,15 +1161,15 @@ public void testLoadInterruptedException() {
     expected =
         assertThrows(UncheckedExecutionException.class, () -> cache.getUnchecked(new Object()));
     assertThat(expected).hasCauseThat().isSameInstanceAs(e);
-    assertTrue(currentThread().interrupted());
+    assertTrue(Thread.interrupted());
     stats = cache.stats();
     assertEquals(2, stats.missCount());
     assertEquals(0, stats.loadSuccessCount());
     assertEquals(2, stats.loadExceptionCount());
     assertEquals(0, stats.hitCount());
 
     cache.refresh(new Object());
-    assertTrue(currentThread().interrupted());
+    assertTrue(Thread.interrupted());
     checkLoggedCause(e);
     stats = cache.stats();
     assertEquals(2, stats.missCount());
@@ -1183,7 +1182,7 @@ public void testLoadInterruptedException() {
         assertThrows(
             ExecutionException.class, () -> cache.get(new Object(), throwing(callableException)));
     assertThat(expected).hasCauseThat().isSameInstanceAs(callableException);
-    assertTrue(currentThread().interrupted());
+    assertTrue(Thread.interrupted());
     stats = cache.stats();
     assertEquals(3, stats.missCount());
     assertEquals(0, stats.loadSuccessCount());
@@ -1192,7 +1191,7 @@ public void testLoadInterruptedException() {
 
     expected = assertThrows(ExecutionException.class, () -> cache.getAll(asList(new Object())));
     assertThat(expected).hasCauseThat().isSameInstanceAs(e);
-    assertTrue(currentThread().interrupted());
+    assertTrue(Thread.interrupted());
     stats = cache.stats();
     assertEquals(4, stats.missCount());
     assertEquals(0, stats.loadSuccessCount());
@@ -1392,7 +1391,7 @@ public void testBulkLoadInterruptedException() {
     ExecutionException expected =
         assertThrows(ExecutionException.class, () -> cache.getAll(asList(new Object())));
     assertThat(expected).hasCauseThat().isSameInstanceAs(e);
-    assertTrue(currentThread().interrupted());
+    assertTrue(Thread.interrupted());
     stats = cache.stats();
     assertEquals(1, stats.missCount());
     assertEquals(0, stats.loadSuccessCount());
@@ -1765,31 +1764,22 @@ public void testLoadingExceptionWithCause() {
     LoadingCache<Object, Object> cacheChecked =
         CacheBuilder.newBuilder().build(exceptionLoader(ee));
 
-    try {
-      cacheUnchecked.get(new Object());
-      fail();
-    } catch (ExecutionException e) {
-      fail();
-    } catch (UncheckedExecutionException caughtEe) {
-      assertThat(caughtEe).hasCauseThat().isSameInstanceAs(uee);
-    }
-
     UncheckedExecutionException caughtUee =
+        assertThrows(UncheckedExecutionException.class, () -> cacheUnchecked.get(new Object()));
+    assertThat(caughtUee).hasCauseThat().isSameInstanceAs(uee);
+
+    caughtUee =
         assertThrows(
             UncheckedExecutionException.class, () -> cacheUnchecked.getUnchecked(new Object()));
     assertThat(caughtUee).hasCauseThat().isSameInstanceAs(uee);
 
     cacheUnchecked.refresh(new Object());
     checkLoggedCause(uee);
 
-    try {
-      cacheUnchecked.getAll(asList(new Object()));
-      fail();
-    } catch (ExecutionException e) {
-      fail();
-    } catch (UncheckedExecutionException caughtEe) {
-      assertThat(caughtEe).hasCauseThat().isSameInstanceAs(uee);
-    }
+    caughtUee =
+        assertThrows(
+            UncheckedExecutionException.class, () -> cacheUnchecked.getAll(asList(new Object())));
+    assertThat(caughtUee).hasCauseThat().isSameInstanceAs(uee);
 
     ExecutionException caughtEe =
         assertThrows(ExecutionException.class, () -> cacheChecked.get(new Object()));
@@ -1818,14 +1808,10 @@ public void testBulkLoadingExceptionWithCause() {
     LoadingCache<Object, Object> cacheChecked =
         CacheBuilder.newBuilder().build(bulkLoader(exceptionLoader(ee)));
 
-    try {
-      cacheUnchecked.getAll(asList(new Object()));
-      fail();
-    } catch (ExecutionException e) {
-      fail();
-    } catch (UncheckedExecutionException caughtEe) {
-      assertThat(caughtEe).hasCauseThat().isSameInstanceAs(uee);
-    }
+    UncheckedExecutionException caughtUee =
+        assertThrows(
+            UncheckedExecutionException.class, () -> cacheUnchecked.getAll(asList(new Object())));
+    assertThat(caughtUee).hasCauseThat().isSameInstanceAs(uee);
 
     ExecutionException caughtEe =
         assertThrows(ExecutionException.class, () -> cacheChecked.getAll(asList(new Object())));
@@ -1897,6 +1883,7 @@ private static void testConcurrentLoadingNull(CacheBuilder<Object, Object> build
         builder.build(
             new CacheLoader<String, String>() {
               @Override
+              @SuppressWarnings("CacheLoaderNull") // test of broken user implementation
               public String load(String key) throws InterruptedException {
                 callCount.incrementAndGet();
                 startSignal.await();

android/guava-tests/test/com/google/common/eventbus/EventBusTest.java

@@ -159,11 +159,7 @@ public void throwExceptionOn(String message) {
           }
         };
     eventBus.register(subscriber);
-    try {
-      eventBus.post(EVENT);
-    } catch (RuntimeException e) {
-      fail("Exception should not be thrown.");
-    }
+    eventBus.post(EVENT);
   }
 
   public void testDeadEventForwarding() {

android/guava-tests/test/com/google/common/util/concurrent/JSR166TestCase.java

@@ -99,6 +99,8 @@
  *       tests.
  * </ul>
  */
+// We call threadUnexpectedException, which does the right thing for errors.
+@SuppressWarnings("AssertionFailureIgnored")
 abstract class JSR166TestCase extends TestCase {
   private static final boolean useSecurityManager = Boolean.getBoolean("jsr166.useSecurityManager");
 
@@ -449,54 +451,46 @@ static void delay(long millis) throws InterruptedException {
   }
 
   /** Waits out termination of a thread pool or fails doing so. */
-  void joinPool(ExecutorService exec) {
+  void joinPool(ExecutorService exec) throws InterruptedException {
     try {
       exec.shutdown();
       assertTrue(
           "ExecutorService did not terminate in a timely manner",
           exec.awaitTermination(2 * LONG_DELAY_MS, MILLISECONDS));
     } catch (SecurityException ok) {
       // Allowed in case test doesn't have privs
-    } catch (InterruptedException ie) {
-      fail("Unexpected InterruptedException");
     }
   }
 
   /**
    * Checks that thread does not terminate within the default millisecond delay of {@code
    * timeoutMillis()}.
    */
-  void assertThreadStaysAlive(Thread thread) {
+  void assertThreadStaysAlive(Thread thread) throws InterruptedException {
     assertThreadStaysAlive(thread, timeoutMillis());
   }
 
   /** Checks that thread does not terminate within the given millisecond delay. */
-  void assertThreadStaysAlive(Thread thread, long millis) {
-    try {
-      // No need to optimize the failing case via Thread.join.
-      delay(millis);
-      assertTrue(thread.isAlive());
-    } catch (InterruptedException ie) {
-      fail("Unexpected InterruptedException");
-    }
+  void assertThreadStaysAlive(Thread thread, long millis) throws InterruptedException {
+    // No need to optimize the failing case via Thread.join.
+    delay(millis);
+    assertTrue(thread.isAlive());
   }
 
   /**
    * Checks that the threads do not terminate within the default millisecond delay of {@code
    * timeoutMillis()}.
    */
-  void assertThreadsStayAlive(Thread... threads) {
+  void assertThreadsStayAlive(Thread... threads) throws InterruptedException {
     assertThreadsStayAlive(timeoutMillis(), threads);
   }
 
   /** Checks that the threads do not terminate within the given millisecond delay. */
-  void assertThreadsStayAlive(long millis, Thread... threads) {
-    try {
-      // No need to optimize the failing case via Thread.join.
-      delay(millis);
-      for (Thread thread : threads) assertTrue(thread.isAlive());
-    } catch (InterruptedException ie) {
-      fail("Unexpected InterruptedException");
+  void assertThreadsStayAlive(long millis, Thread... threads) throws InterruptedException {
+    // No need to optimize the failing case via Thread.join.
+    delay(millis);
+    for (Thread thread : threads) {
+      assertTrue(thread.isAlive());
     }
   }
 

guava-testlib/test/com/google/common/collect/testing/features/FeatureEnumTest.java

@@ -52,8 +52,7 @@ private static void assertGoodTesterAnnotation(Class<? extends Annotation> annot
       try {
         method = annotationClass.getMethod(propertyName);
       } catch (NoSuchMethodException e) {
-        fail(
-            rootLocaleFormat("%s must have a property named '%s'.", annotationClass, propertyName));
+        throw new AssertionError("Annotation is missing required method", e);
       }
       final Class<?> returnType = method.getReturnType();
       assertTrue(

guava-testlib/test/com/google/common/testing/NullPointerTesterTest.java

@@ -56,7 +56,10 @@
  * @author Kevin Bourrillion
  * @author Mick Killianey
  */
-@SuppressWarnings("CheckReturnValue")
+@SuppressWarnings({
+  "CheckReturnValue",
+  "unused", // many methods tested reflectively -- maybe prefer local @Keep annotations?
+})
 public class NullPointerTesterTest extends TestCase {
 
   /** Non-NPE RuntimeException. */
@@ -268,7 +271,7 @@ public void testStaticOneArgMethodsThatShouldPass() throws Exception {
       try {
         new NullPointerTester().testMethodParameter(new OneArg(), method, 0);
       } catch (AssertionError unexpected) {
-        fail("Should not have flagged method " + methodName);
+        throw new AssertionError("Should not have flagged method " + methodName, unexpected);
       }
     }
   }
@@ -293,7 +296,7 @@ public void testNonStaticOneArgMethodsThatShouldPass() throws Exception {
       try {
         new NullPointerTester().testMethodParameter(foo, method, 0);
       } catch (AssertionError unexpected) {
-        fail("Should not have flagged method " + methodName);
+        throw new AssertionError("Should not have flagged method " + methodName, unexpected);
       }
     }
   }