Subject: Re: [PATCH v3] format-patch: assume --cover-letter for diff in multi-patch series

From: Junio C Hamano <[email-masked]>
Date: Wed, 05 Jun 2024 13:44:27 -0700

Rubén Justo <[email-masked]> writes:

> +test_expect_success "format-patch --range-diff, implicit --cover-letter" '
> +	test_must_fail git format-patch --no-cover-letter \
> +		-v2 --range-diff=topic main..unmodified &&
> +	test_must_fail git -c format.coverLetter=no format-patch \
> +		-v2 --range-diff=topic main..unmodified &&
> +	git format-patch -v2 --range-diff=topic main..unmodified &&
> +	test_when_finished "rm v2-000?-*" &&
> +	test_grep "^Range-diff against v1:$" v2-0000-cover-letter.patch
> +'

Isn't this doing three separate things in a single test?  Unless it
is the local convention in this script, let's split them to three.
If "--no-cover-letter" fails to prevent v2-* files from getting
created, it would fail without hitting test_when_finished.  v2 was
already bad enough in that regard, but piling two more things that
could fail on top is making it even worse, no?

> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index ba85b582c5..b96348eebd 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -2492,6 +2492,16 @@ test_expect_success 'interdiff: solo-patch' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'interdiff: multi-patch, implicit --cover-letter' '
> +	test_must_fail git format-patch --no-cover-letter \
> +		--interdiff=boop~2 -2 -v23 &&
> +	test_must_fail git -c format.coverLetter=no format-patch \
> +		--interdiff=boop~2 -2 -v23 &&
> +	git format-patch --interdiff=boop~2 -2 -v23 &&
> +	test_grep "^Interdiff against v22:$" v23-0000-cover-letter.patch &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'format-patch does not respect diff.noprefix' '
>  	git -c diff.noprefix format-patch -1 --stdout >actual &&
>  	grep "^--- a/blorp" actual