Re: [PATCH v4 3/4] cache-tree: subdirectory tests

[Junio C Hamano <gitster@pobox.com>]

David Turner <dturner@twopensource.com> writes:

> Add tests to confirm that invalidation of subdirectories nether over-
> nor under-invalidates.
>
> Signed-off-by: David Turner <dturner@twitter.com>
> ---
>  t/t0090-cache-tree.sh | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
> index 98fb1ab..8437c5f 100755
> --- a/t/t0090-cache-tree.sh
> +++ b/t/t0090-cache-tree.sh
> @@ -21,10 +21,13 @@ test_shallow_cache_tree () {
>  	cmp_cache_tree expect
>  }
>  
> +# Test that the cache-tree for a given directory is invalid.
> +# If no directory is given, check that the root is invalid
>  test_invalid_cache_tree () {
> -	echo "invalid                                   (0 subtrees)" >expect &&
> -	printf "SHA #(ref)  (%d entries, 0 subtrees)\n" $(git ls-files|wc -l) >>expect &&
> -	cmp_cache_tree expect
> +	test-dump-cache-tree >actual &&
> +	sed -e "s/$_x40/SHA/" -e "s/[0-9]* subtrees//g" <actual >filtered &&
> +	expect=$(printf "invalid                                  $1 ()\n") &&

It would be saner to do 'printf "string %s more string" "$1"' than
embedding caller-supplied "$1" inside the format specifier.

> +	fgrep "$expect" filtered

We'd actually want to see fewer uses of 'fgrep' in the tests for two
reasons.

Is having an entry that is invalidated the only thing we care about
in this test?  Shouldn't the caller expect "These subtrees and
nothing else must be invalidated", in which case the helper should
check not just the expected "invalid dir1/" appears in the output
but no other unexpected "invalid somethingelse/" appears (and this
"no other unexpected output" makes use of grep family in tests like
this less desirable).

In other words, wouldn't it be better to do the helper along the
lines of:

	test_invalidated_cache_tree () {
        	if test $# != 0
		then
	        	printf "invalid %s ()\n" "" "$@"
                fi >expect &&
		test-dump-cache-tree |
                sed -n -e '/^invalid /p' >actual &&
                test_cmp expect actual
	}

and use

	test_invalidated_cache_tree dir1

when we expect only dir1 and dir2 (but not dir2 or anything else) is
invalidated?

Thanks.