abhishekrb19 commented on code in PR #16046:
URL: https://github.com/apache/druid/pull/16046#discussion_r1517095801
##########
sql/src/test/java/org/apache/druid/sql/calcite/CalciteSubqueryTest.java:
##########
@@ -675,98 +678,112 @@ public void testEmptyGroupWithOffsetDoesntInfiniteLoop()
public void testMaxSubqueryRows()
{
if ("without memory limit".equals(testName)) {
- expectedException.expect(ResourceLimitExceededException.class);
- expectedException.expectMessage(
- "Cannot issue the query, subqueries generated results beyond
maximum[1] rows. Try setting the "
- + "'maxSubqueryBytes' in the query context to 'auto' for enabling
byte based limit, which chooses an optimal "
- + "limit based on memory size and result's heap usage or manually
configure the values of either 'maxSubqueryBytes' "
- + "or 'maxSubqueryRows' in the query context. Manually alter the
value carefully as it can cause the broker to "
- + "go out of memory."
- );
+ testMaxSubqueryRowsWithoutMemoryLimit();
+ } else {
+ testMaxSubQueryRowsWithLimit();
+ }
+ }
+
+ private void testMaxSubqueryRowsWithoutMemoryLimit()
+ {
+ Throwable exception = assertThrows(ResourceLimitExceededException.class,
() -> {
Map<String, Object> modifiedQueryContext = new HashMap<>(queryContext);
Review Comment:
It would be better to move all the test setup outside the `assertThrows` and
narrow it down to the thing that we're expecting to throw an exception.
##########
sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java:
##########
@@ -12319,61 +12313,64 @@ public void
testRequireTimeConditionLogicalValuePositive()
@Test
public void testRequireTimeConditionSimpleQueryNegative()
{
- msqIncompatible();
- expectedException.expect(CannotBuildQueryException.class);
- expectedException.expectMessage("__time column");
+ Throwable exception = assertThrows(CannotBuildQueryException.class, () -> {
+ msqIncompatible();
Review Comment:
nit: Consider moving `msqIncompatible()` and any other test initialization
outside `assertThrows`.
Same applies to the other tests
##########
sql/src/test/java/org/apache/druid/sql/calcite/CalciteMultiValueStringQueryTest.java:
##########
@@ -123,23 +124,25 @@ public void testMultiValueStringWorksLikeStringGroupBy()
public void testMultiValueStringGroupByDoesNotWork()
{
// Cannot vectorize due to usage of expressions.
Review Comment:
should the comment be removed as well?
--
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]
