Re: [PATCH v4 1/5] stash: improve option parsing test coverage

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 29, 2018, 19:39 UTC
Message-ID: <xmqqr2o2658t.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180328222129.22192-2-joel@teichroeb.net>

Joel Teichroeb <joel@teichroeb.net> writes:

Show 27 quoted lines

> In preparation for converting the stash command incrementally to
> a builtin command, this patch improves test coverage of the option
> parsing. Both for having too many paramerters, or too few.
>
> Signed-off-by: Joel Teichroeb <joel@teichroeb.net>
> ---
>  t/t3903-stash.sh | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index aefde7b17..8a666c60c 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -84,6 +84,17 @@ test_expect_success 'apply stashed changes (including index)' '
>  	test 1 = $(git show HEAD:file)
>  '
>  
> +test_expect_success 'giving too many ref agruments does nothing' '
> +
> +	for type in apply drop pop show "branch stash-branch"
> +	do
> +		test-chmtime =123456789 file &&
> +		test_must_fail git stash $type stash@{0} stash@{1} 2>err &&
> +		test_i18ngrep "Too many" err &&
> +		test 123456789 = $(test-chmtime -v +0 file | sed 's/[^0-9].*$//') || return 1
> +	done
> +'

This is done with "file" whose contents are all different in HEAD, the index and the working tree. If the command tries to "push" by mistake, it will touch the timestamp of "file" and fail the test. That is a reasonable thing to check.

What do stash@{0} and stash{1} record at this point? Would they touch that "file" if the command tries to "apply" or "pop" by mistake? Perhaps it deserves a bit of in-code comment here.

As "drop" or "show" would not touch the working tree anyway, the test for timestamp seems pointless, even though grepping for "Too many" would be a reasonable test.