You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kl...@apache.org on 2019/06/13 17:22:45 UTC

[geode] 05/05: GEODE-6183: Cleanup and rename LocatorIntegrationTest

This is an automated email from the ASF dual-hosted git repository.

klund pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 87c83ee4e2e6214bf2dd65d279520c8d437c56ee
Author: Kirk Lund <kl...@apache.org>
AuthorDate: Thu May 30 13:46:46 2019 -0700

    GEODE-6183: Cleanup and rename LocatorIntegrationTest
    
    * Fixup IDE warnings
    * Reformat test code
    * Update to use AssertJ
---
 ...rJUnitTest.java => LocatorIntegrationTest.java} | 193 +++++++++++----------
 1 file changed, 100 insertions(+), 93 deletions(-)

diff --git a/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorIntegrationTest.java
similarity index 53%
rename from geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorJUnitTest.java
rename to geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorIntegrationTest.java
index 9f0c4ce..3ce6811 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorJUnitTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorIntegrationTest.java
@@ -21,13 +21,11 @@ import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_S
 import static org.apache.geode.distributed.ConfigurationProperties.LOCATOR_WAIT_TIME;
 import static org.apache.geode.distributed.ConfigurationProperties.LOG_FILE;
 import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.apache.geode.distributed.internal.DistributionConfig.GEMFIRE_PREFIX;
 import static org.apache.geode.internal.AvailablePort.SOCKET;
 import static org.apache.geode.internal.AvailablePort.getRandomAvailablePort;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
 
 import java.io.File;
 import java.io.IOException;
@@ -42,13 +40,16 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
 import org.junit.experimental.categories.Category;
 import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+import org.junit.runners.Parameterized.UseParametersRunnerFactory;
 
 import org.apache.geode.SystemConnectException;
-import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.distributed.internal.InternalLocator;
 import org.apache.geode.distributed.internal.ServerLocation;
 import org.apache.geode.distributed.internal.membership.gms.messenger.JGroupsMessenger;
@@ -63,162 +64,168 @@ import org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactor
 
 @Category({MembershipTest.class})
 @RunWith(Parameterized.class)
