Grant Henke 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? Yeah, I think I left these protected and available to change so that you could override them, but now that we have getMiniClusterBuilder we don't need to expose them. 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 fol That's true now, however there may be some test specific flag in the future that needs to be used. I think inheriting the base functionality is the safest approach. -- 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 18:01:51 +0000 Gerrit-HasComments: Yes
