Todd Lipcon has posted comments on this change. Change subject: Fix and improve code coverage ......................................................................
Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/2448/2/README.adoc File README.adoc: Line 242 > Is it no longer true (that lcov produces better HTML)? Or just irrelevant? I dont think lcov would work well with the llvm-generated stuff, but the next version of llvm has an llvm-cov genhtml. Either way I've never really used this, and I doubt anyone else has either, so just removed it. http://gerrit.cloudera.org:8080/#/c/2448/2/build-support/jenkins/build-and-test.sh File build-support/jenkins/build-and-test.sh: Line 306: -r $SOURCE_ROOT/src \ > Note that BUILD_ROOT/src is where you'd find the generated PB source. Shoul I don't think so, because it generates lots of methods which aren't used and I dont think we need to be writing unit tests for them :) Next revision actually changes this a bit, because using src/ as the root confused Sonar (despite the jenkins visualizer being fine). Switching to using SOURCE_ROOT but adding a filter for src/kudu http://gerrit.cloudera.org:8080/#/c/2448/2/src/kudu/util/logging.cc File src/kudu/util/logging.cc: Line 141: needs_flush = false; > This must be single-threaded, otherwise I'd expect TSAN would complain abou yea actually next rev substantially changes this stuff, because it turns out that not only might this be multi-threaded (two threads FATAL at the same time), it could even be re-entrant (if for some reason we crash in the middle of flushing coverage). added various guards to prevent this (gcov_flush needs external synchronization) -- To view, visit http://gerrit.cloudera.org:8080/2448 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If0273fe095d8f160f0068ec421fceb54eda588de Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
