SOLR-5914: Cleanup and fix Solr's test cleanup code.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1582038 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -240,6 +240,9 @@ Other Changes
 * SOLR-5903: SolrCore implements Closeable, cut over to using try-with-resources
   where possible. (Alan Woodward)
 
+* SOLR-5914: Cleanup and fix Solr's test cleanup code. 
+ (Mark Miller, Uwe Schindler)
+
 ==================  4.7.1  ==================
 
 Versions of Major Components

solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestContentStreamDataSource.java

@@ -58,7 +58,6 @@ public class TestContentStreamDataSource extends AbstractDataImportHandlerTestCa
   @After
   public void tearDown() throws Exception {
     jetty.stop();
-    instance.tearDown();
     super.tearDown();
   }
 
@@ -174,9 +173,6 @@ public class TestContentStreamDataSource extends AbstractDataImportHandlerTestCa
       FileUtils.copyFile(getFile(CONF_DIR + "dataconfig-contentstream.xml"), f);
     }
 
-    public void tearDown() throws Exception {
-      recurseDelete(homeDir);
-    }
   }
 
   private JettySolrRunner createJetty(SolrInstance instance) throws Exception {

solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestSolrEntityProcessorEndToEnd.java

@@ -19,7 +19,6 @@ package org.apache.solr.handler.dataimport;
 
 import java.io.File;
 import java.io.IOException;
-import java.net.URL;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -27,7 +26,6 @@ import java.util.Map;
 import java.util.Map.Entry;
 
 import org.apache.commons.io.FileUtils;
-import org.apache.http.client.HttpClient;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.embedded.JettySolrRunner;
 import org.apache.solr.client.solrj.impl.HttpSolrServer;

solr/core/src/test/org/apache/solr/cloud/TestMultiCoreConfBootstrap.java

@@ -78,18 +78,6 @@ public class TestMultiCoreConfBootstrap extends SolrTestCaseJ4 {
     
     zkServer.shutdown();
 
-    String skip = System.getProperty("solr.test.leavedatadir");
-    if (null != skip && 0 != skip.trim().length()) {
-      log.info("NOTE: per solr.test.leavedatadir, dataDir will not be removed: " + dataDir.getAbsolutePath());
-    } else {
-      if (!AbstractSolrTestCase.recurseDelete(dataDir)) {
-        log.warn("!!!! WARNING: best effort to remove " + dataDir.getAbsolutePath() + " FAILED !!!!!");
-      }
-      if (!AbstractSolrTestCase.recurseDelete(dataDir2)) {
-        log.warn("!!!! WARNING: best effort to remove " + dataDir.getAbsolutePath() + " FAILED !!!!!");
-      }
-    }
-
     zkServer = null;
     zkDir = null;
 

solr/core/src/test/org/apache/solr/cloud/TestZkChroot.java

@@ -64,17 +64,6 @@ public class TestZkChroot extends SolrTestCaseJ4 {
     
     zkServer.shutdown();
     
-    String skip = System.getProperty("solr.test.leavedatadir");
-    if (null != skip && 0 != skip.trim().length()) {
-      log.info("NOTE: per solr.test.leavedatadir, dataDir will not be removed: "
-          + dataDir.getAbsolutePath());
-    } else {
-      if (!AbstractSolrTestCase.recurseDelete(dataDir)) {
-        log.warn("!!!! WARNING: best effort to remove "
-            + dataDir.getAbsolutePath() + " FAILED !!!!!");
-      }
-    }
-    
     zkServer = null;
     zkDir = null;
     

solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java

@@ -265,7 +265,6 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
       if (!runner.isStopped())  {
         runner.stop();
       }
-      recurseDelete(solrHomeDirectory);
     }
   }
 

solr/core/src/test/org/apache/solr/handler/component/SpellCheckComponentTest.java

@@ -20,9 +20,9 @@ package org.apache.solr.handler.component;
 import java.io.File;
 import java.util.*;
 
-import org.apache.lucene.util.Constants;
 import org.apache.lucene.util.LuceneTestCase.Slow;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4.SuppressTempDirCleanUp;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SpellingParams;
@@ -42,6 +42,7 @@ import org.junit.Test;
  * @since solr 1.3
  */
 @Slow
+@SuppressTempDirCleanUp(bugUrl = "https://issues.apache.org/jira/browse/SOLR-1877 Spellcheck IndexReader leak bug?")
 public class SpellCheckComponentTest extends SolrTestCaseJ4 {
   static String rh = "spellCheckCompRH";
 

solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java

@@ -22,6 +22,7 @@ import java.util.Map;
 
 import org.apache.lucene.analysis.Token;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4.SuppressTempDirCleanUp;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.SpellingParams;
 import org.apache.solr.common.util.NamedList;
@@ -37,6 +38,7 @@ import org.junit.Test;
 /**
  * Simple tests for {@link DirectSolrSpellChecker}
  */
+@SuppressTempDirCleanUp(bugUrl = "https://issues.apache.org/jira/browse/SOLR-1877 Spellcheck IndexReader leak bug?")
 public class DirectSolrSpellCheckerTest extends SolrTestCaseJ4 {
 
   private static SpellingQueryConverter queryConverter;

solr/core/src/test/org/apache/solr/spelling/SpellCheckCollatorTest.java

@@ -23,6 +23,7 @@ import java.util.Set;
 import org.apache.lucene.util.LuceneTestCase.Slow;
 import org.apache.lucene.util.TestUtil;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4.SuppressTempDirCleanUp;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.GroupParams;
@@ -41,6 +42,7 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 
 @Slow
+@SuppressTempDirCleanUp(bugUrl = "https://issues.apache.org/jira/browse/SOLR-1877 Spellcheck IndexReader leak bug?")
 public class SpellCheckCollatorTest extends SolrTestCaseJ4 {
   @BeforeClass
   public static void beforeClass() throws Exception {

solr/core/src/test/org/apache/solr/spelling/WordBreakSolrSpellCheckerTest.java

@@ -24,6 +24,7 @@ import java.util.Map;
 import org.apache.lucene.analysis.MockAnalyzer;
 import org.apache.lucene.analysis.Token;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4.SuppressTempDirCleanUp;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.handler.component.SpellCheckComponent;
@@ -32,6 +33,7 @@ import org.apache.solr.util.RefCounted;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+@SuppressTempDirCleanUp(bugUrl = "https://issues.apache.org/jira/browse/SOLR-1877 Spellcheck IndexReader leak bug?")
 public class WordBreakSolrSpellCheckerTest extends SolrTestCaseJ4 {
   
   @BeforeClass

solr/solrj/src/test/org/apache/solr/client/solrj/MergeIndexesExampleTestBase.java

@@ -88,15 +88,6 @@ public abstract class MergeIndexesExampleTestBase extends SolrExampleTestBase {
   public void tearDown() throws Exception {
     super.tearDown();
 
-    String skip = System.getProperty("solr.test.leavedatadir");
-    if (null != skip && 0 != skip.trim().length()) {
-      System.err.println("NOTE: per solr.test.leavedatadir, dataDir will not be removed: " + dataDir2.getAbsolutePath());
-    } else {
-      if (!recurseDelete(dataDir2)) {
-        System.err.println("!!!! WARNING: best effort to remove " + dataDir2.getAbsolutePath() + " FAILED !!!!!");
-      }
-    }
-
     cores.shutdown();
     
     if (saveProp == null) System.clearProperty("solr.directoryFactory");

solr/solrj/src/test/org/apache/solr/client/solrj/MultiCoreExampleTestBase.java

@@ -73,14 +73,6 @@ public abstract class MultiCoreExampleTestBase extends SolrExampleTestBase
   public void tearDown() throws Exception {
     super.tearDown();
     
-    String skip = System.getProperty("solr.test.leavedatadir");
-    if (null != skip && 0 != skip.trim().length()) {
-      System.err.println("NOTE: per solr.test.leavedatadir, dataDir2 will not be removed: " + dataDir2.getAbsolutePath());
-    } else {
-      if (!recurseDelete(dataDir2)) {
-        System.err.println("!!!! WARNING: best effort to remove " + dataDir2.getAbsolutePath() + " FAILED !!!!!");
-      }
-    }
     if(solrCore0 != null) solrCore0.shutdown();
     if(solrCore1 != null) solrCore1.shutdown();
     if(solrAdmin != null) solrAdmin.shutdown();

solr/solrj/src/test/org/apache/solr/client/solrj/embedded/AbstractEmbeddedSolrServerTestCase.java

@@ -81,17 +81,6 @@ public abstract class AbstractEmbeddedSolrServerTestCase extends LuceneTestCase
 
     deleteAdditionalFiles();
 
-    File dataDir = new File(tempDir,"data");
-    String skip = System.getProperty("solr.test.leavedatadir");
-    if (null != skip && 0 != skip.trim().length()) {
-      log.info("NOTE: per solr.test.leavedatadir, dataDir will not be removed: " + dataDir.getAbsolutePath());
-    } else {
-      //Removing the temporary directory which contains the index (all other files should have been removed before)
-      if (!AbstractSolrTestCase.recurseDelete(tempDir)) {
-        log.warn("!!!! WARNING: best effort to remove " + dataDir.getAbsolutePath() + " FAILED !!!!!");
-      }
-    }
-
     super.tearDown();
   }
 

solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java

@@ -286,9 +286,6 @@ public abstract class BaseDistributedSearchTestCase extends SolrTestCaseJ4 {
   @Override
   public void tearDown() throws Exception {
     destroyServers();
-    if (!AbstractSolrTestCase.recurseDelete(testDir)) {
-      System.err.println("!!!! WARNING: best effort to remove " + testDir.getAbsolutePath() + " FAILED !!!!!");
-    }
     FieldCache.DEFAULT.purgeAllCaches();   // avoid FC insanity
     super.tearDown();
   }

solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java

@@ -30,11 +30,9 @@ import java.lang.annotation.Inherited;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
-import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -136,6 +134,19 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
     public String bugUrl() default "None";
   }
   
+
+  /**
+   * Annotation for test classes to prevent TEMP_DIR cleanup.
+   */
+  @Documented
+  @Inherited
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target(ElementType.TYPE)
+  public @interface SuppressTempDirCleanUp {
+    /** Point to JIRA entry. */
+    public String bugUrl() default "None";
+  }
+  
   // these are meant to be accessed sequentially, but are volatile just to ensure any test
   // thread will read the latest value
   protected static volatile SSLTestConfig sslConfig;
@@ -174,23 +185,40 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
   @AfterClass
   @SuppressWarnings("unused")
   private static void afterClass() throws Exception {
+    try {
       deleteCore();
       resetExceptionIgnores();
       endTrackingSearchers();
       endTrackingZkClients();
       resetFactory();
       coreName = ConfigSolrXmlOld.DEFAULT_DEFAULT_CORE_NAME;
+    } finally {
+      try {
+        if (dataDir != null && dataDir.exists() && !recurseDelete(dataDir)) {
+          String msg = "!!!! WARNING: best effort to remove "
+              + dataDir.getAbsolutePath() + " FAILED !!!!!";
+          if (RandomizedContext.current().getTargetClass()
+              .isAnnotationPresent(SuppressTempDirCleanUp.class)) {
+            System.err.println(msg);
+          } else {
+            fail(msg);
+          }
+        }
+      } finally {
+        dataDir = null;
         System.clearProperty("jetty.testMode");
         System.clearProperty("tests.shardhandler.randomSeed");
         System.clearProperty("enable.update.log");
         System.clearProperty("useCompoundFile");
         System.clearProperty("urlScheme");
         
-    if(isSSLMode()) {
+        if (isSSLMode()) {
           HttpClientUtil.setConfigurer(new HttpClientConfigurer());
         }
         // clean up static
         sslConfig = null;
+      }
+    }
     
     IpTables.unblockAllPorts();
   }
@@ -514,7 +542,7 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
   /**
    * The directory used to story the index managed by the TestHarness h
    */
-  protected static File dataDir;
+  protected static volatile File dataDir;
   
   // hack due to File dataDir
   protected static String hdfsDataDir;
@@ -634,22 +662,11 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
   public static void deleteCore() {
     log.info("###deleteCore" );
     if (h != null) { h.close(); }
-    if (dataDir != null) {
-      String skip = System.getProperty("solr.test.leavedatadir");
-      if (null != skip && 0 != skip.trim().length()) {
-        System.err.println("NOTE: per solr.test.leavedatadir, dataDir will not be removed: " + dataDir.getAbsolutePath());
-      } else {
-        if (!recurseDelete(dataDir)) {
-          System.err.println("!!!! WARNING: best effort to remove " + dataDir.getAbsolutePath() + " FAILED !!!!!");
-        }
-      }
-    }
 
     if (factoryProp == null) {
       System.clearProperty("solr.directoryFactory");
     }
     
-    dataDir = null;
     solrConfig = null;
     h = null;
     lrf = null;

solr/test-framework/src/java/org/apache/solr/util/AbstractSolrTestCase.java

@@ -129,19 +129,6 @@ public abstract class AbstractSolrTestCase extends SolrTestCaseJ4 {
     return TestHarness.deleteByQuery(q, args);
   }
 
-
-  public static boolean recurseDelete(File f) {
-    if (f.isDirectory()) {
-      for (File sub : f.listFiles()) {
-        if (!recurseDelete(sub)) {
-          System.err.println("!!!! WARNING: best effort to remove " + sub.getAbsolutePath() + " FAILED !!!!!");
-          return false;
-        }
-      }
-    }
-    return f.delete();
-  }
-
   /** @see SolrTestCaseJ4#getFile */
   public static File getFile(String name) {
     return SolrTestCaseJ4.getFile(name);