all: Prefer mock+delegatesTo() over Mockito.spy()

Spies are really magical and easily produce unexpected results. Using them in
tests can easily yield tests that don't do what you think they do. Delegation
is much safer when possible.

Delegation doesn't work when methods `return true`, final methods, and with
restricted visibility, though. So CensusModulesTest and
MaxConnectionIdleManagerTest are left as-is.

1 files changed, 20 insertions, 19 deletions

diff --git a/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java b/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java
index 41c081a1c..9d8bc4f15 100644
--- a/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java
+++ b/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java
@@ -29,6 +29,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.AdditionalAnswers.delegatesTo;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.same;
@@ -37,7 +38,6 @@ import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -165,24 +165,25 @@ public class GrpclbLoadBalancerTest {
         .thenReturn(pickFirstBalancer);
     when(roundRobinBalancerFactory.newLoadBalancer(any(Helper.class)))
         .thenReturn(roundRobinBalancer);
-    mockLbService = spy(new LoadBalancerGrpc.LoadBalancerImplBase() {
-        @Override
-        public StreamObserver<LoadBalanceRequest> balanceLoad(
-            final StreamObserver<LoadBalanceResponse> responseObserver) {
-          StreamObserver<LoadBalanceRequest> requestObserver =
-              mock(StreamObserver.class);
-          Answer<Void> closeRpc = new Answer<Void>() {
-              @Override
-              public Void answer(InvocationOnMock invocation) {
-                responseObserver.onCompleted();
-                return null;
-              }
-            };
-          doAnswer(closeRpc).when(requestObserver).onCompleted();
-          lbRequestObservers.add(requestObserver);
-          return requestObserver;
-        }
-      });
+    mockLbService = mock(LoadBalancerGrpc.LoadBalancerImplBase.class, delegatesTo(
+        new LoadBalancerGrpc.LoadBalancerImplBase() {
+          @Override
+          public StreamObserver<LoadBalanceRequest> balanceLoad(
+              final StreamObserver<LoadBalanceResponse> responseObserver) {
+            StreamObserver<LoadBalanceRequest> requestObserver =
+                mock(StreamObserver.class);
+            Answer<Void> closeRpc = new Answer<Void>() {
+                @Override
+                public Void answer(InvocationOnMock invocation) {
+                  responseObserver.onCompleted();
+                  return null;
+                }
+              };
+            doAnswer(closeRpc).when(requestObserver).onCompleted();
+            lbRequestObservers.add(requestObserver);
+            return requestObserver;
+          }
+        }));
     fakeLbServer = InProcessServerBuilder.forName("fakeLb")
         .directExecutor().addService(mockLbService).build().start();
     doAnswer(new Answer<ManagedChannel>() {