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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 29, 2022, 01:20 UTC
Message-ID: <xmqqv8my4kvu.fsf@gitster.g>
In-Reply-To: <20221128174825.1510407-1-thierry.reding@gmail.com>

Thierry Reding <thierry.reding@gmail.com> writes:

Show 30 quoted lines

> +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.