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

From: Johannes Sixt <j6t@kdbg.org>
Date: Apr 2, 2016, 18:50 UTC
Message-ID: <5700145E.4060308@kdbg.org>
In-Reply-To: <1459619912-5445-7-git-send-email-mehul.jain2029@gmail.com>

Am 02.04.2016 um 19:58 schrieb Mehul Jain:

Show 32 quoted lines

> 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