kgyrtkirk commented on code in PR #16420:
URL: https://github.com/apache/druid/pull/16420#discussion_r1604400193


##########
server/src/test/java/org/apache/druid/server/lookup/cache/LookupLoadingSpecTest.java:
##########
@@ -59,4 +66,62 @@ public void testLoadingOnlyRequiredLookupsWithNullList()
     DruidException exception = Assert.assertThrows(DruidException.class, () -> 
LookupLoadingSpec.loadOnly(null));
     Assert.assertEquals("Expected non-null set of lookups to load.", 
exception.getMessage());
   }
+
+  @MethodSource("provideParamsForTestGetSpecFromContext")
+  @ParameterizedTest
+  public void testGetLookupLoadingSpecFromContext(Map<String, Object> context, 
LookupLoadingSpec defaultSpec, LookupLoadingSpec expectedSpec)
+  {
+    LookupLoadingSpec specFromContext = 
LookupLoadingSpec.getSpecFromContext(context, defaultSpec);
+    Assert.assertEquals(expectedSpec.getMode(), specFromContext.getMode());
+    Assert.assertEquals(expectedSpec.getLookupsToLoad(), 
specFromContext.getLookupsToLoad());
+  }
+
+  public static Collection<Object[]> provideParamsForTestGetSpecFromContext()
+  {
+    ImmutableSet<String> lookupsToLoad = ImmutableSet.of("lookupName1", 
"lookupName2");
+    final ImmutableMap<String, Object> contextWithModeOnlyRequired = 
ImmutableMap.of(
+        LookupLoadingSpec.CTX_LOOKUPS_TO_LOAD, new ArrayList<>(lookupsToLoad),
+        LookupLoadingSpec.CTX_LOOKUP_LOADING_MODE, 
LookupLoadingSpec.Mode.ONLY_REQUIRED);
+    final ImmutableMap<String, Object> contextWithModeNone = ImmutableMap.of(
+        LookupLoadingSpec.CTX_LOOKUP_LOADING_MODE, 
LookupLoadingSpec.Mode.NONE);
+    final ImmutableMap<String, Object> contextWithModeAll = ImmutableMap.of(
+        LookupLoadingSpec.CTX_LOOKUP_LOADING_MODE, LookupLoadingSpec.Mode.ALL);
+    final ImmutableMap<String, Object> contextWithoutLookupKeys = 
ImmutableMap.of();
+
+    // Return params: <context, default LookupLoadingSpec, expected 
LookupLoadingSpec>
+    Object[][] params = new Object[][]{
+        // Default spec is returned in the case of context not having the 
lookup keys.

Review Comment:
   that's great and fine ; but instead of leaving comments about a param's 
intentions - its better to name it with 
[Named](https://junit.org/junit5/docs/5.9.1/api/org.junit.jupiter.api/org/junit/jupiter/api/Named.html)
 ; so a more descriptive name will be shown when the testcase is run - an [SO 
example is here](https://stackoverflow.com/a/66994980/1525291)
   
   



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