Re: [PATCH 04/19] t4202: clarify intent by creating expected content less cleverly

From: Jeff King <peff@peff.net>
Date: Dec 10, 2021, 09:09 UTC
Message-ID: <YbMZMITRqyHmNC7Z@coredump.intra.peff.net>
In-Reply-To: <20211209051115.52629-5-sunshine@sunshineco.com>

On Thu, Dec 09, 2021 at 12:11:00AM -0500, Eric Sunshine wrote:

Show 15 quoted lines

> Several tests assign the output of `$(...)` command substitution to an
> "expect" variable, taking advantage of the fact that `$(...)` folds out
> the final line terminator while leaving internal line terminators
> intact. They do this because the "actual" string with which "expect"
> will be compared is shaped the same way. However, this intent (having
> internal line terminators, but no final line terminator) is not
> necessarily obvious at first glance and may confuse casual readers. The
> intent can be made more obvious by using `printf` instead, with which
> line termination is stated clearly:
> 
>     printf "sixth\nthird"
> 
> In fact, many other tests in this script already use `printf` for
> precisely this purpose, thus it is an established pattern. Therefore,
> convert these tests to employ `printf`, as well.

Seems reasonable. I don't think these tests actually care about the lack of trailing newline, so another option is to use tformat. Or its shorter cousin, --format. E.g.:

Show 6 quoted lines

> -	actual=$(git log --pretty="format:%s" --diff-filter=M HEAD) &&
> -	expect=$(echo second) &&
> -	verbose test "$actual" = "$expect"
> +	git log --pretty="format:%s" --diff-filter=M HEAD >actual &&
> +	printf "second" >expect &&
> +	test_cmp expect actual

becomes:

  git log --format=%s --diff-filter=M HEAD >actual &&
  echo second >expect &&
  test_cmp expect actual

which is even less magical. But if the existing pattern is there in nearby tests, I don't have any problem with following it.

> While at it, modernize the tests to use test_cmp() to compare the
> expected and actual output rather than using the semi-deprecated
> `verbose test "$x" = "$y"`.

Yay. Happy to see more "verbose" calls cleaned up.

-Peff