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 2016/03/16 18:42:05 UTC

incubator-geode git commit: Change DistributedTestCase to extend JUnit3DistributedTestCase.

Repository: incubator-geode
Updated Branches:
  refs/heads/feature/GEODE-1050 216bbbd6f -> 8a1a162a4


Change DistributedTestCase to extend JUnit3DistributedTestCase.

Fix up javadocs in CacheTestCase classes.


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/8a1a162a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/8a1a162a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/8a1a162a

Branch: refs/heads/feature/GEODE-1050
Commit: 8a1a162a4a53f4cfdf5f0d85d52eaac9861c5331
Parents: 216bbbd
Author: Kirk Lund <kl...@apache.org>
Authored: Wed Mar 16 10:41:15 2016 -0700
Committer: Kirk Lund <kl...@apache.org>
Committed: Wed Mar 16 10:41:15 2016 -0700

----------------------------------------------------------------------
 .../gemfire/test/dunit/DistributedTestCase.java | 451 +------------------
 .../cache/internal/JUnit3CacheTestCase.java     |  36 +-
 .../cache/internal/JUnit4CacheTestCase.java     |  64 ++-
 3 files changed, 52 insertions(+), 499 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/8a1a162a/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/DistributedTestCase.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/DistributedTestCase.java b/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/DistributedTestCase.java
index e94c492..65f403b 100755
--- a/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/DistributedTestCase.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/DistributedTestCase.java
@@ -25,6 +25,7 @@ import java.util.Properties;
 import java.util.Set;
 
 import com.gemstone.gemfire.test.dunit.internal.DistributedTestFixture;
+import com.gemstone.gemfire.test.dunit.internal.JUnit3DistributedTestCase;
 import org.apache.logging.log4j.Logger;
 import org.junit.experimental.categories.Category;
 
@@ -70,25 +71,8 @@ import junit.framework.TestCase;
  */
 @Category(DistributedTest.class)
 @SuppressWarnings("serial")
