Re: [PATCH 1/6] t/t5528-push-default: remove redundant test_config lines

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 19, 2013, 19:26 UTC
Message-ID: <7vbo71zyta.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1371640304-26019-2-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 32 quoted lines

> The line
>
>   test_config push.default upstream
>
> appears unnecessarily in two tests, as the final test_push_failure sets
> push.default before pushing anyway.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  t/t5528-push-default.sh | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
> index 4736da8..69ce6bf 100755
> --- a/t/t5528-push-default.sh
> +++ b/t/t5528-push-default.sh
> @@ -48,7 +48,6 @@ test_expect_success '"upstream" pushes to configured upstream' '
>  test_expect_success '"upstream" does not push on unconfigured remote' '
>  	git checkout master &&
>  	test_unconfig branch.master.remote &&
> -	test_config push.default upstream &&
>  	test_commit three &&
>  	test_push_failure upstream
>  '
> @@ -57,7 +56,6 @@ test_expect_success '"upstream" does not push on unconfigured branch' '
>  	git checkout master &&
>  	test_config branch.master.remote parent1 &&
>  	test_unconfig branch.master.merge &&
> -	test_config push.default upstream
>  	test_commit four &&
>  	test_push_failure upstream
>  '

Makes sense, but I wonder if the only remaining test_config should also be eradicated in the test after this one.

    test_expect_success '"upstream" does not push when remotes do not match' '
            git checkout master &&
            test_config branch.master.remote parent1 &&
            test_config branch.master.merge refs/heads/foo &&
            test_config push.default upstream &&
            test_commit five &&
            test_must_fail git push parent2
    '

It uses test_must_fail but that is only because test_push_failure is not equipped to handle anything other than "git push" without arguments. If the helper were updated to do something like this:

	strategy=$1; shift
	...
	test_must_fail git -c push.default="$strategy" push "$@"

then "test_config push.default" here can be removed and all the tests will end with test_push_success or test_push_failure.