dsmiley commented on code in PR #4042:
URL: https://github.com/apache/solr/pull/4042#discussion_r2680023477
##########
solr/core/src/test/org/apache/solr/spelling/suggest/TestPhraseSuggestions.java:
##########
@@ -17,19 +17,43 @@
package org.apache.solr.spelling.suggest;
import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4Test;
+import org.apache.solr.SolrXpathTestCase;
+import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.common.params.SpellingParams;
+import org.apache.solr.util.EmbeddedSolrServerTestRule;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
-public class TestPhraseSuggestions extends SolrTestCaseJ4 {
+public class TestPhraseSuggestions extends SolrXpathTestCase {
static final String URI = "/suggest_wfst";
+ @ClassRule
+ public static final EmbeddedSolrServerTestRule solrTestRule = new
EmbeddedSolrServerTestRule();
+
@BeforeClass
public static void beforeClass() throws Exception {
- initCore("solrconfig-phrasesuggest.xml", "schema-phrasesuggest.xml");
- assertQ(req("qt", URI, "q", "", SpellingParams.SPELLCHECK_BUILD, "true"));
+ // This was part of the SolrTestCaseJ4.setupTestCases method and appears
to be needed. Ugh.
+ // Is this a direction we want, this randomness and need in SolrTestCase?
+ SolrTestCaseJ4Test.newRandomConfig();
Review Comment:
Hmm, this is a struggle/tension. On one hand, I want to see SolrTestCase
leaner; not a kitchen sink that STCJ4 became. And I want it to be usable by
3rd parties as well. Including logic for newRandomConfig definitely fails the
latter. Maybe we need another subclass for first-party. Or have STC be able
to recognize 1P from 3P and be graceful... which I think for some specific
methods it may (and/or STCJ4 does. TEST_HOME() is one). Or maybe put certain
functionality as static methods in other classes, like TEST_HOME() -- like a
1PTest.TEST_Home().
##########
solr/test-framework/src/java/org/apache/solr/SolrXPathTestCase.java:
##########
@@ -0,0 +1,96 @@
+package org.apache.solr;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.response.InputStreamResponseParser;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.util.BaseTestHarness;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import javax.xml.xpath.XPathExpressionException;
+import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+
+public class SolrXpathTestCase extends SolrTestCase {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ /**
+ * Executes a query using SolrClient and validates the XML response against
XPath expressions.
+ * This provides a similar interface to assertQ() in SolrTestCaseJ4 but
works with SolrClient.
+ *
+ * @param req the query parameters
+ * @param tests XPath expressions to validate against the response
+ *
+ * @see SolrTestCaseJ4#assertQ(String, SolrQueryRequest, String...)
+ */
+ public void assertQ(QueryRequest req, String... tests) {
+ try {
+
+ // Process request and extract raw response
+ QueryResponse rsp = req.process(getSolrClient());
+ NamedList<Object> rawResponse = rsp.getResponse();
+ InputStream stream = (InputStream) rawResponse.get("stream");
+ String response = new String(stream.readAllBytes(),
StandardCharsets.UTF_8);
+
+ String results = BaseTestHarness.validateXPath(response, tests);
+
+ if (results != null) {
+ String msg =
+ "REQUEST FAILED: xpath="
+ + results
+ + "\n\txml response was: "
+ + response
+ + "\n\trequest was:"
+ + req.getQueryParams();
+
+ fail(msg);
+ }
+ } catch (XPathExpressionException e1) {
+ throw new RuntimeException("XPath is invalid", e1);
+ } catch (Throwable e3) {
+ log.error("REQUEST FAILED: {}", req.getParams(), e3);
+ throw new RuntimeException("Exception during query", e3);
Review Comment:
Not assertionFailure?
##########
solr/test-framework/src/java/org/apache/solr/SolrXPathTestCase.java:
##########
@@ -0,0 +1,96 @@
+package org.apache.solr;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.response.InputStreamResponseParser;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.util.BaseTestHarness;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import javax.xml.xpath.XPathExpressionException;
+import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+
+public class SolrXpathTestCase extends SolrTestCase {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ /**
+ * Executes a query using SolrClient and validates the XML response against
XPath expressions.
+ * This provides a similar interface to assertQ() in SolrTestCaseJ4 but
works with SolrClient.
+ *
+ * @param req the query parameters
+ * @param tests XPath expressions to validate against the response
+ *
+ * @see SolrTestCaseJ4#assertQ(String, SolrQueryRequest, String...)
+ */
+ public void assertQ(QueryRequest req, String... tests) {
+ try {
+
+ // Process request and extract raw response
+ QueryResponse rsp = req.process(getSolrClient());
+ NamedList<Object> rawResponse = rsp.getResponse();
+ InputStream stream = (InputStream) rawResponse.get("stream");
+ String response = new String(stream.readAllBytes(),
StandardCharsets.UTF_8);
Review Comment:
You are assuming an InputStreamResponseParser requesting XML but don't see
the logic for it.
##########
solr/test-framework/src/java/org/apache/solr/SolrXpathTestCase.java:
##########
@@ -0,0 +1,96 @@
+package org.apache.solr;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.response.InputStreamResponseParser;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.util.BaseTestHarness;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import javax.xml.xpath.XPathExpressionException;
+import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+
+public class SolrXpathTestCase extends SolrTestCase {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ /**
+ * Executes a query using SolrClient and validates the XML response against
XPath expressions.
+ * This provides a similar interface to assertQ() in SolrTestCaseJ4 but
works with SolrClient.
+ *
+ * @param req the query parameters
+ * @param tests XPath expressions to validate against the response
+ *
+ * @see SolrTestCaseJ4#assertQ(String, SolrQueryRequest, String...)
+ */
+ public void assertQ(QueryRequest req, String... tests) {
+ try {
+
+ // Process request and extract raw response
+ QueryResponse rsp = req.process(getSolrClient());
+ NamedList<Object> rawResponse = rsp.getResponse();
+ InputStream stream = (InputStream) rawResponse.get("stream");
+ String response = new String(stream.readAllBytes(),
StandardCharsets.UTF_8);
+
+ String results = BaseTestHarness.validateXPath(response, tests);
+
+ if (results != null) {
+ String msg =
+ "REQUEST FAILED: xpath="
+ + results
+ + "\n\txml response was: "
+ + response
+ + "\n\trequest was:"
+ + req.getQueryParams();
+
+ fail(msg);
+ }
+ } catch (XPathExpressionException e1) {
+ throw new RuntimeException("XPath is invalid", e1);
+ } catch (Throwable e3) {
+ log.error("REQUEST FAILED: {}", req.getParams(), e3);
+ throw new RuntimeException("Exception during query", e3);
+ }
+ }
+
+ /**
+ * Generates a QueryRequest
+ *
+ * @see SolrTestCaseJ4#req(String...)
+ */
+ public static QueryRequest req(String... q) {
+ ModifiableSolrParams params = new ModifiableSolrParams();
+
+ if (q.length == 1) {
+ params.set("q", q);
+ }
+ if (q.length % 2 != 0) {
+ throw new RuntimeException(
+ "The length of the string array (query arguments) needs to be even");
+ }
+ for (int i = 0; i < q.length; i += 2) {
+ params.set(q[i], q[i + 1]);
+ }
+
+ params.set("wt", "xml");
+ params.set("indent", params.get("indent", "off"));
Review Comment:
`wt` should be be set since it's the job of the ResponseParser to declare
it's `wt`
##########
solr/test-framework/src/java/org/apache/solr/SolrXpathTestCase.java:
##########
@@ -0,0 +1,96 @@
+package org.apache.solr;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.response.InputStreamResponseParser;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.util.BaseTestHarness;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import javax.xml.xpath.XPathExpressionException;
+import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+
+public class SolrXpathTestCase extends SolrTestCase {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ /**
+ * Executes a query using SolrClient and validates the XML response against
XPath expressions.
+ * This provides a similar interface to assertQ() in SolrTestCaseJ4 but
works with SolrClient.
+ *
+ * @param req the query parameters
+ * @param tests XPath expressions to validate against the response
+ *
+ * @see SolrTestCaseJ4#assertQ(String, SolrQueryRequest, String...)
+ */
+ public void assertQ(QueryRequest req, String... tests) {
+ try {
+
+ // Process request and extract raw response
+ QueryResponse rsp = req.process(getSolrClient());
+ NamedList<Object> rawResponse = rsp.getResponse();
+ InputStream stream = (InputStream) rawResponse.get("stream");
+ String response = new String(stream.readAllBytes(),
StandardCharsets.UTF_8);
+
+ String results = BaseTestHarness.validateXPath(response, tests);
+
+ if (results != null) {
+ String msg =
+ "REQUEST FAILED: xpath="
+ + results
+ + "\n\txml response was: "
+ + response
+ + "\n\trequest was:"
+ + req.getQueryParams();
+
+ fail(msg);
+ }
+ } catch (XPathExpressionException e1) {
+ throw new RuntimeException("XPath is invalid", e1);
+ } catch (Throwable e3) {
+ log.error("REQUEST FAILED: {}", req.getParams(), e3);
+ throw new RuntimeException("Exception during query", e3);
+ }
+ }
+
+ /**
+ * Generates a QueryRequest
+ *
+ * @see SolrTestCaseJ4#req(String...)
+ */
+ public static QueryRequest req(String... q) {
+ ModifiableSolrParams params = new ModifiableSolrParams();
+
+ if (q.length == 1) {
+ params.set("q", q);
+ }
+ if (q.length % 2 != 0) {
+ throw new RuntimeException(
+ "The length of the string array (query arguments) needs to be even");
+ }
+ for (int i = 0; i < q.length; i += 2) {
+ params.set(q[i], q[i + 1]);
Review Comment:
use "add" not "set" since params can repeat
##########
solr/test-framework/src/java/org/apache/solr/SolrXpathTestCase.java:
##########
@@ -0,0 +1,96 @@
+package org.apache.solr;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.response.InputStreamResponseParser;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.util.BaseTestHarness;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import javax.xml.xpath.XPathExpressionException;
+import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+
+public class SolrXpathTestCase extends SolrTestCase {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ /**
+ * Executes a query using SolrClient and validates the XML response against
XPath expressions.
+ * This provides a similar interface to assertQ() in SolrTestCaseJ4 but
works with SolrClient.
+ *
+ * @param req the query parameters
+ * @param tests XPath expressions to validate against the response
+ *
+ * @see SolrTestCaseJ4#assertQ(String, SolrQueryRequest, String...)
+ */
+ public void assertQ(QueryRequest req, String... tests) {
+ try {
+
+ // Process request and extract raw response
+ QueryResponse rsp = req.process(getSolrClient());
+ NamedList<Object> rawResponse = rsp.getResponse();
+ InputStream stream = (InputStream) rawResponse.get("stream");
+ String response = new String(stream.readAllBytes(),
StandardCharsets.UTF_8);
Review Comment:
call this `responseXml` to clearly show it's format
##########
solr/test-framework/src/java/org/apache/solr/SolrXpathTestCase.java:
##########
@@ -0,0 +1,96 @@
+package org.apache.solr;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.response.InputStreamResponseParser;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.util.BaseTestHarness;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import javax.xml.xpath.XPathExpressionException;
+import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+
+public class SolrXpathTestCase extends SolrTestCase {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ /**
+ * Executes a query using SolrClient and validates the XML response against
XPath expressions.
+ * This provides a similar interface to assertQ() in SolrTestCaseJ4 but
works with SolrClient.
+ *
+ * @param req the query parameters
+ * @param tests XPath expressions to validate against the response
+ *
+ * @see SolrTestCaseJ4#assertQ(String, SolrQueryRequest, String...)
+ */
+ public void assertQ(QueryRequest req, String... tests) {
+ try {
+
+ // Process request and extract raw response
+ QueryResponse rsp = req.process(getSolrClient());
+ NamedList<Object> rawResponse = rsp.getResponse();
+ InputStream stream = (InputStream) rawResponse.get("stream");
+ String response = new String(stream.readAllBytes(),
StandardCharsets.UTF_8);
+
+ String results = BaseTestHarness.validateXPath(response, tests);
+
+ if (results != null) {
+ String msg =
+ "REQUEST FAILED: xpath="
+ + results
+ + "\n\txml response was: "
+ + response
+ + "\n\trequest was:"
+ + req.getQueryParams();
+
+ fail(msg);
+ }
+ } catch (XPathExpressionException e1) {
+ throw new RuntimeException("XPath is invalid", e1);
+ } catch (Throwable e3) {
+ log.error("REQUEST FAILED: {}", req.getParams(), e3);
+ throw new RuntimeException("Exception during query", e3);
+ }
+ }
+
+ /**
+ * Generates a QueryRequest
+ *
+ * @see SolrTestCaseJ4#req(String...)
+ */
+ public static QueryRequest req(String... q) {
+ ModifiableSolrParams params = new ModifiableSolrParams();
+
+ if (q.length == 1) {
+ params.set("q", q);
+ }
+ if (q.length % 2 != 0) {
+ throw new RuntimeException(
+ "The length of the string array (query arguments) needs to be even");
+ }
+ for (int i = 0; i < q.length; i += 2) {
+ params.set(q[i], q[i + 1]);
+ }
+
+ params.set("wt", "xml");
+ params.set("indent", params.get("indent", "off"));
+
+ QueryRequest req = new QueryRequest(params);
+ String path = params.get("qt");
+ if (path != null) {
+ req.setPath(path);
+ }
+ req.setResponseParser(new InputStreamResponseParser("xml"));
Review Comment:
IMO this doesn't belong here.
##########
solr/core/src/test/org/apache/solr/spelling/suggest/TestPhraseSuggestions.java:
##########
@@ -17,19 +17,43 @@
package org.apache.solr.spelling.suggest;
import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.SolrTestCaseJ4Test;
+import org.apache.solr.SolrXpathTestCase;
+import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.common.params.SpellingParams;
+import org.apache.solr.util.EmbeddedSolrServerTestRule;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
-public class TestPhraseSuggestions extends SolrTestCaseJ4 {
+public class TestPhraseSuggestions extends SolrXpathTestCase {
static final String URI = "/suggest_wfst";
+ @ClassRule
+ public static final EmbeddedSolrServerTestRule solrTestRule = new
EmbeddedSolrServerTestRule();
+
@BeforeClass
public static void beforeClass() throws Exception {
- initCore("solrconfig-phrasesuggest.xml", "schema-phrasesuggest.xml");
- assertQ(req("qt", URI, "q", "", SpellingParams.SPELLCHECK_BUILD, "true"));
+ // This was part of the SolrTestCaseJ4.setupTestCases method and appears
to be needed. Ugh.
+ // Is this a direction we want, this randomness and need in SolrTestCase?
+ SolrTestCaseJ4Test.newRandomConfig();
+
+ solrTestRule.startSolr(SolrTestCaseJ4.TEST_HOME());
+ solrTestRule
+ .newCollection()
+ .withConfigSet("../collection1")
+ .withConfigFile("conf/solrconfig-phrasesuggest.xml")
Review Comment:
is that leading "conf" really necessary? I hope not. Something to
improve... the presence of those intermediate conf directories is a sad
accident of history.
##########
solr/test-framework/src/java/org/apache/solr/SolrXPathTestCase.java:
##########
@@ -0,0 +1,96 @@
+package org.apache.solr;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.response.InputStreamResponseParser;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.util.BaseTestHarness;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import javax.xml.xpath.XPathExpressionException;
+import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+
+public class SolrXpathTestCase extends SolrTestCase {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ /**
+ * Executes a query using SolrClient and validates the XML response against
XPath expressions.
+ * This provides a similar interface to assertQ() in SolrTestCaseJ4 but
works with SolrClient.
+ *
+ * @param req the query parameters
+ * @param tests XPath expressions to validate against the response
+ *
+ * @see SolrTestCaseJ4#assertQ(String, SolrQueryRequest, String...)
+ */
+ public void assertQ(QueryRequest req, String... tests) {
+ try {
+
+ // Process request and extract raw response
+ QueryResponse rsp = req.process(getSolrClient());
+ NamedList<Object> rawResponse = rsp.getResponse();
+ InputStream stream = (InputStream) rawResponse.get("stream");
+ String response = new String(stream.readAllBytes(),
StandardCharsets.UTF_8);
+
+ String results = BaseTestHarness.validateXPath(response, tests);
+
+ if (results != null) {
+ String msg =
+ "REQUEST FAILED: xpath="
+ + results
+ + "\n\txml response was: "
+ + response
+ + "\n\trequest was:"
+ + req.getQueryParams();
+
+ fail(msg);
+ }
+ } catch (XPathExpressionException e1) {
+ throw new RuntimeException("XPath is invalid", e1);
+ } catch (Throwable e3) {
+ log.error("REQUEST FAILED: {}", req.getParams(), e3);
+ throw new RuntimeException("Exception during query", e3);
+ }
+ }
+
+ /**
+ * Generates a QueryRequest
+ *
+ * @see SolrTestCaseJ4#req(String...)
+ */
+ public static QueryRequest req(String... q) {
+ ModifiableSolrParams params = new ModifiableSolrParams();
+
+ if (q.length == 1) {
+ params.set("q", q);
+ }
+ if (q.length % 2 != 0) {
+ throw new RuntimeException(
+ "The length of the string array (query arguments) needs to be even");
+ }
+ for (int i = 0; i < q.length; i += 2) {
+ params.set(q[i], q[i + 1]);
+ }
+
+ params.set("wt", "xml");
+ params.set("indent", params.get("indent", "off"));
+
+ QueryRequest req = new QueryRequest(params);
+ String path = params.get("qt");
+ if (path != null) {
+ req.setPath(path);
+ }
+ req.setResponseParser(new InputStreamResponseParser("xml"));
+ return req;
+ }
+
+ public SolrClient getSolrClient(){
+ throw new RuntimeException("This method needs to be overridden");
Review Comment:
then make it abstract?!
--
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]
