Re: [PATCH v3 2/3] test: add test for --[no-]autostash flag

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Mehul Jain <mehul.jain2029@gmail.com> writes:

> Signed-off-by: Mehul Jain <mehul.jain2029@gmail.com>
> ---
>  t/t5520-pull.sh         | 19 +++++++++++++++++++
>  t/t5521-pull-options.sh | 16 ++++++++++++++++

There's no need to split code/test/doc into separate patches, except if
your patches are getting really huge. As a reviewer, I like having tests
and doc in the same patch because they describe the intention of the
programmer.

We try to have a history where each commit is equally good, and with
your version there are two commits where --autostash exists and is
undocumented (which is not catastrophic, though).

> +test_expect_success 'pull --rebase --no-autostash fails with dirty working directory and rebase.autstash set true' '
> +	test_config rebase.autostash true &&
> +	git reset --hard before-rebase &&
> +	echo dirty >new_file &&
> +	git add new_file &&
> +	test_must_fail git pull --rebase --no-autostash . copy
> +'
> +
> +test_expect_success 'pull --rebase --autostash succeeds with dirty working directory and rebase.autstash set false' '
> +	test_config rebase.autostash false &&
> +	git reset --hard before-rebase &&
> +	echo dirty >new_file &&
> +	git add new_file &&
> +	git pull --rebase --autostash . copy &&
> +	test_cmp_rev HEAD^ copy &&
> +	test "$(cat new_file)" = dirty &&
> +	test "$(cat file)" = "modified again"
> +'

Sounds good.

> --- a/t/t5521-pull-options.sh
> +++ b/t/t5521-pull-options.sh
> @@ -62,6 +62,22 @@ test_expect_success 'git pull -v --rebase' '
>  	test_must_be_empty out)
>  '
>  
> +test_expect_success 'git pull --rebase --autostash' '
> +	mkdir clonedrbas &&
> +	(cd clonedrbas  && git init &&
> +	git pull --rebase --autostash "../parent" >out 2>err &&
> +	test -s err &&
> +	test_must_be_empty out)
> +'
> +
> +test_expect_success 'git pull --rebase --no-autostash' '
> +	mkdir clonedrbnas &&
> +	(cd clonedrbnas  && git init &&
> +	git pull --rebase --no-autostash "../parent" >out 2>err &&
> +	test -s err &&
> +	test_must_be_empty out)
> +'

Not sure these tests are needed if you have the ones in t/t5520-pull.sh.
More tests means more time to run so testing twice the same thing has a
cost.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/