Re: [PATCH 1/2] t3507: pin CHERRY_PICK_HEAD absence for a conflicting --no-commit

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sep 4, 2026, 09:57 UTC
Message-ID: <5e77651d-38a1-451e-b96b-33c91c414eb5@gmail.com>
In-Reply-To: <20260903125524.67889-1-f@lex.la>

Hi Alexsei

On 03/09/2026 13:55, Aleksei Sviridkin wrote:

Show 7 quoted lines

> The tests here check the ref after a conflicting pick, after a clean
> pick and after a clean pick under --no-commit, but not after a
> conflicting one under --no-commit.  That is the combination a user
> runs into by accident: the pick stops with conflicts, and the ref
> "git commit" would take the authorship from is not there.
> 
> Pin it next to its siblings.

What does pinning a test mean?

Show 21 quoted lines

> Letting the ref be written under
> --no-commit when the pick conflicts leaves the rest of the cherry-pick
> tests green, so nothing else guards that path.
> 
> Assisted-by: LLM
> Signed-off-by: Aleksei Sviridkin <f@lex.la>
> ---
>   t/t3507-cherry-pick-conflict.sh | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
> index 44596cb1e8..2ce2e88184 100755
> --- a/t/t3507-cherry-pick-conflict.sh
> +++ b/t/t3507-cherry-pick-conflict.sh
> @@ -100,6 +100,12 @@ test_expect_success 'cherry-pick --no-commit does not set CHERRY_PICK_HEAD' '
>   	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
>   '
>   
> +test_expect_success 'failed cherry-pick --no-commit does not set CHERRY_PICK_HEAD' '
> +	pristine_detach initial &&
> +	test_must_fail git cherry-pick --no-commit picked &&

We already have a test that checks the advice that's printed when there are conflicts, so could just add

	test_must_fail git show-ref --verify CHERRY_PICK_HEAD

there. Because that test checks the command's output, we know that the cherry-pick has failed due to conflicts, and not some other reason. Using test_must_fail() here without checking the error message means we don't verify the reason that the cherry-pick failed.

Thanks

Phillip

Show 8 quoted lines

> +	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
> +'
> +
>   test_expect_success 'cherry-pick w/dirty tree does not set CHERRY_PICK_HEAD' '
>   	pristine_detach initial &&
>   	echo foo >foo &&
> 
> base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc