imply-cheddar commented on code in PR #15454:
URL: https://github.com/apache/druid/pull/15454#discussion_r1427492644
##########
server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java:
##########
@@ -848,8 +853,12 @@ public void
testMarkAsUnusedSegmentsInIntervalWithInvalidInterval() throws IOExc
publisher.publishSegment(newSegment1);
publisher.publishSegment(newSegment2);
// invalid interval start > end
- final Interval theInterval =
Intervals.of("2017-10-22T00:00:00.000/2017-10-02T00:00:00.000");
- sqlSegmentsMetadataManager.markAsUnusedSegmentsInInterval(newDataSource,
theInterval);
+ DruidExceptionMatcher.invalidInput().assertThrowsAndMatches(
+ () -> {
+ Interval theInterval =
Intervals.of("2017-10-22T00:00:00.000/2017-10-02T00:00:00.000");
+
sqlSegmentsMetadataManager.markAsUnusedSegmentsInInterval(newDataSource,
theInterval);
+ }
+ );
Review Comment:
I think that this is also pointless. Let's just delete this test
##########
server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java:
##########
@@ -621,7 +622,8 @@ public void testMarkAsUnusedAllSegmentsInDataSource()
Assert.assertNotNull(response.getEntity());
Assert.assertTrue(response.getEntity().toString().contains("java.lang.IllegalArgumentException"));
}
- catch (IllegalArgumentException ignore) {
+ catch (DruidException ignore) {
+ Assert.assertEquals(DruidException.Category.INVALID_INPUT,
ignore.getCategory());
// expected
}
Review Comment:
Can you switch this around to
`DruidExceptionMatcher.invalidInput().assertThrowsAndMatches()`
##########
server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java:
##########
@@ -712,8 +713,12 @@ public void
testMarkAsUsedNonOvershadowedSegmentsInIntervalWithInvalidInterval()
publish(newSegment1, false);
publish(newSegment2, false);
// invalid interval start > end
- final Interval theInterval =
Intervals.of("2017-10-22T00:00:00.000/2017-10-02T00:00:00.000");
-
sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(newDataSource,
theInterval);
+ DruidExceptionMatcher.invalidInput().assertThrowsAndMatches(
+ () -> {
+ Interval theInterval =
Intervals.of("2017-10-22T00:00:00.000/2017-10-02T00:00:00.000");
+
sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(newDataSource,
theInterval);
+ }
+ );
Review Comment:
Actually, when looking at this test, it seems to be throwing when you read
the interval, and that has absolutely no relation to the MetadataManager or any
fo the setup. I think this test is just useless and should be deleted
whole-hog.
--
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]
