Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12042 )
Change subject: java: add support for flaky test reporting ...................................................................... Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/12042/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12042/5//COMMIT_MSG@12 PS5, Line 12: TestResultReporter ResultReporter http://gerrit.cloudera.org:8080/#/c/12042/5/java/gradle/dependencies.gradle File java/gradle/dependencies.gradle: http://gerrit.cloudera.org:8080/#/c/12042/5/java/gradle/dependencies.gradle@45 PS5, Line 45: v20181114", Is this preferred over 9.4.15 because we're on JDK8? http://gerrit.cloudera.org:8080/#/c/12042/5/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java File java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java: http://gerrit.cloudera.org:8080/#/c/12042/5/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@49 PS5, Line 49: private static final Layout LAYOUT = new PatternLayout( : "%d{HH:mm:ss.SSS} [%p - %t] (%F:%L) %m%n"); Is it worth using this in CapturingLogAppender too? http://gerrit.cloudera.org:8080/#/c/12042/5/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/ResultReporter.java File java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/ResultReporter.java: http://gerrit.cloudera.org:8080/#/c/12042/5/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/ResultReporter.java@54 PS5, Line 54: private boolean reportResults = true; Why this default? What happens if we try reporting results but aren't properly configured? http://gerrit.cloudera.org:8080/#/c/12042/5/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java File java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java: http://gerrit.cloudera.org:8080/#/c/12042/5/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java@110 PS5, Line 110: capturer.close(); Would this make sense in the `finally` block? Or is it only here to quiesce further logging before reporting a failure? -- To view, visit http://gerrit.cloudera.org:8080/12042 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I34f88363dbf52c2f7bba50fbfb5de059f88e7f74 Gerrit-Change-Number: 12042 Gerrit-PatchSet: 5 Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Thu, 28 Mar 2019 18:15:29 +0000 Gerrit-HasComments: Yes
