Re: [PATCH v3 4/4] lib-submodule-update: use callbacks in test_submodule_switch_common()

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 12, 2020, 18:46 UTC
Message-ID: <xmqqy2os15s0.fsf@gitster.c.googlers.com>
In-Reply-To: <74e6086da451a4ce2ac52b04a2399ef332d61047.1591897173.git.liu.denton@gmail.com>

Denton Liu <liu.denton@gmail.com> writes:

Show 16 quoted lines

>  test_submodule_switch_common () {
> -	command="$1"
> +	command="$1" # should be a git command
> +	before="$2"
> +	after="$3"
> +
> +	if test -z "$before"
> +	then
> +		before=true
> +	fi
> +
> +	if test -z "$after"
> +	then
> +		after=true
> +	fi
> +

	command="$1" before=${2:-true} after=${3:-true}

should be sufficient? Spelling 'true' as ':' would make it even shorter but probably too cryptic ;-)

Show 11 quoted lines

> diff --git a/t/t3426-rebase-submodule.sh b/t/t3426-rebase-submodule.sh
> index 788605ccc0..d31e6487bd 100755
> --- a/t/t3426-rebase-submodule.sh
> +++ b/t/t3426-rebase-submodule.sh
> @@ -16,11 +16,10 @@ git_rebase () {
>  	git revert HEAD &&
>  	git status -su >actual &&
>  	ls -1pR * >>actual &&
> -	test_cmp expect actual &&
> -	git rebase "$1"
> +	test_cmp expect actual

We used to run "git rebase" in this helper, but that is the thing we expect to succeed or to fail, so with the revamped test framework, this is split out to just be the "before testing" helper function?

Makes sense.

> -test_submodule_switch_func "git_rebase"
> +test_submodule_switch_func "rebase \$arg" "git_rebase"

Show 13 quoted lines

>  git_rebase_interactive () {
>  	git status -su >expect &&
> @@ -34,11 +33,10 @@ git_rebase_interactive () {
>  	ls -1pR * >>actual &&
>  	test_cmp expect actual &&
>  	set_fake_editor &&
> -	echo "fake-editor.sh" >.git/info/exclude &&
> -	git rebase -i "$1"
> +	echo "fake-editor.sh" >.git/info/exclude
>  }
>  
> -test_submodule_switch_func "git_rebase_interactive"
> +test_submodule_switch_func "rebase -i \$arg" "git_rebase_interactive"

Likewise.