-public abstract class DistributedTestCase extends TestCase implements DistributedTestFixture, Serializable {
+public abstract class DistributedTestCase extends JUnit3DistributedTestCase implements DistributedTestFixture, Serializable {
   
-  private static final Logger logger = LogService.getLogger();
-  
-  private static final Set<String> testHistory = new LinkedHashSet<String>();
-
-  /** This VM's connection to the distributed system */
-  private static InternalDistributedSystem system;
-  private static Class lastSystemCreatedInTest;
-  private static Properties lastSystemProperties;
-  private static volatile String testMethodName;
-  
-  /** For formatting timing info */
-  private static final DecimalFormat format = new DecimalFormat("###.###");
-
-  private static boolean reconnect = false;
-
-  private static final boolean logPerTest = Boolean.getBoolean("dunitLogPerTest");
-
   /**
    * Creates a new <code>DistributedTestCase</code> test with the given name.
    */
@@ -96,435 +80,4 @@ public abstract class DistributedTestCase extends TestCase implements Distribute
     super(name);
     DUnitLauncher.launchIfNeeded();
   }
-
-  //---------------------------------------------------------------------------
-  // methods for tests
-  //---------------------------------------------------------------------------
-
-  /**
-   * @deprecated Please override {@link #getDistributedSystemProperties()} instead.
-   */
-  @Deprecated
-  public final void setSystem(final Properties props, final DistributedSystem ds) { // TODO: delete
-    system = (InternalDistributedSystem)ds;
-    lastSystemProperties = props;
-    lastSystemCreatedInTest = getClass(); // used to be getDeclaringClass()
-  }
-  
-  /**
-   * Returns this VM's connection to the distributed system.  If necessary, the
-   * connection will be lazily created using the given {@code Properties}.
-   *
-   * <p>Do not override this method. Override {@link #getDistributedSystemProperties()}
-   * instead.
-   *
-   * <p>Note: "final" was removed so that WANTestBase can override this method.
-   * This was part of the xd offheap merge.
-   *
-   * @since 3.0
-   */
-  public final InternalDistributedSystem getSystem(final Properties props) {
-    // Setting the default disk store name is now done in setUp
-    if (system == null) {
-      system = InternalDistributedSystem.getAnyInstance();
-    }
-    if (system == null || !system.isConnected()) {
-      // Figure out our distributed system properties
-      Properties p = DistributedTestUtils.getAllDistributedSystemProperties(props);
-      lastSystemCreatedInTest = getClass(); // used to be getDeclaringClass()
-      if (logPerTest) {
-        String testMethod = getTestMethodName();
-        String testName = lastSystemCreatedInTest.getName() + '-' + testMethod;
-        String oldLogFile = p.getProperty(DistributionConfig.LOG_FILE_NAME);
-        p.put(DistributionConfig.LOG_FILE_NAME, 
-            oldLogFile.replace("system.log", testName+".log"));
-        String oldStatFile = p.getProperty(DistributionConfig.STATISTIC_ARCHIVE_FILE_NAME);
-        p.put(DistributionConfig.STATISTIC_ARCHIVE_FILE_NAME, 
-            oldStatFile.replace("statArchive.gfs", testName+".gfs"));
-      }
-      system = (InternalDistributedSystem)DistributedSystem.connect(p);
-      lastSystemProperties = p;
-    } else {
-      boolean needNewSystem = false;
-      if(!getClass().equals(lastSystemCreatedInTest)) { // used to be getDeclaringClass()
-        Properties newProps = DistributedTestUtils.getAllDistributedSystemProperties(props);
-        needNewSystem = !newProps.equals(lastSystemProperties);
-        if(needNewSystem) {
-          LogWriterUtils.getLogWriter().info(
-              "Test class has changed and the new DS properties are not an exact match. "
-                  + "Forcing DS disconnect. Old props = "
-                  + lastSystemProperties + "new props=" + newProps);
-        }
-      } else {
-        Properties activeProps = system.getProperties();
-        for (Iterator iter = props.entrySet().iterator();
-        iter.hasNext(); ) {
-          Map.Entry entry = (Map.Entry) iter.next();
-          String key = (String) entry.getKey();
-          String value = (String) entry.getValue();
-          if (!value.equals(activeProps.getProperty(key))) {
-            needNewSystem = true;
-            LogWriterUtils.getLogWriter().info("Forcing DS disconnect. For property " + key
-                                + " old value = " + activeProps.getProperty(key)
-                                + " new value = " + value);
-            break;
-          }
-        }
-      }
-      if(needNewSystem) {
-        // the current system does not meet our needs to disconnect and
-        // call recursively to get a new system.
-        LogWriterUtils.getLogWriter().info("Disconnecting from current DS in order to make a new one");
-        disconnectFromDS();
-        getSystem(props);
-      }
-    }
-    return system;
-  }
-
-  /**
-   * Returns this VM's connection to the distributed system.  If necessary, the
-   * connection will be lazily created using the {@code Properties} returned by
-   * {@link #getDistributedSystemProperties()}.
-   *
-   * <p>Do not override this method. Override {@link #getDistributedSystemProperties()}
-   * instead.
-   *
-   * @see #getSystem(Properties)
-   *
-   * @since 3.0
-   */
-  public final InternalDistributedSystem getSystem() {
-    return getSystem(getDistributedSystemProperties());
-  }
-
-  public final InternalDistributedSystem basicGetSystem() {
-    return system;
-  }
-
-  public final void nullSystem() { // TODO: delete
-    system = null;
-  }
-
-  public static final InternalDistributedSystem getSystemStatic() {
-    return system;
-  }
-
-  /**
-   * Returns a loner distributed system that isn't connected to other vms.
-   * 
-   * @since 6.5
-   */
-  public final InternalDistributedSystem getLonerSystem() {
-    Properties props = getDistributedSystemProperties();
-    props.put(DistributionConfig.MCAST_PORT_NAME, "0");
-    props.put(DistributionConfig.LOCATORS_NAME, "");
-    return getSystem(props);
-  }
-  
-  /**
-   * Returns whether or this VM is connected to a {@link DistributedSystem}.
-   */
-  public final boolean isConnectedToDS() {
-    return system != null && system.isConnected();
-  }
-
-  /**
-   * Returns a {@code Properties} object used to configure a connection to a
-   * {@link DistributedSystem}. Unless overridden, this method will return an
-   * empty {@code Properties} object.
-   *
-   * @since 3.0
-   */
-  public Properties getDistributedSystemProperties() {
-    Properties props = new Properties();
-    return props;
-  }
-
-  public static final void disconnectAllFromDS() {
-    disconnectFromDS();
-    Invoke.invokeInEveryVM(()->disconnectFromDS());
-  }
-
-  /**
-   * Disconnects this VM from the distributed system
-   */
-  public static final void disconnectFromDS() {
-    //setTestMethodName(null);
-    GemFireCacheImpl.testCacheXml = null;
-    if (system != null) {
-      system.disconnect();
-      system = null;
-    }
-    
-    for (;;) {
-      DistributedSystem ds = InternalDistributedSystem.getConnectedInstance();
-      if (ds == null) {
-        break;
-      }
-      try {
-        ds.disconnect();
-      } catch (Exception e) {
-        // ignore
-      }
-    }
-    
-    AdminDistributedSystemImpl ads = AdminDistributedSystemImpl.getConnectedInstance();
-    if (ads != null) {// && ads.isConnected()) {
-      ads.disconnect();
-    }
-  }
-
-  //---------------------------------------------------------------------------
-  // name methods
-  //---------------------------------------------------------------------------
-  
-  public static final String getTestMethodName() {
-    return testMethodName;
-  }
-
-  public static final void setTestMethodName(final String testMethodName) { // TODO: delete
-    DistributedTestCase.testMethodName = testMethodName;
-  }
-  
-  /**
-   * Returns a unique name for this test method.  It is based on the
-   * name of the class as well as the name of the method.
-   */
-  public final String getUniqueName() {
-    return getClass().getSimpleName() + "_" + getName();
-  }
-
-  //---------------------------------------------------------------------------
-  // setup methods
-  //---------------------------------------------------------------------------
-
-  /**
-   * Sets up the DistributedTestCase.
-   *
-   * <p> Do not override this method. Override {@link #preSetUp()} with work that
-   * needs to occur before setUp() or override {@link #postSetUp()} with work
-   * that needs to occur after setUp().
-   */
-  @Override
-  public final void setUp() throws Exception {
-    preSetUp();
-    setUpDistributedTestCase();
-    postSetUp();
-  }
-
-  /**
-   * Sets up DistributedTest in controller and remote VMs. This includes the
-   * defining the test name, setting the default disk store name, logging the
-   * test history, and capturing a creation stack for detecting the source of
-   * incompatible DistributedSystem connections.
-   *
-   * <p>Do not override this method.
-   */
-  private final void setUpDistributedTestCase() {
-    final String className = getClass().getCanonicalName();
-    final String methodName = getName();
-    
-    logTestHistory();
-    
-    setUpVM(methodName, getDefaultDiskStoreName(0, -1, className, methodName));
-    
-    for (int hostIndex = 0; hostIndex < Host.getHostCount(); hostIndex++) {
-      Host host = Host.getHost(hostIndex);
-      for (int vmIndex = 0; vmIndex < host.getVMCount(); vmIndex++) {
-        final String vmDefaultDiskStoreName = getDefaultDiskStoreName(hostIndex, vmIndex, className, methodName);
-        host.getVM(vmIndex).invoke(()->setUpVM(methodName, vmDefaultDiskStoreName));
-      }
-    }
-    
-    logTestStart();
-  }
-
-  /**
-   * {@code preSetUp()} is invoked before {@link #setUpDistributedTestCase()}.
-   *
-   * <p>Override this as needed. Default implementation is empty.
-   */
-  public void preSetUp() throws Exception {
-  }
-
-  /**
-   * {@code postSetUp()} is invoked after {@link #setUpDistributedTestCase()}.
-   *
-   * <p>Override this as needed. Default implementation is empty.
-   */
-  public void postSetUp() throws Exception {
-  }
-  
-  private static final String getDefaultDiskStoreName(final int hostIndex, final int vmIndex, final String className, final String methodName) {
-    return "DiskStore-" + String.valueOf(hostIndex) + "-" + String.valueOf(vmIndex) + "-" + className + "." + methodName; // used to be getDeclaringClass()
-  }
-  
-  private static final void setUpVM(final String methodName, final String defaultDiskStoreName) {
-    setTestMethodName(methodName);
-    GemFireCacheImpl.setDefaultDiskStoreName(defaultDiskStoreName);
-    System.setProperty(HoplogConfig.ALLOW_LOCAL_HDFS_PROP, "true");    
-    setUpCreationStackGenerator();
-  }
-
-  private final void logTestStart() {
-    System.out.println("\n\n[setup] START TEST " + getClass().getSimpleName()+"."+testMethodName+"\n\n");
-  }
-  
-  private static final void setUpCreationStackGenerator() {
-    // the following is moved from InternalDistributedSystem to fix #51058
-    InternalDistributedSystem.TEST_CREATION_STACK_GENERATOR.set(
-        new CreationStackGenerator() {
-          @Override
-          public Throwable generateCreationStack(final DistributionConfig config) {
-            final StringBuilder sb = new StringBuilder();
-            final String[] validAttributeNames = config.getAttributeNames();
-            for (int i = 0; i < validAttributeNames.length; i++) {
-              final String attName = validAttributeNames[i];
-              final Object actualAtt = config.getAttributeObject(attName);
-              String actualAttStr = actualAtt.toString();
-              sb.append("  ");
-              sb.append(attName);
-              sb.append("=\"");
-              if (actualAtt.getClass().isArray()) {
-                actualAttStr = InternalDistributedSystem.arrayToString(actualAtt);
-              }
-              sb.append(actualAttStr);
-              sb.append("\"");
-              sb.append("\n");
-            }
-            return new Throwable("Creating distributed system with the following configuration:\n" + sb.toString());
-          }
-        });
-  }
-  
-  /**
-   * Write a message to the log about what tests have ran previously. This
-   * makes it easier to figure out if a previous test may have caused problems
-   */
-  private final void logTestHistory() {
-    String classname = getClass().getSimpleName();
-    testHistory.add(classname);
-    System.out.println("Previously run tests: " + testHistory);
-  }
-
-  //---------------------------------------------------------------------------
-  // teardown methods
-  //---------------------------------------------------------------------------
-
-  /**
-   * Tears down the DistributedTestCase.
-   *
-   * <p>Do not override this method. Override {@link #preTearDown()} with work that
-   * needs to occur before tearDown() or override {@link #postTearDown()} with work
-   * that needs to occur after tearDown().
-   */
-  @Override
-  public final void tearDown() throws Exception {
-    preTearDown();
-    tearDownDistributedTestCase();
-    postTearDown();
-  }
-
-  private final void tearDownDistributedTestCase() throws Exception {
-    Invoke.invokeInEveryVM(()->tearDownCreationStackGenerator());
-    if (logPerTest) {
-      disconnectFromDS();
-      Invoke.invokeInEveryVM(()->disconnectFromDS());
-    }
-    cleanupAllVms();
-  }
-
-  /**
-   * {@code preTearDown()} is invoked before {@link #tearDownDistributedTestCase()}.
-   *
-   * <p>Override this as needed. Default implementation is empty.
-   */
-  public void preTearDown() throws Exception {
-  }
-
-  /**
-   * {@code postTearDown()} is invoked after {@link #tearDownDistributedTestCase()}.
-   *
-   * <p>Override this as needed. Default implementation is empty.
-   */
-  public void postTearDown() throws Exception {
-  }
-  
-  private static final void cleanupAllVms() {
-    tearDownVM();
-    Invoke.invokeInEveryVM(()->tearDownVM());
-    Invoke.invokeInLocator(()->{
-      DistributionMessageObserver.setInstance(null);
-      DistributedTestUtils.unregisterInstantiatorsInThisVM();
-    });
-    DUnitLauncher.closeAndCheckForSuspects();
-  }
-
-  private static final void tearDownVM() {
-    closeCache();
-
-    // keep alphabetized to detect duplicate lines
-    CacheCreation.clearThreadLocals();
-    CacheServerTestUtil.clearCacheReference();
-    ClientProxyMembershipID.system = null;
-    ClientServerTestCase.AUTO_LOAD_BALANCE = false;
-    ClientStatsManager.cleanupForTests();
-    DiskStoreObserver.setInstance(null);
-    DistributedTestUtils.unregisterInstantiatorsInThisVM();
-    DistributionMessageObserver.setInstance(null);
-    GlobalLockingDUnitTest.region_testBug32356 = null;
-    InitialImageOperation.slowImageProcessing = 0;
-    InternalClientMembership.unregisterAllListeners();
-    LogWrapper.close();
-    MultiVMRegionTestCase.CCRegion = null;
-    QueryObserverHolder.reset();
-    QueryTestUtils.setCache(null);
-    RegionTestCase.preSnapshotRegion = null;
-    SocketCreator.resetHostNameCache();
-    SocketCreator.resolve_dns = true;
-    TcpClient.clearStaticData();
-
-    // clear system properties -- keep alphabetized
-    System.clearProperty("gemfire.log-level");
-    System.clearProperty(HoplogConfig.ALLOW_LOCAL_HDFS_PROP);    
-    System.clearProperty("jgroups.resolve_dns");
-    
-    if (InternalDistributedSystem.systemAttemptingReconnect != null) {
-      InternalDistributedSystem.systemAttemptingReconnect.stopReconnecting();
-    }
-    
-    IgnoredException.removeAllExpectedExceptions();
-  }
-
-  private static final void closeCache() { // TODO: this should move to CacheTestCase
-    GemFireCacheImpl cache = GemFireCacheImpl.getInstance();
-    if (cache != null && !cache.isClosed()) {
-      destroyRegions(cache);
-      cache.close();
-    }
-  }
-  
-  protected static final void destroyRegions(final Cache cache) { // TODO: this should move to CacheTestCase
-    if (cache != null && !cache.isClosed()) {
-      // try to destroy the root regions first so that we clean up any persistent files.
-      for (Iterator itr = cache.rootRegions().iterator(); itr.hasNext();) {
-        Region root = (Region)itr.next();
-        String regionFullPath = root == null ? null : root.getFullPath();
-        // for colocated regions you can't locally destroy a partitioned region.
-        if(root.isDestroyed() || root instanceof HARegion || root instanceof PartitionedRegion) {
-          continue;
-        }
-        try {
-          root.localDestroyRegion("teardown");
-        } catch (Throwable t) {
-          logger.error("Failure during tearDown destroyRegions for " + regionFullPath, t);
-        }
-      }
-    }
-  }
-  
-  private static final void tearDownCreationStackGenerator() {
-    InternalDistributedSystem.TEST_CREATION_STACK_GENERATOR.set(InternalDistributedSystem.DEFAULT_CREATION_STACK_GENERATOR);
-  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/8a1a162a/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/cache/internal/JUnit3CacheTestCase.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/cache/internal/JUnit3CacheTestCase.java b/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/cache/internal/JUnit3CacheTestCase.java
index f7c49fa..afbb29d 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/cache/internal/JUnit3CacheTestCase.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/cache/internal/JUnit3CacheTestCase.java
@@ -46,32 +46,32 @@ public abstract class JUnit3CacheTestCase extends JUnit3DistributedTestCase impl
   }
 
   /**
-   * Creates the <code>Cache</code> for this test that is not connected
-   * to other members
+   * Creates the {@code Cache} for this test that is not connected to other
+   * members.
    */
   public final Cache createLonerCache() {
     return delegate.createLonerCache();
   }
 
   /**
-   * Sets this test up with a CacheCreation as its cache.
-   * Any existing cache is closed. Whoever calls this must also call finishCacheXml
+   * Sets this test up with a {@code CacheCreation} as its cache. Any existing
+   * cache is closed. Whoever calls this must also call {@code finishCacheXml}.
    */
   public static final void beginCacheXml() {
     JUnit4CacheTestCase.beginCacheXml();
   }
 
   /**
-   * Finish what beginCacheXml started. It does this be generating a cache.xml
-   * file and then creating a real cache using that cache.xml.
+   * Finish what {@code beginCacheXml} started. It does this be generating a
+   * cache.xml file and then creating a real cache using that cache.xml.
    */
   public final void finishCacheXml(final String name) {
     delegate.finishCacheXml(name);
   }
 
   /**
-   * Finish what beginCacheXml started. It does this be generating a cache.xml
-   * file and then creating a real cache using that cache.xml.
+   * Finish what {@code beginCacheXml} started. It does this be generating a
+   * cache.xml file and then creating a real cache using that cache.xml.
    */
   public final void finishCacheXml(final String name, final boolean useSchema, final String xmlVersion) {
     delegate.finishCacheXml(name, useSchema, xmlVersion);
@@ -97,7 +97,8 @@ public abstract class JUnit3CacheTestCase extends JUnit3DistributedTestCase impl
   }
 
   /**
-   * creates a client cache from the factory if one does not already exist
+   * Creates a client cache from the factory if one does not already exist.
+   *
    * @since 6.5
    */
   public final ClientCache getClientCache(final ClientCacheFactory factory) {
@@ -105,7 +106,8 @@ public abstract class JUnit3CacheTestCase extends JUnit3DistributedTestCase impl
   }
 
   /**
-   * same as {@link #getCache()} but with casting
+   * Invokes {@link #getCache()} and casts the return to
+   * {@code GemFireCacheImpl}.
    */
   public final GemFireCacheImpl getGemfireCache() { // TODO: remove?
     return delegate.getGemfireCache();
@@ -122,12 +124,16 @@ public abstract class JUnit3CacheTestCase extends JUnit3DistributedTestCase impl
     return JUnit4CacheTestCase.basicGetCache();
   }
 
-  /** Close the cache */
+  /**
+   * Close the cache.
+   */
   public static final void closeCache() {
     JUnit4CacheTestCase.closeCache();
   }
 
-  /** Closed the cache in all VMs. */
+  /**
+   * Closed the cache in all VMs.
+   */
   protected final void closeAllCache() {
     delegate.closeAllCache();
   }
@@ -204,10 +210,8 @@ public abstract class JUnit3CacheTestCase extends JUnit3DistributedTestCase impl
   }
 
   /**
-   * Return a set of disk directories
-   * for persistence tests. These directories
-   * will be automatically cleaned up
-   * on test case closure.
+   * Return a set of disk directories for persistence tests. These directories
+   * will be automatically cleaned up during tear down.
    */
   public static final File[] getDiskDirs() {
     return JUnit4CacheTestCase.getDiskDirs();

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/8a1a162a/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/cache/internal/JUnit4CacheTestCase.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/cache/internal/JUnit4CacheTestCase.java b/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/cache/internal/JUnit4CacheTestCase.java
index 8919aa6..d3988ef 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/cache/internal/JUnit4CacheTestCase.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/cache/internal/JUnit4CacheTestCase.java
@@ -136,8 +136,8 @@ public class JUnit4CacheTestCase extends JUnit4DistributedTestCase implements Ca
   }
 
   /**
-   * Creates the {@code Cache} for this test that is not connected
-   * to other members
+   * Creates the {@code Cache} for this test that is not connected to other
+   * members.
    */
   public final Cache createLonerCache() {
     synchronized(JUnit4CacheTestCase.class) {
@@ -161,8 +161,8 @@ public class JUnit4CacheTestCase extends JUnit4DistributedTestCase implements Ca
   }
 
   /**
-   * Sets this test up with a CacheCreation as its cache.
-   * Any existing cache is closed. Whoever calls this must also call finishCacheXml
+   * Sets this test up with a {@code CacheCreation} as its cache. Any existing
+   * cache is closed. Whoever calls this must also call {@code finishCacheXml}.
    */
   public static final synchronized void beginCacheXml() {
     closeCache();
@@ -170,8 +170,8 @@ public class JUnit4CacheTestCase extends JUnit4DistributedTestCase implements Ca
   }
 
   /**
-   * Finish what beginCacheXml started. It does this be generating a cache.xml
-   * file and then creating a real cache using that cache.xml.
+   * Finish what {@code beginCacheXml} started. It does this be generating a
+   * cache.xml file and then creating a real cache using that cache.xml.
    */
   public final void finishCacheXml(final String name) {
     synchronized(JUnit4CacheTestCase.class) {
@@ -194,8 +194,8 @@ public class JUnit4CacheTestCase extends JUnit4DistributedTestCase implements Ca
   }
 
   /**
-   * Finish what beginCacheXml started. It does this be generating a cache.xml
-   * file and then creating a real cache using that cache.xml.
+   * Finish what {@code beginCacheXml} started. It does this be generating a
+   * cache.xml file and then creating a real cache using that cache.xml.
    */
   public final void finishCacheXml(final String name, final boolean useSchema, final String xmlVersion) {
     synchronized(JUnit4CacheTestCase.class) {
@@ -262,7 +262,7 @@ public class JUnit4CacheTestCase extends JUnit4DistributedTestCase implements Ca
   }
 
   /**
-   * creates a client cache from the factory if one does not already exist
+   * Creates a client cache from the factory if one does not already exist.
    *
    * @since 6.5
    */
@@ -295,7 +295,8 @@ public class JUnit4CacheTestCase extends JUnit4DistributedTestCase implements Ca
   }
 
   /**
-   * same as {@link #getCache()} but with casting
+   * Invokes {@link #getCache()} and casts the return to
+   * {@code GemFireCacheImpl}.
    */
   public final GemFireCacheImpl getGemfireCache() { // TODO: remove?
     return (GemFireCacheImpl)getCache();
@@ -312,13 +313,8 @@ public class JUnit4CacheTestCase extends JUnit4DistributedTestCase implements Ca
     return cache;
   }
 
-//  public static final synchronized void disconnectFromDS() {
-//    closeCache();
-//    JUnit4DistributedTestCase.disconnectFromDS();
-//  }
-
   /**
-   * Close the cache
+   * Close the cache.
    */
   public static final synchronized void closeCache() {
     // Workaround for that fact that some classes are now extending
@@ -372,7 +368,7 @@ public class JUnit4CacheTestCase extends JUnit4DistributedTestCase implements Ca
     postTearDownCacheTestCase();
   }
 
-  public final void tearDownCacheTestCase() { // TODO: make private
+  private final void tearDownCacheTestCase() {
     remoteTearDown();
     Invoke.invokeInEveryVM(()->remoteTearDown());
   }
@@ -414,7 +410,7 @@ public class JUnit4CacheTestCase extends JUnit4DistributedTestCase implements Ca
   }
 
   /**
-   * Returns a region with the given name and attributes
+   * Returns a region with the given name and attributes.
    */
   public final Region createRegion(final String name, final RegionAttributes attributes) throws CacheException {
     return createRegion(name, "root", attributes);
@@ -524,22 +520,6 @@ public class JUnit4CacheTestCase extends JUnit4DistributedTestCase implements Ca
     return removeExceptionTag;
   }
 
-  /**
-   * Used to generate a cache.xml. Basically just a CacheCreation with a few
-   * more methods implemented.
-   */
-  private static final class TestCacheCreation extends CacheCreation {
-    private boolean closed = false;
-    @Override
-    public void close() {
-      this.closed = true;
-    }
-    @Override
-    public boolean isClosed() {
-      return this.closed;
-    }
-  }
-
   public static final File getDiskDir() {
     int vmNum = VM.getCurrentVMNum();
     File dir = new File("diskDir", "disk" + String.valueOf(vmNum)).getAbsoluteFile();
@@ -568,4 +548,20 @@ public class JUnit4CacheTestCase extends JUnit4DistributedTestCase implements Ca
       FileUtil.delete(file);
     }
   }
+
+  /**
+   * Used to generate a cache.xml. Basically just a {@code CacheCreation} with
+   * a few more methods implemented.
+   */
+  private static final class TestCacheCreation extends CacheCreation {
+    private boolean closed = false;
+    @Override
+    public void close() {
+      this.closed = true;
+    }
+    @Override
+    public boolean isClosed() {
+      return this.closed;
+    }
+  }
 }