Subject: Re: [PATCH v3] am: Allow passing --no-verify flag

From: Junio C Hamano <[email-masked]>
Date: Tue, 29 Nov 2022 10:20:05 +0900

Thierry Reding <[email-masked]> writes:

> +test_expect_success 'am with failing applypatch-msg hook (no verify)' '
> +	rm -fr .git/rebase-apply &&
> +	git reset --hard &&
> +	git checkout first &&
> +	test_hook applypatch-msg <<-\EOF &&
> +	exit 1
> +	EOF
> +	git am --no-verify patch1
> +'
> +
>  test_expect_success 'am with pre-applypatch hook' '
>  	rm -fr .git/rebase-apply &&
>  	git reset --hard &&
> @@ -374,6 +384,16 @@ test_expect_success 'am with failing pre-applypatch hook' '
>  	test_cmp_rev first HEAD
>  '
>  
> +test_expect_success 'am with failing pre-applypatch hook (no verify)' '
> +	rm -fr .git/rebase-apply &&
> +	git reset --hard &&
> +	git checkout first &&
> +	test_hook pre-applypatch <<-\EOF &&
> +	exit 1
> +	EOF
> +	git am --no-verify patch1
> +'
> +
>  test_expect_success 'am with post-applypatch hook' '
>  	rm -fr .git/rebase-apply &&
>  	git reset --hard &&

These two tests will still pass if you change the implementation to
run the hook and simply ignore its exit status, but I recall you
making a good argument against that alternative implementation ,in
comparison to "not running the hook at all".

I think these tests should make sure that the hooks did not even
run.  Perhaps by creating a marker file before running "git am",
adding a "rm" that marker file in the hook, and making sure that
the marker file still exists after "git am" returns, or something
like that.