-@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
-public class LocatorJUnitTest {
-
-  private static final int REQUEST_TIMEOUT = 5 * 1000;
+@UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
+public class LocatorIntegrationTest {
 
   private Locator locator;
   private File tmpFile;
   private int port;
 
-  @Parameterized.Parameters
+  @Parameters
   public static Collection<Object> data() {
-    return Arrays.asList(new Object[] {(IntSupplier) () -> 0,
-        (IntSupplier) () -> AvailablePortHelper.getRandomAvailableTCPPort()});
+    return Arrays.asList(new Object[] {
+        (IntSupplier) () -> 0,
+        (IntSupplier) AvailablePortHelper::getRandomAvailableTCPPort});
   }
 
-  @Parameterized.Parameter
+  @Parameter
   public IntSupplier portSupplier;
 
   @Rule
+  public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
+
+  @Rule
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
   @Before
   public void setUp() throws IOException {
     tmpFile = File.createTempFile("locator", ".log");
-    this.port = portSupplier.getAsInt();
+    port = portSupplier.getAsInt();
     deleteLocatorViewFile(port);
   }
 
-  private void deleteLocatorViewFile(int portNumber) {
-    File locatorFile = new File("locator" + portNumber + "view.dat");
-    if (locatorFile.exists()) {
-      locatorFile.delete();
-    }
-  }
-
   @After
   public void tearDown() {
+    JGroupsMessenger.THROW_EXCEPTION_ON_START_HOOK = false;
     if (locator != null) {
       locator.stop();
     }
-    assertEquals(false, Locator.hasLocator());
+    assertThat(Locator.hasLocator()).isFalse();
   }
 
   /**
-   * GEODE-4176: locator creates "locator0view.dat" file when started with port 0
-   *
+   * Fix: locator creates "locator0view.dat" file when started with port 0
    */
   @Test
   public void testThatLocatorDoesNotCreateFileWithZeroPort() throws Exception {
     deleteLocatorViewFile(0);
-    Properties dsprops = new Properties();
-    dsprops.setProperty(MCAST_PORT, "0");
-    dsprops.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
-    dsprops.setProperty(LOCATOR_WAIT_TIME, "1"); // seconds
-    dsprops.setProperty(LOG_FILE, "");
-    locator = Locator.startLocatorAndDS(port, null, dsprops);
+
+    Properties configProperties = new Properties();
+    configProperties.setProperty(MCAST_PORT, "0");
+    configProperties.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
+    configProperties.setProperty(LOCATOR_WAIT_TIME, "1"); // seconds
+    configProperties.setProperty(LOG_FILE, "");
+
+    locator = Locator.startLocatorAndDS(port, null, configProperties);
+
     File viewFile = new File("locator0view.dat");
-    assertFalse("file should not exist: " + viewFile.getAbsolutePath(), viewFile.exists());
+
+    assertThat(viewFile).doesNotExist();
   }
 
   /**
-   * TRAC #45804: if jmx-manager-start is true in a locator then gfsh connect will fail
+   * Fix: if jmx-manager-start is true in a locator then gfsh connect will fail
    */
   @Test
   public void testGfshConnectShouldSucceedIfJmxManagerStartIsTrueInLocator() throws Exception {
-    Properties dsprops = new Properties();
     int jmxPort = getRandomAvailablePort(SOCKET);
-    dsprops.setProperty(MCAST_PORT, "0");
-    dsprops.setProperty(JMX_MANAGER_PORT, "" + jmxPort);
-    dsprops.setProperty(JMX_MANAGER_START, "true");
-    dsprops.setProperty(JMX_MANAGER_HTTP_PORT, "0");
-    dsprops.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
-    dsprops.setProperty(LOG_FILE, "");
-    System.setProperty(DistributionConfig.GEMFIRE_PREFIX + "disableManagement", "false"); // not
-                                                                                          // needed
-    try {
-      locator = Locator.startLocatorAndDS(port, null, dsprops);
-      List<JmxManagerProfile> alreadyManaging =
-          GemFireCacheImpl.getInstance().getJmxManagerAdvisor().adviseAlreadyManaging();
-      assertEquals(1, alreadyManaging.size());
-      assertEquals(GemFireCacheImpl.getInstance().getMyId(),
-          alreadyManaging.get(0).getDistributedMember());
-    } finally {
-      System.clearProperty(DistributionConfig.GEMFIRE_PREFIX + "enabledManagement");
-    }
-  }
 
+    Properties configProperties = new Properties();
+    configProperties.setProperty(MCAST_PORT, "0");
+    configProperties.setProperty(JMX_MANAGER_PORT, "" + jmxPort);
+    configProperties.setProperty(JMX_MANAGER_START, "true");
+    configProperties.setProperty(JMX_MANAGER_HTTP_PORT, "0");
+    configProperties.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
+    configProperties.setProperty(LOG_FILE, "");
+
+    // not needed
+    System.setProperty(GEMFIRE_PREFIX + "disableManagement", "false");
+
+    locator = Locator.startLocatorAndDS(port, null, configProperties);
+    List<JmxManagerProfile> alreadyManaging =
+        GemFireCacheImpl.getInstance().getJmxManagerAdvisor().adviseAlreadyManaging();
+
+    assertThat(alreadyManaging).hasSize(1);
+    assertThat(alreadyManaging.get(0).getDistributedMember())
+        .isEqualTo(GemFireCacheImpl.getInstance().getMyId());
+  }
 
   @Test
   public void testHandlersAreWaitedOn() throws Exception {
-    Properties dsprops = new Properties();
-    int jmxPort = getRandomAvailablePort(SOCKET);
-    dsprops.setProperty(MCAST_PORT, "0");
-    dsprops.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
-    dsprops.setProperty(LOCATOR_WAIT_TIME, "1"); // seconds
-    dsprops.setProperty(LOG_FILE, "");
-    locator = Locator.startLocatorAndDS(port, null, dsprops);
+    Properties configProperties = new Properties();
+    configProperties.setProperty(MCAST_PORT, "0");
+    configProperties.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
+    configProperties.setProperty(LOCATOR_WAIT_TIME, "1"); // seconds
+    configProperties.setProperty(LOG_FILE, "");
+
+    locator = Locator.startLocatorAndDS(port, null, configProperties);
+
     InternalLocator internalLocator = (InternalLocator) locator;
+
     // the locator should always install a SharedConfigurationStatusRequest handler
-    assertTrue(internalLocator.hasHandlerForClass(SharedConfigurationStatusRequest.class));
+    assertThat(internalLocator.hasHandlerForClass(SharedConfigurationStatusRequest.class)).isTrue();
   }
 
-
   @Test
   public void testBasicInfo() throws Exception {
     locator = Locator.startLocator(port, tmpFile);
-    int boundPort = (port == 0) ? locator.getPort() : port;
+    int boundPort = port == 0 ? locator.getPort() : port;
     TcpClient client = new TcpClient();
     String[] info = client.getInfo(InetAddress.getLocalHost(), boundPort);
-    assertNotNull(info);
-    assertTrue(info.length > 1);
+
+    assertThat(info).isNotNull();
+    assertThat(info.length).isGreaterThanOrEqualTo(1);
   }
 
   @Test
   public void testNoThreadLeftBehind() throws Exception {
-    Properties dsprops = new Properties();
-    dsprops.setProperty(MCAST_PORT, "0");
-    dsprops.setProperty(JMX_MANAGER_START, "false");
-    dsprops.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
-
     JGroupsMessenger.THROW_EXCEPTION_ON_START_HOOK = true;
+
+    Properties configProperties = new Properties();
+    configProperties.setProperty(MCAST_PORT, "0");
+    configProperties.setProperty(JMX_MANAGER_START, "false");
+    configProperties.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
+
     int threadCount = Thread.activeCount();
-    try {
-      locator = Locator.startLocatorAndDS(port, new File(""), dsprops);
-      locator.stop();
-      fail("expected an exception");
-    } catch (SystemConnectException expected) {
 
-      for (int i = 0; i < 10; i++) {
-        if (threadCount < Thread.activeCount()) {
-          Thread.sleep(1000);
-        }
-      }
+    Throwable thrown = catchThrowable(
+        () -> locator = Locator.startLocatorAndDS(port, new File(""), configProperties));
+
+    assertThat(thrown).isInstanceOf(SystemConnectException.class);
+
+    for (int i = 0; i < 10; i++) {
       if (threadCount < Thread.activeCount()) {
-        OSProcess.printStacks(0);
-        fail("expected " + threadCount + " threads or fewer but found " + Thread.activeCount()
-            + ".  Check log file for a thread dump.");
+        Thread.sleep(1000);
       }
-    } finally {
-      JGroupsMessenger.THROW_EXCEPTION_ON_START_HOOK = false;
     }
+
+    OSProcess.printStacks(0);
+
+    assertThat(threadCount)
+        .as("Expected " + threadCount + " threads or fewer but found " + Thread.activeCount()
+            + ". Check log file for a thread dump.")
+        .isGreaterThanOrEqualTo(Thread.activeCount());
   }
 
   /**
    * Make sure two ServerLocation objects on different hosts but with the same port are not equal
-   * <p/>
-   * TRAC #42040: LoadBalancing directs all traffic to a single cache server if all servers are
-   * started on the same port
+   *
+   * <p>
+   * Fix: LoadBalancing directs all traffic to a single cache server if all servers are started on
+   * the same port
    */
   @Test
   public void testServerLocationOnDifferentHostsShouldNotTestEqual() {
-    ServerLocation sl1 = new ServerLocation("host1", 777);
-    ServerLocation sl2 = new ServerLocation("host2", 777);
-    if (sl1.equals(sl2)) {
-      fail("ServerLocation instances on different hosts should not test equal");
-    }
+    ServerLocation serverLocation1 = new ServerLocation("host1", 777);
+    ServerLocation serverLocation2 = new ServerLocation("host2", 777);
+
+    assertThat(serverLocation1).isNotEqualTo(serverLocation2);
   }
 
+  private void deleteLocatorViewFile(int portNumber) {
+    File locatorFile = new File("locator" + portNumber + "view.dat");
+    if (locatorFile.exists()) {
+      assertThat(locatorFile.delete()).isTrue();
+    }
+  }
 }