From bd28b92850325fec7641ea9d6adb2739e47381f9 Mon Sep 17 00:00:00 2001
From: Eric Anderson <ejona@google.com>
Date: Mon, 18 Sep 2017 09:54:14 -0700
Subject: [PATCH] Avoid catching AssertionError in tests

Found via ErrorProne
---
 .../src/test/java/io/grpc/ContextTest.java    | 11 +++++---
 .../java/io/grpc/internal/ServerImplTest.java | 27 ++++++++++---------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/context/src/test/java/io/grpc/ContextTest.java b/context/src/test/java/io/grpc/ContextTest.java
index b6ae7d38bb..4f1de7c38e 100644
--- a/context/src/test/java/io/grpc/ContextTest.java
+++ b/context/src/test/java/io/grpc/ContextTest.java
@@ -455,7 +455,7 @@ public class ContextTest {
     assertSame(current, observed);
     assertSame(current, Context.current());
 
-    final Error err = new Error();
+    final TestError err = new TestError();
     try {
       base.wrap(new Runnable() {
         @Override
@@ -464,7 +464,7 @@ public class ContextTest {
         }
       }).run();
       fail("Expected exception");
-    } catch (Error ex) {
+    } catch (TestError ex) {
       assertSame(err, ex);
     }
     assertSame(current, Context.current());
@@ -495,7 +495,7 @@ public class ContextTest {
     assertSame(current, observed);
     assertSame(current, Context.current());
 
-    final Error err = new Error();
+    final TestError err = new TestError();
     try {
       base.wrap(new Callable<Object>() {
         @Override
@@ -504,7 +504,7 @@ public class ContextTest {
         }
       }).call();
       fail("Excepted exception");
-    } catch (Error ex) {
+    } catch (TestError ex) {
       assertSame(err, ex);
     }
     assertSame(current, Context.current());
@@ -987,4 +987,7 @@ public class ContextTest {
       }
     }
   }
+
+  /** Allows more precise catch blocks than plain Error to avoid catching AssertionError. */
+  private static final class TestError extends Error {}
 }
diff --git a/core/src/test/java/io/grpc/internal/ServerImplTest.java b/core/src/test/java/io/grpc/internal/ServerImplTest.java
index 95e8645e02..d95f0b076f 100644
--- a/core/src/test/java/io/grpc/internal/ServerImplTest.java
+++ b/core/src/test/java/io/grpc/internal/ServerImplTest.java
@@ -1027,7 +1027,7 @@ public class ServerImplTest {
     ServerStreamListener mockListener = mock(ServerStreamListener.class);
     listener.setListener(mockListener);
 
-    Throwable expectedT = new AssertionError();
+    TestError expectedT = new TestError();
     doThrow(expectedT).when(mockListener)
         .messagesAvailable(any(StreamListener.MessageProducer.class));
     // Closing the InputStream is done by the delegated listener (generally ServerCallImpl)
@@ -1035,7 +1035,7 @@ public class ServerImplTest {
     try {
       executor.runDueTasks();
       fail("Expected exception");
-    } catch (Throwable t) {
+    } catch (TestError t) {
       assertSame(expectedT, t);
       ensureServerStateNotLeaked();
     }
@@ -1052,7 +1052,7 @@ public class ServerImplTest {
     ServerStreamListener mockListener = mock(ServerStreamListener.class);
     listener.setListener(mockListener);
 
-    Throwable expectedT = new RuntimeException();
+    RuntimeException expectedT = new RuntimeException();
     doThrow(expectedT).when(mockListener)
         .messagesAvailable(any(StreamListener.MessageProducer.class));
     // Closing the InputStream is done by the delegated listener (generally ServerCallImpl)
@@ -1060,7 +1060,7 @@ public class ServerImplTest {
     try {
       executor.runDueTasks();
       fail("Expected exception");
-    } catch (Throwable t) {
+    } catch (RuntimeException t) {
       assertSame(expectedT, t);
       ensureServerStateNotLeaked();
     }
@@ -1077,13 +1077,13 @@ public class ServerImplTest {
     ServerStreamListener mockListener = mock(ServerStreamListener.class);
     listener.setListener(mockListener);
 
-    Throwable expectedT = new AssertionError();
+    TestError expectedT = new TestError();
     doThrow(expectedT).when(mockListener).halfClosed();
     listener.halfClosed();
     try {
       executor.runDueTasks();
       fail("Expected exception");
-    } catch (Throwable t) {
+    } catch (TestError t) {
       assertSame(expectedT, t);
       ensureServerStateNotLeaked();
     }
@@ -1100,13 +1100,13 @@ public class ServerImplTest {
     ServerStreamListener mockListener = mock(ServerStreamListener.class);
     listener.setListener(mockListener);
 
-    Throwable expectedT = new RuntimeException();
+    RuntimeException expectedT = new RuntimeException();
     doThrow(expectedT).when(mockListener).halfClosed();
     listener.halfClosed();
     try {
       executor.runDueTasks();
       fail("Expected exception");
-    } catch (Throwable t) {
+    } catch (RuntimeException t) {
       assertSame(expectedT, t);
       ensureServerStateNotLeaked();
     }
@@ -1123,13 +1123,13 @@ public class ServerImplTest {
     ServerStreamListener mockListener = mock(ServerStreamListener.class);
     listener.setListener(mockListener);
 
-    Throwable expectedT = new AssertionError();
+    TestError expectedT = new TestError();
     doThrow(expectedT).when(mockListener).onReady();
     listener.onReady();
     try {
       executor.runDueTasks();
       fail("Expected exception");
-    } catch (Throwable t) {
+    } catch (TestError t) {
       assertSame(expectedT, t);
       ensureServerStateNotLeaked();
     }
@@ -1146,13 +1146,13 @@ public class ServerImplTest {
     ServerStreamListener mockListener = mock(ServerStreamListener.class);
     listener.setListener(mockListener);
 
-    Throwable expectedT = new RuntimeException();
+    RuntimeException expectedT = new RuntimeException();
     doThrow(expectedT).when(mockListener).onReady();
     listener.onReady();
     try {
       executor.runDueTasks();
       fail("Expected exception");
-    } catch (Throwable t) {
+    } catch (RuntimeException t) {
       assertSame(expectedT, t);
       ensureServerStateNotLeaked();
     }
@@ -1249,4 +1249,7 @@ public class ServerImplTest {
       throw new UnsupportedOperationException();
     }
   }
+
+  /** Allows more precise catch blocks than plain Error to avoid catching AssertionError. */
+  private static final class TestError extends Error {}
 }