Re: [PATCH] Teach checkout the -n|--dry-run option

[Jens Lehmann <Jens.Lehmann@web.de>]

Am 08.05.2011 23:30, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> diff --git a/t/t1000-read-tree-m-3way.sh b/t/t1000-read-tree-m-3way.sh
>> index ca8a409..bcfb5e6 100755
>> --- a/t/t1000-read-tree-m-3way.sh
>> +++ b/t/t1000-read-tree-m-3way.sh
>> @@ -259,6 +259,8 @@ test_expect_success \
>>      "rm -f .git/index AA &&
>>       cp .orig-A/AA AA &&
>>       git update-index --add AA &&
>> +     git read-tree -n -m $tree_O $tree_A $tree_B &&
>> +     test_must_fail check_result &&
> 
> That's a rather sloppy way to test that the command did not do anything to
> compare one possible outcome, instead of verifying that the result is
> identical to the original condition, no?
> 
> How about
> 
> 	...
> 	git update_index --add AA &&
>         git ls-files -s >pre-dry-run &&
>         git read-tree -n -m $tree_O $tree_A $tree_B &&
>         git ls-files -s >post-dry-run &&
>         test_cmp pre-dry-run post-dry-run &&
>         ...

Yeah, that is much better than my first sketch.

> We also should be checking that the command reports it is going to fail
> when it should as well.  Always remember to check both sides of the coin.
> 
>>       git read-tree -m $tree_O $tree_A $tree_B &&
>>       check_result"
> 
> I suspect that it would make sense to replace
> 
> 	git read-tree $args && check_result
> 
> with
>         
> 	read_tree_must_succeed $args
> 
> and
> 
> 	test_must_fail git read-tree $args
> 
> with
> 
> 	read_tree_must_fail $args
> 
> and implement two shell wrappers, perhaps like this.
> 
> read_tree_must_succeed () {
> 	git ls-files -s >pre-dry-run &&
> 	git read-tree -n "$@" &&
>         git ls-files -s >post-dry-run &&
>         test_cmp pre-dry-run post-dry-run &&
>         git read-tree "$@"
> }
> 
> read_tree_must_fail () {
> 	git ls-files -s >pre-dry-run &&
> 	test_must_fail git read-tree -n "$@" &&
>         git ls-files -s >post-dry-run &&
> 	test_must_fail git read-tree "$@"
> }

Thanks, will send an updated patch with better test cases soon ...