Copilot commented on code in PR #4286:
URL: https://github.com/apache/solr/pull/4286#discussion_r3092285964
##########
solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/stream/StreamingTest.java:
##########
@@ -145,16 +146,16 @@ public static void configureCluster() throws Exception {
// Update request shared by many of the tests
private final UpdateRequest helloDocsUpdateRequest =
new UpdateRequest()
- .add(id, "0", "a_s", "hello0", "a_i", "0", "a_f", "1")
+ .add(id, "0", "a_s", "hello0", "a_i", "0", "a_f", "1", "b_f", "1.5")
.add(id, "2", "a_s", "hello0", "a_i", "2", "a_f", "2")
.add(id, "3", "a_s", "hello3", "a_i", "3", "a_f", "3")
- .add(id, "4", "a_s", "hello4", "a_i", "4", "a_f", "4")
+ .add(id, "4", "a_s", "hello4", "a_i", "4", "a_f", "4", "b_f", "4.5")
.add(id, "1", "a_s", "hello0", "a_i", "1", "a_f", "5")
- .add(id, "5", "a_s", "hello3", "a_i", "10", "a_f", "6")
- .add(id, "6", "a_s", "hello4", "a_i", "11", "a_f", "7")
+ .add(id, "5", "a_s", "hello3", "a_i", "10", "a_f", "6", "b_f", "6.5")
+ .add(id, "6", "a_s", "hello4", "a_i", "11", "a_f", "7", "b_f", "7.5")
.add(id, "7", "a_s", "hello3", "a_i", "12", "a_f", "8")
.add(id, "8", "a_s", "hello3", "a_i", "13", "a_f", "9")
- .add(id, "9", "a_s", "hello0", "a_i", "14", "a_f", "10");
+ .add(id, "9", "a_s", "hello0", "a_i", "14", "a_f", "10", "b_f",
"10.5");
Review Comment:
The PR description says `b_f` is only added to 4 of the 10 fixture docs (ids
0,4,5,6), but this change also adds `b_f` to id 9. Please either update the PR
description to match the fixture, or drop `b_f` from the id 9 document if the
intent was to keep it to 4 docs.
##########
solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/stream/StreamingTest.java:
##########
@@ -1660,7 +1661,8 @@ public void testRollupStream() throws Exception {
new MaxMetric("a_f"),
new MeanMetric("a_i"),
new MeanMetric("a_f"),
- new CountMetric()
+ new CountMetric(),
+ new MissingMetric("b_f")
};
Review Comment:
These assertions exercise `MissingMetric` via direct Java construction, but
the feature is primarily for streaming expressions (`/stream`). To guard
against regressions in `Lang` registration and the
`MissingMetric(StreamExpression, StreamFactory)` parsing path, consider adding
at least one test that executes a `/stream` expression including `missing(b_f)`
(or constructs the metric via `StreamFactory.constructMetric(...)`) and asserts
the returned tuples contain the expected `missing(b_f)` value.
--
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]
