ILuffZhe commented on a change in pull request #2657:
URL: https://github.com/apache/calcite/pull/2657#discussion_r781710381



##########
File path: 
elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/AggregationTest.java
##########
@@ -400,6 +400,21 @@ public static void setupInstance() throws Exception {
             "aggregations:{'v1.max.field': 'val1'",
             "'v2.min.field': 'val2'}"))
         .returnsUnordered("v1=7; v2=5");
+  }
+
+  @Test void testNullsSort() {

Review comment:
       Hi @asereda-gs ,
   I've though about creating a new class for testing NULLS SORT before, since 
NULLS SORT is one small sorting feature, I decided to add a dataset to verify 
its function at last, like @zabetak mentioned above.
   What's more, for those contributors who want to add new features or fixing 
bugs in ES module, I think creating different indexes in one class(like 
ElasticSearchAdapterTest) for unit tests is also a good choice.




-- 
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]


Reply via email to