Skip to content

Commit b2731f2

Browse files
committed
core: Delete AbstractTransportTest.clientShutdownBeforeStartRunnable
The test was added in e4e7f3a when InProcess stopped returning a Runnable from start(). In c5a63a1 we realized (indirectly) that there's no point in using the Runnable any more. This test failed with Binder (which seems to have been using the Runnable unnecessarily), and InProcess, Netty, and OkHttp don't use the Runnable. Instead of fixing it, we'll just move toward stopping using Runnable. I'm not removing the Runnable usage from Binder in this commit because this test is currently causing CI failures and I don't want to do a behavior change when fixing it.
1 parent 62cf842 commit b2731f2

File tree

4 files changed

+0
-35
lines changed

4 files changed

+0
-35
lines changed

core/src/testFixtures/java/io/grpc/internal/AbstractTransportTest.java

-20
Original file line numberDiff line numberDiff line change
@@ -350,26 +350,6 @@ public void clientStartStop() throws Exception {
350350
verify(mockClientTransportListener, never()).transportInUse(anyBoolean());
351351
}
352352

353-
@Test
354-
public void clientShutdownBeforeStartRunnable() throws Exception {
355-
server.start(serverListener);
356-
client = newClientTransport(server);
357-
Runnable runnable = client.start(mockClientTransportListener);
358-
// Shutdown before calling 'runnable'
359-
client.shutdown(Status.UNAVAILABLE.withDescription("shutdown called"));
360-
runIfNotNull(runnable);
361-
verify(mockClientTransportListener, timeout(TIMEOUT_MS)).transportTerminated();
362-
// We should verify that clients don't call transportReady() after transportTerminated(), but
363-
// transports do this today and nothing cares. ServerImpl, on the other hand, doesn't appreciate
364-
// the out-of-order calls.
365-
MockServerTransportListener serverTransportListener
366-
= serverListener.takeListenerOrFail(TIMEOUT_MS, TimeUnit.MILLISECONDS);
367-
assertTrue(serverTransportListener.waitForTermination(TIMEOUT_MS, TimeUnit.MILLISECONDS));
368-
// Allow any status as some transports (e.g., Netty) don't communicate the original status when
369-
// shutdown while handshaking. It won't be used anyway, so no big deal.
370-
verify(mockClientTransportListener).transportShutdown(any(Status.class));
371-
}
372-
373353
@Test
374354
public void clientStartAndStopOnceConnected() throws Exception {
375355
server.start(serverListener);

servlet/src/jettyTest/java/io/grpc/servlet/JettyTransportTest.java

-5
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,4 @@ public void clientChecksInboundMetadataSize_header() {}
262262
@Ignore("https://github.com/jetty/jetty.project/issues/11822")
263263
@Test
264264
public void clientChecksInboundMetadataSize_trailer() {}
265-
266-
@Override
267-
@Ignore("Not yet investigated, but has been seen for multiple servlet containers")
268-
@Test
269-
public void clientShutdownBeforeStartRunnable() {}
270265
}

servlet/src/tomcatTest/java/io/grpc/servlet/TomcatTransportTest.java

-5
Original file line numberDiff line numberDiff line change
@@ -274,9 +274,4 @@ public void earlyServerClose_serverFailure_withClientCancelOnListenerClosed() {}
274274
@Ignore("regression since bumping grpc v1.46 to v1.53")
275275
@Test
276276
public void messageProducerOnlyProducesRequestedMessages() {}
277-
278-
@Override
279-
@Ignore("Not yet investigated, but has been seen for multiple servlet containers")
280-
@Test
281-
public void clientShutdownBeforeStartRunnable() {}
282277
}

servlet/src/undertowTest/java/io/grpc/servlet/UndertowTransportTest.java

-5
Original file line numberDiff line numberDiff line change
@@ -308,9 +308,4 @@ public void clientCancel() {}
308308
@Ignore("regression since bumping grpc v1.46 to v1.53")
309309
@Test
310310
public void messageProducerOnlyProducesRequestedMessages() {}
311-
312-
@Override
313-
@Ignore("Not yet investigated, but has been seen for multiple servlet containers")
314-
@Test
315-
public void clientShutdownBeforeStartRunnable() {}
316311
}

0 commit comments

Comments
 (0)