Am 08.05.2011 23:30, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@xxxxxx> 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 ...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html