[GitHub] [phoenix-omid] stoty commented on a change in pull request #89: OMID-198 Replace static ports used for TSO server with random ports i…

2021-02-25 Thread GitBox 


stoty commented on a change in pull request #89:
URL: https://github.com/apache/phoenix-omid/pull/89#discussion_r582668337



##
File path: 
tso-server/src/test/java/org/apache/omid/tso/TestTSOChannelHandlerNetty.java
##
@@ -70,186 +72,197 @@
 private
 RequestProcessor requestProcessor;
 
-// Component under test
-private TSOChannelHandler channelHandler;
-
 @BeforeMethod
 public void beforeTestMethod() {
 MockitoAnnotations.initMocks(this);
-TSOServerConfig config = new TSOServerConfig();
-config.setPort(1434);
-channelHandler = new TSOChannelHandler(config, requestProcessor, new 
NullMetricsProvider());
 }
 
-@AfterMethod
-public void afterTestMethod() throws IOException {
-channelHandler.close();
+private TSOChannelHandler getTSOChannelHandler(int port) {
+TSOServerConfig config = new TSOServerConfig();
+config.setPort(port);
+return new TSOChannelHandler(config, requestProcessor, new 
NullMetricsProvider());
 }
 
 @Test(timeOut = 10_000)
 public void testMainAPI() throws Exception {
-
-// Check initial state
-assertNull(channelHandler.listeningChannel);
-assertNull(channelHandler.channelGroup);
-
-// Check initial connection
-channelHandler.reconnect();
-assertTrue(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 1);
-assertEquals(((InetSocketAddress) 
channelHandler.listeningChannel.getLocalAddress()).getPort(), 1434);
-
-// Check connection close
-channelHandler.closeConnection();
-assertFalse(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 0);
-
-// Check re-closing connection
-channelHandler.closeConnection();
-assertFalse(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 0);
-
-// Check connection after closing
-channelHandler.reconnect();
-assertTrue(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 1);
-
-// Check re-connection
-channelHandler.reconnect();
-assertTrue(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 1);
-
-// Exercise closeable with re-connection trial
-channelHandler.close();
-assertFalse(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 0);
+int port = NetworkUtils.getFreePort();
+TSOChannelHandler channelHandler = getTSOChannelHandler(port);
 try {
+// Check initial state
+assertNull(channelHandler.listeningChannel);
+assertNull(channelHandler.channelGroup);
+
+// Check initial connection
 channelHandler.reconnect();
-} catch (ChannelException e) {
-// Expected: Can't reconnect after closing
+assertTrue(channelHandler.listeningChannel.isOpen());
+assertEquals(channelHandler.channelGroup.size(), 1);
+assertEquals(((InetSocketAddress) 
channelHandler.listeningChannel.getLocalAddress()).getPort(), port);
+
+// Check connection close
+channelHandler.closeConnection();
 assertFalse(channelHandler.listeningChannel.isOpen());
 assertEquals(channelHandler.channelGroup.size(), 0);
-}
 
+// Check re-closing connection
+channelHandler.closeConnection();
+assertFalse(channelHandler.listeningChannel.isOpen());
+assertEquals(channelHandler.channelGroup.size(), 0);
+
+// Check connection after closing
+channelHandler.reconnect();
+assertTrue(channelHandler.listeningChannel.isOpen());
+assertEquals(channelHandler.channelGroup.size(), 1);
+
+// Check re-connection
+channelHandler.reconnect();
+assertTrue(channelHandler.listeningChannel.isOpen());
+assertEquals(channelHandler.channelGroup.size(), 1);
+
+// Exercise closeable with re-connection trial
+channelHandler.close();
+assertFalse(channelHandler.listeningChannel.isOpen());
+assertEquals(channelHandler.channelGroup.size(), 0);
+try {
+channelHandler.reconnect();

Review comment:
   If this is not supposed to succed, then please add an Assert.fail after.





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

[GitHub] [phoenix-omid] stoty commented on a change in pull request #89: OMID-198 Replace static ports used for TSO server with random ports i…

2021-02-25 Thread GitBox 


stoty commented on a change in pull request #89:
URL: https://github.com/apache/phoenix-omid/pull/89#discussion_r582666495



##
File path: 
hbase-client/src/test/java/org/apache/omid/transaction/OmidTestBase.java
##
@@ -99,12 +102,12 @@ public void beforeGroups(ITestContext context) throws 
Exception {
 HBaseTimestampStorageConfig hBaseTimestampStorageConfig = 
injector.getInstance(HBaseTimestampStorageConfig.class);
 tso.startAsync();
 tso.awaitRunning();
-TestUtils.waitForSocketListening("localhost", 1234, 100);
+TestUtils.waitForSocketListening("localhost", port, 100);
 LOG.info("Finished loading TSO");
 context.setAttribute("tso", tso);
 
 OmidClientConfiguration clientConf = new OmidClientConfiguration();
-clientConf.setConnectionString("localhost:1234");
+clientConf.setConnectionString("localhost:"+port);

Review comment:
   please add spaces around "+" here, and every such occurence 

##
File path: 
hbase-client/src/test/java/org/apache/omid/transaction/TestFilters.java
##
@@ -71,9 +71,8 @@ public void testGetWithValueFilter(ITestContext context) 
throws Exception {
 private void testGet(ITestContext context, Filter f) throws Exception {
 
 CommitTable.Client commitTableClient = 
spy(getCommitTable(context).getClient());
-
 HBaseOmidClientConfiguration hbaseOmidClientConf = new 
HBaseOmidClientConfiguration();
-hbaseOmidClientConf.setConnectionString("localhost:1234");
+hbaseOmidClientConf.setConnectionString("localhost:"+port+"");

Review comment:
   The last +"" is not needed, please remove it everywhere in the patch.

##
File path: 
tso-server/src/test/java/org/apache/omid/tso/TestTSOChannelHandlerNetty.java
##
@@ -70,186 +72,197 @@
 private
 RequestProcessor requestProcessor;
 
-// Component under test
-private TSOChannelHandler channelHandler;
-
 @BeforeMethod
 public void beforeTestMethod() {
 MockitoAnnotations.initMocks(this);
-TSOServerConfig config = new TSOServerConfig();
-config.setPort(1434);
-channelHandler = new TSOChannelHandler(config, requestProcessor, new 
NullMetricsProvider());
 }
 
-@AfterMethod
-public void afterTestMethod() throws IOException {
-channelHandler.close();
+private TSOChannelHandler getTSOChannelHandler(int port) {
+TSOServerConfig config = new TSOServerConfig();
+config.setPort(port);
+return new TSOChannelHandler(config, requestProcessor, new 
NullMetricsProvider());
 }
 
 @Test(timeOut = 10_000)
 public void testMainAPI() throws Exception {
-
-// Check initial state
-assertNull(channelHandler.listeningChannel);
-assertNull(channelHandler.channelGroup);
-
-// Check initial connection
-channelHandler.reconnect();
-assertTrue(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 1);
-assertEquals(((InetSocketAddress) 
channelHandler.listeningChannel.getLocalAddress()).getPort(), 1434);
-
-// Check connection close
-channelHandler.closeConnection();
-assertFalse(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 0);
-
-// Check re-closing connection
-channelHandler.closeConnection();
-assertFalse(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 0);
-
-// Check connection after closing
-channelHandler.reconnect();
-assertTrue(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 1);
-
-// Check re-connection
-channelHandler.reconnect();
-assertTrue(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 1);
-
-// Exercise closeable with re-connection trial
-channelHandler.close();
-assertFalse(channelHandler.listeningChannel.isOpen());
-assertEquals(channelHandler.channelGroup.size(), 0);
+int port = NetworkUtils.getFreePort();
+TSOChannelHandler channelHandler = getTSOChannelHandler(port);
 try {
+// Check initial state
+assertNull(channelHandler.listeningChannel);
+assertNull(channelHandler.channelGroup);
+
+// Check initial connection
 channelHandler.reconnect();
-} catch (ChannelException e) {
-// Expected: Can't reconnect after closing
+assertTrue(channelHandler.listeningChannel.isOpen());
+assertEquals(channelHandler.channelGroup.size(), 1);
+assertEquals(((InetSocketAddress) 
channelHandler.listeningChannel.getLocalAddress()).getPort(), port);
+
+// Check connection close
+channelHandler.closeCo