On Sun, Feb 21, 2016 at 1:55 PM, Felipe Gonçalves Assis <felipeg.assis@xxxxxxxxx> wrote: > On 21 February 2016 at 14:52, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: >> On Sun, Feb 21, 2016 at 10:09 AM, Felipe Gonçalves Assis >> <felipeg.assis@xxxxxxxxx> wrote: >>> +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. Looking at the code itself and its history, this seems to be a deliberate decision, so the test may be appropriate, however, I defer to Junio's judgment. >>> +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 The last option would be most straightforward, however, looking at diff.c:parse_rename_score(), I see that it never actually errors out (by returning -1), so you may not (presently) be able to test these cases. It seems (presently) that merge-recursive.c:parse_merge_opt() can only catch a usage error if nothing follows the '='. -- 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