imply-cheddar commented on code in PR #14184:
URL: https://github.com/apache/druid/pull/14184#discussion_r1181390366


##########
server/src/test/java/org/apache/druid/server/QueryResourceTest.java:
##########
@@ -1043,19 +1045,21 @@ public <T> QueryRunner<T> 
getQueryRunnerForIntervals(Query<T> query, Iterable<In
         return (queryPlus, responseContext) -> {
           beforeScheduler.forEach(CountDownLatch::countDown);
 
-          return scheduler.run(
-              scheduler.prioritizeAndLaneQuery(queryPlus, ImmutableSet.of()),
-              new LazySequence<T>(() -> {
-                inScheduler.forEach(CountDownLatch::countDown);
-                try {
-                  // pretend to be a query that is waiting on results
-                  Thread.sleep(500);
-                }
-                catch (InterruptedException ignored) {
-                }
-                // all that waiting for nothing :(
-                return Sequences.empty();
-              })
+          return Sequences.simple(
+              scheduler.run(
+                  scheduler.prioritizeAndLaneQuery(queryPlus, 
ImmutableSet.of()),
+                  new LazySequence<T>(() -> {
+                    inScheduler.forEach(CountDownLatch::countDown);
+                    try {
+                      // pretend to be a query that is waiting on results
+                      Thread.sleep(500);
+                    }
+                    catch (InterruptedException ignored) {
+                    }
+                    // all that waiting for nothing :(
+                    return Sequences.empty();
+                  })
+              ).toList()

Review Comment:
   Why did you need this change in the test?  It looks like you are making the 
test that was doing things semi-lazily to now be explicitly eager, which feels 
like a behavior change?



##########
server/src/test/java/org/apache/druid/server/QueryResourceTest.java:
##########
@@ -896,7 +896,9 @@ public void testTooManyQuery() throws InterruptedException
     assertAsyncResponseAndCountdownOrBlockForever(
         SIMPLE_TIMESERIES_QUERY,
         waitAllFinished,
-        response -> Assert.assertEquals(Response.Status.OK.getStatusCode(), 
response.getStatus())
+        response -> {
+          Assert.assertEquals(Response.Status.OK.getStatusCode(), 
response.getStatus());
+        }

Review Comment:
   This looks superfluous?  I'm guessing you added some lines and then didn't 
clean up the `{}` or is there actually a change in here?



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

Reply via email to