[ 
https://issues.apache.org/jira/browse/SOLR-959?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12671257#action_12671257
 ] 

Shalin Shekhar Mangar commented on SOLR-959:
--------------------------------------------

Thanks Hoss and Akshay.

bq. The most alarming thing at this point is that several other test methods do 
similar stop/start of the master, and yet they work fine (even though the slave 
has no knowledge of the new master port) which suggests to me that the tests 
may be false positives making flawed assumptions.

Hoss, perhaps the master's port did not change for these tests? It would be 
best to stop the slaves and re-create their solrconfig using the new master 
instance's port which is what Akshay has done in the latest patch.

The latest patch looks good, works well. I'll commit this shortly.

> Remove hardcoded port numbers from TestReplicationHandler
> ---------------------------------------------------------
>
>                 Key: SOLR-959
>                 URL: https://issues.apache.org/jira/browse/SOLR-959
>             Project: Solr
>          Issue Type: Test
>          Components: replication (java)
>            Reporter: Hoss Man
>            Priority: Minor
>         Attachments: replicationtest-port-refactor.patch, SOLR-959.patch
>
>
> TestReplicationHandler has a hardcoded port of 9999 in it for the "master".  
> hardcoding port numbers in unit tests is very brittle and error prone and can 
> cause problems.  Ideally tests that aren't explicitly testing network related 
> functionality should avoid dealing with the network at all, but when 
> neccessary it's much better to let the OS pick any available port (as most 
> other solr tests do) then to hardcoded it.
> in TestReplicationHandler things are a little more complicated because the 
> master port number needs to be refered to in the slave config files.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to