AHeise commented on a change in pull request #13778:
URL: https://github.com/apache/flink/pull/13778#discussion_r511081704
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/netty/PartitionRequestClientFactoryTest.java
##########
@@ -131,8 +129,6 @@ public void testNettyClientConnectRetryFailure() throws
Exception {
factory.createPartitionRequestClient(serverAndClient.getConnectionID(0));
Review comment:
While the commit message is quite direct, it seems a bit clumsy.
How about `[refactor][tests][network] Cleanup
PartitionRequestClientFactoryTest`?
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/netty/NettyTestUtil.java
##########
@@ -19,6 +19,7 @@
package org.apache.flink.runtime.io.network.netty;
import org.apache.flink.configuration.Configuration;
+import org.apache.flink.runtime.io.network.ConnectionID;
import org.apache.flink.util.NetUtils;
Review comment:
Add this commit before the test fix? Then you can use the method
directly.
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/netty/PartitionRequestClientFactoryTest.java
##########
@@ -61,15 +61,23 @@
@Test
public void testInterruptsNotCached() throws Exception {
- ConnectionID connectionId = new ConnectionID(new
InetSocketAddress(InetAddress.getLocalHost(), 8080), 0);
- try (AwaitingNettyClient nettyClient = new
AwaitingNettyClient()) {
+ final NettyTestUtil.NettyServerAndClient nettyServerAndClient =
createNettyServerAndClient();
+ ConnectionID connectionId = new ConnectionID(new
InetSocketAddress(
+
nettyServerAndClient.server().getConfig().getServerAddress(),
+
nettyServerAndClient.server().getConfig().getServerPort()),
+ 0);
+ try {
+ AwaitingNettyClient nettyClient = new
AwaitingNettyClient(nettyServerAndClient.client());
PartitionRequestClientFactory factory = new
PartitionRequestClientFactory(nettyClient, 0);
nettyClient.awaitForInterrupts = true;
connectAndInterrupt(factory, connectionId);
nettyClient.awaitForInterrupts = false;
factory.createPartitionRequestClient(connectionId);
+ } finally {
+ nettyServerAndClient.client().shutdown();
+ nettyServerAndClient.server().shutdown();
}
Review comment:
Couldn't you leave it as is and just request a `ConnectionID` from
`AwaitingNettyClient`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
