Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10838 )
Change subject: KUDU-2420: Support parallel java tests ...................................................................... Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/10838/4/java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java File java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java: http://gerrit.cloudera.org:8080/#/c/10838/4/java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java@50 PS4, Line 50: protected int numMasters = 3; : protected int numTabletServers = 3; Shouldn't these be constants? i.e. public static final int NUM_MASTERS? http://gerrit.cloudera.org:8080/#/c/10838/4/java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java: http://gerrit.cloudera.org:8080/#/c/10838/4/java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java@51 PS4, Line 51: return super.getMiniClusterBuilder() Chaining to the superclass' impl makes the overall code flow tougher to follow. In this case, the superclass is just setting the number of tservers and masters, which doesn't add much value, so maybe we can just duplicate that here? Same for the other tests. -- To view, visit http://gerrit.cloudera.org:8080/10838 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I97c6718b022936a8839f957da0928f54ff6e7371 Gerrit-Change-Number: 10838 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 29 Jun 2018 17:53:44 +0000 Gerrit-HasComments: Yes
