Github user eolivelli commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/601#discussion_r216658766
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java ---
@@ -1486,6 +1490,377 @@ public TestQPMain getTestQPMain() {
}
}
+ /**
+ * Verify boot works configuring a MetricsProvider
+ */
+ @Test
+ public void testMetricsProviderLifecycle() throws Exception {
--- End diff --
The whole file need a good refactor as already commented with @lvfangmin
I did not change the overall style of the class in order not to pollute the
patch.
I will be happy to follow up with a refactor of this class.
As told with @lvfangmin it is important to test the lifecycle of
QuorumPeerMain which is actually used in production, instead of
ZooKeeperServerMain which is never used in production sites.
Alternatively I can move these new tests to a new class.
It is not simple to mock QuorumPeerMain without creating an useless test.
I could use PowerMock to remove all real code (TxLog, Database....) and
speed up
As soon as we will have Maven+surefire it will be easier to run single
methods and not be forced to run the whole file (or but a lot of @Ignore)
---