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

[Eric Sunshine <sunshine@sunshineco.com>]

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
>  '