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 6: (4 comments) 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: finish(): all events previously captured in append() are now guaranteed to : * be on disk and visible to readers > I don't think it matters. In fact, the CapturingLogAppender use case is one Ack 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; > The default value doesn't really matter, because consider how the ResultRep Makes sense, thanks for the explanation 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: throw t; > This whole thing is kind of funky. I see, yeah I wasn't sure about how this closeable worked, but i think it is more understandable now. http://gerrit.cloudera.org:8080/#/c/12042/6/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/6/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java@58 PS6, Line 58: skipReporting nit: Might be missing something, shouldn't this be reversed? I.e. skipReporting ? null : new ResultReporter() Seems you're using it as enableReporting, but the name seems a bit confusing. -- 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: 6 Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Fri, 29 Mar 2019 00:08:52 +0000 Gerrit-HasComments: Yes
