You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jj...@apache.org on 2018/11/13 09:15:58 UTC

[geode] branch develop updated: GEODE-5873: Fix VMProvider.invokeInEveryMember (#2812)

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

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


The following commit(s) were added to refs/heads/develop by this push:
     new 7b252a6  GEODE-5873: Fix VMProvider.invokeInEveryMember (#2812)
7b252a6 is described below

commit 7b252a628379d21fcafb34ce96357c434357a108
Author: Juan José Ramos <ju...@users.noreply.github.com>
AuthorDate: Tue Nov 13 09:15:46 2018 +0000

    GEODE-5873: Fix VMProvider.invokeInEveryMember (#2812)
    
    GEODE-5873: Fix VMProvider.invokeInEveryMember
    
    Method `invokeInEveryMember` from `VMProvider` ends up being a no-op
    when the array of members is empty or null.
    
    - Fixed tests `CreateJndiBindingCommandDUnitTest` and
    `DestroyJndiBindingCommandDUnitTest`, they weren't fully validating
    the result.
    - Method `invokeInEveryMember` now validates its parameters, throwing
    an exception when the arguments are invalid.
---
 .../CreateJndiBindingCommandDUnitTest.java         | 14 +++++++++----
 .../DestroyJndiBindingCommandDUnitTest.java        | 23 +++++++++++++---------
 .../apache/geode/test/junit/rules/VMProvider.java  |  6 ++++++
 3 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommandDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommandDUnitTest.java
index 42e206e..4a491dd 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommandDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommandDUnitTest.java
@@ -20,11 +20,13 @@ import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Test;
+import org.junit.experimental.categories.Category;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.NodeList;
 
 import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
+import org.apache.geode.distributed.internal.InternalLocator;
 import org.apache.geode.internal.jndi.JNDIInvoker;
 import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.management.internal.configuration.domain.Configuration;
@@ -33,9 +35,11 @@ import org.apache.geode.test.dunit.IgnoredException;
 import org.apache.geode.test.dunit.SerializableRunnableIF;
 import org.apache.geode.test.dunit.rules.ClusterStartupRule;
 import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.GfshTest;
 import org.apache.geode.test.junit.rules.GfshCommandRule;
 import org.apache.geode.test.junit.rules.VMProvider;
 
+@Category(GfshTest.class)
 public class CreateJndiBindingCommandDUnitTest {
 
   private static MemberVM locator, server1, server2;
@@ -57,10 +61,10 @@ public class CreateJndiBindingCommandDUnitTest {
   }
 
   @Test
-  public void testCreateJndiBinding() throws Exception {
+  public void testCreateJndiBinding() {
     // assert that is no datasource
     VMProvider.invokeInEveryMember(
-        () -> assertThat(JNDIInvoker.getNoOfAvailableDataSources()).isEqualTo(0));
+        () -> assertThat(JNDIInvoker.getNoOfAvailableDataSources()).isEqualTo(0), server1, server2);
 
     // create the binding
     gfsh.executeAndAssertThat(
@@ -69,8 +73,10 @@ public class CreateJndiBindingCommandDUnitTest {
 
     // verify cluster config is updated
     locator.invoke(() -> {
+      InternalLocator internalLocator = ClusterStartupRule.getLocator();
+      assertThat(internalLocator).isNotNull();
       InternalConfigurationPersistenceService ccService =
-          ClusterStartupRule.getLocator().getConfigurationPersistenceService();
+          internalLocator.getConfigurationPersistenceService();
       Configuration configuration = ccService.getConfiguration("cluster");
       String xmlContent = configuration.getCacheXmlContent();
 
@@ -94,7 +100,7 @@ public class CreateJndiBindingCommandDUnitTest {
 
     // verify datasource exists
     VMProvider.invokeInEveryMember(
-        () -> assertThat(JNDIInvoker.getNoOfAvailableDataSources()).isEqualTo(1));
+        () -> assertThat(JNDIInvoker.getNoOfAvailableDataSources()).isEqualTo(1), server1, server2);
 
     // bounce server1
     server1.stop(false);
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DestroyJndiBindingCommandDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DestroyJndiBindingCommandDUnitTest.java
index f4c3cd6..97f0170 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DestroyJndiBindingCommandDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DestroyJndiBindingCommandDUnitTest.java
@@ -26,6 +26,7 @@ import org.w3c.dom.Element;
 import org.w3c.dom.NodeList;
 
 import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
+import org.apache.geode.distributed.internal.InternalLocator;
 import org.apache.geode.internal.jndi.JNDIInvoker;
 import org.apache.geode.management.internal.configuration.domain.Configuration;
 import org.apache.geode.management.internal.configuration.utils.XmlUtils;
@@ -35,7 +36,7 @@ import org.apache.geode.test.junit.categories.GfshTest;
 import org.apache.geode.test.junit.rules.GfshCommandRule;
 import org.apache.geode.test.junit.rules.VMProvider;
 
-@Category({GfshTest.class})
+@Category(GfshTest.class)
 public class DestroyJndiBindingCommandDUnitTest {
 
   private static MemberVM locator, server1, server2;
@@ -60,20 +61,24 @@ public class DestroyJndiBindingCommandDUnitTest {
   }
 
   @Test
-  public void testDestroyJndiBinding() throws Exception {
+  public void testDestroyJndiBinding() {
     // assert that there is a datasource
-    VMProvider.invokeInEveryMember(
-        () -> assertThat(JNDIInvoker.getBindingNamesRecursively(JNDIInvoker.getJNDIContext()))
-            .containsKey("jndi1").containsValue(
-                "org.apache.geode.internal.datasource.GemFireBasicDataSource"));
+    VMProvider
+        .invokeInEveryMember(
+            () -> assertThat(JNDIInvoker.getBindingNamesRecursively(JNDIInvoker.getJNDIContext()))
+                .containsKey("java:jndi1").containsValue(
+                    "org.apache.geode.internal.datasource.GemFireBasicDataSource"),
+            server1, server2);
 
     gfsh.executeAndAssertThat("destroy jndi-binding --name=jndi1").statusIsSuccess()
         .tableHasColumnOnlyWithValues("Member", "server-1", "server-2");
 
     // verify cluster config is updated
     locator.invoke(() -> {
+      InternalLocator internalLocator = ClusterStartupRule.getLocator();
+      assertThat(internalLocator).isNotNull();
       InternalConfigurationPersistenceService ccService =
-          ClusterStartupRule.getLocator().getConfigurationPersistenceService();
+          internalLocator.getConfigurationPersistenceService();
       Configuration configuration = ccService.getConfiguration("cluster");
       Document document = XmlUtils.createDocumentFromXml(configuration.getCacheXmlContent());
       NodeList jndiBindings = document.getElementsByTagName("jndi-binding");
@@ -91,9 +96,9 @@ public class DestroyJndiBindingCommandDUnitTest {
       assertThat(found).isFalse();
     });
 
-    // verify datasource exists
+    // verify datasource does not exists
     VMProvider.invokeInEveryMember(
-        () -> assertThat(JNDIInvoker.getNoOfAvailableDataSources()).isEqualTo(0));
+        () -> assertThat(JNDIInvoker.getNoOfAvailableDataSources()).isEqualTo(0), server1, server2);
 
     // bounce server1 and assert that there is still no datasource received from cluster config
     server1.stop(false);
diff --git a/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/VMProvider.java b/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/VMProvider.java
index 4176a44..9294f6f 100644
--- a/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/VMProvider.java
+++ b/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/VMProvider.java
@@ -19,6 +19,7 @@ import java.io.File;
 import java.util.Arrays;
 
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang.ArrayUtils;
 
 import org.apache.geode.test.dunit.AsyncInvocation;
 import org.apache.geode.test.dunit.SerializableCallableIF;
@@ -27,7 +28,12 @@ import org.apache.geode.test.dunit.VM;
 import org.apache.geode.test.dunit.rules.ClusterStartupRule;
 
 public abstract class VMProvider {
+
   public static void invokeInEveryMember(SerializableRunnableIF runnableIF, VMProvider... members) {
+    if (ArrayUtils.isEmpty(members)) {
+      throw new IllegalArgumentException("Array of members must not be null nor empty.");
+    }
+
     Arrays.stream(members).forEach(member -> member.invoke(runnableIF));
   }