GEODE-6221: Cleanup some Protobuf tests (apache#3039)

GEODE-6221: Cleanup some Protobuf tests * clarify the doc comments for DistributedRestoreSystemProperties. * Remove usage of JUnit4CacheTestCase from geode-protobuf. * Don't use DUnit embedded locator for testing against. I'm not 100% this will fix all the flakiness, but it should be an improvement.
IronOnet · Dec 28, 2018 · 724fdde · 724fdde
1 parent 4d77573
commit 724fdde
Show file tree

Hide file tree

Showing 4 changed files with 132 additions and 110 deletions.
diff --git a/...t/src/main/java/org/apache/geode/test/dunit/rules/DistributedRestoreSystemProperties.java b/...t/src/main/java/org/apache/geode/test/dunit/rules/DistributedRestoreSystemProperties.java
@@ -18,7 +18,7 @@
 
 /**
  * Distributed version of RestoreSystemProperties which affects all DUnit JVMs including the Locator
- * JVM.
+ * JVM and the test JVM.
  */
 public class DistributedRestoreSystemProperties extends AbstractDistributedRule {
 

diff --git a/...de/internal/protocol/protobuf/v1/acceptance/LocatorConnectionAuthenticationDUnitTest.java b/...de/internal/protocol/protobuf/v1/acceptance/LocatorConnectionAuthenticationDUnitTest.java
@@ -15,64 +15,95 @@
 
 package org.apache.geode.internal.protocol.protobuf.v1.acceptance;
 
-import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
 import java.net.Socket;
 import java.util.Properties;
 
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.server.CacheServer;
 import org.apache.geode.distributed.ConfigurationProperties;
 import org.apache.geode.distributed.Locator;
-import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol;
 import org.apache.geode.internal.protocol.protobuf.v1.ConnectionAPI;
 import org.apache.geode.internal.protocol.protobuf.v1.MessageUtil;
 import org.apache.geode.internal.protocol.protobuf.v1.ProtobufRequestUtilities;
 import org.apache.geode.internal.protocol.protobuf.v1.serializer.ProtobufProtocolSerializer;
 import org.apache.geode.internal.protocol.protobuf.v1.state.exception.ConnectionStateException;
 import org.apache.geode.security.SimpleTestSecurityManager;
-import org.apache.geode.test.dunit.Host;
 import org.apache.geode.test.dunit.IgnoredException;
-import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
+import org.apache.geode.test.dunit.VM;
 import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
 import org.apache.geode.test.junit.categories.ClientServerTest;
 
 /**
  * Test sending ProtoBuf messages to the locator, with a security manager configured on the locator
  */
 @Category(ClientServerTest.class)
-public class LocatorConnectionAuthenticationDUnitTest extends JUnit4CacheTestCase {
+public class LocatorConnectionAuthenticationDUnitTest {
   @Rule
-  public final DistributedRestoreSystemProperties restoreSystemProperties =
+  public final DistributedRestoreSystemProperties restoreDistributedSystemProperties =
       new DistributedRestoreSystemProperties();
+
   private int locatorPort;
 
+  // only used for cleanup on VM 1.
+  private static Locator locator;
+  private VM locatorVM;
+  private Cache cache;
+
   @Before
   public void setup() throws IOException {
     // Start a new locator with authorization
-    locatorPort = Host.getHost(0).getVM(0).invoke(() -> {
+    locatorVM = VM.getVM(0);
+    locatorPort = locatorVM.invoke(() -> {
       System.setProperty("geode.feature-protobuf-protocol", "true");
       Properties props = new Properties();
       props.setProperty(ConfigurationProperties.SECURITY_MANAGER,
           SimpleTestSecurityManager.class.getName());
-      Locator locator = Locator.startLocatorAndDS(0, null, props);
+      locator = Locator.startLocatorAndDS(0, null, props);
       return locator.getPort();
     });
 
     startCacheWithCacheServer(locatorPort);
   }
 
+  @After
+  public void tearDown() {
+    try {
+      locatorVM.invoke(() -> locator.stop());
+      locator = null;
+    } finally {
+      cache.close();
+      cache = null;
+    }
+  }
+
+  private void startCacheWithCacheServer(int locatorPort) throws IOException {
+    System.setProperty("geode.feature-protobuf-protocol", "true");
+
+    Properties props = new Properties();
+    props.setProperty(ConfigurationProperties.LOCATORS, "localhost[" + locatorPort + "]");
+    props.setProperty("security-username", "cluster");
+    props.setProperty("security-password", "cluster");
+    props.setProperty(ConfigurationProperties.USE_CLUSTER_CONFIGURATION, "true");
+    cache = new CacheFactory(props).create();
+    final CacheServer cacheServer = cache.addCacheServer();
+    cacheServer.setPort(0);
+    cacheServer.start();
+  }
+
   private Socket createSocket() throws IOException {
-    Host host = Host.getHost(0);
-    Socket socket = new Socket(host.getHostName(), locatorPort);
+    Socket socket = new Socket(VM.getHostName(), locatorPort);
     MessageUtil.sendHandshake(socket);
     MessageUtil.verifyHandshakeSuccess(socket);
     return socket;
@@ -89,19 +120,18 @@ public void authorizedClientCanGetServersIfSecurityIsEnabled() throws Throwable
             .putCredentials("security-username", "cluster").putCredentials("security-password",
                 "cluster"))
         .build();
-    ClientProtocol.Message GetServerRequestMessage = ClientProtocol.Message.newBuilder()
+    ClientProtocol.Message getServerRequestMessage = ClientProtocol.Message.newBuilder()
         .setGetServerRequest(ProtobufRequestUtilities.createGetServerRequest()).build();
 
     ProtobufProtocolSerializer protobufProtocolSerializer = new ProtobufProtocolSerializer();
 
     try (Socket socket = createSocket()) {
       protobufProtocolSerializer.serialize(authorization, socket.getOutputStream());
-
       ClientProtocol.Message authorizationResponse =
           protobufProtocolSerializer.deserialize(socket.getInputStream());
-      assertEquals(true, authorizationResponse.getHandshakeResponse().getAuthenticated());
-      protobufProtocolSerializer.serialize(GetServerRequestMessage, socket.getOutputStream());
+      assertTrue(authorizationResponse.getHandshakeResponse().getAuthenticated());
 
+      protobufProtocolSerializer.serialize(getServerRequestMessage, socket.getOutputStream());
       ClientProtocol.Message GetServerResponseMessage =
           protobufProtocolSerializer.deserialize(socket.getInputStream());
       assertTrue("Got response: " + GetServerResponseMessage,
@@ -130,20 +160,4 @@ public void unauthorizedClientCannotGetServersIfSecurityIsEnabled() throws Throw
           getServerRequestMessage.getErrorResponse());
     }
   }
-
-
-  private Integer startCacheWithCacheServer(int locatorPort) throws IOException {
-    System.setProperty("geode.feature-protobuf-protocol", "true");
-
-    Properties props = new Properties();
-    props.setProperty(ConfigurationProperties.LOCATORS, "localhost[" + locatorPort + "]");
-    props.setProperty("security-username", "cluster");
-    props.setProperty("security-password", "cluster");
-    props.setProperty(ConfigurationProperties.USE_CLUSTER_CONFIGURATION, "true");
-    InternalCache cache = getCache(props);
-    CacheServer cacheServer = cache.addCacheServer();
-    cacheServer.setPort(0);
-    cacheServer.start();
-    return cacheServer.getPort();
-  }
 }
diff --git a/...org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java b/...org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java
@@ -30,11 +30,8 @@
 import org.junit.experimental.categories.Category;
 
 import org.apache.geode.Statistics;
-import org.apache.geode.cache.server.CacheServer;
 import org.apache.geode.distributed.ConfigurationProperties;
-import org.apache.geode.distributed.Locator;
-import org.apache.geode.distributed.internal.InternalDistributedSystem;
-import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.distributed.DistributedSystem;
 import org.apache.geode.internal.protocol.protobuf.statistics.ProtobufClientStatistics;
 import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes;
 import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol;
@@ -43,34 +40,54 @@
 import org.apache.geode.internal.protocol.protobuf.v1.ProtobufRequestUtilities;
 import org.apache.geode.internal.protocol.protobuf.v1.serializer.ProtobufProtocolSerializer;
 import org.apache.geode.internal.protocol.protobuf.v1.serializer.exception.InvalidProtocolMessageException;
-import org.apache.geode.test.dunit.DistributedTestUtils;
 import org.apache.geode.test.dunit.Host;
 import org.apache.geode.test.dunit.IgnoredException;
-import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
 import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
+import org.apache.geode.test.dunit.rules.MemberVM;
 import org.apache.geode.test.junit.categories.ClientServerTest;
 
 /*
  * Test sending ProtoBuf messages to the locator
  */
 @Category(ClientServerTest.class)
-public class LocatorConnectionDUnitTest extends JUnit4CacheTestCase {
+public class LocatorConnectionDUnitTest {
+
+  // Has to be distributed because how else would we query the members?
+  @Rule
+  public ClusterStartupRule clusterStartupRule = new ClusterStartupRule(2);
 
   @Rule
   public final DistributedRestoreSystemProperties restoreSystemProperties =
       new DistributedRestoreSystemProperties();
 
+  private MemberVM locatorVM;
+  private int locatorPort;
+  private String hostName;
+
   @Before
-  public void setup() throws IOException {
-    Host.getLocator().invoke(() -> System.setProperty("geode.feature-protobuf-protocol", "true"));
+  public void setup() {
+    for (VM vm : VM.getAllVMs()) {
+      vm.invoke(() -> System.setProperty("geode.feature-protobuf-protocol", "true"));
+    }
+
+    Properties properties = new Properties();
+    properties.put(ConfigurationProperties.STATISTIC_SAMPLING_ENABLED, "true");
+    properties.put(ConfigurationProperties.STATISTIC_SAMPLE_RATE, "100");
+
+    clusterStartupRule.getVM(0)
+        .invoke(() -> System.setProperty("geode.feature-protobuf-protocol", "true"));
+    locatorVM = clusterStartupRule.startLocatorVM(0, properties, 0);
+    locatorPort = locatorVM.invoke(() -> ClusterStartupRule.getLocator().getPort());
+
+    clusterStartupRule.startServerVM(1, locatorPort);
 
-    startCacheWithCacheServer();
+    hostName = Host.getHost(0).getHostName();
   }
 
   private Socket createSocket() throws IOException {
-    Host host = Host.getHost(0);
-    int locatorPort = DistributedTestUtils.getDUnitLocatorPort();
-    Socket socket = new Socket(host.getHostName(), locatorPort);
+    Socket socket = new Socket(hostName, locatorPort);
     MessageUtil.sendHandshake(socket);
     MessageUtil.verifyHandshakeSuccess(socket);
     return socket;
@@ -151,53 +168,52 @@ public void testInvalidOperationReturnsFailure()
     ignoredInvalidExecutionContext.remove();
   }
 
-  private Statistics getStatistics() {
-    InternalDistributedSystem distributedSystem =
-        (InternalDistributedSystem) Locator.getLocator().getDistributedSystem();
-
+  private static Statistics getStatistics() {
+    final DistributedSystem distributedSystem =
+        ClusterStartupRule.getLocator().getDistributedSystem();
     Statistics[] protobufServerStats = distributedSystem.findStatisticsByType(
         distributedSystem.findType(ProtobufClientStatistics.PROTOBUF_CLIENT_STATISTICS));
     assertEquals(1, protobufServerStats.length);
     return protobufServerStats[0];
   }
 
   private Long getBytesReceived() {
-    return Host.getLocator().invoke(() -> {
+    return locatorVM.invoke(() -> {
       Statistics statistics = getStatistics();
       return statistics.get("bytesReceived").longValue();
     });
   }
 
   private Long getBytesSent() {
-    return Host.getLocator().invoke(() -> {
+    return locatorVM.invoke(() -> {
       Statistics statistics = getStatistics();
       return statistics.get("bytesSent").longValue();
     });
   }
 
   private Long getMessagesReceived() {
-    return Host.getLocator().invoke(() -> {
+    return locatorVM.invoke(() -> {
       Statistics statistics = getStatistics();
       return statistics.get("messagesReceived").longValue();
     });
   }
 
   private Long getMessagesSent() {
-    return Host.getLocator().invoke(() -> {
+    return locatorVM.invoke(() -> {
       Statistics statistics = getStatistics();
       return statistics.get("messagesSent").longValue();
     });
   }
 
   private Integer getClientConnectionStarts() {
-    return Host.getLocator().invoke(() -> {
+    return locatorVM.invoke(() -> {
       Statistics statistics = getStatistics();
       return statistics.get("clientConnectionStarts").intValue();
     });
   }
 
   private Integer getClientConnectionTerminations() {
-    return Host.getLocator().invoke(() -> {
+    return locatorVM.invoke(() -> {
       Statistics statistics = getStatistics();
       return statistics.get("clientConnectionTerminations").intValue();
     });
@@ -215,7 +231,7 @@ private void validateGetAvailableServersResponse(
 
   private void validateStats(long messagesReceived, long messagesSent, long bytesReceived,
       long bytesSent, int clientConnectionStarts, int clientConnectionTerminations) {
-    Host.getLocator().invoke(() -> {
+    locatorVM.invoke(() -> {
       await().untilAsserted(() -> {
         Statistics statistics = getStatistics();
         assertEquals(0, statistics.get("currentClientConnections"));
@@ -230,22 +246,4 @@ private void validateStats(long messagesReceived, long messagesSent, long bytesR
       });
     });
   }
-
-  @Override
-  public Properties getDistributedSystemProperties() {
-    Properties properties = super.getDistributedSystemProperties();
-    properties.put(ConfigurationProperties.STATISTIC_SAMPLING_ENABLED, "true");
-    properties.put(ConfigurationProperties.STATISTIC_SAMPLE_RATE, "100");
-    return properties;
-  }
-
-  private Integer startCacheWithCacheServer() throws IOException {
-    System.setProperty("geode.feature-protobuf-protocol", "true");
-
-    InternalCache cache = getCache();
-    CacheServer cacheServer = cache.addCacheServer();
-    cacheServer.setPort(0);
-    cacheServer.start();
-    return cacheServer.getPort();
-  }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -18,7 +18,7 @@ @@
     /**
      * Distributed version of RestoreSystemProperties which affects all DUnit JVMs including the Locator
-     * JVM.
+     * JVM and the test JVM.
      */
     public class DistributedRestoreSystemProperties extends AbstractDistributedRule {
@@ Expand Down @@