[geode] branch develop updated: GEODE-6420: Cleanup GetRegionsFunctionTest

[kl...@apache.org]

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


The following commit(s) were added to refs/heads/develop by this push:
     new 32e1182  GEODE-6420: Cleanup GetRegionsFunctionTest
32e1182 is described below

commit 32e118289bc6f9b66300b18094bb161226cf8f91
Author: Kirk Lund <kl...@apache.org>
AuthorDate: Fri Feb 15 13:52:14 2019 -0800

    GEODE-6420: Cleanup GetRegionsFunctionTest
    
    * Reimplement GetRegionsFunctionCacheFactoryTest with Mockito.verify.
    * Combine GetRegionsFunctionCacheFactoryTest and GetRegionsFunctionTest.
    * Cleanup GetRegionsFunctionTest including addition of assertions.
    * Give type to GetRegionsFunction and its ResultSender results.
---
 .../internal/cli/functions/GetRegionsFunction.java |   7 +-
 .../GetRegionsFunctionCacheFactoryTest.java        |  84 ----------------
 .../cli/functions/GetRegionsFunctionTest.java      | 107 +++++++++++----------
 3 files changed, 60 insertions(+), 138 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunction.java
index fbdd9a4..12bd19f 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunction.java
@@ -26,12 +26,12 @@ import org.apache.geode.management.internal.cli.domain.RegionInformation;
 /**
  * Function that retrieves regions hosted on every member
  */
