Fix: test cases

Author: AgraVator

api/src/test/java/io/grpc/NameResolverTest.java

@@ -105,7 +105,7 @@ public void args() {
   }
 
   private NameResolver.Args createArgs() {
-    ChannelConfigurator channelConfigurator = mock(ChannelConfigurator.class);
+    ChannelConfigurator channelConfigurator = new ChannelConfigurator() {};
     return NameResolver.Args.newBuilder()
         .setDefaultPort(defaultPort)
         .setProxyDetector(proxyDetector)
@@ -123,7 +123,13 @@ private NameResolver.Args createArgs() {
 
   @Test
   public void args_childChannelConfigurator() {
-    ChannelConfigurator channelConfigurator = mock(ChannelConfigurator.class);
+    final ManagedChannelBuilder<?>[] capturedBuilder = new ManagedChannelBuilder<?>[1];
+    ChannelConfigurator channelConfigurator = new ChannelConfigurator() {
+      @Override
+      public void configureChannelBuilder(ManagedChannelBuilder<?> builder) {
+        capturedBuilder[0] = builder;
+      }
+    };
 
     SynchronizationContext realSyncContext = new SynchronizationContext(
         new Thread.UncaughtExceptionHandler() {
@@ -148,7 +154,7 @@ public void uncaughtException(Thread t, Throwable e) {
     // Validate configurator accepts builders
     ManagedChannelBuilder<?> mockBuilder = mock(ManagedChannelBuilder.class);
     configurator.configureChannelBuilder(mockBuilder);
-    verify(channelConfigurator).configureChannelBuilder(mockBuilder);
+    assertThat(capturedBuilder[0]).isSameInstanceAs(mockBuilder);
   }
 
   @Test

core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java

@@ -26,7 +26,6 @@
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -52,7 +51,9 @@
 import io.grpc.MetricSink;
 import io.grpc.NameResolver;
 import io.grpc.NameResolverRegistry;
+import io.grpc.NoopMetricSink;
 import io.grpc.StaticTestingClassLoader;
+import io.grpc.Uri;
 import io.grpc.internal.ManagedChannelImplBuilder.ChannelBuilderDefaultPortProvider;
 import io.grpc.internal.ManagedChannelImplBuilder.ClientTransportFactoryBuilder;
 import io.grpc.internal.ManagedChannelImplBuilder.FixedPortProvider;
@@ -77,7 +78,6 @@
 import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameter;
 import org.junit.runners.Parameterized.Parameters;
-import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnit;
 import org.mockito.junit.MockitoRule;
@@ -786,7 +786,7 @@ public void setNameResolverExtArgs() {
 
   @Test
   public void childChannelConfigurator_setsField() {
-    ChannelConfigurator configurator = mock(ChannelConfigurator.class);
+    ChannelConfigurator configurator = new ChannelConfigurator() {};
     assertSame(builder, builder.childChannelConfigurator(configurator));
     assertSame(configurator, builder.channelConfigurator);
   }
@@ -801,8 +801,14 @@ public void childChannelConfigurator_propagatesMetricsAndInterceptors_xdsTarget(
     when(mockClientTransportFactory.getSupportedSocketAddressTypes())
         .thenReturn(Collections.singleton(InetSocketAddress.class));
 
-    MetricSink mockMetricSink = mock(MetricSink.class);
-    ClientInterceptor mockInterceptor = mock(ClientInterceptor.class);
+    MetricSink mockMetricSink = new NoopMetricSink();
+    ClientInterceptor mockInterceptor = new ClientInterceptor() {
+      @Override
+      public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(
+          MethodDescriptor<ReqT, RespT> method, CallOptions callOptions, Channel next) {
+        return next.newCall(method, callOptions);
+      }
+    };
 
     // Define the Configurator
     ChannelConfigurator configurator = new ChannelConfigurator() {
@@ -814,23 +820,47 @@ public void configureChannelBuilder(ManagedChannelBuilder<?> builder) {
       }
     };
 
-    // Mock NameResolver.Factory to capture Args
-    NameResolver.Factory mockNameResolverFactory = mock(NameResolver.Factory.class);
-    when(mockNameResolverFactory.getDefaultScheme()).thenReturn("xds");
-    NameResolver mockNameResolver = mock(NameResolver.class);
-    when(mockNameResolver.getServiceAuthority()).thenReturn("foo.authority");
-    ArgumentCaptor<NameResolver.Args> argsCaptor = ArgumentCaptor.forClass(NameResolver.Args.class);
-    if (enableRfc3986UrisParam) {
-      when(mockNameResolverFactory.newNameResolver((io.grpc.Uri) any(),
-          argsCaptor.capture())).thenReturn(mockNameResolver);
-    } else {
-      when(mockNameResolverFactory.newNameResolver((URI) any(),
-          argsCaptor.capture())).thenReturn(mockNameResolver);
-    }
+    // Use NameResolver.Factory to capture Args
+    final NameResolver.Args[] capturedArgs = new NameResolver.Args[1];
+    final boolean[] newNameResolverCalled = new boolean[1];
+
+    NameResolver realNameResolver = new NameResolver() {
+      @Override
+      public String getServiceAuthority() {
+        return "foo.authority";
+      }
 
-    // Use the configurator and the mock factory
+      @Override
+      public void start(Listener2 listener) {}
+
+      @Override
+      public void shutdown() {}
+    };
+
+    NameResolver.Factory realNameResolverFactory = new NameResolver.Factory() {
+      @Override
+      public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
+        newNameResolverCalled[0] = true;
+        capturedArgs[0] = args;
+        return realNameResolver;
+      }
+
+      @Override
+      public NameResolver newNameResolver(Uri targetUri, NameResolver.Args args) {
+        newNameResolverCalled[0] = true;
+        capturedArgs[0] = args;
+        return realNameResolver;
+      }
+
+      @Override
+      public String getDefaultScheme() {
+        return "xds";
+      }
+    };
+
+    // Use the configurator and the custom factory
     NameResolverRegistry registry = new NameResolverRegistry();
-    registry.register(new NameResolverFactoryToProviderFacade(mockNameResolverFactory));
+    registry.register(new NameResolverFactoryToProviderFacade(realNameResolverFactory));
 
     ManagedChannelBuilder<?> parentBuilder = new ManagedChannelImplBuilder(
         "xds:///my-service-target",
@@ -843,28 +873,25 @@ public void configureChannelBuilder(ManagedChannelBuilder<?> builder) {
     grpcCleanupRule.register(channel);
 
     // Verify that newNameResolver was called
-    if (enableRfc3986UrisParam) {
-      verify(mockNameResolverFactory).newNameResolver((io.grpc.Uri) any(), any());
-    } else {
-      verify(mockNameResolverFactory).newNameResolver((URI) any(), any());
-    }
+    assertThat(newNameResolverCalled[0]).isTrue();
 
     // Extract the childChannelConfigurator from Args
-    NameResolver.Args args = argsCaptor.getValue();
+    NameResolver.Args args = capturedArgs[0];
     ChannelConfigurator channelConfiguratorInArgs = args.getChildChannelConfigurator();
     assertNotNull("Child channel configurator should be present in NameResolver.Args",
         channelConfiguratorInArgs);
 
     // Verify the configurator is the one we passed
     assertThat(channelConfiguratorInArgs).isSameInstanceAs(configurator);
 
-    // Verify the configurator logically applies (by running it on a mock)
-    ManagedChannelBuilder<?> mockChildBuilder = mock(ManagedChannelBuilder.class);
-    // Stub addMetricSink to return the builder to avoid generic return type issues
-    doReturn(mockChildBuilder).when(mockChildBuilder).addMetricSink(any());
+    // Verify the configurator logically applies (by running it on a real builder)
+    ManagedChannelImplBuilder childBuilder = new ManagedChannelImplBuilder(
+        "xds:///child-service-target",
+        mockClientTransportFactoryBuilder,
+        new FixedPortProvider(DUMMY_PORT));
 
-    configurator.configureChannelBuilder(mockChildBuilder);
-    verify(mockChildBuilder).addMetricSink(mockMetricSink);
+    configurator.configureChannelBuilder(childBuilder);
+    assertThat(childBuilder.metricSinks).contains(mockMetricSink);
   }
 
   @Test

rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java

@@ -18,7 +18,6 @@
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.truth.Truth.assertThat;
-import static org.mockito.AdditionalAnswers.delegatesTo;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.argThat;
@@ -53,6 +52,7 @@
 import io.grpc.LoadBalancer.Subchannel;
 import io.grpc.LoadBalancer.SubchannelPicker;
 import io.grpc.LoadBalancer.SubchannelStateListener;
+import io.grpc.LongCounterMetricInstrument;
 import io.grpc.ManagedChannel;
 import io.grpc.ManagedChannelBuilder;
 import io.grpc.Metadata;
@@ -61,7 +61,6 @@
 import io.grpc.MetricInstrument;
 import io.grpc.MetricRecorder;
 import io.grpc.MetricRecorder.Registration;
-import io.grpc.MetricSink;
 import io.grpc.NameResolver.ConfigOrError;
 import io.grpc.NoopMetricSink;
 import io.grpc.ServerCall;
@@ -95,6 +94,7 @@
 import java.util.Map;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import javax.annotation.Nonnull;
 import org.junit.After;
 import org.junit.Before;
@@ -361,7 +361,7 @@ public void metricsWithRealChannel() throws Exception {
         .directExecutor()
         .build()
         .start());
-    MetricSink metrics = mock(MetricSink.class, delegatesTo(new NoopMetricSink()));
+    VerificationMetricSink metrics = new VerificationMetricSink();
     ManagedChannel channel = grpcCleanupRule.register(
         InternalManagedChannelBuilder.addMetricSink(
             InProcessChannelBuilder.forName("fake-bigtable.googleapis.com")
@@ -379,12 +379,7 @@ public void metricsWithRealChannel() throws Exception {
     assertThat(recorder.awaitCompletion(10, TimeUnit.SECONDS)).isTrue();
     assertThat(recorder.getError()).isNull();
 
-    verify(metrics).addLongCounter(
-        eqMetricInstrumentName("grpc.lb.rls.default_target_picks"),
-        eq(1L),
-        eq(Arrays.asList("directaddress:///fake-bigtable.googleapis.com", "localhost:8972",
-            "defaultTarget", "complete")),
-        eq(Arrays.asList("customvalue")));
+    metrics.awaitCall();
   }
 
   @Test
@@ -931,4 +926,34 @@ public void registerBackendResponse(boolean throttled) {
     }
   }
 
+  private static final class VerificationMetricSink extends NoopMetricSink {
+    private final AtomicBoolean called =
+        new AtomicBoolean();
+
+    @Override
+    public void addLongCounter(
+        LongCounterMetricInstrument metricInstrument,
+        long value,
+        List<String> requiredLabelValues,
+        List<String> optionalLabelValues) {
+      if (metricInstrument.getName().equals("grpc.lb.rls.default_target_picks")
+          && value == 1L
+          && requiredLabelValues.equals(Arrays.asList(
+              "directaddress:///fake-bigtable.googleapis.com", "localhost:8972",
+              "defaultTarget", "complete"))
+          && optionalLabelValues.equals(Arrays.asList("customvalue"))) {
+        called.set(true);
+      }
+    }
+
+    public void awaitCall() throws InterruptedException {
+      long start = System.currentTimeMillis();
+      while (!called.get()) {
+        if (System.currentTimeMillis() - start > 5000) {
+          throw new AssertionError("Timed out waiting for metric sink call");
+        }
+        Thread.sleep(50);
+      }
+    }
+  }
 }

xds/src/test/java/io/grpc/xds/FakeControlPlaneXdsIntegrationTest.java

@@ -22,13 +22,6 @@
 import static io.grpc.xds.XdsTestControlPlaneService.ADS_TYPE_URL_CDS;
 import static io.grpc.xds.XdsTestControlPlaneService.ADS_TYPE_URL_EDS;
 import static org.junit.Assert.assertEquals;
-import static org.mockito.AdditionalAnswers.delegatesTo;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyList;
-import static org.mockito.ArgumentMatchers.anyLong;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.timeout;
-import static org.mockito.Mockito.verify;
 
 import com.github.xds.type.v3.TypedStruct;
 import com.google.common.collect.ImmutableMap;
@@ -66,18 +59,20 @@
 import io.grpc.InsecureServerCredentials;
 import io.grpc.InternalFeatureFlags;
 import io.grpc.LoadBalancerRegistry;
+import io.grpc.LongCounterMetricInstrument;
 import io.grpc.ManagedChannel;
 import io.grpc.ManagedChannelBuilder;
 import io.grpc.Metadata;
 import io.grpc.MethodDescriptor;
-import io.grpc.MetricSink;
 import io.grpc.NoopMetricSink;
 import io.grpc.Server;
 import io.grpc.testing.protobuf.SimpleRequest;
 import io.grpc.testing.protobuf.SimpleResponse;
 import io.grpc.testing.protobuf.SimpleServiceGrpc;
 import java.net.InetSocketAddress;
 import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -377,12 +372,12 @@ public void pingPong_logicalDns_authorityOverride() {
   }
 
   @Test
-  public void childChannelConfigurator_passesMetricSinkToChannel_E2E() {
-    MetricSink mockSink = mock(MetricSink.class, delegatesTo(new NoopMetricSink()));
+  public void childChannelConfigurator_passesMetricSinkToChannel_E2E() throws Exception {
+    CountingMetricSink sink = new CountingMetricSink();
     ChannelConfigurator configurator = new ChannelConfigurator() {
       @Override
       public void configureChannelBuilder(ManagedChannelBuilder<?> builder) {
-        builder.addMetricSink(mockSink);
+        builder.addMetricSink(sink);
       }
     };
 
@@ -398,21 +393,20 @@ public void configureChannelBuilder(ManagedChannelBuilder<?> builder) {
 
       // The xDS client inside the channel configurator will have created an ADS stream.
       // The metric sink should have received attempt or connection metrics.
-      verify(mockSink, timeout(5000).atLeastOnce())
-          .addLongCounter(any(), anyLong(), anyList(), anyList());
+      sink.awaitCall();
     } finally {
       channel.shutdownNow();
     }
   }
 
   @Test
   public void childChannelConfigurator_passesMetricSinkToServer_E2E() throws Exception {
-    MetricSink mockSink = mock(MetricSink.class, delegatesTo(new NoopMetricSink()));
+    CountingMetricSink sink = new CountingMetricSink();
     ChannelConfigurator configurator = new ChannelConfigurator() {
       @Override
       public void configureChannelBuilder(ManagedChannelBuilder<?> builder) {
         // Child channels (xDS client connections) created by this server get the sink.
-        builder.addMetricSink(mockSink);
+        builder.addMetricSink(sink);
       }
     };
 
@@ -428,10 +422,33 @@ public void configureChannelBuilder(ManagedChannelBuilder<?> builder) {
 
     try {
       // The server xDS client will connect to control plane to get LDS.
-      verify(mockSink, timeout(5000).atLeastOnce())
-          .addLongCounter(any(), anyLong(), anyList(), anyList());
+      sink.awaitCall();
     } finally {
       childServer.shutdownNow();
     }
   }
+
+  private static final class CountingMetricSink extends NoopMetricSink {
+    private final AtomicInteger count =
+        new AtomicInteger();
+
+    @Override
+    public void addLongCounter(
+        LongCounterMetricInstrument metricInstrument,
+        long value,
+        List<String> requiredLabelValues,
+        List<String> optionalLabelValues) {
+      count.incrementAndGet();
+    }
+
+    public void awaitCall() throws InterruptedException {
+      long start = System.currentTimeMillis();
+      while (count.get() == 0) {
+        if (System.currentTimeMillis() - start > 5000) {
+          throw new AssertionError("Timed out waiting for metric sink call");
+        }
+        Thread.sleep(50);
+      }
+    }
+  }
 }