Re: [PATCH v2 6/7] t5520: reduce commom lines of code

[Johannes Sixt <j6t@kdbg.org>]

Am 02.04.2016 um 19:58 schrieb Mehul Jain:
> These two tests are almost similar and thus can be folded in a for-loop.
>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Mehul Jain <mehul.jain2029@gmail.com>
> ---
>   t/t5520-pull.sh | 16 +++++++---------
>   1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index fb9f845..e12af96 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -298,15 +298,13 @@ test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
>   	test_pull_autostash_fail --rebase --no-autostash
>   '
>
> -test_expect_success 'pull --autostash (without --rebase) should error out' '
> -	test_must_fail git pull --autostash . copy 2>err &&
> -	test_i18ngrep "only valid with --rebase" err
> -'
> -
> -test_expect_success 'pull --no-autostash (without --rebase) should error out' '
> -	test_must_fail git pull --no-autostash . copy 2>err &&
> -	test_i18ngrep "only valid with --rebase" err
> -'
> +for i in --autostash --no-autostash
> +do
> +	test_expect_success "pull $i (without --rebase) is illegal" '
> +		test_must_fail git pull $i . copy 2>err &&
> +		test_i18ngrep "only valid with --rebase" err
> +	'
> +done

Hm. If the implementation of test_expect_success uses the variable, too, 
its value is lost when the test snippet runs. Fortunately, it does not.

You can make this code a bit more robust by using double-quotes around 
the test code so that $i is expanded before test_expect_success is 
evaluated.

You could also change the variable name, but to be sufficiently safe, 
you would have to use an unsightly long name. 'opt' would be just as bad 
as 'i'.

>
>   test_expect_success 'pull.rebase' '
>   	git reset --hard before-rebase &&
>

-- Hannes