dsmiley commented on code in PR #1374:
URL: https://github.com/apache/solr/pull/1374#discussion_r1117988442
##########
solr/test-framework/src/java/org/apache/solr/util/SolrClientTestRule.java:
##########
@@ -135,14 +135,16 @@ protected void create(NewCollectionBuilder b) throws
SolrServerException, IOExce
req.process(getAdminClient());
}
- /** Provides a SolrClient instance for administration actions */
+ /**
+ * Provides a SolrClient instance for administration actions. The caller
doesn't need to close it
+ */
public abstract SolrClient getAdminClient();
- /** Provides a SolrClient instance for collection1 */
+ /** Provides a SolrClient instance for collection1. The caller doesn't need
to close it */
public SolrClient getSolrClient() {
return getSolrClient("collection1");
}
-
+ /** Provides a SolrClient instance for provided name. The caller doesn't
need to close it */
Review Comment:
"For provided name"? And what is this "name"?
##########
solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java:
##########
@@ -65,11 +66,8 @@ public void before() throws Exception {
@After
public void after() throws Exception {
- if (jetty != null) {
- jetty.stop();
- jetty = null;
- }
- client = null;
+ solrClientTestRule.reset();
+ SolrJettyTestRule.client = null;
Review Comment:
Looking at the state of SolrJettyTestRule right now, it's already *not*
static and is public -- which is good. Your PR doesn't compile. I suggest
never pushing code that doesn't even compile.
##########
solr/test-framework/src/java/org/apache/solr/util/SolrJettyTestRule.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.util;
+
+import static org.apache.solr.SolrTestCaseJ4.DEFAULT_TEST_CORENAME;
+import static org.apache.solr.SolrTestCaseJ4.getHttpSolrClient;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.embedded.JettyConfig;
+import org.apache.solr.embedded.JettySolrRunner;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SolrJettyTestRule extends SolrClientTestRule {
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private SolrClient client = null;
+ private JettySolrRunner jetty;
+ private SolrClient adminClient = null;
+
+ private ConcurrentHashMap<String, SolrClient> clients =
+ new ConcurrentHashMap<>(); // TODO close them
Review Comment:
That's a critical TODO by the way, and an easy one to get done.
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
