{"thread":{"id":"24511","subject":"[PATCH v2 0/7] Detailed test coverage reports for Git","startedAt":"2010-07-25T14:40:54Z","lastAt":"2010-07-25T19:27:19Z","messageCount":17,"participants":["Ævar Arnfjörð Bjarmason","Jonathan Nieder"],"isPatch":true,"patchVersion":2,"patchTotal":7},"messages":[{"id":"146282","messageId":"1280068861-17701-1-git-send-email-avarab@gmail.com","threadId":"24511","inReplyTo":null,"subject":"[PATCH v2 0/7] Detailed test coverage reports for Git","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-25T14:40:54Z","receivedAt":"2010-07-25T14:40:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is v2 of the test coverage series. It addresses all the points\nthat were raised for v1. Here's the diffstat against v1:\n    \n     .gitignore |   15 +++------------\n     Makefile   |   19 +++++++++----------\n     t/README   |   21 ++++++++++++++++-----\n     3 files changed, 28 insertions(+), 27 deletions(-)\n\nAnd the diff since v2:\n    \n    diff --git a/.gitignore b/.gitignore\n    index e02f1f9..baed247 100644\n    --- a/.gitignore\n    +++ b/.gitignore\n    @@ -207,12 +207,3 @@\n    -/*.gcda\n    -/*.gcno\n    -/*.gcov\n    -/builtin/*.gcda\n    -/builtin/*.gcno\n    -/builtin/*.gcov\n    -/xdiff/*.gcda\n    -/xdiff/*.gcno\n    -/xdiff/*.gcov\n    -/compat/*.gcda\n    -/compat/*.gcno\n    -/compat/*.gcov\n    +*.gcda\n    +*.gcno\n    +*.gcov\n    diff --git a/Makefile b/Makefile\n    index c35c348..b6975aa 100644\n    --- a/Makefile\n    +++ b/Makefile\n    @@ -2282,0 +2283 @@ coverage:\n    +object_dirs := $(sort $(dir $(OBJECTS)))\n    @@ -2284,4 +2285,3 @@ coverage-clean:\n    -\t$(RM) *.gcov *.gcda *.gcno\n    -\t$(RM) builtin/*.gcov\n    -\t$(RM) builtin/*.gcda\n    -\t$(RM) builtin/*.gcno\n    +\t$(RM) $(addsuffix *.gcov,$(object_dirs))\n    +\t$(RM) $(addsuffix *.gcda,$(object_dirs))\n    +\t$(RM) $(addsuffix *.gcno,$(object_dirs))\n    @@ -2298,4 +2298,3 @@ coverage-report:\n    -\tgcov -b *.c\n    -\tgcov -b -o builtin builtin/*.c\n    -\tgcov -b -o xdiff xdiff/*.c\n    -\tgcov -b -o compat compat/*.c\n    +\tfor dir in $(object_dirs); do \\\n    +\t\tgcov --preserve-paths --branch-probabilities --all-blocks --object-directory=$$dir $$dir*.c; \\\n    +\tdone\n    @@ -2303 +2302 @@ coverage-report:\n    -coverage-report-untested-functions:\n    +coverage-untested-functions: coverage-report\n    @@ -2308 +2307 @@ coverage-report-untested-functions:\n    -coverage-report-cover-db:\n    +coverage-report-cover-db: coverage-report\n    diff --git a/t/README b/t/README\n    index 718f35d..400e2da 100644\n    --- a/t/README\n    +++ b/t/README\n    @@ -273,0 +274,9 @@ Do:\n    +   Don't blindly follow test coverage metrics, they're a good way to\n    +   spot if you've missed something. If a new function you added\n    +   doesn't have any coverage you're probably doing something wrong,\n    +   but having 100% coverage doesn't necessarily mean that you tested\n    +   everything.\n    +\n    +   Tests that are likely to smoke out future regressions are better\n    +   than tests that just inflate the coverage metrics.\n    +\n    @@ -518,3 +527,5 @@ Test coverage\n    -You can use the coverage tests to find out if your tests are really\n    -testing your code code. To do that, run the coverage target at the\n    -top-level (not in the t/ directory):\n    +You can use the coverage tests to find code paths that are not being\n    +used or properly exercised yet.\n    +\n    +To do that, run the coverage target at the top-level (not in the t/\n    +directory):\n    @@ -532 +543 @@ functions:\n    -    make coverage-report-untested-functions\n    +    make coverage-untested-functions\n    @@ -537 +548 @@ Devel::Cover module. To install it do:\n    -   # On Debian:\n    +   # On Debian or Ubuntu:\n    \nI also rewrote some of the commit messages.\n    \nÆvar Arnfjörð Bjarmason (7):\n  gitignore: Ignore files generated by \"make coverage\"\n  Makefile: Include subdirectories in \"make cover\" reports\n  Makefile: Split out the untested functions target\n  Makefile: Add coverage-report-cover-db target\n  Makefile: Add coverage-report-cover-db-html target\n  t/README: A new section about test coverage\n  t/README: Add a note about the dangers of coverage chasing\n\n .gitignore |    6 ++++++\n Makefile   |   17 +++++++++++++++--\n t/README   |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 72 insertions(+), 2 deletions(-)\n"},{"id":"146284","messageId":"1280068861-17701-2-git-send-email-avarab@gmail.com","threadId":"24511","inReplyTo":"1280068861-17701-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 1/7] gitignore: Ignore files generated by \"make coverage\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-25T14:40:55Z","receivedAt":"2010-07-25T14:40:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"make coverage\" support added by Thomas Rast in 901c369af5 didn't\ncontain a corresponding patch to patch .gitignore.\n\nChange gitignore to ignore the *.gcda, *.gcno and *.gcov files\ngenerated by GCC and our coverage invocations.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n .gitignore |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 14e2b6b..0e61845 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -204,3 +204,7 @@\n *.pdb\n /Debug/\n /Release/\n+*.gcda\n+*.gcno\n+*.gcov\n+/coverage-untested-functions\n-- \n1.7.0.4\n"},{"id":"146283","messageId":"1280068861-17701-3-git-send-email-avarab@gmail.com","threadId":"24511","inReplyTo":"1280068861-17701-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 2/7] Makefile: Include subdirectories in \"make cover\" reports","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-25T14:40:56Z","receivedAt":"2010-07-25T14:40:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"We generate profiling files in all the $(OBJECTS) dirs. Aggregate\nresults from there, and add them to the corresponding clean target.\n\nAlso expand the gcov arguments. Generate reports for things like \"x()\n|| y()\" using --all-blocks, and add --preserve-paths since we're\nprofiling in subdirectories now.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile |    9 +++++++--\n 1 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex bc3c570..a95d260 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2280,8 +2280,11 @@ coverage:\n \t$(MAKE) coverage-build\n \t$(MAKE) coverage-report\n \n+object_dirs := $(sort $(dir $(OBJECTS)))\n coverage-clean:\n-\trm -f *.gcda *.gcno\n+\t$(RM) $(addsuffix *.gcov,$(object_dirs))\n+\t$(RM) $(addsuffix *.gcda,$(object_dirs))\n+\t$(RM) $(addsuffix *.gcno,$(object_dirs))\n \n COVERAGE_CFLAGS = $(CFLAGS) -O0 -ftest-coverage -fprofile-arcs\n COVERAGE_LDFLAGS = $(CFLAGS)  -O0 -lgcov\n@@ -2292,7 +2295,9 @@ coverage-build: coverage-clean\n \t\t-j1 test\n \n coverage-report:\n-\tgcov -b *.c\n+\tfor dir in $(object_dirs); do \\\n+\t\tgcov --preserve-paths --branch-probabilities --all-blocks --object-directory=$$dir $$dir*.c; \\\n+\tdone\n \tgrep '^function.*called 0 ' *.c.gcov \\\n \t\t| sed -e 's/\\([^:]*\\)\\.gcov: *function \\([^ ]*\\) called.*/\\1: \\2/' \\\n \t\t| tee coverage-untested-functions\n-- \n1.7.0.4\n"},{"id":"146286","messageId":"1280068861-17701-4-git-send-email-avarab@gmail.com","threadId":"24511","inReplyTo":"1280068861-17701-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 3/7] Makefile: Split out the untested functions target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-25T14:40:57Z","receivedAt":"2010-07-25T14:40:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the coverage-report target so that it doesn't generate the\ncoverage-untested-functions file by default. I'm adding more targets\nfor doing various things with the gcov files, and they shouldn't all\nrun by default.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex a95d260..e9f8ef8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2298,6 +2298,8 @@ coverage-report:\n \tfor dir in $(object_dirs); do \\\n \t\tgcov --preserve-paths --branch-probabilities --all-blocks --object-directory=$$dir $$dir*.c; \\\n \tdone\n+\n+coverage-untested-functions: coverage-report\n \tgrep '^function.*called 0 ' *.c.gcov \\\n \t\t| sed -e 's/\\([^:]*\\)\\.gcov: *function \\([^ ]*\\) called.*/\\1: \\2/' \\\n \t\t| tee coverage-untested-functions\n-- \n1.7.0.4\n"},{"id":"146285","messageId":"1280068861-17701-5-git-send-email-avarab@gmail.com","threadId":"24511","inReplyTo":"1280068861-17701-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 4/7] Makefile: Add coverage-report-cover-db target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-25T14:40:58Z","receivedAt":"2010-07-25T14:40:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a target to convert the *.gcov files to a Devel::Cover\ndatabase. That database can subsequently be formatted by the cover(1)\ntool, which is included with Devel::Cover.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n .gitignore |    1 +\n Makefile   |    4 ++++\n 2 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 0e61845..bd53c02 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -208,3 +208,4 @@\n *.gcno\n *.gcov\n /coverage-untested-functions\n+/cover_db\ndiff --git a/Makefile b/Makefile\nindex e9f8ef8..f2c680d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2303,3 +2303,7 @@ coverage-untested-functions: coverage-report\n \tgrep '^function.*called 0 ' *.c.gcov \\\n \t\t| sed -e 's/\\([^:]*\\)\\.gcov: *function \\([^ ]*\\) called.*/\\1: \\2/' \\\n \t\t| tee coverage-untested-functions\n+\n+coverage-report-cover-db: coverage-report\n+\tgcov2perl -db cover_db *.gcov\n+\n-- \n1.7.0.4\n"},{"id":"146287","messageId":"1280068861-17701-6-git-send-email-avarab@gmail.com","threadId":"24511","inReplyTo":"1280068861-17701-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 5/7] Makefile: Add coverage-report-cover-db-html target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-25T14:40:59Z","receivedAt":"2010-07-25T14:40:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a target to generate a detailed HTML report for the entire Git\ncodebase using Devel::Cover's cover(1) tool. Output it in\ncover_db_html instead of the default cover_db, so that it isn't mixed\nup with our raw report files.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n .gitignore |    1 +\n Makefile   |    2 ++\n 2 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex bd53c02..baed247 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -209,3 +209,4 @@\n *.gcov\n /coverage-untested-functions\n /cover_db\n+/cover_db_html\ndiff --git a/Makefile b/Makefile\nindex f2c680d..b6975aa 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2307,3 +2307,5 @@ coverage-untested-functions: coverage-report\n coverage-report-cover-db: coverage-report\n \tgcov2perl -db cover_db *.gcov\n \n+coverage-report-cover-db-html: coverage-report-cover-db\n+\tcover -report html -outputdir cover_db_html cover_db\n-- \n1.7.0.4\n"},{"id":"146288","messageId":"1280068861-17701-7-git-send-email-avarab@gmail.com","threadId":"24511","inReplyTo":"1280068861-17701-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 6/7] t/README: A new section about test coverage","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-25T14:41:00Z","receivedAt":"2010-07-25T14:41:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Document how test writers can generate coverage reports, to ensure\nthat their codepaths are being properly exercised.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/README |   42 ++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 42 insertions(+), 0 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 0d1183c..2fa6744 100644\n--- a/t/README\n+++ b/t/README\n@@ -267,6 +267,9 @@ Do:\n \tgit merge hla &&\n \tgit push gh &&\n \ttest ...\n+    \n+ - Check the test coverage for your tests. See the \"Test coverage\"\n+   below.\n \n Don't:\n \n@@ -508,3 +511,42 @@ the purpose of t0000-basic.sh, which is to isolate that level of\n validation in one place.  Your test also ends up needing\n updating when such a change to the internal happens, so do _not_\n do it and leave the low level of validation to t0000-basic.sh.\n+\n+Test coverage\n+-------------\n+\n+You can use the coverage tests to find code paths that are not being\n+used or properly exercised yet.\n+\n+To do that, run the coverage target at the top-level (not in the t/\n+directory):\n+\n+    make coverage\n+    \n+That'll compile Git with GCC's coverage arguments, and generate a test\n+report with gcov after the tests finish. Running the coverage tests\n+can take a while, since running the tests in parallel is incompatible\n+with GCC's coverage mode.\n+\n+After the tests have run you can generate a list of untested\n+functions:\n+\n+    make coverage-untested-functions\n+\n+You can also generate a detailed per-file HTML report using the\n+Devel::Cover module. To install it do:\n+\n+   # On Debian or Ubuntu:\n+   sudo aptitude install libdevel-cover-perl\n+\n+   # From the CPAN with cpanminus\n+   curl -L http://cpanmin.us | perl - --sudo --self-upgrade\n+   cpanm --sudo Devel::Cover\n+   \n+Then, at the top-level:\n+\n+    make coverage-report-cover-db-html\n+\n+That'll generate a detailed cover report in the \"cover_db_html\"\n+directory, which you can then copy to a webserver, or inspect locally\n+in a browser.\n-- \n1.7.0.4\n"},{"id":"146289","messageId":"1280068861-17701-8-git-send-email-avarab@gmail.com","threadId":"24511","inReplyTo":"1280068861-17701-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 7/7] t/README: Add a note about the dangers of coverage chasing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-25T14:41:01Z","receivedAt":"2010-07-25T14:41:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Having no coverage at all is almost always a bad sign, but trying to\nattain 100% coverage everywhere is usually a waste of time. Add a\nparagraph to explain this to future test writers.\n\nInspired-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/README |    9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 2fa6744..400e2da 100644\n--- a/t/README\n+++ b/t/README\n@@ -271,6 +271,15 @@ Do:\n  - Check the test coverage for your tests. See the \"Test coverage\"\n    below.\n \n+   Don't blindly follow test coverage metrics, they're a good way to\n+   spot if you've missed something. If a new function you added\n+   doesn't have any coverage you're probably doing something wrong,\n+   but having 100% coverage doesn't necessarily mean that you tested\n+   everything.\n+\n+   Tests that are likely to smoke out future regressions are better\n+   than tests that just inflate the coverage metrics.\n+\n Don't:\n \n  - exit() within a <script> part.\n-- \n1.7.0.4\n"},{"id":"146292","messageId":"20100725160545.GA25169@burratino","threadId":"24511","inReplyTo":"1280068861-17701-8-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 7/7] t/README: Add a note about the dangers of coverage chasing","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-25T16:05:45Z","receivedAt":"2010-07-25T16:05:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> +   Don't blindly follow test coverage metrics\n\nHmph, that is just common sense, while “you should really not be\npaying any attention to your code while writing tests” is not.  I even\nprefer the text without this patch applied.  So forget I said anything;\nI can find a way to hint at that in t/README later. :)\n\nJonathan\n"},{"id":"146295","messageId":"20100725163742.GA25658@burratino","threadId":"24511","inReplyTo":"1280068861-17701-2-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 1/7] gitignore: Ignore files generated by \"make coverage\"","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-25T16:37:42Z","receivedAt":"2010-07-25T16:37:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nÆvar Arnfjörð Bjarmason wrote:\n\n> Change gitignore to ignore the *.gcda, *.gcno and *.gcov files\n> generated by GCC and our coverage invocations.\n\nGood idea.  The following is nitpicking of the worst kind; sorry.\n\nIt is easier to maintain frequently-changing lists like .gitignore\nand the #include lines in source files if new additions go in some\nlogical place in the middle instead of the end.  There is less\nlock contention that way. :)\n\nSo maybe:\n\ndiff --git i/.gitignore w/.gitignore\nindex 0e61845..57f79ef 100644\n--- i/.gitignore\n+++ w/.gitignore\n@@ -181,6 +181,10 @@\n *.[aos]\n *.py[co]\n .depend/\n+*.gcda\n+*.gcno\n+*.gcov\n+/coverage-untested-functions\n *+\n /config.mak\n /autom4te.cache\n@@ -204,7 +208,3 @@\n *.pdb\n /Debug/\n /Release/\n-*.gcda\n-*.gcno\n-*.gcov\n-/coverage-untested-functions\n-- \n"},{"id":"146297","messageId":"20100725170242.GB25658@burratino","threadId":"24511","inReplyTo":"1280068861-17701-3-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 2/7] Makefile: Include subdirectories in \"make cover\" reports","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-25T17:02:42Z","receivedAt":"2010-07-25T17:02:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> We generate profiling files in all the $(OBJECTS) dirs. Aggregate\n> results from there, and add them to the corresponding clean target.\n> \n> Also expand the gcov arguments. Generate reports for things like \"x()\n> || y()\" using --all-blocks, and add --preserve-paths since we're\n> profiling in subdirectories now.\n\nAll good things.  It might be good to add a GCOVFLAGS that can be\nadded to the \"make\" command line, then.\n\n> +++ b/Makefile\n> @@ -2280,8 +2280,11 @@ coverage:\n>  \t$(MAKE) coverage-build\n>  \t$(MAKE) coverage-report\n>  \n> +object_dirs := $(sort $(dir $(OBJECTS)))\n>  coverage-clean:\n> -\trm -f *.gcda *.gcno\n> +\t$(RM) $(addsuffix *.gcov,$(object_dirs))\n> +\t$(RM) $(addsuffix *.gcda,$(object_dirs))\n> +\t$(RM) $(addsuffix *.gcno,$(object_dirs))\n>  COVERAGE_CFLAGS = $(CFLAGS) -O0 -ftest-coverage -fprofile-arcs\n\nWhat about coverage-untested-functions?\n\n[...]\n> @@ -2292,7 +2295,9 @@ coverage-build: coverage-clean\n>  \t\t-j1 test\n>  \n>  coverage-report:\n> -\tgcov -b *.c\n> +\tfor dir in $(object_dirs); do \\\n> +\t\tgcov --preserve-paths --branch-probabilities --all-blocks --object-directory=$$dir $$dir*.c; \\\n> +\tdone\n\nThis will not error out if \"for\" fails; maybe it would make sense to use\nset -e or exit.\n\nMore importantly, it spews quite a lot of output:\n\n| for dir in ./ block-sha1/ builtin/ compat/ xdiff/; do \\\n|                 gcov --preserve-paths --branch-probabilities --all-blocks --object-directory=$dir $dir*.c || exit; \\\n|         done\n| ./abspath.gcno:version '405p', prefer '404*'\n| ./abspath.gcda:version '405p', prefer version '404*'\n| ./advice.gcno:version '405p', prefer '404*'\n[...]\n| ./ctype.gcno:cannot open graph file\n[...]\n| File 'wt-status.c'\n| Lines executed:36.26% of 535\n| Branches executed:39.78% of 279\n| Taken at least once:20.43% of 279\n| No calls\n| wt-status.c:creating 'wt-status.c.gcov'\n|\n| File 'write_or_die.c'\n[...]\n| block-sha1/sha1.c:creating 'block-sha1#sha1.c.gcov'\n|\n| builtin/add.gcno:version '405p', prefer '404*'\n[...]\n| compat/basename.gcno:cannot open graph file\n| compat/cygwin.gcno:cannot open graph file\n[...]\n\nCan gcov be convinced to be a little quieter (i.e., only useful warnings)?\n(I don’t know; just asking.)\n\nHere are the changes I squashed in for testing; please feel free to\ntake or leave what you like.\n\ndiff --git i/Makefile w/Makefile\nindex a95d260..b791ad5 100644\n--- i/Makefile\n+++ w/Makefile\n@@ -1485,6 +1485,7 @@ ifndef V\n \tQUIET_BUILT_IN = @echo '   ' BUILTIN $@;\n \tQUIET_GEN      = @echo '   ' GEN $@;\n \tQUIET_LNCP     = @echo '   ' LN/CP $@;\n+\tQUIET_GCOV     = @echo '   ' GCOV $@;\n \tQUIET_SUBDIR0  = +@subdir=\n \tQUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \\\n \t\t\t $(MAKE) $(PRINT_DIR) -C $$subdir\n@@ -2285,9 +2286,11 @@ coverage-clean:\n \t$(RM) $(addsuffix *.gcov,$(object_dirs))\n \t$(RM) $(addsuffix *.gcda,$(object_dirs))\n \t$(RM) $(addsuffix *.gcno,$(object_dirs))\n+\t$(RM) coverage-untested-functions\n \n COVERAGE_CFLAGS = $(CFLAGS) -O0 -ftest-coverage -fprofile-arcs\n COVERAGE_LDFLAGS = $(CFLAGS)  -O0 -lgcov\n+GCOVFLAGS = --preserve-paths --branch-probabilities --all-blocks\n \n coverage-build: coverage-clean\n \t$(MAKE) CFLAGS=\"$(COVERAGE_CFLAGS)\" LDFLAGS=\"$(COVERAGE_LDFLAGS)\" all\n@@ -2295,9 +2298,9 @@ coverage-build: coverage-clean\n \t\t-j1 test\n \n coverage-report:\n-\tfor dir in $(object_dirs); do \\\n-\t\tgcov --preserve-paths --branch-probabilities --all-blocks --object-directory=$$dir $$dir*.c; \\\n+\t$(QUIET_GCOV)for dir in $(object_dirs); do \\\n+\t\tgcov $(GCOVFLAGS) --object-directory=$$dir $$dir*.c || exit; \\\n \tdone\n \tgrep '^function.*called 0 ' *.c.gcov \\\n \t\t| sed -e 's/\\([^:]*\\)\\.gcov: *function \\([^ ]*\\) called.*/\\1: \\2/' \\\n-\t\t| tee coverage-untested-functions\n+\t\t> coverage-untested-functions\n-- \n"},{"id":"146301","messageId":"20100725171331.GA8604@burratino","threadId":"24511","inReplyTo":"1280068861-17701-5-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 4/7] Makefile: Add coverage-report-cover-db target","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-25T17:13:31Z","receivedAt":"2010-07-25T17:13:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> +++ b/Makefile\n> @@ -2303,3 +2303,7 @@ coverage-untested-functions: coverage-report\n>  \tgrep '^function.*called 0 ' *.c.gcov \\\n>  \t\t| sed -e 's/\\([^:]*\\)\\.gcov: *function \\([^ ]*\\) called.*/\\1: \\2/' \\\n>  \t\t| tee coverage-untested-functions\n> +\n> +coverage-report-cover-db: coverage-report\n> +\tgcov2perl -db cover_db *.gcov\n> +\n> -- \n\nnitpick: trailing newline.\n\nThis target is not included in .PHONY and it does not seem to be\nphony after all, anyway.  Why not use something like the following?\n\n cover_db: coverage-report\n\tgcov2perl...\n\nIt seems more intuitive to me, and once the makefile learns to track\nhow long a coverage-report remains valid it would allow avoiding a\nrebuild of the cover_db.\n\ndiff --git i/Makefile w/Makefile\nindex f2c680d..c59740c 100644\n--- i/Makefile\n+++ w/Makefile\n@@ -2282,6 +2282,7 @@ coverage:\n \n object_dirs := $(sort $(dir $(OBJECTS)))\n coverage-clean:\n+\t$(RM) cover_db\n \t$(RM) $(addsuffix *.gcov,$(object_dirs))\n \t$(RM) $(addsuffix *.gcda,$(object_dirs))\n \t$(RM) $(addsuffix *.gcno,$(object_dirs))\n@@ -2304,6 +2305,5 @@ coverage-untested-functions: coverage-report\n \t\t| sed -e 's/\\([^:]*\\)\\.gcov: *function \\([^ ]*\\) called.*/\\1: \\2/' \\\n \t\t| tee coverage-untested-functions\n \n-coverage-report-cover-db: coverage-report\n+cover_db: coverage-report\n \tgcov2perl -db cover_db *.gcov\n-\n-- \n"},{"id":"146299","messageId":"20100725171733.GB8604@burratino","threadId":"24511","inReplyTo":"1280068861-17701-6-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 5/7] Makefile: Add coverage-report-cover-db-html target","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-25T17:17:33Z","receivedAt":"2010-07-25T17:17:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> Add a target to generate a detailed HTML report for the entire Git\n> codebase using Devel::Cover's cover(1) tool. Output it in\n> cover_db_html instead of the default cover_db, so that it isn't mixed\n> up with our raw report files.\n\nOh!  Apparently these are directories. ;-)\n\ndiff --git i/.gitignore w/.gitignore\nindex baed247..54280af 100644\n--- i/.gitignore\n+++ w/.gitignore\n@@ -209,4 +209,4 @@\n *.gcov\n /coverage-untested-functions\n /cover_db\n-/cover_db_html\n+/cover_db_html/\ndiff --git i/Makefile w/Makefile\nindex b6975aa..0fdb434 100644\n--- i/Makefile\n+++ w/Makefile\n@@ -2282,6 +2282,7 @@ coverage:\n \n object_dirs := $(sort $(dir $(OBJECTS)))\n coverage-clean:\n+\t$(RM) -r cover_db_html\n \t$(RM) $(addsuffix *.gcov,$(object_dirs))\n \t$(RM) $(addsuffix *.gcda,$(object_dirs))\n \t$(RM) $(addsuffix *.gcno,$(object_dirs))\n@@ -2307,5 +2308,5 @@ coverage-untested-functions: coverage-report\n coverage-report-cover-db: coverage-report\n \tgcov2perl -db cover_db *.gcov\n \n-coverage-report-cover-db-html: coverage-report-cover-db\n+cover_db_html: coverage-report-cover-db\n \tcover -report html -outputdir cover_db_html cover_db\n-- \n"},{"id":"146300","messageId":"20100725172012.GC8604@burratino","threadId":"24511","inReplyTo":"1280068861-17701-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 0/7] Detailed test coverage reports for Git","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-25T17:20:12Z","receivedAt":"2010-07-25T17:20:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> Ævar Arnfjörð Bjarmason (7):\n>   gitignore: Ignore files generated by \"make coverage\"\n>   Makefile: Include subdirectories in \"make cover\" reports\n>   Makefile: Split out the untested functions target\n>   Makefile: Add coverage-report-cover-db target\n>   Makefile: Add coverage-report-cover-db-html target\n>   t/README: A new section about test coverage\n>   t/README: Add a note about the dangers of coverage chasing\n\nWith whatever subset of the changes I have hinted at seems\nsuitable,\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nThanks.\n"},{"id":"146308","messageId":"AANLkTikHXRJcuv35uFrv_g1umVdUM-QD80brFGj94E3N@mail.gmail.com","threadId":"24511","inReplyTo":"20100725172012.GC8604@burratino","subject":"Re: [PATCH v2 0/7] Detailed test coverage reports for Git","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-25T17:46:27Z","receivedAt":"2010-07-25T17:46:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Jul 25, 2010 at 17:20, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> Ævar Arnfjörð Bjarmason (7):\n>>   gitignore: Ignore files generated by \"make coverage\"\n>>   Makefile: Include subdirectories in \"make cover\" reports\n>>   Makefile: Split out the untested functions target\n>>   Makefile: Add coverage-report-cover-db target\n>>   Makefile: Add coverage-report-cover-db-html target\n>>   t/README: A new section about test coverage\n>>   t/README: Add a note about the dangers of coverage chasing\n>\n> With whatever subset of the changes I have hinted at seems\n> suitable,\n>\n> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nAll the changes you made look good, I approve of having them squashed\nwhen this is applied. Thanks.\n"},{"id":"146310","messageId":"20100725174832.GB9146@burratino","threadId":"24511","inReplyTo":"AANLkTikHXRJcuv35uFrv_g1umVdUM-QD80brFGj94E3N@mail.gmail.com","subject":"Re: [PATCH v2 0/7] Detailed test coverage reports for Git","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-25T17:48:32Z","receivedAt":"2010-07-25T17:48:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> On Sun, Jul 25, 2010 at 17:20, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>> Ævar Arnfjörð Bjarmason wrote:\n\n>>> Ævar Arnfjörð Bjarmason (7):\n>>>   gitignore: Ignore files generated by \"make coverage\"\n>>>   Makefile: Include subdirectories in \"make cover\" reports\n>>>   Makefile: Split out the untested functions target\n>>>   Makefile: Add coverage-report-cover-db target\n>>>   Makefile: Add coverage-report-cover-db-html target\n>>>   t/README: A new section about test coverage\n>>>   t/README: Add a note about the dangers of coverage chasing\n[...]\n> All the changes you made look good, I approve of having them squashed\n> when this is applied. Thanks.\n\nI hope not. :)  The change to coverage-clean suggested in my reply to\npatch 4 needs at least a \"-r\" after the $(RM), since I had not noticed\nthat cover_db is a directory.\n"},{"id":"146317","messageId":"AANLkTilcivnC1ZIZ1vDKmMZwCc7Hdc9gSTNYzQEYM08z@mail.gmail.com","threadId":"24511","inReplyTo":"20100725160545.GA25169@burratino","subject":"Re: [PATCH v2 7/7] t/README: Add a note about the dangers of coverage chasing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-25T19:27:19Z","receivedAt":"2010-07-25T19:27:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Jul 25, 2010 at 16:05, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> +   Don't blindly follow test coverage metrics\n>\n> Hmph, that is just common sense,\n\nYou'd be surprised at how uncommon it is when people have 98% coverage\nand try to painfully squeeze out that last 2% :)\n\n> while “you should really not be paying any attention to your code\n> while writing tests” is not.  I even prefer the text without this\n> patch applied.  So forget I said anything; I can find a way to hint\n> at that in t/README later. :)\n\nI don't know whether it should be applied. I just wrote a short\nsummary in response to the previous commentary.\n"}]}