Re: [PATCH 3/7] t5528-push-default.sh: add helper functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Matthieu Moy <Matthieu.Moy@xxxxxxx> writes:

> Signed-off-by: Matthieu Moy <Matthieu.Moy@xxxxxxx>
> ---
>  t/t5528-push-default.sh |   45 ++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 38 insertions(+), 7 deletions(-)

The use of helpers makes the body of the test easier to follow.

> diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
> index c334c51..da7d3d8 100755
> --- a/t/t5528-push-default.sh
> +++ b/t/t5528-push-default.sh
> @@ -13,16 +13,47 @@ test_expect_success 'setup bare remotes' '
>  	git push parent2 HEAD
>  '
>  
> +# $1 = local revision
> +# $2 = remote revision (tested to be equal to the local one)
> +check_pushed_commit () {
> +	git log -1 --format='%h %s' >expect &&
> +	git --git-dir=repo1 log -1 --format='%h %s' "$2" >actual &&
> +	test_cmp expect actual
> +}

Hmph.  How is $1 used in the above to make it compare between local and
remote?  Does the first one need to have "$1" before " >expect"?

> +# $1 = push.default value
> +# $2 = expected target branch for the push
> +test_push_success () {
> +	git -c push.default="$1" push &&
> +	check_pushed_commit HEAD "$2"
> +}
> +
> +# $1 = push.default value
> +# other arguments = target branches that should not be touched
> +test_push_failure () {
> +	push_default=$1 &&
> +	shift &&
> +	if test $# -gt 0
> +	then
> +		# branch may not exist
> +		test_might_fail git --git-dir=repo1 \
> +			log --no-walk --format='%h %s' "$@" >expect
> +	fi &&
> +	test_must_fail git -c push.default="$1" &&

What subcommand does this run with one-shot override configuration?  Do
we need " push" before " &&"?

> +	if test $# -gt 0
> +	then
> +		test_might_fail git --git-dir=repo1 \
> +			log -1 --format='%h %s' "$@" >actual
> +	fi &&
> +	test_cmp expect actual
> +}

This allows us to look at not just the failure of the push operation,
but also inspects the resulting repository to make sure things that must
stay intact do, which is very nice.  The callers can even pass "--all"
to it, instead of enumerating the branches, which would be very useful
when testing a single-branch modes like `current` and `upstream`.

>  test_expect_success '"upstream" pushes to configured upstream' '
>  	git checkout master &&
>  	test_config branch.master.remote parent1 &&
>  	test_config branch.master.merge refs/heads/foo &&
> -	test_config push.default upstream &&
>  	test_commit two &&
> -	git push &&
> -	echo two >expect &&
> -	git --git-dir=repo1 log -1 --format=%s foo >actual &&
> -	test_cmp expect actual
> +	test_push_success upstream foo
>  '
>
>  test_expect_success '"upstream" does not push on unconfigured remote' '
> @@ -30,7 +61,7 @@ test_expect_success '"upstream" does not push on unconfigured remote' '
>  	test_unconfig branch.master.remote &&
>  	test_config push.default upstream &&
>  	test_commit three &&
> -	test_must_fail git push
> +	test_push_failure upstream master
>  '

... and we can use --all not master here, right?

>  test_expect_success '"upstream" does not push on unconfigured branch' '
> @@ -39,7 +70,7 @@ test_expect_success '"upstream" does not push on unconfigured branch' '
>  	test_unconfig branch.master.merge &&
>  	test_config push.default upstream
>  	test_commit four &&
> -	test_must_fail git push
> +	test_push_failure upstream master
>  '

Same here.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]