dsmiley commented on code in PR #4699:
URL: https://github.com/apache/solr/pull/4699#discussion_r3709366256
##########
solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerDistribTest.java:
##########
@@ -61,7 +60,7 @@ private LukeResponse requestLuke(ModifiableSolrParams extra)
throws Exception {
handle.put(LukeRequestHandler.KEY_DISTINCT, SKIP);
handle.put(LukeRequestHandler.KEY_TOP_TERMS, SKIP);
handle.put(LukeRequestHandler.KEY_HISTOGRAM, SKIP);
- QueryResponse qr = query(params);
+ QueryResponse qr = query("/admin/luke", params);
Review Comment:
no; don't use QueryRequest/QueryResponse for non-SearchHandler
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -439,9 +439,8 @@ public void testExampleConfig() throws Exception {
String url = solrTestRule.getBaseUrl();
try (SolrClient adminClient = getHttpSolrClient(url)) {
SolrQuery q = new SolrQuery();
- q.set("qt", CommonParams.SYSTEM_INFO_PATH);
- QueryResponse rsp = adminClient.query(q);
+ QueryResponse rsp = new QueryRequest(CommonParams.SYSTEM_INFO_PATH,
q).process(adminClient);
Review Comment:
please no
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -699,10 +698,11 @@ public void testErrorHandling() throws Exception {
SolrClient client = getSolrClient();
SolrQuery query = new SolrQuery();
Review Comment:
SolrQuery should only be used for SearchHandler.
##########
solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java:
##########
@@ -598,18 +600,22 @@ protected void commit() throws Exception {
}
}
+ protected QueryResponse queryRandomShard(ModifiableSolrParams params)
+ throws SolrServerException, IOException {
+ return queryRandomShard(params.get(CommonParams.QT, "/select"), params);
+ }
+
/**
* Queries a random shard; nothing more.
*
* <p>WARNING: tests should generally not call this as it doesn't compare to
the control client
*/
- protected QueryResponse queryRandomShard(ModifiableSolrParams params)
+ protected QueryResponse queryRandomShard(String requestHandler,
ModifiableSolrParams params)
Review Comment:
No *Modifiable*SolrParams please
##########
solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java:
##########
@@ -1665,14 +1665,15 @@ private void createNewCollection(final String
collection) throws InterruptedExce
}
@Override
- protected QueryResponse queryRandomShard(ModifiableSolrParams params)
+ protected QueryResponse queryRandomShard(String requestHandler,
ModifiableSolrParams params)
Review Comment:
maybe out of scope but it's a shame to see ModifiableSolrParams instead of
simply SolrParams
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -699,10 +698,11 @@ public void testErrorHandling() throws Exception {
SolrClient client = getSolrClient();
SolrQuery query = new SolrQuery();
- query.set(CommonParams.QT, "/analysis/field");
query.set(AnalysisParams.FIELD_TYPE, "pint");
query.set(AnalysisParams.FIELD_VALUE, "ignore_exception");
- SolrException ex = expectThrows(SolrException.class, () ->
client.query(query));
+ SolrException ex =
+ expectThrows(
+ SolrException.class, () -> new QueryRequest("/analysis/field",
query).process(client));
Review Comment:
Use GSR
##########
solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java:
##########
@@ -598,18 +600,22 @@ protected void commit() throws Exception {
}
}
+ protected QueryResponse queryRandomShard(ModifiableSolrParams params)
Review Comment:
Please just refer to SolrParams here
##########
solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java:
##########
@@ -1482,13 +1481,12 @@ public void doTestIllegalFilePaths() {
@Test
public void testFileListShouldReportErrorsWhenTheyOccur() throws Exception {
SolrQuery q = new SolrQuery();
- q.add("qt", "/replication")
- .add("wt", "json")
+ q.add("wt", "json")
.add("command", "filelist")
.add(
"generation",
"-2"); // A 'generation' value not matching any commit point
should cause error.
- QueryResponse response = followerClient.query(q);
+ QueryResponse response = new QueryRequest("/replication",
q).process(followerClient);
Review Comment:
sigh...
##########
solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java:
##########
@@ -660,7 +675,9 @@ protected QueryResponse query(boolean setDistribParams,
SolrParams p) throws Exc
int which = r.nextInt(clients.size());
SolrClient client = clients.get(which);
try {
- QueryResponse rsp1 = client.query(new
ModifiableSolrParams(params));
+ QueryResponse rsp1 =
+ new QueryRequest(requestHandler, new
ModifiableSolrParams(params))
Review Comment:
hmm; not sure why we copy
##########
solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java:
##########
@@ -732,11 +732,10 @@ private String getStringOrNull(NamedList<Object> details,
String keyName) {
private NamedList<Object> getFollowerDetails() throws SolrServerException,
IOException {
ModifiableSolrParams params = new ModifiableSolrParams();
- params.set(CommonParams.QT, "/replication");
params.set("command", "details");
params.set("follower", "true");
- QueryResponse response = followerClient.query(params);
+ QueryResponse response = new QueryRequest("/replication",
params).process(followerClient);
Review Comment:
no; use GSR here
--
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]
