Copilot commented on code in PR #8121:
URL: https://github.com/apache/incubator-seata/pull/8121#discussion_r3330619233
##########
extensions/rpc/seata-http/src/test/java/org/apache/seata/integration/http/HttpTest.java:
##########
@@ -39,14 +40,24 @@
class HttpTest {
- private static final String HOST = "http://127.0.0.1:8081";;
+ private static final int PORT = findAvailablePort();
+ private static final String HOST = "http://127.0.0.1:"; + PORT;
private static final String TEST_EXCEPTION = "/testException";
private static final String GET_PATH = "/testGet";
private static final String POST_PATH = "/testPost";
- public static final String XID = "127.0.0.1:8081:87654321";
+ public static final String XID = "127.0.0.1:" + PORT + ":87654321";
private static final int PARAM_TYPE_MAP = 1;
private static final int PARAM_TYPE_BEAN = 2;
+ private static int findAvailablePort() {
+ try (ServerSocket socket = new ServerSocket(0)) {
+ socket.setReuseAddress(true);
+ return socket.getLocalPort();
+ } catch (IOException e) {
+ return 8081;
+ }
Review Comment:
`findAvailablePort()` silently falls back to `8081` if binding a temporary
socket fails. This can mask the real error and reintroduce port-collision
flakiness (especially when tests are run concurrently), making failures harder
to diagnose. It’s better to fail fast with a clear exception.
##########
test-suite/test-new-version/src/test/java/org/apache/seata/core/rpc/netty/ChannelManagerTestHelper.java:
##########
@@ -31,7 +30,8 @@ public static ConcurrentMap<String, Channel>
getChannelConcurrentMap(AbstractNet
}
public static Channel getChannel(TmNettyRemotingClient client) {
- return
getChannelManager(client).acquireChannel(ProtocolTestConstants.MOCK_SERVER_ADDRESS);
+ String serverAddress = System.getProperty("service.mock.grouplist",
"0.0.0.0:10091");
+ return getChannelManager(client).acquireChannel(serverAddress);
Review Comment:
The default fallback address `0.0.0.0:10091` is not a valid remote connect
target (0.0.0.0 is a bind-any address, and clients typically cannot connect to
it). If the system property is missing, this helper will attempt to connect to
an unusable host and tests will fail in a non-obvious way.
##########
mock-server/src/main/java/org/apache/seata/mockserver/MockServer.java:
##########
@@ -45,93 +44,190 @@ public class MockServer {
protected static final Logger LOGGER =
LoggerFactory.getLogger(MockServer.class);
- private static ThreadPoolExecutor workingThreads;
- private static MockNettyRemotingServer nettyRemotingServer;
+ public static final int MOCK_DEFAULT_PORT = 10091;
+ public static final String MOCK_SEATA_PORT_KEY = "SEATA_MOCK_PORT";
- private static volatile boolean inited = false;
- private static volatile int actualPort;
+ private ThreadPoolExecutor workingThreads;
+ private MockNettyRemotingServer nettyRemotingServer;
+ private MockCoordinator coordinator;
+ private int port;
+ private volatile boolean started = false;
- public static final int MOCK_DEFAULT_PORT = 10091;
- public static String MOCK_SEATA_PORT_KEY = "SEATA_MOCK_PORT";
+ public MockServer() {}
/**
- * The entry point of application.
+ * Start this mock server instance on the specified port.
+ * If port is 0, a random available port will be assigned.
*
- * @param args the input arguments
+ * @param port the port to listen on, 0 for random port
*/
- public static void main(String[] args) {
- SpringApplication.run(MockServer.class, args);
- int port = NumberUtils.toInt(System.getenv(MOCK_SEATA_PORT_KEY),
MOCK_DEFAULT_PORT);
+ public synchronized void start(int port) {
+ start(port, new MockCoordinator());
+ }
- if (args != null && args.length > 0) {
- try {
- port = Integer.parseInt(args[0]);
- } catch (NumberFormatException e) {
- LOGGER.error("Invalid port number provided, using default
port: {}", port, e);
- }
+ /**
+ * Start this mock server instance on the specified port with the given
coordinator.
+ *
+ * @param port the port to listen on, 0 for random port
+ * @param coordinator the mock coordinator to use
+ */
+ public synchronized void start(int port, MockCoordinator coordinator) {
+ if (started) {
+ return;
+ }
+ if (port == 0) {
+ port = findAvailablePort();
}
- start(port);
+ ConfigurationCache.clear();
+ System.clearProperty(ConfigurationKeys.SERVER_SERVICE_PORT_CAMEL);
+ System.clearProperty("server.port");
+
+ workingThreads = new ThreadPoolExecutor(
+ 50,
+ 50,
+ 500,
+ TimeUnit.SECONDS,
+ new LinkedBlockingQueue<>(20000),
+ new ThreadPoolExecutor.CallerRunsPolicy());
Review Comment:
The project typically creates managed thread pools via
`ThreadPoolExecutorFactory` (e.g. it standardizes thread naming and can route
to alternative providers like virtual threads). This mock server now uses the
JDK default `ThreadFactory`, which makes thread dumps/logging harder and
bypasses the central thread-pool provider mechanism.
##########
test-suite/test-new-version/src/test/java/org/apache/seata/core/rpc/netty/v1/ProtocolV1Server.java:
##########
@@ -75,16 +84,14 @@ protected void initChannel(Channel channel) throws
Exception {
String host = "0.0.0.0";
ChannelFuture future = serverBootstrap.bind(new
InetSocketAddress(host, port));
- ChannelFuture channelFuture =
future.addListener((ChannelFutureListener) future1 -> {
- if (!future1.isSuccess()) {
- throw new RuntimeException("Server start fail !",
future1.cause());
- }
- });
-
try {
- channelFuture.await(5000, TimeUnit.MILLISECONDS);
+ future.await(5000, TimeUnit.MILLISECONDS);
+ if (!future.isSuccess()) {
+ throw new RuntimeException("Server start fail!",
future.cause());
+ }
+ this.port = ((InetSocketAddress)
future.channel().localAddress()).getPort();
Review Comment:
`future.await(5000, TimeUnit.MILLISECONDS)` returns `false` on timeout, but
the return value is ignored. In that case `future.isSuccess()` will still be
false and `future.cause()` may be null, leading to a misleading "start fail"
exception instead of a clear timeout. Also, the interrupt status is lost when
catching `InterruptedException`.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
