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.
---
