bruno-roustant commented on a change in pull request #2472:
URL: https://github.com/apache/lucene-solr/pull/2472#discussion_r603910635



##########
File path: 
solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java
##########
@@ -302,6 +305,26 @@ public void doTestHandlerPathUnchanged() throws Exception {
     assertEquals("/replication", ReplicationHandler.PATH);
   }
 
+  @Test
+  public void testShardsWhitelist() throws Exception {
+    // Run another test with shards whitelist enabled and whitelist is empty.
+    // Expect an exception because the leader URL is not allowed.
+    systemClearPropertySolrDisableShardsWhitelist();

Review comment:
       For backward compatibility I have added 
systemSetPropertySolrDisableShardsWhitelist("true") in setUp() and 
systemClearPropertySolrDisableShardsWhitelist() in tearDown() as this is 
already done in other tests (otherwise most test methods fail).
   So here the idea is to have one method that does not disable shard 
whitelisting. Another approach could be to move 
systemSetPropertySolrDisableShardsWhitelist() to each test method, but this 
doesn't improve readibility nor scalability if we increase the number of test 
cases. I would prefer to keep this call here as we are effectively in a test 
class that requires backqard compatibility about whitelisting.




-- 
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...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to