dsmiley commented on code in PR #4451:
URL: https://github.com/apache/solr/pull/4451#discussion_r3270723648
##########
gradle/testing/randomization.gradle:
##########
@@ -133,6 +133,7 @@ configure(allprojects.findAll {project ->
project.path.startsWith(":solr")}) {
testOptions += [
[propName: 'tests.src.home', value: null, description: "See
SOLR-14023."],
[propName: 'solr.tests.use.numeric.points', value: null,
description: "Point implementation to use (true=numerics, false=trie)."],
+ [propName: 'tests.ssl', value: false, description: "Force SSL on for
all tests that support it (respects @SuppressSSL)."],
Review Comment:
I'll create a separate PR for this build convenience thing. It's also
debatable it should be elevated to be in this list.
##########
solr/core/src/test/org/apache/solr/search/TestSolrJ.java:
##########
@@ -1,188 +0,0 @@
-/*
Review Comment:
wasn't a test, it's really some benchmark-like thing from ancient times that
never should have been merged IMO
##########
solr/test-framework/src/java/org/apache/solr/client/solrj/apache/package-info.java:
##########
@@ -1,20 +0,0 @@
-/*
- * 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.
- */
-
-/** Apache HttpClient based {@link org.apache.solr.client.solrj.SolrClient}
implementations */
-@Deprecated
-package org.apache.solr.client.solrj.apache;
Review Comment:
Removing this whole package :-) bye bye HttpSolrClient,
CloudLegacySolrClient, etc.
##########
solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java:
##########
@@ -2474,67 +2467,22 @@ public static
CollectionAdminRequest.RequestStatusResponse waitForAsyncClusterRe
rsp -> rsp.getRequestStatus().isFinal());
}
- /**
- * A variant of {@link
org.apache.solr.client.solrj.impl.CloudSolrClient.Builder} that will
- * randomize some internal settings.
- */
- public static class RandomizingCloudHttp2SolrClientBuilder extends
CloudSolrClient.Builder {
Review Comment:
basically I renamed the http2 one to RandomizingCloudSolrClientBuilder
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java:
##########
@@ -465,6 +465,9 @@ public abstract static class BuilderBase<B extends
BuilderBase<?, ?>, C extends
*/
@SuppressWarnings("unchecked")
public B withHttpClient(C httpSolrClient) {
+ if (this.baseSolrUrl == null) {
Review Comment:
Belongs in another PR.
##########
solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java:
##########
@@ -579,6 +577,10 @@ public synchronized void stop() throws Exception {
IOUtils.closeQuietly(jettySolrClient);
jettySolrClient = null;
+ if (enableProxy) {
Review Comment:
moved; related to a tricky test fix that's kinda related to the transition
##########
solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java:
##########
@@ -55,6 +56,7 @@
import org.junit.Test;
import org.mockito.Mockito;
[email protected](bugUrl =
"https://issues.apache.org/jira/browse/SOLR-17810";)
Review Comment:
I told the contribute there that this is needed
##########
solr/core/src/test/org/apache/solr/util/TestCborDataFormat.java:
##########
@@ -135,8 +135,10 @@ private byte[] runQuery(String testCollection,
CloudSolrClient client, String wt
request.setResponseParser(new InputStreamResponseParser(wt));
}
result = client.request(request, testCollection);
- InputStream inputStream = (InputStream) result.get("stream");
Review Comment:
This is done in several tests because we *must* close the stream now.
Previously we could get away with not doing so.
##########
solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java:
##########
@@ -166,14 +167,14 @@ public void fixShardCount(int count) {
}
protected volatile JettySolrRunner controlJetty;
- protected final List<SolrClient> clients = Collections.synchronizedList(new
ArrayList<>());
+ protected final List<HttpSolrClient> clients =
Collections.synchronizedList(new ArrayList<>());
Review Comment:
using this more specifc type allows getBaseURL
##########
solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java:
##########
@@ -378,17 +382,22 @@ protected void createServers(int numServers) throws
Exception {
controlJettyDir, useJettyDataDir ? getDataDir(testDir +
"/control/data") : null);
controlJetty.start();
try (CloudSolrClient client = createCloudClient("control_collection")) {
- assertEquals(
- 0,
- CollectionAdminRequest.createCollection("control_collection",
"conf1", 1, 1)
- .setCreateNodeSet(controlJetty.getNodeName())
- .process(client)
- .getStatus());
+ RetryUtil.retryOnException(
Review Comment:
I don't love the retry but it's needed sometimes
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
