GEODE-2730: Refactor ServerStarterRule and LocatorStarterRule

- Introduced LocalServerStarterRule and LocalLocatorStarterRule to eventually replace LocatorStarterRule and ServerStarterRule. - The new rules will *only* start a member through the rule's before() method. Members will *never* be started on Rule instantiation. - The new rules are backed by Builders that use AvailablePort.Keeper to reserve the ports that a member will use when it eventually starts. The Keepers are released just before starting the member, with the goal of minimizing test flakiness caused by BindExceptions when a port is already in use.
stefanma · Mar 30, 2017 · 50f6d1e · 50f6d1e
1 parent f3a2ef4
commit 50f6d1e
Show file tree

Hide file tree

Showing 102 changed files with 1,074 additions and 422 deletions.
diff --git a/...ssembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java b/...ssembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
@@ -14,15 +14,15 @@
  */
 package org.apache.geode.rest.internal.web;
 
-import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import org.apache.geode.cache.RegionShortcut;
 import org.apache.geode.security.TestSecurityManager;
-import org.apache.geode.test.dunit.rules.ServerStarterRule;
+import org.apache.geode.test.dunit.rules.LocalServerStarterRule;
+import org.apache.geode.test.dunit.rules.ServerStarterBuilder;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.apache.geode.test.junit.categories.SecurityTest;
 import org.apache.http.HttpResponse;
