suneet-s commented on code in PR #12472:
URL: https://github.com/apache/druid/pull/12472#discussion_r857246712
##########
processing/src/test/java/org/apache/druid/query/timeboundary/TimeBoundaryQueryRunnerTest.java:
##########
@@ -216,6 +247,22 @@ public void testTimeBoundary()
Assert.assertEquals(DateTimes.of("2011-04-15T00:00:00.000Z"), maxTime);
}
+ @Test(expected = UOE.class)
+ @SuppressWarnings("unchecked")
+ public void testTimeBoundaryArrayResults()
+ {
+ TimeBoundaryQuery timeBoundaryQuery = Druids.newTimeBoundaryQueryBuilder()
+ .dataSource("testing")
+ .bound(null)
+ .build();
+ ResponseContext context = ConcurrentResponseContext.createEmpty();
+ context.initializeMissingSegments();
+ new TimeBoundaryQueryQueryToolChest().resultsAsArrays(
+ timeBoundaryQuery,
+ runner.run(QueryPlus.wrap(timeBoundaryQuery), context)
+ ).toList();
+ }
+
Review Comment:
I haven't thought much about this specific example, but here are my 2 cents
> Or create TimeBoundaryQueryRunnerTest objects in toolchest tests to use
the setup code
This seems a little bit like an anti-pattern, so I'd stay away from it.
> One way to move these to toolchest tests could be to copy some of the
runner and data creation code in toolchest tests too.
Usually, the pattern I've seen in instances like this is to move this common
setup logic into some shared class, and then use that class in both test
classes. So similar to this suggestion, but not copying the code over directly.
In this case the refactoring seems tedious, and I don't think needs to be
done as part of this change.
--
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]
