Skip to content

Commit ea74446

Browse files
authored
binder: Fix a "fail the setup transaction" test case. (#12434)
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. The fix is as simple as mocking transact() to throw rather than return false.
1 parent 6da93db commit ea74446

File tree

1 file changed

+63
-11
lines changed

1 file changed

+63
-11
lines changed

binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java

Lines changed: 63 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,21 @@
2020
import static org.mockito.ArgumentMatchers.any;
2121
import static org.mockito.ArgumentMatchers.anyInt;
2222
import static org.mockito.ArgumentMatchers.isNull;
23-
import static org.mockito.Mockito.when;
23+
import static org.mockito.Mockito.doThrow;
2424
import static org.robolectric.Shadows.shadowOf;
2525

2626
import android.os.IBinder;
2727
import android.os.Looper;
2828
import android.os.Parcel;
29+
import android.os.RemoteException;
2930
import com.google.common.collect.ImmutableList;
3031
import io.grpc.Attributes;
32+
import io.grpc.ServerStreamTracer;
3133
import io.grpc.Status;
3234
import io.grpc.internal.FixedObjectPool;
3335
import io.grpc.internal.MockServerTransportListener;
36+
import io.grpc.internal.ObjectPool;
37+
import java.util.List;
3438
import java.util.concurrent.ScheduledExecutorService;
3539
import org.junit.Before;
3640
import org.junit.Rule;
@@ -60,26 +64,74 @@ public final class BinderServerTransportTest {
6064

6165
@Before
6266
public void setUp() throws Exception {
63-
transport =
64-
new BinderServerTransport(
65-
new FixedObjectPool<>(executorService),
66-
Attributes.EMPTY,
67-
ImmutableList.of(),
68-
OneWayBinderProxy.IDENTITY_DECORATOR,
69-
mockBinder);
7067
transportListener = new MockServerTransportListener(transport);
7168
}
7269

70+
// Provide defaults so that we can "include only relevant details in tests."
71+
BinderServerTransportBuilder newBinderServerTransportBuilder() {
72+
return new BinderServerTransportBuilder()
73+
.setExecutorServicePool(new FixedObjectPool<>(executorService))
74+
.setAttributes(Attributes.EMPTY)
75+
.setStreamTracerFactories(ImmutableList.of())
76+
.setBinderDecorator(OneWayBinderProxy.IDENTITY_DECORATOR)
77+
.setCallbackBinder(mockBinder);
78+
}
79+
7380
@Test
74-
public void testSetupTransactionFailureCausesMultipleShutdowns_b153460678() throws Exception {
81+
public void testSetupTransactionFailureReportsMultipleTerminations_b153460678() throws Exception {
7582
// Make the binder fail the setup transaction.
76-
when(mockBinder.transact(anyInt(), any(Parcel.class), isNull(), anyInt())).thenReturn(false);
83+
doThrow(new RemoteException())
84+
.when(mockBinder)
85+
.transact(anyInt(), any(Parcel.class), isNull(), anyInt());
86+
transport = newBinderServerTransportBuilder().setCallbackBinder(mockBinder).build();
87+
shadowOf(Looper.getMainLooper()).idle();
7788
transport.start(transportListener);
7889

79-
// Now shut it down.
90+
// Now shut it down externally *before* executing Runnables scheduled on the executor.
8091
transport.shutdownNow(Status.UNKNOWN.withDescription("reasons"));
8192
shadowOf(Looper.getMainLooper()).idle();
8293

8394
assertThat(transportListener.isTerminated()).isTrue();
8495
}
96+
97+
static class BinderServerTransportBuilder {
98+
ObjectPool<ScheduledExecutorService> executorServicePool;
99+
Attributes attributes;
100+
List<ServerStreamTracer.Factory> streamTracerFactories;
101+
OneWayBinderProxy.Decorator binderDecorator;
102+
IBinder callbackBinder;
103+
104+
public BinderServerTransport build() {
105+
return new BinderServerTransport(
106+
executorServicePool, attributes, streamTracerFactories, binderDecorator, callbackBinder);
107+
}
108+
109+
public BinderServerTransportBuilder setExecutorServicePool(
110+
ObjectPool<ScheduledExecutorService> executorServicePool) {
111+
this.executorServicePool = executorServicePool;
112+
return this;
113+
}
114+
115+
public BinderServerTransportBuilder setAttributes(Attributes attributes) {
116+
this.attributes = attributes;
117+
return this;
118+
}
119+
120+
public BinderServerTransportBuilder setStreamTracerFactories(
121+
List<ServerStreamTracer.Factory> streamTracerFactories) {
122+
this.streamTracerFactories = streamTracerFactories;
123+
return this;
124+
}
125+
126+
public BinderServerTransportBuilder setBinderDecorator(
127+
OneWayBinderProxy.Decorator binderDecorator) {
128+
this.binderDecorator = binderDecorator;
129+
return this;
130+
}
131+
132+
public BinderServerTransportBuilder setCallbackBinder(IBinder callbackBinder) {
133+
this.callbackBinder = callbackBinder;
134+
return this;
135+
}
136+
}
85137
}

0 commit comments

Comments
 (0)