From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 07 Oct 2017 02:29:19 GMT
Subject: Re: [PATCH 2/2] tests: fix diff order arguments in test_cmp
Message-ID: <xmqqwp47u1vk.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171006190006.19623-2-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

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