Re: [PATCH v15 7/7] t/t7507: tests for broken behavior of status

From: Junio C Hamano <gitster@pobox.com>
Date: May 2, 2016, 23:07 UTC
Message-ID: <xmqq7ffcqct1.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1462046616-2582-7-git-send-email-pranit.bauva@gmail.com>

Pranit Bauva <pranit.bauva@gmail.com> writes:

Show 19 quoted lines

> Variable named 'verbose' in builtin/commit.c is consumed by git-status
> and git-commit so if a new verbose related behavior is introduced in
> git-commit, then it should not affect the behavior of git-status.
>
> One previous commit (title: commit: add a commit.verbose config
> variable) introduced a new config variable named commit.verbose,
> so care should be taken that it would not affect the behavior of
> status.
>
> Another previous commit (title: "parse-options.c: make OPTION_COUNTUP
> respect "unspecified" values") changes the initial value of verbose
> from 0 to -1. This can cause git-status to display a verbose output even
> when it isn't supposed to.
>
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
>
> ---
> This is a split off from the previous patch 6/6 as suggested by Eric
> Sunshine.

If these are documenting what your previous patches broke, then there test body should describe what should happen, and then if it is broken, use test_expect_failure, no?

Your first test does "run status with commit.verbose is set, and make sure the "diff --git" does not appear", which is correct, so if it does not work, test_expect_failure would be the right thing to use.

These, especially the latter, look rather unpleasant regressions to me, and the main commit.verbose change would need to be held back before they are fixed.

Show 23 quoted lines

> ---
>  t/t7507-commit-verbose.sh | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
> index 2bb6d8d..00e0c3d 100755
> --- a/t/t7507-commit-verbose.sh
> +++ b/t/t7507-commit-verbose.sh
> @@ -144,4 +144,14 @@ do
>  	"
>  done
>  
> +test_expect_success 'status ignores commit.verbose=true' '
> +	git -c commit.verbose=true status >actual &&
> +	! grep "^diff --git" actual
> +'
> +
> +test_expect_success 'status does not verbose without --verbose' '
> +	git status >actual &&
> +	! grep "^diff --git" actual
> +'
> +
>  test_done