[ https://issues.apache.org/jira/browse/SOLR-13664?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hoss Man updated SOLR-13664: ---------------------------- Attachment: SOLR-13664.patch Status: Open (was: Open) Testing of the last patch uncovered 3 classes of problems in our existing tests: # tests trying to call {{FileUtils.deleteDirectory(initCoreDataDir);}} after calling {{deleteCore()}} (specifically to work around this bug!) that now get NPE #* Example: TestRecovery # tests that need to be able to write files to {{initCoreDataDir}} *before* calling {{initCore()}} _and_ call {{initCore()}} + {{deleteCore()}} in the intividual test method life cycle #* the patch already ensures {{initCoreDataDir}} is created before the subclass is initialized, so tests that just used a single {{initCore()}} call for all test methods would be fine -- it's only tests that also {{deleteCore()}} in {{@After}} methods that are problems #* Example: QueryElevationComponentTest, SolrCoreCheckLockOnStartupTest * one test that doesn't even use {{initCore()}} -- it builds it's own TestHarness/CoreContainer using {{initCoreDataDir}} directly -- but like #2, calls {{deleteCore()}} in {{@After}} methods (to leverate the common cleanup of the TestHarness) ** Example: SolrMetricsIntegrationTest Based on these classes of problems, I think the best way forward is to update the existing patch to: * make the {{initAndGetDataDir()}} private helper method I introduced in the last patch public and change it to return the {{File}} (not String) and beef up it's javadocs * deprecate {{initCoreDataDir}} and change all existing direct uses of it in our tests to use the helper method This makes fixing the existing test problems trivial: just replace all uses of {{initCoreDataDir}} to {{initAndGetDataDir()}} ... any logic attempting to seed/inspect the dataDir prior to {{initCore()}} will initialize the directory that will be used by the next subsequent {{initCore()}} call. ---- I've attached an updated patch with this changes, but! ... for completeness, I think it's important to also consider about how this will impact any existing third-party tests downstream users may have written that subclass {{SolrTestCaseJ4}: * if they don't refer to {{initCoreDataDir}} directly, then like the existing patch the only change in behavior they should notice is that if their tests call {{deleteCore()}} any subsequent {{initCore()}} calls won't be polluted with the old data. * if they do refer to {{initCoreDataDir}} directly in tests, then that usage _may_ continue to work as is if the usage is only "between" calls to {{initCore()}} and {{deleteCore()}} (ie: to inspect the data dir) * if they attempt to use {{initCoreDataDir}} _after_ calling {{deleteCore()}} (either directly, or indirectly by referencing it before a call to {{initCore()}} in a test lifecycle that involves multiple {{initCore()}} + {{deleteCore()}} pairs) then they will start getting NPEs and will need to change their test to use {{initAndGetDataDir()}} directly. I think the tradeoff of fixing this bug vs the impact on end users is worth making this change: right now the bug can silently affect users w/weird results, but any tests that are impacted adversly by this change will trigger loud NPEs and have an easy fix we can mention in the ugprade notes. > SolrTestCaseJ4.deleteCore() does not delete/clean dataDir > ---------------------------------------------------------- > > Key: SOLR-13664 > URL: https://issues.apache.org/jira/browse/SOLR-13664 > Project: Solr > Issue Type: Bug > Security Level: Public(Default Security Level. Issues are Public) > Reporter: Hoss Man > Assignee: Hoss Man > Priority: Major > Attachments: SOLR-13664.patch, SOLR-13664.patch, SOLR-13664.patch > > > In spite of what it's javadocs say, {{SolrTestCaseJ4.deleteCore()}} does > nothing to delete the dataDir used by the TestHarness > The git history is a bit murky, so i'm not entirely certain when this stoped > working, but I suspect it happened as part of the overall cleanup regarding > test temp dirs and the use of {{LuceneTestCase.createTempDir(...) -> > TestRuleTemporaryFilesCleanup}} > While this is not problematic in many test classes, where a single > {{initCore(...) is called in a {{@BeforeClass}} and the test then re-uses > that SolrCore for all test methods and relies on {{@AfterClass > SolrTestCaseJ4.teardownTestCases()}} to call {{deleteCore()}}, it's > problematic in test classes where {{deleteCore()}} is explicitly called in an > {{@After}} method to ensure a unique core (w/unique dataDir) is used for each > test method. > (there are currently about 61 tests that call {{deleteCore()}} directly) -- This message was sent by Atlassian JIRA (v7.6.14#76016) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org