ctubbsii commented on a change in pull request #1525:
URL: https://github.com/apache/zookeeper/pull/1525#discussion_r521825196
##########
File path:
zookeeper-server/src/test/java/org/apache/zookeeper/server/ZooKeeperServerTest.java
##########
@@ -135,4 +138,32 @@ public void testInvalidSnapshot() {
}
}
+ @Test
+ public void testClientZxidAhead() {
+ ZooKeeperServer zooKeeperServer = new ZooKeeperServer();
+ final ZKDatabase zkDatabase = new
ZKDatabase(mock(FileTxnSnapLog.class));
+ zooKeeperServer.setZKDatabase(zkDatabase);
+
+ final ByteBuffer output = ByteBuffer.allocate(30);
+ // serialize a connReq
+ output.putInt(1);
+ // lastZxid
+ output.putLong(99L);
+ output.putInt(500);
+ output.putLong(123L);
+ output.putInt(1);
+ output.put((byte) 1);
+ output.put((byte) 1);
+ output.flip();
+
+ try {
+ final NIOServerCnxn nioServerCnxn = mock(NIOServerCnxn.class);
+ zooKeeperServer.processConnectRequest(nioServerCnxn, output);
+ } catch (Exception e) {
+ // expect
+ assertTrue(TestServerCnxn.instanceofCloseRequestException(e));
Review comment:
> I don't agree with subclassing for testing is a good strategy.
> A more preferred way is mocking
To be clear, I don't think it's necessarily a *good* strategy, just that in
*some* circumstances it might be an appropriate way to test something that you
don't otherwise want accessible with higher visibility. I agree mocking is
better, whenever possible.
> `VisibleForTesting` annotation: I don't have much experience with that,
but that must be a reason why projects of Hadoop are using it all over the
place.
I would caution against the idea that popularity is a good measure of
reason. :wink:
My 2 cents already having been provided, I don't see any serious problems
with any of the discussed strategies. They all seem to have some pros and cons.
:smiley_cat:
----------------------------------------------------------------
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:
[email protected]
