Akshat-Jain commented on code in PR #16328:
URL: https://github.com/apache/druid/pull/16328#discussion_r1578968058
##########
server/src/test/java/org/apache/druid/query/lookup/LookupReferencesManagerTest.java:
##########
@@ -765,6 +777,91 @@ public void testCoordinatorLookupSync() throws Exception
}
+ public static Object[]
parametersForTestCoordinatorSelectiveLoadingOfLookups()
+ {
+ return new Object[] {
+ // load all lookups
+ new Object[]{
+ null
+ },
+ // don't load any lookups
+ new Object[]{
+ Collections.emptyList()
+ },
+ // only load these lookups
+ new Object[]{
+ Arrays.asList("testLookup1", "testLookup2")
+ },
+ };
+ }
+
+ @Test
+ @Parameters
+ public void testCoordinatorSelectiveLoadingOfLookups(List<String>
lookupsToLoad) throws Exception
+ {
+ LookupExtractorFactoryContainer container1 = new
LookupExtractorFactoryContainer(
+ "0",
+ new MapLookupExtractorFactory(
+ ImmutableMap.of(
+ "key1",
+ "value1"
+ ), true
+ )
+ );
+
+ LookupExtractorFactoryContainer container2 = new
LookupExtractorFactoryContainer(
+ "0",
+ new MapLookupExtractorFactory(
+ ImmutableMap.of(
+ "key2",
+ "value2"
+ ), true
+ )
+ );
+
+ LookupExtractorFactoryContainer container3 = new
LookupExtractorFactoryContainer(
+ "0",
+ new MapLookupExtractorFactory(
+ ImmutableMap.of(
+ "key3",
+ "value3"
+ ), true
+ )
+ );
+ EasyMock.reset(config);
+ EasyMock.reset(druidLeaderClient);
+ Map<String, Object> lookupMap = new HashMap<>();
+ lookupMap.put("testLookup1", container1);
+ lookupMap.put("testLookup2", container2);
+ lookupMap.put("testLookup3", container3);
+ String strResult = mapper.writeValueAsString(lookupMap);
+ Request request = new Request(HttpMethod.GET, new
URL("http://localhost:1234/xx";));
+ EasyMock.expect(config.getLookupTier()).andReturn(LOOKUP_TIER);
+ EasyMock.expect(config.getLookupsToLoad()).andReturn(lookupsToLoad);
+ EasyMock.replay(config);
+ EasyMock.expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
+ .andReturn(request);
+ StringFullResponseHolder responseHolder = new StringFullResponseHolder(
+ newEmptyResponse(HttpResponseStatus.OK),
+ StandardCharsets.UTF_8
+ ).addChunk(strResult);
+ EasyMock.expect(druidLeaderClient.go(request)).andReturn(responseHolder);
+ EasyMock.replay(druidLeaderClient);
+
+ lookupReferencesManager.start();
+
+ for (String lookupName : lookupMap.keySet()) {
+ if (lookupsToLoad == null || lookupsToLoad.contains(lookupName)) {
+ Assert.assertEquals(Optional.of(lookupMap.get(lookupName)),
lookupReferencesManager.get(lookupName));
+ } else {
+
Assert.assertFalse(lookupReferencesManager.get(lookupName).isPresent());
Review Comment:
@kfaraz To achieve that, we'd need multiple private methods:
1. Method for the common init stuff.
2. Method for creating LookupExtractorFactoryContainer object, and returning
it to the individual tests for assertion purposes.
I don't think this would be cleaner than the current approach of
parameterized test. Do you have any strict reason for breaking it up into
separate tests?
I specifically went with the parameterized approach here as that's cleaner
IMO. If it helps, I can maybe add more comments explaining the assertions?
Appreciate your thoughts on this, thanks! :)
--
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]
