Re: [PATCH v2 02/14] t5403: refactor rebase post-checkout hook tests

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 9, 2021, 18:24 UTC
Message-ID: <xmqqk0gdskkh.fsf@gitster.g>
In-Reply-To: <a67a5a03b94ccbaec85da2c93ce5bbffcd5cfac9.1638975482.git.gitgitgadget@gmail.com>

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 5 quoted lines

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> These tests only test the default backend and do not check that the
> arguments passed to the hook are correct. Fix this by running the
> tests with both backends and adding checks for the hook arguments.

Nice.

Show 33 quoted lines

> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  t/t5403-post-checkout-hook.sh | 42 ++++++++++++++++++++++-------------
>  1 file changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> index 1ec9e23be75..272b02687ba 100755
> --- a/t/t5403-post-checkout-hook.sh
> +++ b/t/t5403-post-checkout-hook.sh
> @@ -49,23 +49,33 @@ test_expect_success 'post-checkout receives the right args when not switching br
>  	test $old = $new && test $flag = 0
>  '
>  
> -test_expect_success 'post-checkout is triggered on rebase' '
> -	test_when_finished "rm -f .git/post-checkout.args" &&
> -	git checkout -b rebase-test main &&
> -	rm -f .git/post-checkout.args &&
> -	git rebase rebase-on-me &&
> -	read old new flag <.git/post-checkout.args &&
> -	test $old != $new && test $flag = 1
> -'
> +test_rebase () {
> +	args="$*" &&
> +	test_expect_success "post-checkout is triggered on rebase $args" '
> +		test_when_finished "rm -f .git/post-checkout.args" &&
> +		git checkout -B rebase-test main &&
> +		rm -f .git/post-checkout.args &&
> +		git rebase $args rebase-on-me &&
> +		read old new flag <.git/post-checkout.args &&
> +		test_cmp_rev main $old &&
> +		test_cmp_rev rebase-on-me $new &&
> +		test $flag = 1
> +	'

OK, so we now make sure $old and $new are what we expect, in addition to what $flag we got. And the change from -b to -B is understandable as this needs to be prepared to run more than once.

Show 19 quoted lines

> -test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
> -	test_when_finished "rm -f .git/post-checkout.args" &&
> -	git checkout -b ff-rebase-test rebase-on-me^ &&
> -	rm -f .git/post-checkout.args &&
> -	git rebase rebase-on-me &&
> -	read old new flag <.git/post-checkout.args &&
> -	test $old != $new && test $flag = 1
> -'
> +	test_expect_success "post-checkout is triggered on rebase $args with fast-forward" '
> +		test_when_finished "rm -f .git/post-checkout.args" &&
> +		git checkout -B ff-rebase-test rebase-on-me^ &&
> +		rm -f .git/post-checkout.args &&
> +		git rebase $args rebase-on-me &&
> +		read old new flag <.git/post-checkout.args &&
> +		test_cmp_rev rebase-on-me^ $old &&
> +		test_cmp_rev rebase-on-me $new &&
> +		test $flag = 1
> +	'
> +}

Likewise.

> +test_rebase --apply &&
> +test_rebase --merge

I am not sure if "&&" is appropriate here.  It is like saying

	test_expect_success "test on apply" '
		body of apply test
	' &&

	test_expect_success "test on merge" '
		body of merge test
	'

no? In other words, even if somebody broke the apply backend, we still are interested in seeing the merge backend succeed.

>  test_expect_success 'post-checkout hook is triggered by clone' '
>  	mkdir -p templates/hooks &&