kfaraz commented on code in PR #16420:
URL: https://github.com/apache/druid/pull/16420#discussion_r1596842747
##########
server/src/test/java/org/apache/druid/server/lookup/cache/LookupLoadingSpecTest.java:
##########
@@ -59,4 +61,68 @@ public void testLoadingOnlyRequiredLookupsWithNullList()
DruidException exception = Assert.assertThrows(DruidException.class, () ->
LookupLoadingSpec.loadOnly(null));
Assert.assertEquals("Expected non-null set of lookups to load.",
exception.getMessage());
}
+
+ @Test
+ public void testCreateLookupLoadingSpecFromContext()
+ {
+ ImmutableSet<String> lookupsToLoad = ImmutableSet.of("lookupName1",
"lookupName2");
+
+ // Default spec is returned in the case of context not having the lookup
keys.
+ Assert.assertEquals(
+ LookupLoadingSpec.ALL,
+ LookupLoadingSpec.createFromContext(
+ ImmutableMap.of(),
+ LookupLoadingSpec.ALL
+ )
+ );
+
+ // Default spec is returned in the case of context not having the lookup
keys.
+ Assert.assertEquals(
+ LookupLoadingSpec.NONE,
+ LookupLoadingSpec.createFromContext(
+ ImmutableMap.of(),
+ LookupLoadingSpec.NONE
+ )
+ );
+
+ // Only required lookups are returned in the case of context having the
lookup keys.
+ Assert.assertEquals(
+ LookupLoadingSpec.loadOnly(lookupsToLoad),
Review Comment:
```suggestion
LookupLoadingSpec.loadOnly(ImmutableSet.of("lookup1", "lookup2")),
```
##########
server/src/test/java/org/apache/druid/server/lookup/cache/LookupLoadingSpecTest.java:
##########
@@ -59,4 +61,68 @@ public void testLoadingOnlyRequiredLookupsWithNullList()
DruidException exception = Assert.assertThrows(DruidException.class, () ->
LookupLoadingSpec.loadOnly(null));
Assert.assertEquals("Expected non-null set of lookups to load.",
exception.getMessage());
}
+
+ @Test
+ public void testCreateLookupLoadingSpecFromContext()
+ {
+ ImmutableSet<String> lookupsToLoad = ImmutableSet.of("lookupName1",
"lookupName2");
+
+ // Default spec is returned in the case of context not having the lookup
keys.
+ Assert.assertEquals(
+ LookupLoadingSpec.ALL,
+ LookupLoadingSpec.createFromContext(
+ ImmutableMap.of(),
+ LookupLoadingSpec.ALL
+ )
+ );
+
+ // Default spec is returned in the case of context not having the lookup
keys.
+ Assert.assertEquals(
+ LookupLoadingSpec.NONE,
+ LookupLoadingSpec.createFromContext(
+ ImmutableMap.of(),
+ LookupLoadingSpec.NONE
+ )
+ );
+
+ // Only required lookups are returned in the case of context having the
lookup keys.
+ Assert.assertEquals(
+ LookupLoadingSpec.loadOnly(lookupsToLoad),
+ LookupLoadingSpec.createFromContext(
+ ImmutableMap.of(
+ LookupLoadingSpec.CTX_LOOKUPS_TO_LOAD, new
ArrayList<>(lookupsToLoad),
Review Comment:
```suggestion
LookupLoadingSpec.CTX_LOOKUPS_TO_LOAD,
Arrays.asList("lookup1", "lookup2"),
```
##########
server/src/test/java/org/apache/druid/server/lookup/cache/LookupLoadingSpecTest.java:
##########
@@ -59,4 +61,68 @@ public void testLoadingOnlyRequiredLookupsWithNullList()
DruidException exception = Assert.assertThrows(DruidException.class, () ->
LookupLoadingSpec.loadOnly(null));
Assert.assertEquals("Expected non-null set of lookups to load.",
exception.getMessage());
}
+
+ @Test
+ public void testCreateLookupLoadingSpecFromContext()
+ {
+ ImmutableSet<String> lookupsToLoad = ImmutableSet.of("lookupName1",
"lookupName2");
Review Comment:
You can remove this variable as it is used in only one test case. See the
other comment.
##########
server/src/test/java/org/apache/druid/server/lookup/cache/LookupLoadingSpecTest.java:
##########
@@ -59,4 +61,68 @@ public void testLoadingOnlyRequiredLookupsWithNullList()
DruidException exception = Assert.assertThrows(DruidException.class, () ->
LookupLoadingSpec.loadOnly(null));
Assert.assertEquals("Expected non-null set of lookups to load.",
exception.getMessage());
}
+
+ @Test
+ public void testCreateLookupLoadingSpecFromContext()
+ {
+ ImmutableSet<String> lookupsToLoad = ImmutableSet.of("lookupName1",
"lookupName2");
+
+ // Default spec is returned in the case of context not having the lookup
keys.
+ Assert.assertEquals(
+ LookupLoadingSpec.ALL,
+ LookupLoadingSpec.createFromContext(
+ ImmutableMap.of(),
+ LookupLoadingSpec.ALL
+ )
+ );
+
+ // Default spec is returned in the case of context not having the lookup
keys.
+ Assert.assertEquals(
+ LookupLoadingSpec.NONE,
+ LookupLoadingSpec.createFromContext(
+ ImmutableMap.of(),
+ LookupLoadingSpec.NONE
+ )
+ );
Review Comment:
Put the empty context cases in a separate test method.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java:
##########
@@ -71,46 +73,115 @@
import java.io.IOException;
import java.util.HashMap;
+import java.util.Map;
public class ClientCompactionTaskQuerySerdeTest
{
+ static {
+ NullHandling.initializeForTests();
+ }
+
private static final RowIngestionMetersFactory ROW_INGESTION_METERS_FACTORY =
new TestUtils().getRowIngestionMetersFactory();
private static final CoordinatorClient COORDINATOR_CLIENT = new
NoopCoordinatorClient();
private static final AppenderatorsManager APPENDERATORS_MANAGER = new
TestAppenderatorsManager();
+ private static final ObjectMapper MAPPER =
setupInjectablesInObjectMapper(new DefaultObjectMapper());
+
+ private static final IndexSpec INDEX_SPEC = IndexSpec.builder()
+
.withDimensionCompression(CompressionStrategy.LZ4)
+
.withMetricCompression(CompressionStrategy.LZF)
+
.withLongEncoding(LongEncodingStrategy.LONGS)
+ .build();
+ private static final IndexSpec INDEX_SPEC_FOR_INTERMEDIATE_PERSISTS =
IndexSpec.builder()
+
.withDimensionCompression(CompressionStrategy.LZ4)
+
.withMetricCompression(CompressionStrategy.UNCOMPRESSED)
+
.withLongEncoding(LongEncodingStrategy.AUTO)
+
.build();
+ private static final ClientCompactionIOConfig CLIENT_COMPACTION_IO_CONFIG =
new ClientCompactionIOConfig(
+ new ClientCompactionIntervalSpec(
+ Intervals.of("2019/2020"),
+ "testSha256OfSortedSegmentIds"),
Review Comment:
Can be in a single line.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java:
##########
@@ -71,46 +73,115 @@
import java.io.IOException;
import java.util.HashMap;
+import java.util.Map;
public class ClientCompactionTaskQuerySerdeTest
{
+ static {
+ NullHandling.initializeForTests();
+ }
+
private static final RowIngestionMetersFactory ROW_INGESTION_METERS_FACTORY =
new TestUtils().getRowIngestionMetersFactory();
private static final CoordinatorClient COORDINATOR_CLIENT = new
NoopCoordinatorClient();
private static final AppenderatorsManager APPENDERATORS_MANAGER = new
TestAppenderatorsManager();
+ private static final ObjectMapper MAPPER =
setupInjectablesInObjectMapper(new DefaultObjectMapper());
+
+ private static final IndexSpec INDEX_SPEC = IndexSpec.builder()
+
.withDimensionCompression(CompressionStrategy.LZ4)
+
.withMetricCompression(CompressionStrategy.LZF)
+
.withLongEncoding(LongEncodingStrategy.LONGS)
+ .build();
+ private static final IndexSpec INDEX_SPEC_FOR_INTERMEDIATE_PERSISTS =
IndexSpec.builder()
+
.withDimensionCompression(CompressionStrategy.LZ4)
+
.withMetricCompression(CompressionStrategy.UNCOMPRESSED)
+
.withLongEncoding(LongEncodingStrategy.AUTO)
+
.build();
+ private static final ClientCompactionIOConfig CLIENT_COMPACTION_IO_CONFIG =
new ClientCompactionIOConfig(
+ new ClientCompactionIntervalSpec(
+ Intervals.of("2019/2020"),
+ "testSha256OfSortedSegmentIds"),
+ true
+ );
+ private static final ClientCompactionTaskGranularitySpec
CLIENT_COMPACTION_TASK_GRANULARITY_SPEC =
+ new ClientCompactionTaskGranularitySpec(Granularities.DAY,
Granularities.HOUR, true);
+ private static final ClientCompactionTaskDimensionsSpec
CLIENT_COMPACTION_TASK_DIMENSIONS_SPEC =
+ new
ClientCompactionTaskDimensionsSpec(DimensionsSpec.getDefaultSchemas(ImmutableList.of("ts",
"dim")));
+ private static final AggregatorFactory[] METRICS_SPEC = new
AggregatorFactory[] {new CountAggregatorFactory("cnt")};
+ private static final ClientCompactionTaskTransformSpec
CLIENT_COMPACTION_TASK_TRANSFORM_SPEC =
+ new ClientCompactionTaskTransformSpec(new SelectorDimFilter("dim1",
"foo", null));
+ private static final DynamicPartitionsSpec DYNAMIC_PARTITIONS_SPEC = new
DynamicPartitionsSpec(100, 30000L);
+ private static final SegmentsSplitHintSpec SEGMENTS_SPLIT_HINT_SPEC = new
SegmentsSplitHintSpec(new HumanReadableBytes(100000L), 10);
Review Comment:
Rather than declaring all of these constants and reusing the same values in
all the tests, just add private methods `createCompactionTuningConfig` and
`createCompactionTaskQuery`.
--
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]
