diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 13907c3d1c1..58b9b04e3a5 100644 --- a/solr/CHANGES.txt +++ b/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 diff --git a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestContentStreamDataSource.java b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestContentStreamDataSource.java index a209fa8e374..396ce64cbef 100644 --- a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestContentStreamDataSource.java +++ b/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 { diff --git a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestSolrEntityProcessorEndToEnd.java b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestSolrEntityProcessorEndToEnd.java index 5aa55e71241..d9d010e8922 100644 --- a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestSolrEntityProcessorEndToEnd.java +++ b/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; diff --git a/solr/core/src/test/org/apache/solr/cloud/TestMultiCoreConfBootstrap.java b/solr/core/src/test/org/apache/solr/cloud/TestMultiCoreConfBootstrap.java index 0e977317ee1..09fe71724f5 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestMultiCoreConfBootstrap.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestMultiCoreConfBootstrap.java @@ -77,18 +77,6 @@ public class TestMultiCoreConfBootstrap extends SolrTestCaseJ4 { cores.shutdown(); 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; diff --git a/solr/core/src/test/org/apache/solr/cloud/TestZkChroot.java b/solr/core/src/test/org/apache/solr/cloud/TestZkChroot.java index b025d03e906..0820cc9d50b 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestZkChroot.java +++ b/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; diff --git a/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java index 5954e773ab1..bb9001e9773 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java +++ b/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); } } diff --git a/solr/core/src/test/org/apache/solr/handler/component/SpellCheckComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/SpellCheckComponentTest.java index 81280606189..0f675ea4f40 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/SpellCheckComponentTest.java +++ b/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"; diff --git a/solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java b/solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java index 044778ccefd..a208f7b11af 100644 --- a/solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java +++ b/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; diff --git a/solr/core/src/test/org/apache/solr/spelling/SpellCheckCollatorTest.java b/solr/core/src/test/org/apache/solr/spelling/SpellCheckCollatorTest.java index 5e71e14d95d..8ebebf0fa10 100644 --- a/solr/core/src/test/org/apache/solr/spelling/SpellCheckCollatorTest.java +++ b/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 { diff --git a/solr/core/src/test/org/apache/solr/spelling/WordBreakSolrSpellCheckerTest.java b/solr/core/src/test/org/apache/solr/spelling/WordBreakSolrSpellCheckerTest.java index dfa4a6c0bc7..f3578307e67 100644 --- a/solr/core/src/test/org/apache/solr/spelling/WordBreakSolrSpellCheckerTest.java +++ b/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 diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/MergeIndexesExampleTestBase.java b/solr/solrj/src/test/org/apache/solr/client/solrj/MergeIndexesExampleTestBase.java index 369bcdbb3b9..42f83a1d932 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/MergeIndexesExampleTestBase.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/MergeIndexesExampleTestBase.java @@ -87,15 +87,6 @@ public abstract class MergeIndexesExampleTestBase extends SolrExampleTestBase { @Override 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(); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/MultiCoreExampleTestBase.java b/solr/solrj/src/test/org/apache/solr/client/solrj/MultiCoreExampleTestBase.java index 56fec2821ce..723bdb2f093 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/MultiCoreExampleTestBase.java +++ b/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(); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/AbstractEmbeddedSolrServerTestCase.java b/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/AbstractEmbeddedSolrServerTestCase.java index a42e79e80ba..0525f373259 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/AbstractEmbeddedSolrServerTestCase.java +++ b/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(); } diff --git a/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java b/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java index 11f88831e7b..9aa669af64e 100644 --- a/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java +++ b/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(); } diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java index 862b4c46dab..10318dbaf52 100644 --- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java +++ b/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 { - deleteCore(); - resetExceptionIgnores(); - endTrackingSearchers(); - endTrackingZkClients(); - resetFactory(); - coreName = ConfigSolrXmlOld.DEFAULT_DEFAULT_CORE_NAME; - System.clearProperty("jetty.testMode"); - System.clearProperty("tests.shardhandler.randomSeed"); - System.clearProperty("enable.update.log"); - System.clearProperty("useCompoundFile"); - System.clearProperty("urlScheme"); - - if(isSSLMode()) { - HttpClientUtil.setConfigurer(new HttpClientConfigurer()); + 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()) { + HttpClientUtil.setConfigurer(new HttpClientConfigurer()); + } + // clean up static + sslConfig = null; + } } - // 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; diff --git a/solr/test-framework/src/java/org/apache/solr/util/AbstractSolrTestCase.java b/solr/test-framework/src/java/org/apache/solr/util/AbstractSolrTestCase.java index b2bb7d3b744..9e2dca0676c 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/AbstractSolrTestCase.java +++ b/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);