Re: [PATCH] ls-files: update test style

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Jun 23 2022, Li Linchao via GitGitGadget wrote:

> From: Li Linchao <lilinchao@oschina.cn>
>
> Update test style in t/t30[*].sh for uniformity, that's to
> keep test title the same line with helper function itself.

We have a few of these sorts of old style tests, and it's good to update
them.

>     Write test code like this:
> diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
> index 48cec4e5f88..76361b92336 100755
> --- a/t/t3001-ls-files-others-exclude.sh
> +++ b/t/t3001-ls-files-others-exclude.sh
> @@ -67,26 +67,26 @@ echo '!*.2
>  
>  allignores='.gitignore one/.gitignore one/two/.gitignore'
>  
> -test_expect_success \
> -    'git ls-files --others with various exclude options.' \
> -    'git ls-files --others \
> +test_expect_success 'git ls-files --others with various exclude options.' '
> +	git ls-files --others \
>         --exclude=\*.6 \
>         --exclude-per-directory=.gitignore \
>         --exclude-from=.git/ignore \
>         >output &&

This though really stops too short, here we end up with:

	<TAB>git-ls-files --others \
	<7 spaces>--exclude [...]

> -     test_cmp expect output'
> +     test_cmp expect output

And you've space-indented this test_cmp, presumably the below has the
same issues (I didn't check in detail)

Instead the argument lists should be <TAB><TAB> indented, and the rest
should be TAB indented.

> +'
>  
>  # Test \r\n (MSDOS-like systems)
>  printf '*.1\r\n/*.3\r\n!*.6\r\n' >.gitignore
>  
> -test_expect_success \
> -    'git ls-files --others with \r\n line endings.' \
> -    'git ls-files --others \
> +test_expect_success 'git ls-files --others with \r\n line endings.' '
> +	git ls-files --others \
>         --exclude=\*.6 \
>         --exclude-per-directory=.gitignore \
>         --exclude-from=.git/ignore \
>         >output &&
> -     test_cmp expect output'
> +     test_cmp expect output
> +'

Aside from the above I think it's also worth incorporating all the
"printf", "echo", "cat" etc. that we do into the "test_expect_success"
themselves, and if they're needed by more than one test perhaps make
them a "setup" helper function (which would test_when_finished "rm -f
.gitignore" clean up after itself).

That's obviously bigger than some whitespace changes, so we could punt
on it for now, but as we're looking at this anyway we could convert
fully to a more modern style in a follow-up commit...

> -test_expect_success \
> -    'git ls-files with path restriction with --.' \
> -    'git ls-files --others -- path0 >output &&
> +test_expect_success 'git ls-files with path restriction with --.' '
> +    git ls-files --others -- path0 >output &&
>  	test_cmp output - <<EOF
>  path0
>  EOF
>  '

On the topic of leaving things on the table: here we could use "<<-EOF"
(or actually better "<<-\EOF") instead, and indent the here-doc, as we
usually do.