dsmiley commented on code in PR #3197:
URL: https://github.com/apache/solr/pull/3197#discussion_r1962529603
##########
solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java:
##########
@@ -1027,6 +1028,64 @@ public static String assertJQ(SolrQueryRequest req,
double delta, String... test
}
}
+ public static <T> String assertThatJQ(SolrQueryRequest req, Matcher<T> test)
throws Exception {
+ return assertThatJQ(req, "", test);
+ }
+
+ /**
+ * Validates a query completes and, using JSON deserialization, returns an
object that passes the
+ * given Matcher test.
+ *
+ * <p>Please use this with care: this makes it easy to match complete
structures, but doing so can
+ * result in fragile tests if you are matching more than what you want to
test.
+ *
+ * @param req Solr request to execute
+ * @param message Failure message for test
+ * @param test Matcher for the given object returned from deserializing the
response
+ * @return The request response as a JSON String if the test matcher passes
+ */
+ @SuppressWarnings("unchecked")
+ public static <T> String assertThatJQ(SolrQueryRequest req, String message,
Matcher<T> test)
Review Comment:
You are adding more methods that uses the almost but not quite yet
deprecated TestHarness. Can you please instead consider changing the approach
of this method to look like `org.apache.solr.SolrTestCaseHS#assertJQ` which
takes a SolrClient? Let's say you agree to do that. My next thought is...
should SolrTestCase have such extensive utility methods or should they live on
a helper class (a new one I guess). I lean to the latter but I haven't thought
about that much yet.
##########
solr/test-framework/src/java/org/apache/solr/util/SolrMatchers.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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 java.util.List;
+import org.hamcrest.Description;
+import org.hamcrest.Matcher;
+import org.hamcrest.TypeSafeDiagnosingMatcher;
+
+public class SolrMatchers {
Review Comment:
add a bit of javadoc please. Like reference "Hamcrest", and javadoc on the
subListMatches method.
##########
solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java:
##########
@@ -1027,6 +1028,64 @@ public static String assertJQ(SolrQueryRequest req,
double delta, String... test
}
}
+ public static <T> String assertThatJQ(SolrQueryRequest req, Matcher<T> test)
throws Exception {
+ return assertThatJQ(req, "", test);
+ }
+
+ /**
+ * Validates a query completes and, using JSON deserialization, returns an
object that passes the
+ * given Matcher test.
+ *
+ * <p>Please use this with care: this makes it easy to match complete
structures, but doing so can
+ * result in fragile tests if you are matching more than what you want to
test.
+ *
+ * @param req Solr request to execute
+ * @param message Failure message for test
+ * @param test Matcher for the given object returned from deserializing the
response
+ * @return The request response as a JSON String if the test matcher passes
+ */
+ @SuppressWarnings("unchecked")
+ public static <T> String assertThatJQ(SolrQueryRequest req, String message,
Matcher<T> test)
+ throws Exception {
+ SolrParams params = null;
+ try {
+ params = req.getParams();
Review Comment:
you could get & set at the declaration rather than split across the `try`.
Could then be final.
--
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
