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

[Jonathan Nieder <jrnieder@gmail.com>]

Ævar Arnfjörð Bjarmason wrote:

> Document how test writers can generate coverage reports

Very neat!

> --- 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.

> @@ -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 said, with or without any of the changes implied above,

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.