dsmiley commented on a change in pull request #152:
URL: https://github.com/apache/solr/pull/152#discussion_r643465722



##########
File path: 
solr/core/src/test/org/apache/solr/cloud/OverseerModifyCollectionTest.java
##########
@@ -42,29 +39,27 @@ public static void setupCluster() throws Exception {
   }
 
   @Test
+  @SuppressWarnings({"unchecked"})

Review comment:
       We try to avoid writing code in a way that has warnings so that we 
needn't suppress them.  Sometimes it's not avoidable.  I can tell here that 
it's because you called `SolrTestCaseJ4.map`.  Let's call Map.of instead.  Or 
even Collections.singletonMap because we might take this change to 8x and Java 
8 doesn't have Map.of.

##########
File path: 
solr/core/src/test/org/apache/solr/cloud/OverseerModifyCollectionTest.java
##########
@@ -42,29 +39,27 @@ public static void setupCluster() throws Exception {
   }
 
   @Test
+  @SuppressWarnings({"unchecked"})
   public void testModifyColl() throws Exception {
 
     final String collName = "modifyColl";
 
     CollectionAdminRequest.createCollection(collName, "conf1", 1, 2)
         .process(cluster.getSolrClient());
 
-    // TODO create a modifyCollection() method on CollectionAdminRequest
-    ModifiableSolrParams p1 = new ModifiableSolrParams();
-    p1.add("collection", collName);
-    p1.add("action", "MODIFYCOLLECTION");
-    p1.add("collection.configName", "conf2");
-    cluster.getSolrClient().request(new GenericSolrRequest(POST, 
COLLECTIONS_HANDLER_PATH, p1));
+    //Modify configSet
+    RequestStatusState requestStatusState = 
CollectionAdminRequest.modifyCollection(collName,
+            map("collection.configName", "conf2")
+    ).processAndWait(cluster.getSolrClient(), DEFAULT_TIMEOUT);
+    assertEquals(requestStatusState.getKey(), "completed");

Review comment:
       RequestStatusState is an enum, and is thus immutable.  Why not just do 
an instance check?

##########
File path: 
solr/core/src/test/org/apache/solr/cloud/OverseerModifyCollectionTest.java
##########
@@ -42,29 +39,27 @@ public static void setupCluster() throws Exception {
   }
 
   @Test
+  @SuppressWarnings({"unchecked"})
   public void testModifyColl() throws Exception {
 
     final String collName = "modifyColl";
 
     CollectionAdminRequest.createCollection(collName, "conf1", 1, 2)
         .process(cluster.getSolrClient());
 
-    // TODO create a modifyCollection() method on CollectionAdminRequest
-    ModifiableSolrParams p1 = new ModifiableSolrParams();
-    p1.add("collection", collName);
-    p1.add("action", "MODIFYCOLLECTION");
-    p1.add("collection.configName", "conf2");
-    cluster.getSolrClient().request(new GenericSolrRequest(POST, 
COLLECTIONS_HANDLER_PATH, p1));
+    //Modify configSet
+    RequestStatusState requestStatusState = 
CollectionAdminRequest.modifyCollection(collName,
+            map("collection.configName", "conf2")
+    ).processAndWait(cluster.getSolrClient(), DEFAULT_TIMEOUT);
+    assertEquals(requestStatusState.getKey(), "completed");

Review comment:
       And I presume the changes here fail without the underlying fix?




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to