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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 24, 2010, 21:25 UTC
Message-ID: <20100724212542.GA5610@burratino>
In-Reply-To: <1280004663-4887-7-git-send-email-avarab@gmail.com>

Ævar Arnfjörð Bjarmason wrote:

> Document how test writers can generate coverage reports

Very neat!

Show 11 quoted lines

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

Show 11 quoted lines

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