Skip to content

SOLR-18332: More 'qt' removal, rd 4 #4731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gerlowskija wants to merge 7 commits into
base: main
Choose a base branch
from
Open

SOLR-18332: More 'qt' removal, rd 4 #4731

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f1f2aef
Add rh-aware overrides in SolrTestCaseJ4 methods
gerlowskija Aug 7, 2026
7e1db1e
SOLR-18332: More qt-removal from tests, rd 3
gerlowskija Aug 8, 2026
b5e6629
Replace existing pattern with reqWithPath
gerlowskija Aug 10, 2026
75838e7
Replace qt usage in suggest+spellcheck tests
gerlowskija Aug 9, 2026
4033f8c
Merge branch 'main' into qt-removal-rd-4
gerlowskija Aug 13, 2026
c7073e8
Convert RTG/recovery/versions tests to reqWithPath
gerlowskija Aug 13, 2026
c16e85e
Convert update-processor, schema, and misc handler tests to reqWithPath
gerlowskija Aug 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 12 additions & 25 deletions solr/core/src/test/org/apache/solr/DisMaxRequestHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public void doTestSomeStuff(final String qt) {

assertQ(
"multi qf",
req("q", "cool", "qt", qt, "qf", "subject", "qf", "features_t"),
reqWithPath(qt, "q", "cool", "qf", "subject", "qf", "features_t"),
"//*[@numFound='3']");

assertQ(
Expand All @@ -137,19 +137,18 @@ public void doTestSomeStuff(final String qt) {

assertQ(
"boost query",
req("q", "cool stuff", "qt", qt, "bq", "subject:hell^400"),
reqWithPath(qt, "q", "cool stuff", "bq", "subject:hell^400"),
"//*[@numFound='3']",
"//result/doc[1]/str[@name='id'][.='666']",
"//result/doc[2]/str[@name='id'][.='42']",
"//result/doc[3]/str[@name='id'][.='8675309']");

assertQ(
"multi boost query",
req(
reqWithPath(
qt,

@epugh epugh Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the parameter qt can be renamed to path? Just from readnig the diff, I think this is now a variable holding the specific path?

@gerlowskija gerlowskija Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good thought - if I do eventually succeed in getting 'qt' removed then variable names like this won't make any sense.

OTOH, renaming this variable (and others like it) will make the diff much less homogeneous. Right now every line in this diff is a req -> reqWithPath swap. But if you as a reviewer don't mind adding that noise to the diff (?), then I don't either I guess

@epugh epugh Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind noise... have you seen my pr's!???? Ooh, let me go fix that random typo ;-)

@epugh epugh Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but seriously, for me success means the letters q & t never show up in our code base next to each other.

"q",
"cool stuff",
"qt",
qt,
"bq",
"subject:hell^400",
"bq",
Expand All @@ -172,29 +171,19 @@ public void doTestSomeStuff(final String qt) {

assertQ(
"relying on ALTQ from config",
req(
"qt", qt,
"fq", "id:666",
"facet", "false"),
reqWithPath(qt, "fq", "id:666", "facet", "false"),
"//*[@numFound='1']");

assertQ(
"explicit ALTQ",
req(
"qt", qt,
"q.alt", "id:9999",
"fq", "id:666",
"facet", "false"),
reqWithPath(qt, "q.alt", "id:9999", "fq", "id:666", "facet", "false"),
"//*[@numFound='0']");

assertQ(
"no query slop == no match", req("qt", qt, "q", "\"cool chick\""), "//*[@numFound='0']");
"no query slop == no match", reqWithPath(qt, "q", "\"cool chick\""), "//*[@numFound='0']");
assertQ(
"query slop == match",
req(
"qt", qt,
"qs", "2",
"q", "\"cool chick\""),
reqWithPath(qt, "qs", "2", "q", "\"cool chick\""),
"//*[@numFound='1']");
}

Expand Down Expand Up @@ -228,11 +217,10 @@ public void testExtraBlankBQ() throws Exception {
Pattern p_bool = Pattern.compile("\\(subject:hell\\s*subject:cool\\)");
String resp =
h.query(
req(
reqWithPath(
"/dismax",
"q",
"cool stuff",
"qt",
"/dismax",
"bq",
"subject:hell OR subject:cool",
CommonParams.DEBUG_QUERY,
Expand All @@ -242,11 +230,10 @@ public void testExtraBlankBQ() throws Exception {

resp =
h.query(
req(
reqWithPath(
"/dismax",
"q",
"cool stuff",
"qt",
"/dismax",
"bq",
"subject:hell OR subject:cool",
"bq",
Expand Down
18 changes: 9 additions & 9 deletions solr/core/src/test/org/apache/solr/MinimalSchemaTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,12 @@ public void testSimpleQueries() {
@Test
public void testLuke() {

assertQ("basic luke request failed", req("qt", "/admin/luke"), "//int[@name='numDocs'][.='2']");
assertQ(
"basic luke request failed", reqWithPath("/admin/luke"), "//int[@name='numDocs'][.='2']");

assertQ(
"luke show schema failed",
req(
"qt", "/admin/luke",
"show", "schema"),
reqWithPath("/admin/luke", "show", "schema"),
"//int[@name='numDocs'][.='2']",
"//null[@name='uniqueKeyField']");
}
Expand Down Expand Up @@ -111,11 +110,12 @@ public void testAllConfiguredHandlers() {

assertQ(
"failure w/handler: '" + handler + "'",
req(
"qt", handler,
// this should be fairly innocuous for any type of query
"q", "foo:bar",
"omitHeader", "false"),
reqWithPath(
handler, // this should be fairly innocuous for any type of query

@epugh epugh Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wonder if using the Solr specific term handler still makes sense, or we should just say path?

@gerlowskija gerlowskija Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love the use of "handler" as it's very v1 specific and a bit vestigial. But I think there is a bit of a difference in terms of how we use it colloquially: I think most folks use "Handler" to refer specifically to the last path-segment (i.e. /export) and "path" to talk about the whole path (i.e. /solr/collection1/export).

It'd be nice if we had a v1/v2 agnostic name for "that part of the path that comes after the collection/core name", but right now I think "handler" is kindof serving that role.

"q",
"foo:bar",
"omitHeader",
"false"),
"//lst[@name='responseHeader']");
} catch (Exception e) {
throw new RuntimeException("exception w/handler: '" + handler + "'", e);
Expand Down
2 changes: 1 addition & 1 deletion solr/core/src/test/org/apache/solr/core/AlternateDirectoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public static void beforeClass() throws Exception {
}

public void testAltDirectoryUsed() {
assertQ(req("q", "*:*", "qt", "/select"));
assertQ(reqWithPath("/select", "q", "*:*"));
assertTrue(TestFSDirectoryFactory.openCalled);
assertTrue(TestIndexReaderFactory.newReaderCalled);
}
Expand Down
4 changes: 2 additions & 2 deletions solr/core/src/test/org/apache/solr/core/RequestHandlersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ public void testLazyLoading() {
// But it should behave just like the 'defaults' request handler above
assertQ(
"lazy handler returns fewer matches",
req("q", "id:[42 TO 47]", "qt", "/lazy"),
reqWithPath("/lazy", "q", "id:[42 TO 47]"),
"*[count(//doc)=4]");

assertQ(
"lazy handler includes highlighting",
req("q", "name:Zapp OR title:General", "qt", "/lazy"),
reqWithPath("/lazy", "q", "name:Zapp OR title:General"),
"//lst[@name='highlighting']");
}

Expand Down
32 changes: 16 additions & 16 deletions solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,13 @@ private void assertHistoBucket(int slot, int in) {
public void testLuke() {

// test that Luke can handle all the field types
assertQ(req("qt", "/admin/luke", "id", "SOLR1000"));
assertQ(reqWithPath("/admin/luke", "id", "SOLR1000"));

final int numFlags = EnumSet.allOf(FieldFlag.class).size();

assertQ(
"Not all flags (" + numFlags + ") mentioned in info->key",
req("qt", "/admin/luke"),
reqWithPath("/admin/luke"),
numFlags + "=count(//lst[@name='info']/lst[@name='key']/str)");

// code should be the same for all fields, but just in case do several
Expand All @@ -129,7 +129,7 @@ public void testLuke() {
final String xp = getFieldXPathPrefix(f);
assertQ(
"Not as many schema flags as expected (" + numFlags + ") for " + f,
req("qt", "/admin/luke", "fl", f),
reqWithPath("/admin/luke", "fl", f),
numFlags + "=string-length(" + xp + "[@name='schema'])");
}

Expand All @@ -140,13 +140,13 @@ public void testLuke() {
final String xp = getFieldXPathPrefix(f);
assertQ(
"Not as many index flags as expected (" + numFlags + ") for " + f,
req("qt", "/admin/luke", "fl", f),
reqWithPath("/admin/luke", "fl", f),
numFlags + "=string-length(" + xp + "[@name='index'])");

final String hxp = getFieldXPathHistogram(f);
assertQ(
"Historgram field should be present for field " + f,
req("qt", "/admin/luke", "fl", f),
reqWithPath("/admin/luke", "fl", f),
hxp + "[@name='histogram']");
}
}
Expand All @@ -169,7 +169,7 @@ private static String dynfield(String field) {

@Test
public void testFlParam() {
SolrQueryRequest req = req("qt", "/admin/luke", "fl", "solr_t solr_s", "show", "all");
SolrQueryRequest req = reqWithPath("/admin/luke", "fl", "solr_t solr_s", "show", "all");
try {
// First, determine that the two fields ARE there
String response = h.query(req);
Expand All @@ -186,7 +186,7 @@ public void testFlParam() {
TestHarness.validateXPath(response, getFieldXPathPrefix(f) + "[@name='index']"));
}
// Insure * works
req = req("qt", "/admin/luke", "fl", "*");
req = reqWithPath("/admin/luke", "fl", "*");
response = h.query(req);
for (String f :
Arrays.asList("solr_t", "solr_s", "solr_ti", "solr_td", "solr_dt", "solr_b")) {
Expand All @@ -202,25 +202,25 @@ public void testNumTerms() {
final String f = "name";
for (String n : new String[] {"2", "3", "100", "99999"}) {
assertQ(
req("qt", "/admin/luke", "fl", f, "numTerms", n),
reqWithPath("/admin/luke", "fl", f, "numTerms", n),
field(f) + "lst[@name='topTerms']/int[@name='Apache']",
field(f) + "lst[@name='topTerms']/int[@name='Solr']",
"count(" + field(f) + "lst[@name='topTerms']/int)=2");
}

assertQ(
req("qt", "/admin/luke", "fl", f, "numTerms", "1"),
reqWithPath("/admin/luke", "fl", f, "numTerms", "1"),
// no guarantee which one we find
"count(" + field(f) + "lst[@name='topTerms']/int)=1");

assertQ(
req("qt", "/admin/luke", "fl", f, "numTerms", "0"),
reqWithPath("/admin/luke", "fl", f, "numTerms", "0"),
"count(" + field(f) + "lst[@name='topTerms']/int)=0");

// field with no terms shouldn't error
for (String n : new String[] {"0", "1", "2", "100", "99999"}) {
assertQ(
req("qt", "/admin/luke", "fl", "bogus_s", "numTerms", n),
reqWithPath("/admin/luke", "fl", "bogus_s", "numTerms", n),
"count(" + field(f) + "lst[@name='topTerms']/int)=0");
}
}
Expand All @@ -234,7 +234,7 @@ public void testNullFactories() throws Exception {

try {
assertQ(
req("qt", "/admin/luke", "show", "schema"),
reqWithPath("/admin/luke", "show", "schema"),
"//lst[@name='custom_tc_string']/lst[@name='indexAnalyzer']",
"//lst[@name='custom_tc_string']/lst[@name='queryAnalyzer']",
"0=count(//lst[@name='custom_tc_string']/lst[@name='indexAnalyzer']/lst[@name='filters'])",
Expand All @@ -249,7 +249,7 @@ public void testNullFactories() throws Exception {
}

public void testCopyFieldLists() throws Exception {
SolrQueryRequest req = req("qt", "/admin/luke", "show", "schema");
SolrQueryRequest req = reqWithPath("/admin/luke", "show", "schema");

String xml = h.query(req);
String r =
Expand Down Expand Up @@ -284,7 +284,7 @@ public void testCatchAllCopyField() throws Exception {
"<copyField source=\"*\" dest=\"catchall_t\"/> is missing from the schema",
foundCatchAllCopyField);

SolrQueryRequest req = req("qt", "/admin/luke", "show", "schema", "indent", "on");
SolrQueryRequest req = reqWithPath("/admin/luke", "show", "schema", "indent", "on");
String xml = h.query(req);
String result =
TestHarness.validateXPath(
Expand Down Expand Up @@ -323,7 +323,7 @@ public void testIndexFlagsWithDeletedDocs() throws Exception {

assertQ(
"index flags should be present for solr_s despite deletion in segment",
req("qt", "/admin/luke", "fl", "solr_s"),
reqWithPath("/admin/luke", "fl", "solr_s"),
getFieldXPathPrefix("solr_s") + "[@name='index']");

// Now test the inverse: delete the edges and keep the middle. The first term
Expand All @@ -342,7 +342,7 @@ public void testIndexFlagsWithDeletedDocs() throws Exception {

assertQ(
"index flags should be present for solr_s when edges are deleted",
req("qt", "/admin/luke", "fl", "solr_s"),
reqWithPath("/admin/luke", "fl", "solr_s"),
getFieldXPathPrefix("solr_s") + "[@name='index']");
} finally {
deleteCore();
Expand Down
13 changes: 7 additions & 6 deletions solr/core/src/test/org/apache/solr/handler/admin/SegmentsInfoRequestHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,15 @@ public static void afterClass() throws Exception {
public void testSegmentInfos() {
assertQ(
"Unexpected number of segments returned",
req("qt", "/admin/segments"),
reqWithPath("/admin/segments"),
NUM_SEGMENTS + "=count(//lst[@name='segments']/lst)");
}

@Test
public void testSegmentInfosVersion() {
assertQ(
"Unexpected number of segments returned",
req("qt", "/admin/segments"),
reqWithPath("/admin/segments"),
NUM_SEGMENTS
+ "=count(//lst[@name='segments']/lst/str[@name='version'][.='"
+ Version.LATEST
Expand All @@ -129,14 +129,15 @@ public void testSegmentNames() throws IOException {

return null;
});
assertQ("Unexpected segment names returned", req("qt", "/admin/segments"), segmentNamePatterns);
assertQ(
"Unexpected segment names returned", reqWithPath("/admin/segments"), segmentNamePatterns);
}

@Test
public void testSegmentInfosData() {
assertQ(
"Unexpected document counts in result",
req("qt", "/admin/segments"),
reqWithPath("/admin/segments"),
// #Document
(DOC_COUNT * 2) + "=sum(//lst[@name='segments']/lst/int[@name='size'])",
// #Deletes
Expand All @@ -147,7 +148,7 @@ public void testSegmentInfosData() {
public void testCoreInfo() {
assertQ(
"Missing core info",
req("qt", "/admin/segments", "coreInfo", "true"),
reqWithPath("/admin/segments", "coreInfo", "true"),
"boolean(//lst[@name='info']/lst[@name='core'])");
}

Expand Down Expand Up @@ -193,7 +194,7 @@ public void testFieldInfo() throws Exception {
});
assertQ(
"Unexpected field infos returned",
req("qt", "/admin/segments", "fieldInfo", "true"),
reqWithPath("/admin/segments", "fieldInfo", "true"),
segmentNamePatterns);
}
}
Loading
Loading