-public class GetRegionsFunction implements InternalFunction {
+public class GetRegionsFunction implements InternalFunction<Void> {
 
   private static final long serialVersionUID = 1L;
 
   @Override
-  public void execute(FunctionContext functionContext) {
+  public void execute(FunctionContext<Void> functionContext) {
     try {
       Cache cache = functionContext.getCache();
       Set<Region<?, ?>> regions = cache.rootRegions(); // should never return a null
@@ -45,7 +45,8 @@ public class GetRegionsFunction implements InternalFunction {
           RegionInformation regInfo = new RegionInformation(region, true);
           regionInformationSet.add(regInfo);
         }
-        functionContext.getResultSender().lastResult(regionInformationSet.toArray());
+        functionContext.getResultSender()
+            .lastResult(regionInformationSet.toArray(new RegionInformation[0]));
       }
     } catch (Exception e) {
       functionContext.getResultSender().sendException(e);
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionCacheFactoryTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionCacheFactoryTest.java
deleted file mode 100644
index f9c1a1c..0000000
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionCacheFactoryTest.java
+++ /dev/null
@@ -1,84 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
- * agreements. See the NOTICE file distributed with this work for additional information regarding
- * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance with the License. You may obtain a
- * copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software distributed under the License
- * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
- * or implied. See the License for the specific language governing permissions and limitations under
- * the License.
- */
-
-package org.apache.geode.management.internal.cli.functions;
-
-import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-import java.util.concurrent.atomic.AtomicBoolean;
-
-import org.awaitility.core.ConditionTimeoutException;
-import org.junit.Before;
-import org.junit.Test;
-
-import org.apache.geode.cache.Cache;
-import org.apache.geode.cache.CacheFactory;
-import org.apache.geode.cache.execute.FunctionContext;
-import org.apache.geode.cache.execute.ResultSender;
-
-public class GetRegionsFunctionCacheFactoryTest {
-
-  private enum STATE {
-    INITIAL, BLOCKING, FINISHED
-  };
-
-  private static GetRegionsFunctionCacheFactoryTest.STATE lockingThreadState =
-      GetRegionsFunctionCacheFactoryTest.STATE.INITIAL;
-  private static AtomicBoolean lockAquired = new AtomicBoolean(false);
-  private static AtomicBoolean functionExecuted = new AtomicBoolean(false);
-
-  private GetRegionsFunction getRegionsFunction;
-  private FunctionContext functionContext;
-
-  @Before
-  public void before() {
-    getRegionsFunction = new GetRegionsFunction();
-    functionContext = mock(FunctionContext.class);
-  }
-
-  @Test
-  public void doNotUseCacheFacotryToGetCache() throws Exception {
-    // start a thread that would hold on to the CacheFactory's class lock
-    new Thread(() -> {
-      synchronized (CacheFactory.class) {
-        lockAquired.set(true);
-        lockingThreadState = GetRegionsFunctionCacheFactoryTest.STATE.BLOCKING;
-        try {
-          await().untilTrue(functionExecuted);
-        } catch (ConditionTimeoutException e) {
-          e.printStackTrace();
-          lockingThreadState = GetRegionsFunctionCacheFactoryTest.STATE.FINISHED;
-        }
-      }
-    }).start();
-
-    // wait till the blocking thread aquired the lock on CacheFactory
-    await().untilTrue(lockAquired);
-    when(functionContext.getCache()).thenReturn(mock(Cache.class));
-    when(functionContext.getResultSender()).thenReturn(mock(ResultSender.class));
-
-    // execute a function that would get the cache, make sure that's not waiting on the lock
-    // of CacheFactory
-    getRegionsFunction.execute(functionContext);
-    assertThat(lockingThreadState).isEqualTo(lockingThreadState.BLOCKING);
-
-
-    // this will make the awaitility call in the thread return
-    functionExecuted.set(true);
-  }
-}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionTest.java
index dd49ab6..5e1ca6b 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionTest.java
@@ -14,102 +14,107 @@
  */
 package org.apache.geode.management.internal.cli.functions;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.isNull;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
 
-import java.util.HashSet;
-import java.util.Set;
+import java.util.Collections;
 
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.contrib.java.lang.system.RestoreSystemProperties;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 
-import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.DataPolicy;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionAttributes;
 import org.apache.geode.cache.Scope;
 import org.apache.geode.cache.execute.FunctionContext;
 import org.apache.geode.cache.execute.ResultSender;
-import org.apache.geode.distributed.internal.DistributionConfig;
-import org.apache.geode.distributed.internal.InternalDistributedSystem;
-import org.apache.geode.internal.cache.GemFireCacheImpl;
-import org.apache.geode.internal.cache.LocalRegion;
-import org.apache.geode.internal.cache.control.InternalResourceManager;
+import org.apache.geode.management.internal.cli.domain.RegionInformation;
 
+/**
+ * Characterization unit tests for {@code GetRegionsFunction};
+ */
 public class GetRegionsFunctionTest {
 
-  @Rule
-  public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
-
   @Mock
-  private RegionAttributes regionAttributes;
+  private RegionAttributes<Object, Object> regionAttributes;
 
   @Mock
-  private LocalRegion region;
+  private Region<Object, Object> region;
 
   @Mock
-  private GemFireCacheImpl cache;
+  private FunctionContext<Void> functionContext;
 
   @Mock
-  private InternalResourceManager internalResourceManager;
+  private ResultSender<RegionInformation[]> resultSender;
 
-  @Mock
-  private FunctionContext functionContext;
-
-  private final TestResultSender testResultSender = new TestResultSender();
-  private final Set<Region<?, ?>> regions = new HashSet<>();
-  private final Set<Region<?, ?>> subregions = new HashSet<>();
   private final GetRegionsFunction getRegionsFunction = new GetRegionsFunction();
 
   @Before
-  public void before() {
-    System.setProperty(DistributionConfig.GEMFIRE_PREFIX + "statsDisabled", "true");
-
+  public void setUp() {
     initMocks(this);
 
-    when(cache.getCancelCriterion()).thenReturn(mock(CancelCriterion.class));
-    when(cache.getDistributedSystem()).thenReturn(mock(InternalDistributedSystem.class));
-    when(cache.getResourceManager()).thenReturn(internalResourceManager);
-    when(functionContext.getResultSender()).thenReturn(testResultSender);
-    when(functionContext.getCache()).thenReturn(cache);
+    when(functionContext.<RegionInformation[]>getResultSender()).thenReturn(resultSender);
   }
 
   @Test
-  public void testExecuteWithoutRegions() {
+  public void lastResultIsNullWhenThereAreNoRegions() {
+    Cache cacheFromFunctionContext = mock(Cache.class);
+    when(cacheFromFunctionContext.rootRegions()).thenReturn(Collections.emptySet());
+    when(functionContext.getCache()).thenReturn(cacheFromFunctionContext);
+
     getRegionsFunction.execute(functionContext);
+
+    verify(resultSender).lastResult(isNull());
   }
 
   @Test
-  public void testExecuteWithRegions() {
-    when(cache.rootRegions()).thenReturn(regions);
-    when(region.getFullPath()).thenReturn("/MyRegion");
+  public void lastResultHasRegionInformationForRegion() {
+    String regionNameInCacheFromFunctionContext = "MyRegion";
+    Cache cacheFromFunctionContext = cacheWithOneRootRegion(regionNameInCacheFromFunctionContext);
+    when(functionContext.getCache()).thenReturn(cacheFromFunctionContext);
 
-    when(region.getParentRegion()).thenReturn(null);
-    when(region.subregions(true)).thenReturn(subregions);
-    when(region.subregions(false)).thenReturn(subregions);
-    when(region.getAttributes()).thenReturn(regionAttributes);
-    when(regionAttributes.getDataPolicy()).thenReturn(mock(DataPolicy.class));
-    when(regionAttributes.getScope()).thenReturn(mock(Scope.class));
-    regions.add(region);
     getRegionsFunction.execute(functionContext);
+
+    assertThat(lastResultFrom(resultSender).getValue())
+        .extracting(r -> r.getPath())
+        .containsExactly(regionNameInCacheFromFunctionContext);
   }
 
-  private static class TestResultSender implements ResultSender {
+  @Test
+  public void getsCacheFromFunctionContext() {
+    Cache cacheFromFunctionContext = mock(Cache.class);
+    when(cacheFromFunctionContext.rootRegions()).thenReturn(Collections.emptySet());
+    when(functionContext.getCache()).thenReturn(cacheFromFunctionContext);
 
-    @Override
-    public void lastResult(final Object lastResult) {}
+    getRegionsFunction.execute(functionContext);
 
-    @Override
-    public void sendResult(final Object oneResult) {}
+    verify(functionContext).getCache();
+  }
 
-    @Override
-    public void sendException(final Throwable t) {
-      throw new RuntimeException(t);
-    }
+  private Cache cacheWithOneRootRegion(String regionName) {
+    Cache cache = mock(Cache.class);
+    when(regionAttributes.getDataPolicy()).thenReturn(mock(DataPolicy.class));
+    when(regionAttributes.getScope()).thenReturn(mock(Scope.class));
+    when(region.getFullPath()).thenReturn(Region.SEPARATOR + regionName);
+    when(region.subregions(anyBoolean())).thenReturn(Collections.emptySet());
+    when(region.getAttributes()).thenReturn(regionAttributes);
+    when(cache.rootRegions()).thenReturn(Collections.singleton(region));
+    return cache;
   }
 
+  private ArgumentCaptor<RegionInformation[]> lastResultFrom(
+      ResultSender<RegionInformation[]> resultSender) {
+    ArgumentCaptor<RegionInformation[]> lastResult =
+        ArgumentCaptor.forClass(RegionInformation[].class);
+    verify(resultSender).lastResult(lastResult.capture());
+    return lastResult;
+  }
 }