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