Github user lvfangmin commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/622#discussion_r218880876
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java ---
@@ -1861,6 +1862,76 @@ public void testFaultyMetricsProviderOnConfigure()
throws Exception {
Assert.assertTrue("complains about metrics provider
MetricsProviderLifeCycleException", found);
}
+ /**
+ * Test leader/leader compatibility with/without CloseSessionTxn, so
that
+ * we can gradually rollout this code and rollback if there is problem.
+ */
+ @Test
+ public void testCloseSessionTxnCompatile() throws Exception {
--- End diff --
When I was looking for places to put these kind of tests, this is the
classes where I found most suitable, although I agree it's kind of overloaded.
This test is testing a single thing about compatibility, but that's
debatable depends how much granularity you define a single purpose test.
I'm not against of moving this out, create a separate test file, but
considering we'll remove the closeSessionTxnEnabled option (we have started
roll out this code to prod, and haven't seen problem, so it's likely I'll
remove it here soon), that means this test will only include a single case
leaderEnabled and followerEnabled, let me know if you think it's still worth to
move out.
---
