zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From anmolnar <...@git.apache.org>
Subject [GitHub] zookeeper pull request #652: ZOOKEEPER-3156: Add in option to canonicalize h...
Date Tue, 02 Oct 2018 12:55:41 GMT
Github user anmolnar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/652#discussion_r221936001
  
    --- Diff: src/java/main/org/apache/zookeeper/ClientCnxn.java ---
    @@ -793,7 +794,87 @@ public RWServerFoundException(String msg) {
                 super(msg);
             }
         }
    -    
    +
    +    static class MockableInetSocketAddress {
    --- End diff --
    
    I agree. Unfortunately methods of `InetSocketAddress` and `InetAddress` are final and
cannot be mocked. There's a Mockito extension which could help, but it's quite recent and
an opt-in feature:
    https://github.com/mockito/mockito/wiki/What's-new-in-Mockito-2#mock-the-unmockable-opt-in-mocking-of-final-classesmethods
    
    Implementing wrapper classes is good approach, but I wouldn't put them into `ClientCnxn.java`.
I suggest creating a new file with wrapper classes (nit: renaming them to \*Wrapper instead
of Mockable\* would be better) and also put `getServerPrincipal()` into this file as well.



---

Mime
View raw message