Hi Jonathan, Jonathan Nieder writes: > I suspect this test would be easier to understand if split up (yes, I > know I'm guilty of the same in other tests from this file). I sort of tried to follow what was done in the rest of the file. If this style is desired instead, I'll post an alternative series instead. > test_expect_success 'setup: clear [merge] configuration' ' > test_might_fail git config --unset-all merge.log && > test_might_fail git config --unset-all merge.summary > ' > > test_expect_success 'set up FETCH_HEAD' ' > git checkout master && > git fetch . left > ' Okay. > test_expect_success 'merge.log=3 limits shortlog length' ' > test_might_fail git config --unset merge.log && > test_might_fail git config --unset merge.summary && Redundant, as you pointed out in another email. > cat >expected <<-\EOF && > Left #3 > ... > EOF > git -c merge.log=3 fmt-merge-msg <.git/FETCH_HEAD >msg && > > tail -n 2 msg >actual && > test_cmp expected actual > ' Ok. I don't like the `tail` thing. Why are you doing it instead of comparing full outputs like the tests in the rest of the file? > test_expect_success 'shortlog length defaults to 20' ' > [...] > ' Actually, we can't test this unless we actually create a branch with 20 commits and merge it. I've dropped this- you might like to add it during the cleanup. > test_expect_success 'merge.log=5 does not limit shortlog length' ' > [...] > ' Okay. > test_expect_success 'merge.log=6 does not limit shortlog length' ' > [...] > ' I thought this was a bit of an overkill, so I removed it. > test_expect_success 'merge.log=0 disables shortlog' ' > [...] > ' Okay. > test_expect_success 'merge.log=-1 does something sane' ' > [...] > ' Thanks! I found a bug and fixed it (patch will come in a moment). > I can clean up the earlier tests afterwards. :) Okay, sounds good. -- Ram -- 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