Re: [PATCH 2/2] tests: fix diff order arguments in test_cmp

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 7, 2017, 02:29 UTC
Message-ID: <xmqqwp47u1vk.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171006190006.19623-2-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 13 quoted lines

> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
> index ec5f530102..42f584f8b3 100755
> --- a/t/t4205-log-pretty-formats.sh
> +++ b/t/t4205-log-pretty-formats.sh
> @@ -590,7 +590,7 @@ test_expect_success '%(trailers:unfold) unfolds trailers' '
>  test_expect_success ':only and :unfold work together' '
>  	git log --no-walk --pretty="%(trailers:only:unfold)" >actual &&
>  	git log --no-walk --pretty="%(trailers:unfold:only)" >reverse &&
> -	test_cmp actual reverse &&
> +	test_cmp reverse actual &&
>  	{
>  		grep -v patch.description <trailers | unfold &&
>  		echo

This test is trying to see that giving the two modifers in any order produces identical results, so the result of swaping is no more or no less correct than the original, because what this test calls "actual" is no less authoritative result than what is in "reverse" at this point, which is the reason why we prefer "expect actual" in the typical use of test_cmp.

Renaming <actual, reverse> to (rather meaningless) <one, two> or <only-unfold, unfold-only> might make the intention a bit clearer.

THanks.