You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/10/23 18:51:15 UTC
[GitHub] [flink] AHeise commented on a change in pull request #13778: [FLINK-19791][network][test] Connect to an opened port instead of 8080
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:
users@infra.apache.org