Re: [PATCH v12 4/5] t7507-commit-verbose: improve test coverage by testing number of diffs

> On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> Make the fake "editor" store output of grep in a file so that we can
>> see how many diffs were contained in the message and use them in
>> individual tests where ever it is required. Also use write_script()
>> to create the fake "editor".
>>
>> A subsequent commit will introduce scenarios where it is important to be
>> able to exactly determine how many diffs were present.
>
> These two sentences are backward. The "subsequent commit" bit is
> justification for why you are making the "editor" store the output,
> thus it belongs with the first paragraph. The bit about write_script()
> is just a minor aside which can go in its own paragraph.
>
> I think it's also important to explain that you're changing the
> behavior of write_script() so that it always succeeds, regardless of
> whether grep found diff headers or not, and to give the reason for
> making this change ("so that you don't have to use 'test_must_fail'
> for cases when no diff headers are expected and can instead easily use
> 'test_line_count = 0'").
>
> The patch itself looks fine and is easy enough to read.

>
>> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
>> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
>> ---
>> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
>> index 2ddf28c..0f28a86 100755
>> --- a/t/t7507-commit-verbose.sh
>> +++ b/t/t7507-commit-verbose.sh
>> @@ -3,11 +3,10 @@
>>  test_description='verbose commit template'
>>  . ./test-lib.sh
>>
>> -cat >check-for-diff <<EOF
>> -#!$SHELL_PATH
>> -exec grep '^diff --git' "\$1"
>> +write_script "check-for-diff" <<\EOF &&
>> +grep '^diff --git' "$1" >out
>> +exit 0
>>  EOF
>> -chmod +x check-for-diff
>>  test_set_editor "$PWD/check-for-diff"
>>
>>  cat >message <<'EOF'
>> @@ -23,7 +22,8 @@ test_expect_success 'setup' '
>>  '
>>
>>  test_expect_success 'initial commit shows verbose diff' '
>> -       git commit --amend -v
>> +       git commit --amend -v &&
>> +       test_line_count = 1 out
>>  '
>>
>>  test_expect_success 'second commit' '
>> @@ -39,13 +39,15 @@ check_message() {
>>
>>  test_expect_success 'verbose diff is stripped out' '
>>         git commit --amend -v &&
>> -       check_message message
>> +       check_message message &&
>> +       test_line_count = 1 out
>>  '
>>
>>  test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '
>>         git config diff.mnemonicprefix true &&
>>         git commit --amend -v &&
>> -       check_message message
>> +       check_message message &&
>> +       test_line_count = 1 out
>>  '