On 21 February 2016 at 14:52, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Sun, Feb 21, 2016 at 10:09 AM, Felipe Gonçalves Assis
> <felipeg.as...@gmail.com> wrote:
>> merge-recursive: test rename threshold option
>
>> +       git read-tree --reset -u HEAD &&
>> +       test_must_fail git merge-recursive HEAD^ -- HEAD master &&
>> +       check_find_renames_50
>> +'
>> +
>> +test_expect_success 'low rename threshold' '
>> +       git read-tree --reset -u HEAD &&
>> +       test_must_fail git merge-recursive --rename-threshold=25 HEAD^ -- 
>> HEAD master &&
>> +       check_find_renames_25
>> [...]
>> +       check_find_renames_75
>> [...]
>> +       check_find_renames_100
>> +'
>> +
>> +test_expect_success 'rename threshold is truncated' '
>> +       git read-tree --reset -u HEAD &&
>> +       test_must_fail git merge-recursive --rename-threshold=200% HEAD^ -- 
>> HEAD master &&
>> +       check_find_renames_100
>> +'
>
> Is this truncation documented behavior or is it just a detail of the
> current implementation. (Genuine question; I haven't checked the
> documentation or source.) If just an implementation detail, then it
> might not be desirable to formalize it via a test.
>

Not documented. I will remove this. If you prefer to have it
documented and the test added back later, I can do that.

>> +test_expect_success 'last wins in --rename-threshold=<m> 
>> --rename-threshold=<n>' '
>> +       git read-tree --reset -u HEAD &&
>> +       test_must_fail git merge-recursive --rename-threshold=25 
>> --rename-threshold=75 HEAD^ -- HEAD master &&
>> +       check_find_renames_75
>> +'
>
> Would it make sense to add tests checking that invalid
> --rename-threshold= arguments, such as negative and non-numbers,
> correctly error out?
>

I guess so. Can I ask you for a suggestion on how to check this?

Given that merges here usually fail anyway because of the conflicts,
what is the best way of checking the effect of an argument rejection?
1. Check that the merge fails but the index is not changed
2. Check for a specific exit code
3. Use another setup so that the merges succeed


Thanks for the detailed review. Already working on the other comments.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to