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

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

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Thu, Jul 10, 2014 at 8:31 PM, David Turner <dturner@twopensource.com> wrote:
>> Add tests to confirm that invalidation of subdirectories neither over-
>> nor under-invalidates.
>>
>> Signed-off-by: David Turner <dturner@twitter.com>
>> ---
>>  t/t0090-cache-tree.sh | 26 +++++++++++++++++++++++---
>>  1 file changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
>> index 98fb1ab..3a3342e 100755
>> --- a/t/t0090-cache-tree.sh
>> +++ b/t/t0090-cache-tree.sh
>> @@ -22,9 +22,10 @@ test_shallow_cache_tree () {
>>  }
>>
>>  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
>> +       printf "invalid                                  %s ()\n" "" "$@" >expect &&

Hmm.  This will always expect that the top-level is invalid, even
when $# is 0.  It is OK if you never need to use this to test that a
cache-tree is fully valid, but is it something we want to check?

Existing tests are mostly about "cache-tree is populated fully at a
few strategic, well known and easy places and then it degrades over
time", but after all your series is adding more places to that set
of "a few places", so we may want to make sure that future breakages
to the new code paths that "repair" the cache-tree are caught by
these tests.

>> +       test-dump-cache-tree | \
>
> nit: unnecessary backslash

Good eyes ;-)

>> +       sed -n -e "s/[0-9]* subtrees//" -e '/#(ref)/d' -e '/^invalid /p' >actual &&

Is the second one to remove "#(ref)", which appears for a good
"reference" cache tree entry shown for comparison, necessary?  Do
they ever begin with "invalid"?  If they ever begin with "invalid"
that itself may even be a noteworthy breakage to catch, no?

>> +       test_cmp expect actual
>>  }
>>
>>  test_no_cache_tree () {
>> @@ -49,6 +50,25 @@ test_expect_success 'git-add invalidates cache-tree' '
>>         test_invalid_cache_tree
>>  '
>>
>> +test_expect_success 'git-add in subdir invalidates cache-tree' '
>> +       test_when_finished "git reset --hard; git read-tree HEAD" &&
>> +       mkdir dirx &&
>> +       echo "I changed this file" >dirx/foo &&
>> +       git add dirx/foo &&
>> +       test_invalid_cache_tree
>> +'
>> +
>> +test_expect_success 'git-add in subdir does not invalidate sibling cache-tree' '
>> +       git tag no-children &&
>> +       test_when_finished "git reset --hard no-children; git read-tree HEAD" &&
>> +       mkdir dir1 dir2 &&
>> +       test_commit dir1/a &&
>> +       test_commit dir2/b &&
>> +       echo "I changed this file" >dir1/a &&
>> +       git add dir1/a &&
>> +       test_invalid_cache_tree dir1/
>> +'
>> +
>>  test_expect_success 'update-index invalidates cache-tree' '
>>         test_when_finished "git reset --hard; git read-tree HEAD" &&
>>         echo "I changed this file" >foo &&
>> --
>> 2.0.0.390.gcb682f8