From 75fe3c19328508bbec84e150ef582f6221cecc61 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Tue, 21 Oct 2025 15:47:01 -0700 Subject: [PATCH] binder: Fix a "fail the setup transaction" test case. The setup transaction hasn't actually been failing in this test case since #8987. That's when I changed Robolectric tests to start ignoring the return value of transact() to match how real Android doesn't expose the peers' return value for 'oneway' cross-process transactions. --- .../internal/BinderServerTransportTest.java | 74 ++++++++++++++++--- 1 file changed, 63 insertions(+), 11 deletions(-) diff --git a/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java b/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java index 42bfa387044..b3b99ae34e8 100644 --- a/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java +++ b/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java @@ -20,17 +20,21 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.isNull; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.doThrow; import static org.robolectric.Shadows.shadowOf; import android.os.IBinder; import android.os.Looper; import android.os.Parcel; +import android.os.RemoteException; import com.google.common.collect.ImmutableList; import io.grpc.Attributes; +import io.grpc.ServerStreamTracer; import io.grpc.Status; import io.grpc.internal.FixedObjectPool; import io.grpc.internal.MockServerTransportListener; +import io.grpc.internal.ObjectPool; +import java.util.List; import java.util.concurrent.ScheduledExecutorService; import org.junit.Before; import org.junit.Rule; @@ -60,26 +64,74 @@ public final class BinderServerTransportTest { @Before public void setUp() throws Exception { - transport = - new BinderServerTransport( - new FixedObjectPool<>(executorService), - Attributes.EMPTY, - ImmutableList.of(), - OneWayBinderProxy.IDENTITY_DECORATOR, - mockBinder); transportListener = new MockServerTransportListener(transport); } + // Provide defaults so that we can "include only relevant details in tests." + BinderServerTransportBuilder newBinderServerTransportBuilder() { + return new BinderServerTransportBuilder() + .setExecutorServicePool(new FixedObjectPool<>(executorService)) + .setAttributes(Attributes.EMPTY) + .setStreamTracerFactories(ImmutableList.of()) + .setBinderDecorator(OneWayBinderProxy.IDENTITY_DECORATOR) + .setCallbackBinder(mockBinder); + } + @Test - public void testSetupTransactionFailureCausesMultipleShutdowns_b153460678() throws Exception { + public void testSetupTransactionFailureReportsMultipleTerminations_b153460678() throws Exception { // Make the binder fail the setup transaction. - when(mockBinder.transact(anyInt(), any(Parcel.class), isNull(), anyInt())).thenReturn(false); + doThrow(new RemoteException()) + .when(mockBinder) + .transact(anyInt(), any(Parcel.class), isNull(), anyInt()); + transport = newBinderServerTransportBuilder().setCallbackBinder(mockBinder).build(); + shadowOf(Looper.getMainLooper()).idle(); transport.start(transportListener); - // Now shut it down. + // Now shut it down externally *before* executing Runnables scheduled on the executor. transport.shutdownNow(Status.UNKNOWN.withDescription("reasons")); shadowOf(Looper.getMainLooper()).idle(); assertThat(transportListener.isTerminated()).isTrue(); } + + static class BinderServerTransportBuilder { + ObjectPool executorServicePool; + Attributes attributes; + List streamTracerFactories; + OneWayBinderProxy.Decorator binderDecorator; + IBinder callbackBinder; + + public BinderServerTransport build() { + return new BinderServerTransport( + executorServicePool, attributes, streamTracerFactories, binderDecorator, callbackBinder); + } + + public BinderServerTransportBuilder setExecutorServicePool( + ObjectPool executorServicePool) { + this.executorServicePool = executorServicePool; + return this; + } + + public BinderServerTransportBuilder setAttributes(Attributes attributes) { + this.attributes = attributes; + return this; + } + + public BinderServerTransportBuilder setStreamTracerFactories( + List streamTracerFactories) { + this.streamTracerFactories = streamTracerFactories; + return this; + } + + public BinderServerTransportBuilder setBinderDecorator( + OneWayBinderProxy.Decorator binderDecorator) { + this.binderDecorator = binderDecorator; + return this; + } + + public BinderServerTransportBuilder setCallbackBinder(IBinder callbackBinder) { + this.callbackBinder = callbackBinder; + return this; + } + } }