Re: [PATCH] t: use LF variable defined in the test harness

From: Taylor Blau <me@ttaylorr.com>
Date: Sep 4, 2019, 00:29 UTC
Message-ID: <20190904002930.GA76383@syl.lan>
In-Reply-To: <xmqqwoeprsp1.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Tue, Sep 03, 2019 at 02:11:22PM -0700, Junio C Hamano wrote:

> A few test scripts assign a single LF to $LF, but that is already
> given by test-lib.sh to everybody.

I didn't know that 't/test-lib.sh' provided '$LF' (as I'm sure was the case for the respective authors of those tests below ;-)), so removing the bespoke definitions and relying on the one already defined makes good sense.

> Remove the unnecessary reassignment.

I did find a couple of extra spots when I went looking through 't' myself. I ran:

  $ git grep "='$" -- t

which in addition to the three spots below turned up a couple more interesting locations, as well as a few false positives. They are:

  - t/t3005: this script calls the variable '$new_line', but could be
    renamed to LF and then removed in a second patch

  - t/valgrind/analyze.sh: this script also calls the variable
    '$new_line', but does not ever source test-lib.sh, so I think this
    spot can be ignored.

So I think that my recommendation is to either:

  * introduce a prepratory patch that updates t3005 to rename
    '$new_line' to '$LF', and then amend the patch below to also blow
    away t3005's use of '$LF', or

  * do both steps in one larger patch

Show 48 quoted lines

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  t/t3404-rebase-interactive.sh | 2 --
>  t/t4013-diff-various.sh       | 3 ---
>  t/t5515-fetch-merge-logic.sh  | 3 ---
>  3 files changed, 8 deletions(-)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 461dd539ff..31114d0bbb 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -155,8 +155,6 @@ test_expect_success 'rebase -x with empty command fails' '
>  	test_i18ncmp expected actual
>  '
>
> -LF='
> -'
>  test_expect_success 'rebase -x with newline in command fails' '
>  	test_when_finished "git rebase --abort ||:" &&
>  	test_must_fail env git rebase -x "a${LF}b" @ 2>actual &&
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index a9054d2db1..5ac94b390d 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -7,9 +7,6 @@ test_description='Various diff formatting options'
>
>  . ./test-lib.sh
>
> -LF='
> -'
> -
>  test_expect_success setup '
>
>  	GIT_AUTHOR_DATE="2006-06-26 00:00:00 +0000" &&
> diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
> index e55d8474ef..961eb35c99 100755
> --- a/t/t5515-fetch-merge-logic.sh
> +++ b/t/t5515-fetch-merge-logic.sh
> @@ -12,9 +12,6 @@ GIT_TEST_PROTOCOL_VERSION=
>
>  . ./test-lib.sh
>
> -LF='
> -'
> -
>  test_expect_success setup '
>  	GIT_AUTHOR_DATE="2006-06-26 00:00:00 +0000" &&
>  	GIT_COMMITTER_DATE="2006-06-26 00:00:00 +0000" &&

The rest of the patch here looks obviously good.

Thanks, Taylor