Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Zookeeper >> mail # dev >> Review Request 19508: ZOOKEEPER-1894: ObserverTest.testObserver fails consistently


Copy link to this message
-
Re: Review Request 19508: ZOOKEEPER-1894: ObserverTest.testObserver fails consistently

This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19508/#review38412
Thanks Michi for the patch, it looks very nice. I've just few minor suggestions, kindly see it.
http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/19508/#comment70565>

    Since we are touching this, can we append the strings instead of concatenation ?
    
    like:
    
    sw.append(HostNameUtils.getHostString(clientAddr));
    sw.append(":");
    sw.append(String.valueOf(clientAddr.getPort()));
    

http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/test/org/apache/zookeeper/common/HostNameUtilsTest.java
<https://reviews.apache.org/r/19508/#comment70561>

    Very good test cases.
    
    It would be nice if we add the message section in the Assert.assertEquals("message", expAg, actualArg) like:
    
    Assert.assertEquals("Failed to resolve InetSocketAddress with no host to 0.0.0.0", socketAddress.getAddress().getHostAddress(),  HostNameUtils.getHostString(socketAddress);
    
    Also it would be great if we can modify other assertions present in the tests too.
    
    
- Rakesh R
On March 20, 2014, 11:52 p.m., michim wrote: