You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/09/09 18:25:24 UTC

[GitHub] [solr] risdenk opened a new pull request, #1003: SOLR-16408: TestHdfsUpdateLog, TestRecoveryHdfs, and TestRandomCollapseQParserPlugin try to deleteCore twice

risdenk opened a new pull request, #1003:
URL: https://github.com/apache/solr/pull/1003

   https://issues.apache.org/jira/browse/SOLR-16408
   
   This uses `SolrTestCaseJ4` AfterClass to cleanup and call `deleteCore` instead of trying to do this in `AfterClass` in each of the tests themselves.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] risdenk merged pull request #1003: SOLR-16408: TestHdfsUpdateLog, TestRecoveryHdfs, and TestRandomCollapseQParserPlugin call SolrTestCaseJ4.teardownTestCases instead of deleteCore

Posted by GitBox <gi...@apache.org>.
risdenk merged PR #1003:
URL: https://github.com/apache/solr/pull/1003


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] risdenk commented on a diff in pull request #1003: SOLR-16408: TestHdfsUpdateLog, TestRecoveryHdfs, and TestRandomCollapseQParserPlugin try to deleteCore twice

Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1003:
URL: https://github.com/apache/solr/pull/1003#discussion_r972266388


##########
solr/modules/hdfs/src/test/org/apache/solr/hdfs/search/TestRecoveryHdfs.java:
##########
@@ -109,17 +109,13 @@ public static void afterClass() throws Exception {
     IOUtils.closeQuietly(fs);
     fs = null;
     try {
-      deleteCore();

Review Comment:
   I wonder if it actually makes more sense to call the super afterclass method here instead - since that would clean stuff up in reverse order potentially. At least in the case of HDFS it makes sense to shutdown HDFS after shutting down solr I think...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] risdenk commented on a diff in pull request #1003: SOLR-16408: TestHdfsUpdateLog, TestRecoveryHdfs, and TestRandomCollapseQParserPlugin try to deleteCore twice

Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1003:
URL: https://github.com/apache/solr/pull/1003#discussion_r974562990


##########
solr/modules/hdfs/src/test/org/apache/solr/hdfs/search/TestRecoveryHdfs.java:
##########
@@ -109,17 +109,13 @@ public static void afterClass() throws Exception {
     IOUtils.closeQuietly(fs);
     fs = null;
     try {
-      deleteCore();

Review Comment:
   I did this in 74d2c561532cc6c335d3aa5cc8416511f0f51a48 and think its a lot cleaner.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] risdenk commented on pull request #1003: SOLR-16408: TestHdfsUpdateLog, TestRecoveryHdfs, and TestRandomCollapseQParserPlugin try to deleteCore twice

Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1003:
URL: https://github.com/apache/solr/pull/1003#issuecomment-1242436026

   No idea. There are a bunch of tests that od this in before/after test, but only a few in beforeclass/afterclass. The changes in [SOLR-16187](https://issues.apache.org/jira/browse/SOLR-16187) at least gave a clue as to why this was hanging.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] cpoerschke commented on a diff in pull request #1003: SOLR-16408: TestHdfsUpdateLog, TestRecoveryHdfs, and TestRandomCollapseQParserPlugin try to deleteCore twice

Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #1003:
URL: https://github.com/apache/solr/pull/1003#discussion_r973005178


##########
solr/core/src/test/org/apache/solr/search/TestRandomCollapseQParserPlugin.java:
##########
@@ -82,7 +82,6 @@ public static void buildIndexAndClient() throws Exception {
 
   @AfterClass
   public static void cleanupStatics() {

Review Comment:
   How about marking `NULL_POLICIES` final to make it clearer that it is static but not being cleaned up here because it's never changed in the `BeforeClass` code?



##########
solr/modules/hdfs/src/test/org/apache/solr/hdfs/search/TestRecoveryHdfs.java:
##########
@@ -109,17 +109,13 @@ public static void afterClass() throws Exception {
     IOUtils.closeQuietly(fs);
     fs = null;
     try {
-      deleteCore();
+      HdfsTestUtil.teardownClass(dfsCluster);
     } finally {
-      try {
-        HdfsTestUtil.teardownClass(dfsCluster);
-      } finally {
-        dfsCluster = null;
-        hdfsUri = null;
-        System.clearProperty("solr.ulog.dir");
-        System.clearProperty("test.build.data");
-        System.clearProperty("test.cache.data");
-      }
+      dfsCluster = null;
+      hdfsUri = null;
+      System.clearProperty("solr.ulog.dir");
+      System.clearProperty("test.build.data");
+      System.clearProperty("test.cache.data");

Review Comment:
   Looks like `HdfsTestUtil.teardownClass` already does this.
   ```suggestion
   ```



##########
solr/modules/hdfs/src/test/org/apache/solr/hdfs/update/TestHdfsUpdateLog.java:
##########
@@ -72,17 +72,13 @@ public static void afterClass() throws Exception {
     IOUtils.closeQuietly(fs);
     fs = null;
     try {
-      deleteCore();
+      HdfsTestUtil.teardownClass(dfsCluster);
     } finally {
-      try {
-        HdfsTestUtil.teardownClass(dfsCluster);
-      } finally {
-        dfsCluster = null;
-        hdfsUri = null;
-        System.clearProperty("solr.ulog.dir");
-        System.clearProperty("test.build.data");
-        System.clearProperty("test.cache.data");
-      }
+      dfsCluster = null;
+      hdfsUri = null;
+      System.clearProperty("solr.ulog.dir");
+      System.clearProperty("test.build.data");
+      System.clearProperty("test.cache.data");

Review Comment:
   Looks like `HdfsTestUtil.teardownClass` already does this.
   ```suggestion
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org