kbendick commented on code in PR #4495:
URL: https://github.com/apache/iceberg/pull/4495#discussion_r842323064


##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -72,4 +78,62 @@ protected boolean supportsNamespaceProperties() {
   protected boolean supportsServerSideRetry() {
     return true;
   }
+
+  /* RESTCatalog specific tests */
+
+  @Test
+  public void testConfigRoute() {
+    RESTClient testClient = new RESTClient() {
+      @Override
+      public void head(String path, Consumer<ErrorResponse> errorHandler) {
+        throw new UnsupportedOperationException("Should not be called for 
testConfigRoute");
+      }
+
+      @Override
+      public <T extends RESTResponse> T delete(String path, Class<T> 
responseType,
+                                               Consumer<ErrorResponse> 
errorHandler) {
+        throw new UnsupportedOperationException("Should not be called for 
testConfigRoute");
+      }
+
+      @Override
+      public <T extends RESTResponse> T get(String path, Class<T> 
responseType, Consumer<ErrorResponse> errorHandler) {
+        return (T) RESTCatalogConfigResponse
+            .builder()
+            .withDefaults(ImmutableMap.of())
+            .withOverrides(ImmutableMap.of("cache-enabled", "false"))
+            .build();
+      }
+
+      @Override
+      public <T extends RESTResponse> T post(String path, RESTRequest body, 
Class<T> responseType,
+                                             Consumer<ErrorResponse> 
errorHandler) {
+        throw new UnsupportedOperationException("Should not be called for 
testConfigRoute");
+      }
+
+      @Override
+      public void close() {
+      }
+    };
+
+    RESTCatalog restCat = new RESTCatalog((config) -> testClient);
+    Map<String, String> initialConfig = ImmutableMap.of("uri", 
"http://localhost:8080";);
+
+    restCat.setConf(new Configuration());
+    restCat.initialize("prod", initialConfig);
+
+    Assert.assertNotNull("Should have sever-provided config keys", 
restCat.properties().get("cache-enabled"));
+  }
+
+  @Test
+  public void testInitializeWithBadArguments() {
+    AssertHelpers.assertThrows("Configuration passed to initialize cannot be 
null",
+        IllegalArgumentException.class,
+        "Invalid configuration: null",
+        () -> catalog().initialize("prod", null));
+
+    AssertHelpers.assertThrows("Configuration passed to initialize must have 
uri",
+        IllegalArgumentException.class,
+        "REST Catalog server URI is required",
+        () -> catalog().initialize("prod", ImmutableMap.of()));

Review Comment:
   Now that we've refactored to not have such a large surface area in 
`VisibleForTesting`, I'm going to move these tests over to a test class for 
`TestHTTPClientFactory`.
   
   They're failing now because I updated `RESTCatalog`'s fetch server 
configuration function to use the passed in adaptor (via `clientBuilder`).



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