[GitHub] [lucene-solr] bruno-roustant commented on a change in pull request #2472: SOLR-15217: Use shardsWhitelist in ReplicationHandler.

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org