Re: [PATCH 6/6] t/README: A new section about test coverage

On Sat, Jul 24, 2010 at 21:25, Jonathan Nieder [off-list ref] wrote:

Ævar Arnfjörð Bjarmason wrote:

quoted

Document how test writers can generate coverage reports

Very neat!

Thanks for the review.

quoted

--- a/t/README
+++ b/t/README

@@ -267,6 +267,9 @@ Do:

      git merge hla &&
      git push gh &&
      test ...
+
+ - Check the test coverage for your tests. See the "Test coverage"
+   below.

 Don't:

I have a moment’s hesitation reading this, because I suspect test
coverage checking would be most useful if test authors were _not_ to
pay too much attention to it.

Imagine that the git test suite is almost perfect, so it checks all
the important behavior of git, including edge cases (yes, unlikely,
but bear with me for a moment).  Then the test coverage data would be
very useful indeed: it would point out code that is not actually
needed for anything.

However, if new authors make 99% coverage a goal while writing
tests, the result will be lots of useless tests that check
behavior no one cares about and less useful coverage information.

What I was going for here is that you should try to make sure that the
code you're adding is covered by tests by running the coverage tests.

I.e. if I add a new function "blah" to git-whatever which is
implemented by the "do_blah" function checking if every line of
"do_blah" is covered is an excellent indicator of whether that code is
being exhaustively tested, as opposed to just superficially tested.

In most cases a low test coverage counts is telling about the overall
quality of the tests.

But, the wording can probably be improved. Do you have a suggestion
for the above intent compressed into a sentence or two? I can't come
up with anything right now.

quoted

@@ -508,3 +511,40 @@ the purpose of t0000-basic.sh, which is to isolate that level of

 validation in one place.  Your test also ends up needing
 updating when such a change to the internal happens, so do _not_
 do it and leave the low level of validation to t0000-basic.sh.
+
+Test coverage
+-------------
+
+You can use the coverage tests to find out if your tests are really
+testing your code code. To do that, run the coverage target at the
+top-level (not in the t/ directory):

In other words, I would rather the rationale here read:

       You can use the coverage tests to find code paths that are not being
       properly exercised yet. To do that...

I think it is great if people write new tests that do not exercise
their own code but instead explore related behavior.

That wording is better, thanks.

That said, with or without any of the changes implied above,

Reviewed-by: Jonathan Nieder <redacted>

Thanks.