@@ -41,11 +41,12 @@ public class RestSecurityIntegrationTest {
   protected static final String REGION_NAME = "AuthRegion";
 
   @ClassRule
-  public static ServerStarterRule serverStarter = new ServerStarterRule()
-      .withProperty(TestSecurityManager.SECURITY_JSON,
-          "org/apache/geode/management/internal/security/clientServer.json")
-      .withProperty(SECURITY_MANAGER, TestSecurityManager.class.getName()).withRestService()
-      .startServer();
+  public static LocalServerStarterRule serverStarter =
+      new ServerStarterBuilder().withSecurityManager(TestSecurityManager.class)
+          .withProperty(TestSecurityManager.SECURITY_JSON,
+              "org/apache/geode/management/internal/security/clientServer.json")
+          .withRestService().buildInThisVM();
+
   private final GeodeRestClient restClient =
       new GeodeRestClient("localhost", serverStarter.getHttpPort());
 

diff --git a/...embly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java b/...embly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java
@@ -28,6 +28,8 @@
 import org.apache.geode.rest.internal.web.controllers.Customer;
 import org.apache.geode.rest.internal.web.controllers.RedactingPostProcessor;
 import org.apache.geode.security.TestSecurityManager;
+import org.apache.geode.test.dunit.rules.LocalServerStarterRule;
+import org.apache.geode.test.dunit.rules.ServerStarterBuilder;
 import org.apache.geode.test.dunit.rules.ServerStarterRule;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.apache.geode.test.junit.categories.SecurityTest;
@@ -47,12 +49,12 @@
 public class RestSecurityPostProcessorTest {
 
   @ClassRule
-  public static ServerStarterRule serverStarter = new ServerStarterRule()
-      .withProperty(TestSecurityManager.SECURITY_JSON,
-          "org/apache/geode/management/internal/security/clientServer.json")
-      .withProperty(SECURITY_MANAGER, TestSecurityManager.class.getName())
-      .withProperty(SECURITY_POST_PROCESSOR, RedactingPostProcessor.class.getName())
-      .withRestService().startServer();
+  public static LocalServerStarterRule serverStarter =
+      new ServerStarterBuilder().withSecurityManager(TestSecurityManager.class)
+          .withProperty(TestSecurityManager.SECURITY_JSON,
+              "org/apache/geode/management/internal/security/clientServer.json")
+          .withProperty(SECURITY_POST_PROCESSOR, RedactingPostProcessor.class.getName())
+          .withRestService().buildInThisVM();
 
   private final GeodeRestClient restClient =
       new GeodeRestClient("localhost", serverStarter.getHttpPort());

diff --git a/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityWithSSLTest.java b/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityWithSSLTest.java
@@ -16,7 +16,6 @@
 package org.apache.geode.rest.internal.web;
 
 import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
-import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
 import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_ENABLED_COMPONENTS;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE;
@@ -28,10 +27,10 @@
 import static org.apache.geode.distributed.ConfigurationProperties.START_DEV_REST_API;
 import static org.junit.Assert.assertEquals;
 
-import org.apache.geode.internal.AvailablePortHelper;
 import org.apache.geode.internal.security.SecurableCommunicationChannel;
 import org.apache.geode.security.SimpleTestSecurityManager;
-import org.apache.geode.test.dunit.rules.ServerStarterRule;
+import org.apache.geode.test.dunit.rules.LocalServerStarterRule;
+import org.apache.geode.test.dunit.rules.ServerStarterBuilder;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.apache.geode.test.junit.categories.SecurityTest;
 import org.apache.http.HttpResponse;
@@ -40,36 +39,26 @@
 import org.junit.experimental.categories.Category;
 
 import java.net.URL;
-import java.util.Properties;
 
 @Category({IntegrationTest.class, SecurityTest.class})
 public class RestSecurityWithSSLTest {
+  private static URL KEYSTORE_URL =
+      RestSecurityWithSSLTest.class.getClassLoader().getResource("ssl/trusted.keystore");
 
-  private static int restPort = AvailablePortHelper.getRandomAvailableTCPPort();
   @Rule
-  public ServerStarterRule serverStarter = new ServerStarterRule();
+  public LocalServerStarterRule serverStarter = new ServerStarterBuilder().withRestService()
+      .withProperty(SECURITY_MANAGER, SimpleTestSecurityManager.class.getName())
+      .withProperty(SSL_ENABLED_COMPONENTS, SecurableCommunicationChannel.WEB.getConstant())
+      .withProperty(SSL_KEYSTORE, KEYSTORE_URL.getPath())
+      .withProperty(SSL_KEYSTORE_PASSWORD, "password").withProperty(SSL_KEYSTORE_TYPE, "JKS")
+      .withProperty(SSL_TRUSTSTORE, KEYSTORE_URL.getPath())
+      .withProperty(SSL_TRUSTSTORE_PASSWORD, "password")
+      .withProperty(SSL_PROTOCOLS, "TLSv1.2,TLSv1.1").buildInThisVM();
 
   @Test
   public void testRestSecurityWithSSL() throws Exception {
-    URL keystoreUrl =
-        RestSecurityWithSSLTest.class.getClassLoader().getResource("ssl/trusted.keystore");
-
-    Properties properties = new Properties();
-    properties.setProperty(SECURITY_MANAGER, SimpleTestSecurityManager.class.getName());
-    properties.setProperty(START_DEV_REST_API, "true");
-    properties.setProperty(HTTP_SERVICE_BIND_ADDRESS, "localhost");
-    properties.setProperty(HTTP_SERVICE_PORT, restPort + "");
-    properties.setProperty(SSL_ENABLED_COMPONENTS, SecurableCommunicationChannel.WEB.getConstant());
-    properties.setProperty(SSL_KEYSTORE, keystoreUrl.getPath());
-    properties.setProperty(SSL_KEYSTORE_PASSWORD, "password");
-    properties.setProperty(SSL_KEYSTORE_TYPE, "JKS");
-    properties.setProperty(SSL_TRUSTSTORE, keystoreUrl.getPath());
-    properties.setProperty(SSL_TRUSTSTORE_PASSWORD, "password");
-    properties.setProperty(SSL_PROTOCOLS, "TLSv1.2,TLSv1.1");
-
-    serverStarter.withProperties(properties).startServer();
-
-    GeodeRestClient restClient = new GeodeRestClient("localhost", restPort, true);
+    GeodeRestClient restClient =
+        new GeodeRestClient("localhost", serverStarter.getHttpPort(), true);
     HttpResponse response = restClient.doGet("/servers", "cluster", "cluster");
 
     assertEquals(200, GeodeRestClient.getCode(response));

diff --git a/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java b/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java
@@ -17,6 +17,8 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import org.apache.geode.test.dunit.rules.LocalServerStarterRule;
+import org.apache.geode.test.dunit.rules.ServerStarterBuilder;
 import org.apache.geode.test.dunit.rules.ServerStarterRule;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.apache.geode.test.junit.categories.RestAPITest;
@@ -31,14 +33,13 @@
 public class RestServersJUnitTest {
 
   @ClassRule
-  public static ServerStarterRule serverStarter =
-      new ServerStarterRule().withRestService(true).startServer();
+  public static LocalServerStarterRule serverStarter =
+      new ServerStarterBuilder().withRestService().buildInThisVM();
 
   private static GeodeRestClient restClient;
 
   @BeforeClass
   public static void before() throws Exception {
-    assertThat(serverStarter.getHttpPort()).isEqualTo(7070);
     restClient = new GeodeRestClient("localhost", serverStarter.getHttpPort());
   }
 
@@ -53,6 +54,6 @@ public void testServerStartedOnDefaultPort() throws Exception {
     HttpResponse response = restClient.doGet("/servers", null, null);
     JSONArray body = GeodeRestClient.getJsonArray(response);
     assertThat(body.length()).isEqualTo(1);
-    assertThat(body.getString(0)).isEqualTo("http://localhost:7070");
+    assertThat(body.getString(0)).isEqualTo("http://localhost:" + serverStarter.getHttpPort());
   }
 }
diff --git a/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java b/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java
@@ -21,6 +21,8 @@
 
 import org.apache.geode.internal.i18n.LocalizedStrings;
 import org.apache.geode.security.SimpleTestSecurityManager;
+import org.apache.geode.test.dunit.rules.LocalServerStarterRule;
+import org.apache.geode.test.dunit.rules.ServerStarterBuilder;
 import org.apache.geode.test.dunit.rules.ServerStarterRule;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.apache.geode.test.junit.categories.RestAPITest;
@@ -34,9 +36,8 @@
 public class SwaggerVerificationTest {
 
   @ClassRule
-  public static ServerStarterRule serverStarter = new ServerStarterRule()
-      .withProperty(SECURITY_MANAGER, SimpleTestSecurityManager.class.getName()).withRestService()
-      .startServer();
+  public static LocalServerStarterRule serverStarter = new ServerStarterBuilder()
+      .withSecurityManager(SimpleTestSecurityManager.class).withRestService().buildInThisVM();
 
   private GeodeRestClient restClient;
 

diff --git a/geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/HttpClientRule.java b/geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/HttpClientRule.java
@@ -41,6 +41,11 @@ public class HttpClientRule extends ExternalResource {
   private HttpHost host;
   private HttpClient httpClient;
   private HttpContext context;
+  private LocalLocatorStarterRule locatorStarterRule;
+
+  public HttpClientRule(LocalLocatorStarterRule locatorStarterRule) {
+    this.locatorStarterRule = locatorStarterRule;
+  }
 
   public HttpClientRule(int port) {
     this("localhost", port);
@@ -53,6 +58,10 @@ public HttpClientRule(String hostName, int port) {
 
   @Before
   protected void before() {
+    if (locatorStarterRule != null) {
+      this.hostName = "localhost";
+      this.port = locatorStarterRule.getHttpPort();
+    }
     host = new HttpHost(hostName, port);
     httpClient = HttpClients.createDefault();
     context = new BasicHttpContext();

diff --git a/geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b/geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java
@@ -21,7 +21,8 @@
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionShortcut;
 import org.apache.geode.test.dunit.rules.HttpClientRule;
-import org.apache.geode.test.dunit.rules.ServerStarterRule;
+import org.apache.geode.test.dunit.rules.LocalServerStarterRule;
+import org.apache.geode.test.dunit.rules.ServerStarterBuilder;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.apache.http.HttpResponse;
 import org.apache.http.util.EntityUtils;
@@ -34,8 +35,8 @@
 public class PulseDataExportTest {
 
   @Rule
-  public ServerStarterRule server = new ServerStarterRule().withJMXManager().startServer()
-      .createRegion(RegionShortcut.REPLICATE, "regionA");
+  public LocalServerStarterRule server = new ServerStarterBuilder().withJMXManager()
+      .withRegion(RegionShortcut.REPLICATE, "regionA").buildInThisVM();
 
   @Rule
   public HttpClientRule client = new HttpClientRule(server.getHttpPort());

diff --git a/geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseVerificationTest.java b/geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseVerificationTest.java
@@ -15,12 +15,12 @@
 
 package org.apache.geode.tools.pulse;
 
-import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import org.apache.geode.security.SimpleTestSecurityManager;
 import org.apache.geode.test.dunit.rules.HttpClientRule;
-import org.apache.geode.test.dunit.rules.LocatorStarterRule;
+import org.apache.geode.test.dunit.rules.LocatorStarterBuilder;
+import org.apache.geode.test.dunit.rules.LocalLocatorStarterRule;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.apache.http.HttpResponse;
 import org.junit.ClassRule;
@@ -33,11 +33,11 @@
 public class PulseVerificationTest {
 
   @ClassRule
-  public static LocatorStarterRule locator = new LocatorStarterRule()
-      .withProperty(SECURITY_MANAGER, SimpleTestSecurityManager.class.getName()).startLocator();
+  public static LocalLocatorStarterRule locator = new LocatorStarterBuilder()
+      .withSecurityManager(SimpleTestSecurityManager.class).buildInThisVM();
 
   @Rule
-  public HttpClientRule client = new HttpClientRule(locator.getHttpPort());
+  public HttpClientRule client = new HttpClientRule(locator);
 
   @Test
   public void loginWithIncorrectPassword() throws Exception {

diff --git a/geode-core/src/main/java/org/apache/geode/internal/AvailablePort.java b/geode-core/src/main/java/org/apache/geode/internal/AvailablePort.java
@@ -20,6 +20,7 @@
 
 import java.io.IOException;
 import java.io.PrintStream;
+import java.io.Serializable;
 import java.net.*;
 import java.util.Enumeration;
 import java.util.Random;
@@ -448,8 +449,8 @@ public static int getRandomAvailablePortInRange(int rangeBase, int rangeTop, int
    * smaller that can cause bug 46690
    *
    */
-  public static class Keeper {
-    private final ServerSocket ss;
+  public static class Keeper implements Serializable {
+    private transient final ServerSocket ss;
     private final int port;
 
     public Keeper(ServerSocket ss, int port) {

diff --git a/geode-core/src/test/java/org/apache/geode/cache/ConnectionPoolDUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/ConnectionPoolDUnitTest.java
@@ -5040,7 +5040,7 @@ public void run() {
   // vm0.invoke(new CacheSerializableRunnable("Create Cache Server") {
   // public void run2() throws CacheException {
   // AttributesFactory factory = getBridgeServerRegionAttributes(null,null);
-  // Region region = createRegion(name, factory.create());
+  // Region region = withRegion(name, factory.create());
   // pause(1000);
   // try {
   // startBridgeServer(0);
@@ -5064,7 +5064,7 @@ public void run() {
 
   // ClientServerTestCase.configureConnectionPool(factory,host0,port,-1,true,-1,-1, null);
 
-  // createRegion(name, factory.create());
+  // withRegion(name, factory.create());
   // }
   // };
 

diff --git a/geode-core/src/test/java/org/apache/geode/cache/ProxyJUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/ProxyJUnitTest.java
@@ -1031,7 +1031,7 @@ public void afterDestroy(EntryEvent e) {
     // // on a non-proxy lru
     // {
     // af.setDataPolicy(DataPolicy.NORMAL);
-    // Region r = this.c.createRegion("rLRU", af.create());
+    // Region r = this.c.withRegion("rLRU", af.create());
     // clearCallbackState();
     // assertTrue(clInvokeCount == 0);
     // for (int i=0; i < 10; i++) {
@@ -1048,7 +1048,7 @@ public void afterDestroy(EntryEvent e) {
         fail("expected IllegalStateException");
       } catch (IllegalStateException expected) {
       }
-      // Region r = this.c.createRegion("rEMPTY", af.create());
+      // Region r = this.c.withRegion("rEMPTY", af.create());
       // clearCallbackState();
       // assertTrue(clInvokeCount == 0);
       // for (int i=0; i < 10; i++) {

diff --git a/...e-core/src/test/java/org/apache/geode/cache/partition/PartitionRegionHelperDUnitTest.java b/...e-core/src/test/java/org/apache/geode/cache/partition/PartitionRegionHelperDUnitTest.java
@@ -74,7 +74,7 @@ public void testAssignBucketsToPartitions() throws Throwable {
     VM vm1 = host.getVM(1);
     VM vm2 = host.getVM(2);
 
-    SerializableRunnable createPrRegion = new SerializableRunnable("createRegion") {
+    SerializableRunnable createPrRegion = new SerializableRunnable("withRegion") {
       public void run() {
         Cache cache = getCache();
         AttributesFactory attr = new AttributesFactory();
@@ -143,7 +143,7 @@ public void testAssignBucketsToPartitions_FPR() throws Throwable {
     VM vm2 = host.getVM(2);
     VM vm3 = host.getVM(3);
 
-    SerializableRunnable createPrRegion1 = new SerializableRunnable("createRegion") {
+    SerializableRunnable createPrRegion1 = new SerializableRunnable("withRegion") {
       public void run() {
         Cache cache = getCache();
         FixedPartitionAttributes fpa1 =
@@ -163,7 +163,7 @@ public void run() {
       }
     };
     vm0.invoke(createPrRegion1);
-    SerializableRunnable createPrRegion2 = new SerializableRunnable("createRegion") {
+    SerializableRunnable createPrRegion2 = new SerializableRunnable("withRegion") {
       public void run() {
         Cache cache = getCache();
         FixedPartitionAttributes fpa1 =
@@ -183,7 +183,7 @@ public void run() {
       }
     };
     vm1.invoke(createPrRegion2);
-    SerializableRunnable createPrRegion3 = new SerializableRunnable("createRegion") {
+    SerializableRunnable createPrRegion3 = new SerializableRunnable("withRegion") {
       public void run() {
         Cache cache = getCache();
         FixedPartitionAttributes fpa1 =
@@ -240,7 +240,7 @@ public void run() {
     vm0.invoke(checkAssignment);
 
 
-    SerializableRunnable createPrRegion4 = new SerializableRunnable("createRegion") {
+    SerializableRunnable createPrRegion4 = new SerializableRunnable("withRegion") {
       public void run() {
         Cache cache = getCache();
         AttributesFactory attr = new AttributesFactory();
@@ -269,7 +269,7 @@ public void run() {
     };
     vm3.invoke(createPrRegion4);
 
-    SerializableRunnable checkMembers = new SerializableRunnable("createRegion") {
+    SerializableRunnable checkMembers = new SerializableRunnable("withRegion") {
       public void run() {
         Cache cache = getCache();
         Region region = cache.getRegion("region1");
@@ -550,7 +550,7 @@ public void testMoveSingleBucket() {
     VM vm1 = host.getVM(1);
     VM vm2 = host.getVM(2);
 
-    SerializableCallable createPrRegion = new SerializableCallable("createRegion") {
+    SerializableCallable createPrRegion = new SerializableCallable("withRegion") {
       public Object call() {
         Cache cache = getCache();
         AttributesFactory attr = new AttributesFactory();
@@ -647,7 +647,7 @@ public void testMovePercentage() {
     VM vm1 = host.getVM(1);
     VM vm2 = host.getVM(2);
 
-    SerializableCallable createPrRegion = new SerializableCallable("createRegion") {
+    SerializableCallable createPrRegion = new SerializableCallable("withRegion") {
       public Object call() {
         Cache cache = getCache();
         AttributesFactory attr = new AttributesFactory();