zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From eolivelli <...@git.apache.org>
Subject [GitHub] zookeeper pull request #601: ZOOKEEPER-3123 MetricsProvider Lifecycle in Zoo...
Date Tue, 11 Sep 2018 12:59:52 GMT
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) 



---

Mime
View raw message