[PATCH] ls-files: update test style

[PATCH] ls-files: update test style

[Li Linchao via GitGitGadget]

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.

And update t/README to describe this test style.

Signed-off-by: Li Linchao <lilinchao@oschina.cn>
---
    ls-files: update test style
    
    Update test style in t/t30[*].sh for uniformity, that's to keep test
    title the same line with helper function itself.
    
    And update t/README to describe this test style.
    
    Signed-off-by: Li Linchao lilinchao@oschina.cn

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1269

 t/README                           | 15 ++++++++++++++
 t/t3001-ls-files-others-exclude.sh | 24 +++++++++++-----------
 t/t3002-ls-files-dashpath.sh       | 33 +++++++++++++-----------------
 t/t3020-ls-files-error-unmatch.sh  | 12 +++++------
 t/t3060-ls-files-with-tree.sh      |  6 +++---
 5 files changed, 50 insertions(+), 40 deletions(-)

diff --git a/t/README b/t/README
index 309a31133c6..70205fba41b 100644
--- a/t/README
+++ b/t/README
@@ -560,6 +560,21 @@ Here are the "do's:"
    Even code that isn't a test per se, but merely some setup code
    should be inside a test assertion.
 
+ - Keep test title the same line with test helper function itself,
+   and end the line with a single quote.
+
+   Take test_expect_success helper for example, write it like:
+
+  test_expect_success 'test title to describe this test case' '
+  # test body
+  '
+
+   Instead of:
+
+  test_expect_success \
+  'test title to describe this test case' \
+  '# test body'
+
  - Chain your test assertions
 
    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 &&
-     test_cmp expect output'
+     test_cmp expect output
+'
 
 # 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
+'
 
 test_expect_success 'setup skip-worktree gitignore' '
 	git add $allignores &&
@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '
 	rm $allignores
 '
 
-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 &&
-     test_cmp expect output'
+     test_cmp expect output
+'
 
 test_expect_success !SANITIZE_LEAK 'restore gitignore' '
 	git checkout --ignore-skip-worktree-bits $allignores &&
diff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh
index 54d22a45dfb..adbe96fa2df 100755
--- a/t/t3002-ls-files-dashpath.sh
+++ b/t/t3002-ls-files-dashpath.sh
@@ -16,15 +16,14 @@ filesystem.
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-test_expect_success \
-	setup \
-	'echo frotz >path0 &&
+test_expect_success 'setup' '
+    echo frotz >path0 &&
 	echo frotz >./-foo &&
-	echo frotz >./--'
+	echo frotz >./--
+'
 
-test_expect_success \
-    'git ls-files without path restriction.' \
-    'git ls-files --others >output &&
+test_expect_success 'git ls-files without path restriction.' '
+    git ls-files --others >output &&
      test_cmp output - <<EOF
 --
 -foo
@@ -33,33 +32,29 @@ path0
 EOF
 '
 
-test_expect_success \
-    'git ls-files with path restriction.' \
-    'git ls-files --others path0 >output &&
+test_expect_success 'git ls-files with path restriction.' '
+    git ls-files --others path0 >output &&
 	test_cmp output - <<EOF
 path0
 EOF
 '
 
-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
 '
 
-test_expect_success \
-    'git ls-files with path restriction with -- --.' \
-    'git ls-files --others -- -- >output &&
+test_expect_success 'git ls-files with path restriction with -- --.' '
+    git ls-files --others -- -- >output &&
 	test_cmp output - <<EOF
 --
 EOF
 '
 
-test_expect_success \
-    'git ls-files with no path restriction.' \
-    'git ls-files --others -- >output &&
+test_expect_success 'git ls-files with no path restriction.' '
+    git ls-files --others -- >output &&
 	test_cmp output - <<EOF
 --
 -foo
diff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh
index 2cbcbc0721b..8dd520bb331 100755
--- a/t/t3020-ls-files-error-unmatch.sh
+++ b/t/t3020-ls-files-error-unmatch.sh
@@ -19,12 +19,12 @@ test_expect_success 'setup' '
 	git commit -m "add foo bar"
 '
 
-test_expect_success \
-    'git ls-files --error-unmatch should fail with unmatched path.' \
-    'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'
+test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '
+    test_must_fail git ls-files --error-unmatch foo bar-does-not-match
+'
 
-test_expect_success \
-    'git ls-files --error-unmatch should succeed with matched paths.' \
-    'git ls-files --error-unmatch foo bar'
+test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '
+    git ls-files --error-unmatch foo bar
+'
 
 test_done
diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh
index b257c792a46..c350b4641f3 100755
--- a/t/t3060-ls-files-with-tree.sh
+++ b/t/t3060-ls-files-with-tree.sh
@@ -62,9 +62,9 @@ test_expect_success 'git ls-files --with-tree should succeed from subdir' '
 	)
 '
 
-test_expect_success \
-    'git ls-files --with-tree should add entries from named tree.' \
-    'test_cmp expected output'
+test_expect_success 'git ls-files --with-tree should add entries from named tree.' '
+	test_cmp expected output
+'
 
 test_expect_success 'no duplicates in --with-tree output' '
 	git ls-files --with-tree=HEAD >actual &&

base-commit: 5b71c59bc3b9365075e2a175aa7b6f2b0c84ce44
-- 
gitgitgadget

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

[Ævar Arnfjörð Bjarmason]

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.

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

[Junio C Hamano]

"Li Linchao via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/t/README b/t/README
> index 309a31133c6..70205fba41b 100644
> --- a/t/README
> +++ b/t/README
> @@ -560,6 +560,21 @@ Here are the "do's:"
>     Even code that isn't a test per se, but merely some setup code
>     should be inside a test assertion.
>  
> + - Keep test title the same line with test helper function itself,
> +   and end the line with a single quote.
> +
> +   Take test_expect_success helper for example, write it like:
> +
> +  test_expect_success 'test title to describe this test case' '
> +  # test body
> +  '

If you want to show the pretty layout, then the test body should
be shown indented, i.e.

	test_expect_success 'title' '
		... test body ...
	'

But I am not sure if this belongs to the existing "Do's and don'ts"
section, which lists tips that matter for correctness.

This new one certainly encouraged as a more modern style, but is not
about correctness at all.

A separate "recommended style" section might make sense, but there
will be a lot more entries, like when to quote and not to quote EOF
marker for here-document, indenting the body of here-document, etc.

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

[lilinchao]

>
>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. 
Yes. Currently there are at least 400+ old style tests :). It not easy for me
to fix them all at once with some magic regex expressions, so I'm not going
to update them all in one patch. But I think, first of all, we can explicitly
document which test style we prefer first.
>
>>     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 [...]
> 
OK.
>> -     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. 
OK.
>
>> +'
>> 
>>  # 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). 
Yes, make sense.
>
>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. 
OK, will do.

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

[lilinchao]

>"Li Linchao via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> diff --git a/t/README b/t/README
>> index 309a31133c6..70205fba41b 100644
>> --- a/t/README
>> +++ b/t/README
>> @@ -560,6 +560,21 @@ Here are the "do's:"
>>     Even code that isn't a test per se, but merely some setup code
>>     should be inside a test assertion.
>> 
>> + - Keep test title the same line with test helper function itself,
>> +   and end the line with a single quote.
>> +
>> +   Take test_expect_success helper for example, write it like:
>> +
>> +  test_expect_success 'test title to describe this test case' '
>> +  # test body
>> +  '
>
>If you want to show the pretty layout, then the test body should
>be shown indented, i.e.
>
>	test_expect_success 'title' '
>	... test body ...
>	'
OK.
>
>But I am not sure if this belongs to the existing "Do's and don'ts"
>section, which lists tips that matter for correctness.
>
>This new one certainly encouraged as a more modern style, but is not
>about correctness at all.
OK, I will remove it from "Do's and don'ts" section.
>
>A separate "recommended style" section might make sense, but there
>will be a lot more entries, like when to quote and not to quote EOF
>marker for here-document, indenting the body of here-document, etc.
Yes, a "recommended style" section needed.

[PATCH v2] ls-files: update test style

[Li Linchao via GitGitGadget]

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.

And update t/README to describe this test style.

Signed-off-by: Li Linchao <lilinchao@oschina.cn>
---
    ls-files: update test style
    
    Update test style in t/t30[*].sh for uniformity, that's to keep test
    title the same line with helper function itself.
    
    And update t/README to describe this test style.
    
    Signed-off-by: Li Linchao lilinchao@oschina.cn

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1269

Range-diff vs v1:

 1:  e18a6172cff ! 1:  775c17499d5 ls-files: update test style
     @@ Commit message
          Signed-off-by: Li Linchao <lilinchao@oschina.cn>
      
       ## t/README ##
     -@@ t/README: Here are the "do's:"
     -    Even code that isn't a test per se, but merely some setup code
     -    should be inside a test assertion.
     +@@ t/README: This test harness library does the following things:
     +    consistently when command line arguments --verbose (or -v),
     +    --debug (or -d), and --immediate (or -i) is given.
       
     -+ - Keep test title the same line with test helper function itself,
     -+   and end the line with a single quote.
     ++Recommended style
     ++-----------------
     ++Here are some recommented style that you should follow when you write some test cases.
     ++
     ++ - Keep test title the same line with test helper function itself.
      +
      +   Take test_expect_success helper for example, write it like:
      +
     -+  test_expect_success 'test title to describe this test case' '
     -+  # test body
     ++  test_expect_success 'test title' '
     ++  ... test body ...
      +  '
      +
      +   Instead of:
      +
      +  test_expect_success \
     -+  'test title to describe this test case' \
     -+  '# test body'
     ++  'test title' \
     ++  '... test body ...'
     ++
     ++
     ++ - End the line with a single quote.
     ++
     ++ - Indent the body of here-document, and cut off prefix TAB by using
     ++ "<<-":
      +
     -  - Chain your test assertions
     ++  test_expect_success 'test something' '
     ++      cat >expect <<-\EOF &&
     ++      one
     ++      two
     ++      three
     ++      EOF
     ++      test_something > actual &&
     ++      test_cmp expect actual
     ++  '
     ++
     ++   Instead of:
     ++
     ++  test_expect_success 'test something' '
     ++      cat >expect <<\EOF &&
     ++  one
     ++  two
     ++  three
     ++  EOF
     ++      test_something > actual &&
     ++      test_cmp expect actual
     ++  '
     ++
     ++
     + Do's & don'ts
     + -------------
       
     -    Write test code like this:
      
       ## t/t3001-ls-files-others-exclude.sh ##
      @@ t/t3001-ls-files-others-exclude.sh: echo '!*.2
     @@ t/t3001-ls-files-others-exclude.sh: echo '!*.2
              --exclude=\*.6 \
              --exclude-per-directory=.gitignore \
              --exclude-from=.git/ignore \
     -        >output &&
     +-       >output &&
      -     test_cmp expect output'
     -+     test_cmp expect output
     ++	>output &&
     ++	test_cmp expect output
      +'
       
       # Test \r\n (MSDOS-like systems)
     @@ t/t3001-ls-files-others-exclude.sh: echo '!*.2
              --exclude=\*.6 \
              --exclude-per-directory=.gitignore \
              --exclude-from=.git/ignore \
     -        >output &&
     +-       >output &&
      -     test_cmp expect output'
     -+     test_cmp expect output
     ++	>output &&
     ++	test_cmp expect output
      +'
       
       test_expect_success 'setup skip-worktree gitignore' '
     @@ t/t3001-ls-files-others-exclude.sh: test_expect_success 'setup skip-worktree git
              --exclude=\*.6 \
              --exclude-per-directory=.gitignore \
              --exclude-from=.git/ignore \
     -        >output &&
     +-       >output &&
      -     test_cmp expect output'
     -+     test_cmp expect output
     ++	>output &&
     ++	test_cmp expect output
      +'
       
       test_expect_success !SANITIZE_LEAK 'restore gitignore' '
       	git checkout --ignore-skip-worktree-bits $allignores &&
     +@@ t/t3001-ls-files-others-exclude.sh: test_expect_success 'pattern matches prefix completely' '
     + '
     + 
     + test_expect_success 'ls-files with "**" patterns' '
     +-	cat <<\EOF >expect &&
     +-a.1
     +-one/a.1
     +-one/two/a.1
     +-three/a.1
     +-EOF
     ++	cat <<-\EOF >expect &&
     ++	a.1
     ++	one/a.1
     ++	one/two/a.1
     ++	three/a.1
     ++	EOF
     + 	git ls-files -o -i --exclude "**/a.1" >actual &&
     + 	test_cmp expect actual
     + '
      
       ## t/t3002-ls-files-dashpath.sh ##
      @@ t/t3002-ls-files-dashpath.sh: filesystem.
     @@ t/t3002-ls-files-dashpath.sh: filesystem.
      -	setup \
      -	'echo frotz >path0 &&
      +test_expect_success 'setup' '
     -+    echo frotz >path0 &&
     ++	echo frotz >path0 &&
       	echo frotz >./-foo &&
      -	echo frotz >./--'
      +	echo frotz >./--
     @@ t/t3002-ls-files-dashpath.sh: filesystem.
      -test_expect_success \
      -    'git ls-files without path restriction.' \
      -    'git ls-files --others >output &&
     +-     test_cmp output - <<EOF
     +---
     +--foo
     +-output
     +-path0
     +-EOF
      +test_expect_success 'git ls-files without path restriction.' '
     -+    git ls-files --others >output &&
     -      test_cmp output - <<EOF
     - --
     - -foo
     -@@ t/t3002-ls-files-dashpath.sh: path0
     - EOF
     ++	test_when_finished "rm -f expect" &&
     ++	git ls-files --others >output &&
     ++	cat >expect <<-\EOF &&
     ++	--
     ++	-foo
     ++	output
     ++	path0
     ++	EOF
     ++	test_cmp output expect
       '
       
      -test_expect_success \
      -    'git ls-files with path restriction.' \
      -    'git ls-files --others path0 >output &&
     +-	test_cmp output - <<EOF
     +-path0
     +-EOF
      +test_expect_success 'git ls-files with path restriction.' '
     -+    git ls-files --others path0 >output &&
     - 	test_cmp output - <<EOF
     - path0
     - EOF
     ++	test_when_finished "rm -f expect" &&
     ++	git ls-files --others path0 >output &&
     ++	cat >expect <<-\EOF &&
     ++	path0
     ++	EOF
     ++	test_cmp output expect
       '
       
      -test_expect_success \
      -    'git ls-files with path restriction with --.' \
      -    'git ls-files --others -- path0 >output &&
     +-	test_cmp output - <<EOF
     +-path0
     +-EOF
      +test_expect_success 'git ls-files with path restriction with --.' '
     -+    git ls-files --others -- path0 >output &&
     - 	test_cmp output - <<EOF
     - path0
     - EOF
     ++	test_when_finished "rm -f expect" &&
     ++	git ls-files --others -- path0 >output &&
     ++	cat >expect <<-\EOF &&
     ++	path0
     ++	EOF
     ++	test_cmp output expect
       '
       
      -test_expect_success \
      -    'git ls-files with path restriction with -- --.' \
      -    'git ls-files --others -- -- >output &&
     +-	test_cmp output - <<EOF
     +---
     +-EOF
      +test_expect_success 'git ls-files with path restriction with -- --.' '
     -+    git ls-files --others -- -- >output &&
     - 	test_cmp output - <<EOF
     - --
     - EOF
     ++	test_when_finished "rm -f expect" &&
     ++	git ls-files --others -- -- >output &&
     ++	cat >expect <<-\EOF &&
     ++	--
     ++	EOF
     ++	test_cmp output expect
       '
       
      -test_expect_success \
      -    'git ls-files with no path restriction.' \
      -    'git ls-files --others -- >output &&
     +-	test_cmp output - <<EOF
     +---
     +--foo
     +-output
     +-path0
     +-EOF
      +test_expect_success 'git ls-files with no path restriction.' '
     -+    git ls-files --others -- >output &&
     - 	test_cmp output - <<EOF
     - --
     - -foo
     ++	test_when_finished "rm -f expect" &&
     ++	git ls-files --others -- >output &&
     ++	cat >expect <<-\EOF &&
     ++	--
     ++	-foo
     ++	output
     ++	path0
     ++	EOF
     ++	test_cmp output expect
     ++
     + '
     + 
     + test_done
     +
     + ## t/t3007-ls-files-recurse-submodules.sh ##
     +@@ t/t3007-ls-files-recurse-submodules.sh: test_expect_success '--recurse-submodules does not support --error-unmatch' '
     + 
     + test_incompatible_with_recurse_submodules () {
     + 	test_expect_success "--recurse-submodules and $1 are incompatible" "
     +-		test_must_fail git ls-files --recurse-submodules $1 2>actual &&
     +-		test_i18ngrep 'unsupported mode' actual
     ++	test_must_fail git ls-files --recurse-submodules $1 2>actual &&
     ++	test_i18ngrep 'unsupported mode' actual
     + 	"
     + }
     + 
      
       ## t/t3020-ls-files-error-unmatch.sh ##
      @@ t/t3020-ls-files-error-unmatch.sh: test_expect_success 'setup' '
     @@ t/t3020-ls-files-error-unmatch.sh: test_expect_success 'setup' '
      -    'git ls-files --error-unmatch should fail with unmatched path.' \
      -    'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'
      +test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '
     -+    test_must_fail git ls-files --error-unmatch foo bar-does-not-match
     ++	test_must_fail git ls-files --error-unmatch foo bar-does-not-match
      +'
       
      -test_expect_success \
      -    'git ls-files --error-unmatch should succeed with matched paths.' \
      -    'git ls-files --error-unmatch foo bar'
      +test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '
     -+    git ls-files --error-unmatch foo bar
     ++	git ls-files --error-unmatch foo bar
      +'
       
       test_done
      
       ## t/t3060-ls-files-with-tree.sh ##
     +@@ t/t3060-ls-files-with-tree.sh: a scenario known to trigger a crash with some versions of git.
     + '
     + . ./test-lib.sh
     + 
     +-test_expect_success setup '
     ++test_expect_success 'setup' '
     + 
     + 	# The bug we are exercising requires a fair number of entries
     + 	# in a sub-directory so that add_index_entry will trigger a
      @@ t/t3060-ls-files-with-tree.sh: test_expect_success 'git ls-files --with-tree should succeed from subdir' '
       	)
       '


 t/README                               | 47 ++++++++++++++
 t/t3001-ls-files-others-exclude.sh     | 42 ++++++-------
 t/t3002-ls-files-dashpath.sh           | 86 ++++++++++++++------------
 t/t3007-ls-files-recurse-submodules.sh |  4 +-
 t/t3020-ls-files-error-unmatch.sh      | 12 ++--
 t/t3060-ls-files-with-tree.sh          |  8 +--
 6 files changed, 126 insertions(+), 73 deletions(-)

diff --git a/t/README b/t/README
index 309a31133c6..845af1883c3 100644
--- a/t/README
+++ b/t/README
@@ -547,6 +547,53 @@ This test harness library does the following things:
    consistently when command line arguments --verbose (or -v),
    --debug (or -d), and --immediate (or -i) is given.
 
+Recommended style
+-----------------
+Here are some recommented style that you should follow when you write some test cases.
+
+ - Keep test title the same line with test helper function itself.
+
+   Take test_expect_success helper for example, write it like:
+
+  test_expect_success 'test title' '
+  ... test body ...
+  '
+
+   Instead of:
+
+  test_expect_success \
+  'test title' \
+  '... test body ...'
+
+
+ - End the line with a single quote.
+
+ - Indent the body of here-document, and cut off prefix TAB by using
+ "<<-":
+
+  test_expect_success 'test something' '
+      cat >expect <<-\EOF &&
+      one
+      two
+      three
+      EOF
+      test_something > actual &&
+      test_cmp expect actual
+  '
+
+   Instead of:
+
+  test_expect_success 'test something' '
+      cat >expect <<\EOF &&
+  one
+  two
+  three
+  EOF
+      test_something > actual &&
+      test_cmp expect actual
+  '
+
+
 Do's & don'ts
 -------------
 
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index 48cec4e5f88..e07ac6c6dce 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 &&
-     test_cmp expect output'
+	>output &&
+	test_cmp expect output
+'
 
 # 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'
+	>output &&
+	test_cmp expect output
+'
 
 test_expect_success 'setup skip-worktree gitignore' '
 	git add $allignores &&
@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '
 	rm $allignores
 '
 
-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 &&
-     test_cmp expect output'
+	>output &&
+	test_cmp expect output
+'
 
 test_expect_success !SANITIZE_LEAK 'restore gitignore' '
 	git checkout --ignore-skip-worktree-bits $allignores &&
@@ -283,12 +283,12 @@ test_expect_success 'pattern matches prefix completely' '
 '
 
 test_expect_success 'ls-files with "**" patterns' '
-	cat <<\EOF >expect &&
-a.1
-one/a.1
-one/two/a.1
-three/a.1
-EOF
+	cat <<-\EOF >expect &&
+	a.1
+	one/a.1
+	one/two/a.1
+	three/a.1
+	EOF
 	git ls-files -o -i --exclude "**/a.1" >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh
index 54d22a45dfb..4dd24550eba 100755
--- a/t/t3002-ls-files-dashpath.sh
+++ b/t/t3002-ls-files-dashpath.sh
@@ -16,56 +16,62 @@ filesystem.
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-test_expect_success \
-	setup \
-	'echo frotz >path0 &&
+test_expect_success 'setup' '
+	echo frotz >path0 &&
 	echo frotz >./-foo &&
-	echo frotz >./--'
+	echo frotz >./--
+'
 
-test_expect_success \
-    'git ls-files without path restriction.' \
-    'git ls-files --others >output &&
-     test_cmp output - <<EOF
---
--foo
-output
-path0
-EOF
+test_expect_success 'git ls-files without path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others >output &&
+	cat >expect <<-\EOF &&
+	--
+	-foo
+	output
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction.' \
-    'git ls-files --others path0 >output &&
-	test_cmp output - <<EOF
-path0
-EOF
+test_expect_success 'git ls-files with path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others path0 >output &&
+	cat >expect <<-\EOF &&
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction with --.' \
-    'git ls-files --others -- path0 >output &&
-	test_cmp output - <<EOF
-path0
-EOF
+test_expect_success 'git ls-files with path restriction with --.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- path0 >output &&
+	cat >expect <<-\EOF &&
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction with -- --.' \
-    'git ls-files --others -- -- >output &&
-	test_cmp output - <<EOF
---
-EOF
+test_expect_success 'git ls-files with path restriction with -- --.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- -- >output &&
+	cat >expect <<-\EOF &&
+	--
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with no path restriction.' \
-    'git ls-files --others -- >output &&
-	test_cmp output - <<EOF
---
--foo
-output
-path0
-EOF
+test_expect_success 'git ls-files with no path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- >output &&
+	cat >expect <<-\EOF &&
+	--
+	-foo
+	output
+	path0
+	EOF
+	test_cmp output expect
+
 '
 
 test_done
diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
index dd7770e85de..2a057e987fe 100755
--- a/t/t3007-ls-files-recurse-submodules.sh
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -301,8 +301,8 @@ test_expect_success '--recurse-submodules does not support --error-unmatch' '
 
 test_incompatible_with_recurse_submodules () {
 	test_expect_success "--recurse-submodules and $1 are incompatible" "
-		test_must_fail git ls-files --recurse-submodules $1 2>actual &&
-		test_i18ngrep 'unsupported mode' actual
+	test_must_fail git ls-files --recurse-submodules $1 2>actual &&
+	test_i18ngrep 'unsupported mode' actual
 	"
 }
 
diff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh
index 2cbcbc0721b..133593d23c0 100755
--- a/t/t3020-ls-files-error-unmatch.sh
+++ b/t/t3020-ls-files-error-unmatch.sh
@@ -19,12 +19,12 @@ test_expect_success 'setup' '
 	git commit -m "add foo bar"
 '
 
-test_expect_success \
-    'git ls-files --error-unmatch should fail with unmatched path.' \
-    'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'
+test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '
+	test_must_fail git ls-files --error-unmatch foo bar-does-not-match
+'
 
-test_expect_success \
-    'git ls-files --error-unmatch should succeed with matched paths.' \
-    'git ls-files --error-unmatch foo bar'
+test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '
+	git ls-files --error-unmatch foo bar
+'
 
 test_done
diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh
index b257c792a46..52f76f7b57f 100755
--- a/t/t3060-ls-files-with-tree.sh
+++ b/t/t3060-ls-files-with-tree.sh
@@ -10,7 +10,7 @@ a scenario known to trigger a crash with some versions of git.
 '
 . ./test-lib.sh
 
-test_expect_success setup '
+test_expect_success 'setup' '
 
 	# The bug we are exercising requires a fair number of entries
 	# in a sub-directory so that add_index_entry will trigger a
@@ -62,9 +62,9 @@ test_expect_success 'git ls-files --with-tree should succeed from subdir' '
 	)
 '
 
-test_expect_success \
-    'git ls-files --with-tree should add entries from named tree.' \
-    'test_cmp expected output'
+test_expect_success 'git ls-files --with-tree should add entries from named tree.' '
+	test_cmp expected output
+'
 
 test_expect_success 'no duplicates in --with-tree output' '
 	git ls-files --with-tree=HEAD >actual &&

base-commit: e4a4b31577c7419497ac30cebe30d755b97752c5
-- 
gitgitgadget

[PATCH v3] ls-files: update test style

[Li Linchao via GitGitGadget]

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,
and fix some indentions.

Add a new section "recommended style" in t/README to
encourage people to use more modern style in test.

Signed-off-by: Li Linchao <lilinchao@oschina.cn>
---
    ls-files: update test style
    
    Update test style in t/t30[*].sh for uniformity, that's to keep test
    title the same line with helper function itself.
    
    And update t/README to describe this test style.
    
    Signed-off-by: Li Linchao lilinchao@oschina.cn

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1269

Range-diff vs v2:

 1:  775c17499d5 ! 1:  b3d80bd9cd5 ls-files: update test style
     @@ Commit message
          ls-files: update test style
      
          Update test style in t/t30[*].sh for uniformity, that's to
     -    keep test title the same line with helper function itself.
     +    keep test title the same line with helper function itself,
     +    and fix some indentions.
      
     -    And update t/README to describe this test style.
     +    Add a new section "recommended style" in t/README to
     +    encourage people to use more modern style in test.
      
          Signed-off-by: Li Linchao <lilinchao@oschina.cn>
      
     @@ t/README: This test harness library does the following things:
       
      +Recommended style
      +-----------------
     -+Here are some recommented style that you should follow when you write some test cases.
     ++Here are some recommented styles when writing test case.
      +
      + - Keep test title the same line with test helper function itself.
      +
     @@ t/README: This test harness library does the following things:
      +
      + - End the line with a single quote.
      +
     -+ - Indent the body of here-document, and cut off prefix TAB by using
     -+ "<<-":
     ++ - Indent the body of here-document, and use "<< -" instead of "<<" to strip prefix TAB:
      +
      +  test_expect_success 'test something' '
      +      cat >expect <<-\EOF &&
     @@ t/t3002-ls-files-dashpath.sh: filesystem.
       
       test_done
      
     - ## t/t3007-ls-files-recurse-submodules.sh ##
     -@@ t/t3007-ls-files-recurse-submodules.sh: test_expect_success '--recurse-submodules does not support --error-unmatch' '
     - 
     - test_incompatible_with_recurse_submodules () {
     - 	test_expect_success "--recurse-submodules and $1 are incompatible" "
     --		test_must_fail git ls-files --recurse-submodules $1 2>actual &&
     --		test_i18ngrep 'unsupported mode' actual
     -+	test_must_fail git ls-files --recurse-submodules $1 2>actual &&
     -+	test_i18ngrep 'unsupported mode' actual
     - 	"
     - }
     - 
     -
       ## t/t3020-ls-files-error-unmatch.sh ##
      @@ t/t3020-ls-files-error-unmatch.sh: test_expect_success 'setup' '
       	git commit -m "add foo bar"


 t/README                           | 46 ++++++++++++++++
 t/t3001-ls-files-others-exclude.sh | 42 +++++++--------
 t/t3002-ls-files-dashpath.sh       | 86 ++++++++++++++++--------------
 t/t3020-ls-files-error-unmatch.sh  | 12 ++---
 t/t3060-ls-files-with-tree.sh      |  8 +--
 5 files changed, 123 insertions(+), 71 deletions(-)

diff --git a/t/README b/t/README
index 309a31133c6..5e0539412b4 100644
--- a/t/README
+++ b/t/README
@@ -547,6 +547,52 @@ This test harness library does the following things:
    consistently when command line arguments --verbose (or -v),
    --debug (or -d), and --immediate (or -i) is given.
 
+Recommended style
+-----------------
+Here are some recommented styles when writing test case.
+
+ - Keep test title the same line with test helper function itself.
+
+   Take test_expect_success helper for example, write it like:
+
+  test_expect_success 'test title' '
+  ... test body ...
+  '
+
+   Instead of:
+
+  test_expect_success \
+  'test title' \
+  '... test body ...'
+
+
+ - End the line with a single quote.
+
+ - Indent the body of here-document, and use "<< -" instead of "<<" to strip prefix TAB:
+
+  test_expect_success 'test something' '
+      cat >expect <<-\EOF &&
+      one
+      two
+      three
+      EOF
+      test_something > actual &&
+      test_cmp expect actual
+  '
+
+   Instead of:
+
+  test_expect_success 'test something' '
+      cat >expect <<\EOF &&
+  one
+  two
+  three
+  EOF
+      test_something > actual &&
+      test_cmp expect actual
+  '
+
+
 Do's & don'ts
 -------------
 
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index 48cec4e5f88..e07ac6c6dce 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 &&
-     test_cmp expect output'
+	>output &&
+	test_cmp expect output
+'
 
 # 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'
+	>output &&
+	test_cmp expect output
+'
 
 test_expect_success 'setup skip-worktree gitignore' '
 	git add $allignores &&
@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '
 	rm $allignores
 '
 
-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 &&
-     test_cmp expect output'
+	>output &&
+	test_cmp expect output
+'
 
 test_expect_success !SANITIZE_LEAK 'restore gitignore' '
 	git checkout --ignore-skip-worktree-bits $allignores &&
@@ -283,12 +283,12 @@ test_expect_success 'pattern matches prefix completely' '
 '
 
 test_expect_success 'ls-files with "**" patterns' '
-	cat <<\EOF >expect &&
-a.1
-one/a.1
-one/two/a.1
-three/a.1
-EOF
+	cat <<-\EOF >expect &&
+	a.1
+	one/a.1
+	one/two/a.1
+	three/a.1
+	EOF
 	git ls-files -o -i --exclude "**/a.1" >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh
index 54d22a45dfb..4dd24550eba 100755
--- a/t/t3002-ls-files-dashpath.sh
+++ b/t/t3002-ls-files-dashpath.sh
@@ -16,56 +16,62 @@ filesystem.
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-test_expect_success \
-	setup \
-	'echo frotz >path0 &&
+test_expect_success 'setup' '
+	echo frotz >path0 &&
 	echo frotz >./-foo &&
-	echo frotz >./--'
+	echo frotz >./--
+'
 
-test_expect_success \
-    'git ls-files without path restriction.' \
-    'git ls-files --others >output &&
-     test_cmp output - <<EOF
---
--foo
-output
-path0
-EOF
+test_expect_success 'git ls-files without path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others >output &&
+	cat >expect <<-\EOF &&
+	--
+	-foo
+	output
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction.' \
-    'git ls-files --others path0 >output &&
-	test_cmp output - <<EOF
-path0
-EOF
+test_expect_success 'git ls-files with path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others path0 >output &&
+	cat >expect <<-\EOF &&
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction with --.' \
-    'git ls-files --others -- path0 >output &&
-	test_cmp output - <<EOF
-path0
-EOF
+test_expect_success 'git ls-files with path restriction with --.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- path0 >output &&
+	cat >expect <<-\EOF &&
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction with -- --.' \
-    'git ls-files --others -- -- >output &&
-	test_cmp output - <<EOF
---
-EOF
+test_expect_success 'git ls-files with path restriction with -- --.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- -- >output &&
+	cat >expect <<-\EOF &&
+	--
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with no path restriction.' \
-    'git ls-files --others -- >output &&
-	test_cmp output - <<EOF
---
--foo
-output
-path0
-EOF
+test_expect_success 'git ls-files with no path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- >output &&
+	cat >expect <<-\EOF &&
+	--
+	-foo
+	output
+	path0
+	EOF
+	test_cmp output expect
+
 '
 
 test_done
diff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh
index 2cbcbc0721b..133593d23c0 100755
--- a/t/t3020-ls-files-error-unmatch.sh
+++ b/t/t3020-ls-files-error-unmatch.sh
@@ -19,12 +19,12 @@ test_expect_success 'setup' '
 	git commit -m "add foo bar"
 '
 
-test_expect_success \
-    'git ls-files --error-unmatch should fail with unmatched path.' \
-    'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'
+test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '
+	test_must_fail git ls-files --error-unmatch foo bar-does-not-match
+'
 
-test_expect_success \
-    'git ls-files --error-unmatch should succeed with matched paths.' \
-    'git ls-files --error-unmatch foo bar'
+test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '
+	git ls-files --error-unmatch foo bar
+'
 
 test_done
diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh
index b257c792a46..52f76f7b57f 100755
--- a/t/t3060-ls-files-with-tree.sh
+++ b/t/t3060-ls-files-with-tree.sh
@@ -10,7 +10,7 @@ a scenario known to trigger a crash with some versions of git.
 '
 . ./test-lib.sh
 
-test_expect_success setup '
+test_expect_success 'setup' '
 
 	# The bug we are exercising requires a fair number of entries
 	# in a sub-directory so that add_index_entry will trigger a
@@ -62,9 +62,9 @@ test_expect_success 'git ls-files --with-tree should succeed from subdir' '
 	)
 '
 
-test_expect_success \
-    'git ls-files --with-tree should add entries from named tree.' \
-    'test_cmp expected output'
+test_expect_success 'git ls-files --with-tree should add entries from named tree.' '
+	test_cmp expected output
+'
 
 test_expect_success 'no duplicates in --with-tree output' '
 	git ls-files --with-tree=HEAD >actual &&

base-commit: e4a4b31577c7419497ac30cebe30d755b97752c5
-- 
gitgitgadget

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

[Junio C Hamano]

"Li Linchao via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/t/README b/t/README
> index 309a31133c6..5e0539412b4 100644
> --- a/t/README
> +++ b/t/README
> @@ -547,6 +547,52 @@ This test harness library does the following things:
>     consistently when command line arguments --verbose (or -v),
>     --debug (or -d), and --immediate (or -i) is given.
>  
> +Recommended style
> +-----------------
> +Here are some recommented styles when writing test case.
> +
> + - Keep test title the same line with test helper function itself.
> +
> +   Take test_expect_success helper for example, write it like:
> +
> +  test_expect_success 'test title' '
> +  ... test body ...
> +  '

Indent the body further to the right?

> + - Indent the body of here-document, and use "<< -" instead of "<<" to strip prefix TAB:

Overly long line.

Did you mean to have a space between "<<" and "-"?

"prefix TAB" -> "leading TABs used for indentation" (plural is the
important part)?  

Mention end of here-document marker should by default be quoted,
unless the body needs $variable_interpolation?

> +  test_expect_success 'test something' '
> +      cat >expect <<-\EOF &&
> +      one
> +      two
> +      three
> +      EOF
> +      test_something > actual &&
> +      test_cmp expect actual
> +  '
> +
> +   Instead of:
> +
> +  test_expect_success 'test something' '
> +      cat >expect <<\EOF &&
> +  one
> +  two
> +  three
> +  EOF
> +      test_something > actual &&
> +      test_cmp expect actual
> +  '


Thanks.

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

[lilinchao]

>"Li Linchao via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> diff --git a/t/README b/t/README
>> index 309a31133c6..5e0539412b4 100644
>> --- a/t/README
>> +++ b/t/README
>> @@ -547,6 +547,52 @@ This test harness library does the following things:
>>     consistently when command line arguments --verbose (or -v),
>>     --debug (or -d), and --immediate (or -i) is given.
>> 
>> +Recommended style
>> +-----------------
>> +Here are some recommented styles when writing test case.
>> +
>> + - Keep test title the same line with test helper function itself.
>> +
>> +   Take test_expect_success helper for example, write it like:
>> +
>> +  test_expect_success 'test title' '
>> +  ... test body ...
>> +  '
>
>Indent the body further to the right?
>
>> + - Indent the body of here-document, and use "<< -" instead of "<<" to strip prefix TAB:
>
>Overly long line.
>
>Did you mean to have a space between "<<" and "-"? 
Ops, I'll fix it. This came from an extension in VS code which is to preview AsciiDoc,
it mistakenly render "<<-" into "<←", so I put a space between "<<" and "-".
>
>"prefix TAB" -> "leading TABs used for indentation" (plural is the
>important part)?  
>
>Mention end of here-document marker should by default be quoted,
>unless the body needs $variable_interpolation? 
Sorry, I don't get it. I don't see many of ending "EOF" are quoted in our tests.
>
>> +  test_expect_success 'test something' '
>> +      cat >expect <<-\EOF &&
>> +      one
>> +      two
>> +      three
>> +      EOF
>> +      test_something > actual &&
>> +      test_cmp expect actual
>> +  '
>> +
>> +   Instead of:
>> +
>> +  test_expect_success 'test something' '
>> +      cat >expect <<\EOF &&
>> +  one
>> +  two
>> +  three
>> +  EOF
>> +      test_something > actual &&
>> +      test_cmp expect actual
>> +  '
>
>
>Thanks.

[PATCH v4] ls-files: update test style

[Li Linchao via GitGitGadget]

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,
and fix some indentions.

Add a new section "recommended style" in t/README to
encourage people to use more modern style in test.

Signed-off-by: Li Linchao <lilinchao@oschina.cn>
---
    ls-files: update test style
    
    Update test style in t/t30[*].sh for uniformity, that's to keep test
    title the same line with helper function itself.
    
    And update t/README to describe this test style.
    
    Signed-off-by: Li Linchao lilinchao@oschina.cn

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1269

Range-diff vs v3:

 1:  b3d80bd9cd5 ! 1:  d0ee9836f23 ls-files: update test style
     @@ t/README: This test harness library does the following things:
      +   Take test_expect_success helper for example, write it like:
      +
      +  test_expect_success 'test title' '
     -+  ... test body ...
     ++      ... test body ...
      +  '
      +
      +   Instead of:
      +
      +  test_expect_success \
     -+  'test title' \
     -+  '... test body ...'
     ++      'test title' \
     ++      '... test body ...'
      +
      +
      + - End the line with a single quote.
      +
     -+ - Indent the body of here-document, and use "<< -" instead of "<<" to strip prefix TAB:
     ++ - Indent the body of here-document, and use "<<-" instead of "<<"
     ++   to strip leading TABs used for indentation:
      +
      +  test_expect_success 'test something' '
      +      cat >expect <<-\EOF &&


 t/README                           | 47 ++++++++++++++++
 t/t3001-ls-files-others-exclude.sh | 42 +++++++--------
 t/t3002-ls-files-dashpath.sh       | 86 ++++++++++++++++--------------
 t/t3020-ls-files-error-unmatch.sh  | 12 ++---
 t/t3060-ls-files-with-tree.sh      |  8 +--
 5 files changed, 124 insertions(+), 71 deletions(-)

diff --git a/t/README b/t/README
index 309a31133c6..f662471338d 100644
--- a/t/README
+++ b/t/README
@@ -547,6 +547,53 @@ This test harness library does the following things:
    consistently when command line arguments --verbose (or -v),
    --debug (or -d), and --immediate (or -i) is given.
 
+Recommended style
+-----------------
+Here are some recommented styles when writing test case.
+
+ - Keep test title the same line with test helper function itself.
+
+   Take test_expect_success helper for example, write it like:
+
+  test_expect_success 'test title' '
+      ... test body ...
+  '
+
+   Instead of:
+
+  test_expect_success \
+      'test title' \
+      '... test body ...'
+
+
+ - End the line with a single quote.
+
+ - Indent the body of here-document, and use "<<-" instead of "<<"
+   to strip leading TABs used for indentation:
+
+  test_expect_success 'test something' '
+      cat >expect <<-\EOF &&
+      one
+      two
+      three
+      EOF
+      test_something > actual &&
+      test_cmp expect actual
+  '
+
+   Instead of:
+
+  test_expect_success 'test something' '
+      cat >expect <<\EOF &&
+  one
+  two
+  three
+  EOF
+      test_something > actual &&
+      test_cmp expect actual
+  '
+
+
 Do's & don'ts
 -------------
 
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index 48cec4e5f88..e07ac6c6dce 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 &&
-     test_cmp expect output'
+	>output &&
+	test_cmp expect output
+'
 
 # 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'
+	>output &&
+	test_cmp expect output
+'
 
 test_expect_success 'setup skip-worktree gitignore' '
 	git add $allignores &&
@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '
 	rm $allignores
 '
 
-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 &&
-     test_cmp expect output'
+	>output &&
+	test_cmp expect output
+'
 
 test_expect_success !SANITIZE_LEAK 'restore gitignore' '
 	git checkout --ignore-skip-worktree-bits $allignores &&
@@ -283,12 +283,12 @@ test_expect_success 'pattern matches prefix completely' '
 '
 
 test_expect_success 'ls-files with "**" patterns' '
-	cat <<\EOF >expect &&
-a.1
-one/a.1
-one/two/a.1
-three/a.1
-EOF
+	cat <<-\EOF >expect &&
+	a.1
+	one/a.1
+	one/two/a.1
+	three/a.1
+	EOF
 	git ls-files -o -i --exclude "**/a.1" >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh
index 54d22a45dfb..4dd24550eba 100755
--- a/t/t3002-ls-files-dashpath.sh
+++ b/t/t3002-ls-files-dashpath.sh
@@ -16,56 +16,62 @@ filesystem.
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-test_expect_success \
-	setup \
-	'echo frotz >path0 &&
+test_expect_success 'setup' '
+	echo frotz >path0 &&
 	echo frotz >./-foo &&
-	echo frotz >./--'
+	echo frotz >./--
+'
 
-test_expect_success \
-    'git ls-files without path restriction.' \
-    'git ls-files --others >output &&
-     test_cmp output - <<EOF
---
--foo
-output
-path0
-EOF
+test_expect_success 'git ls-files without path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others >output &&
+	cat >expect <<-\EOF &&
+	--
+	-foo
+	output
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction.' \
-    'git ls-files --others path0 >output &&
-	test_cmp output - <<EOF
-path0
-EOF
+test_expect_success 'git ls-files with path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others path0 >output &&
+	cat >expect <<-\EOF &&
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction with --.' \
-    'git ls-files --others -- path0 >output &&
-	test_cmp output - <<EOF
-path0
-EOF
+test_expect_success 'git ls-files with path restriction with --.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- path0 >output &&
+	cat >expect <<-\EOF &&
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction with -- --.' \
-    'git ls-files --others -- -- >output &&
-	test_cmp output - <<EOF
---
-EOF
+test_expect_success 'git ls-files with path restriction with -- --.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- -- >output &&
+	cat >expect <<-\EOF &&
+	--
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with no path restriction.' \
-    'git ls-files --others -- >output &&
-	test_cmp output - <<EOF
---
--foo
-output
-path0
-EOF
+test_expect_success 'git ls-files with no path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- >output &&
+	cat >expect <<-\EOF &&
+	--
+	-foo
+	output
+	path0
+	EOF
+	test_cmp output expect
+
 '
 
 test_done
diff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh
index 2cbcbc0721b..133593d23c0 100755
--- a/t/t3020-ls-files-error-unmatch.sh
+++ b/t/t3020-ls-files-error-unmatch.sh
@@ -19,12 +19,12 @@ test_expect_success 'setup' '
 	git commit -m "add foo bar"
 '
 
-test_expect_success \
-    'git ls-files --error-unmatch should fail with unmatched path.' \
-    'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'
+test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '
+	test_must_fail git ls-files --error-unmatch foo bar-does-not-match
+'
 
-test_expect_success \
-    'git ls-files --error-unmatch should succeed with matched paths.' \
-    'git ls-files --error-unmatch foo bar'
+test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '
+	git ls-files --error-unmatch foo bar
+'
 
 test_done
diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh
index b257c792a46..52f76f7b57f 100755
--- a/t/t3060-ls-files-with-tree.sh
+++ b/t/t3060-ls-files-with-tree.sh
@@ -10,7 +10,7 @@ a scenario known to trigger a crash with some versions of git.
 '
 . ./test-lib.sh
 
-test_expect_success setup '
+test_expect_success 'setup' '
 
 	# The bug we are exercising requires a fair number of entries
 	# in a sub-directory so that add_index_entry will trigger a
@@ -62,9 +62,9 @@ test_expect_success 'git ls-files --with-tree should succeed from subdir' '
 	)
 '
 
-test_expect_success \
-    'git ls-files --with-tree should add entries from named tree.' \
-    'test_cmp expected output'
+test_expect_success 'git ls-files --with-tree should add entries from named tree.' '
+	test_cmp expected output
+'
 
 test_expect_success 'no duplicates in --with-tree output' '
 	git ls-files --with-tree=HEAD >actual &&

base-commit: e4a4b31577c7419497ac30cebe30d755b97752c5
-- 
gitgitgadget

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

[Junio C Hamano]

"lilinchao@oschina.cn" <lilinchao@oschina.cn> writes:

>>Mention end of here-document marker should by default be quoted,
>>unless the body needs $variable_interpolation? 
> Sorry, I don't get it. I don't see many of ending "EOF" are quoted in our tests.

We encourage to quote the end marker when there is no interpolation
in the here-document, so that ones that do not quote can stand out
and grab attention by developers who are reading the tests, i.e.

	cmd <<-EOF
	$variable interpolated
	EOF

The fact that EOF is not quoted serves as a signal to readers that
they may want to inspect the here-doc text carefully with the
variable interpolation in mind.  If the EOF marker is quoted,

	cmd <<-\EOF
	literal here text
	EOF

even if there are many lines in the here-document, the readers may
not have to be so careful---what is used is what they see (modulo
removing the leading indentation tabs).

[PATCH v5] ls-files: update test style

[Li Linchao via GitGitGadget]

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,
and fix some indentions.

Add a new section "recommended style" in t/README to
encourage people to use more modern style in test.

Signed-off-by: Li Linchao <lilinchao@oschina.cn>
---
    ls-files: update test style
    
    Update test style in t/t30[*].sh for uniformity, that's to keep test
    title the same line with helper function itself.
    
    And update t/README to describe this test style.
    
    Signed-off-by: Li Linchao lilinchao@oschina.cn

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1269

Range-diff vs v4:

 1:  d0ee9836f23 ! 1:  6cbb4e5fd49 ls-files: update test style
     @@ t/README: This test harness library does the following things:
      +      test_cmp expect actual
      +  '
      +
     ++ - Quote or escape the EOF at the head of a here document when
     ++   there is no variable interpolation in it:
     ++
     ++  cmd <<-"EOF"
     ++  literal here-doc text
     ++  EOF
     ++
     ++  Or:
     ++
     ++  cmd <<-\EOF
     ++  literal here-doc text
     ++  EOF
     ++
      +
       Do's & don'ts
       -------------


 t/README                           | 60 +++++++++++++++++++++
 t/t3001-ls-files-others-exclude.sh | 42 +++++++--------
 t/t3002-ls-files-dashpath.sh       | 86 ++++++++++++++++--------------
 t/t3020-ls-files-error-unmatch.sh  | 12 ++---
 t/t3060-ls-files-with-tree.sh      |  8 +--
 5 files changed, 137 insertions(+), 71 deletions(-)

diff --git a/t/README b/t/README
index 309a31133c6..7d63697fb52 100644
--- a/t/README
+++ b/t/README
@@ -547,6 +547,66 @@ This test harness library does the following things:
    consistently when command line arguments --verbose (or -v),
    --debug (or -d), and --immediate (or -i) is given.
 
+Recommended style
+-----------------
+Here are some recommented styles when writing test case.
+
+ - Keep test title the same line with test helper function itself.
+
+   Take test_expect_success helper for example, write it like:
+
+  test_expect_success 'test title' '
+      ... test body ...
+  '
+
+   Instead of:
+
+  test_expect_success \
+      'test title' \
+      '... test body ...'
+
+
+ - End the line with a single quote.
+
+ - Indent the body of here-document, and use "<<-" instead of "<<"
+   to strip leading TABs used for indentation:
+
+  test_expect_success 'test something' '
+      cat >expect <<-\EOF &&
+      one
+      two
+      three
+      EOF
+      test_something > actual &&
+      test_cmp expect actual
+  '
+
+   Instead of:
+
+  test_expect_success 'test something' '
+      cat >expect <<\EOF &&
+  one
+  two
+  three
+  EOF
+      test_something > actual &&
+      test_cmp expect actual
+  '
+
+ - Quote or escape the EOF at the head of a here document when
+   there is no variable interpolation in it:
+
+  cmd <<-"EOF"
+  literal here-doc text
+  EOF
+
+  Or:
+
+  cmd <<-\EOF
+  literal here-doc text
+  EOF
+
+
 Do's & don'ts
 -------------
 
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index 48cec4e5f88..e07ac6c6dce 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 &&
-     test_cmp expect output'
+	>output &&
+	test_cmp expect output
+'
 
 # 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'
+	>output &&
+	test_cmp expect output
+'
 
 test_expect_success 'setup skip-worktree gitignore' '
 	git add $allignores &&
@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '
 	rm $allignores
 '
 
-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 &&
-     test_cmp expect output'
+	>output &&
+	test_cmp expect output
+'
 
 test_expect_success !SANITIZE_LEAK 'restore gitignore' '
 	git checkout --ignore-skip-worktree-bits $allignores &&
@@ -283,12 +283,12 @@ test_expect_success 'pattern matches prefix completely' '
 '
 
 test_expect_success 'ls-files with "**" patterns' '
-	cat <<\EOF >expect &&
-a.1
-one/a.1
-one/two/a.1
-three/a.1
-EOF
+	cat <<-\EOF >expect &&
+	a.1
+	one/a.1
+	one/two/a.1
+	three/a.1
+	EOF
 	git ls-files -o -i --exclude "**/a.1" >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh
index 54d22a45dfb..4dd24550eba 100755
--- a/t/t3002-ls-files-dashpath.sh
+++ b/t/t3002-ls-files-dashpath.sh
@@ -16,56 +16,62 @@ filesystem.
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-test_expect_success \
-	setup \
-	'echo frotz >path0 &&
+test_expect_success 'setup' '
+	echo frotz >path0 &&
 	echo frotz >./-foo &&
-	echo frotz >./--'
+	echo frotz >./--
+'
 
-test_expect_success \
-    'git ls-files without path restriction.' \
-    'git ls-files --others >output &&
-     test_cmp output - <<EOF
---
--foo
-output
-path0
-EOF
+test_expect_success 'git ls-files without path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others >output &&
+	cat >expect <<-\EOF &&
+	--
+	-foo
+	output
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction.' \
-    'git ls-files --others path0 >output &&
-	test_cmp output - <<EOF
-path0
-EOF
+test_expect_success 'git ls-files with path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others path0 >output &&
+	cat >expect <<-\EOF &&
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction with --.' \
-    'git ls-files --others -- path0 >output &&
-	test_cmp output - <<EOF
-path0
-EOF
+test_expect_success 'git ls-files with path restriction with --.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- path0 >output &&
+	cat >expect <<-\EOF &&
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction with -- --.' \
-    'git ls-files --others -- -- >output &&
-	test_cmp output - <<EOF
---
-EOF
+test_expect_success 'git ls-files with path restriction with -- --.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- -- >output &&
+	cat >expect <<-\EOF &&
+	--
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with no path restriction.' \
-    'git ls-files --others -- >output &&
-	test_cmp output - <<EOF
---
--foo
-output
-path0
-EOF
+test_expect_success 'git ls-files with no path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- >output &&
+	cat >expect <<-\EOF &&
+	--
+	-foo
+	output
+	path0
+	EOF
+	test_cmp output expect
+
 '
 
 test_done
diff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh
index 2cbcbc0721b..133593d23c0 100755
--- a/t/t3020-ls-files-error-unmatch.sh
+++ b/t/t3020-ls-files-error-unmatch.sh
@@ -19,12 +19,12 @@ test_expect_success 'setup' '
 	git commit -m "add foo bar"
 '
 
-test_expect_success \
-    'git ls-files --error-unmatch should fail with unmatched path.' \
-    'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'
+test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '
+	test_must_fail git ls-files --error-unmatch foo bar-does-not-match
+'
 
-test_expect_success \
-    'git ls-files --error-unmatch should succeed with matched paths.' \
-    'git ls-files --error-unmatch foo bar'
+test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '
+	git ls-files --error-unmatch foo bar
+'
 
 test_done
diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh
index b257c792a46..52f76f7b57f 100755
--- a/t/t3060-ls-files-with-tree.sh
+++ b/t/t3060-ls-files-with-tree.sh
@@ -10,7 +10,7 @@ a scenario known to trigger a crash with some versions of git.
 '
 . ./test-lib.sh
 
-test_expect_success setup '
+test_expect_success 'setup' '
 
 	# The bug we are exercising requires a fair number of entries
 	# in a sub-directory so that add_index_entry will trigger a
@@ -62,9 +62,9 @@ test_expect_success 'git ls-files --with-tree should succeed from subdir' '
 	)
 '
 
-test_expect_success \
-    'git ls-files --with-tree should add entries from named tree.' \
-    'test_cmp expected output'
+test_expect_success 'git ls-files --with-tree should add entries from named tree.' '
+	test_cmp expected output
+'
 
 test_expect_success 'no duplicates in --with-tree output' '
 	git ls-files --with-tree=HEAD >actual &&

base-commit: e4a4b31577c7419497ac30cebe30d755b97752c5
-- 
gitgitgadget

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

[Junio C Hamano]

"Li Linchao via GitGitGadget" <gitgitgadget@gmail.com> writes:

> 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,
> and fix some indentions.
>
> Add a new section "recommended style" in t/README to
> encourage people to use more modern style in test.
>
> Signed-off-by: Li Linchao <lilinchao@oschina.cn>
> ---
>     ls-files: update test style
>     
>     Update test style in t/t30[*].sh for uniformity, that's to keep test
>     title the same line with helper function itself.
>     
>     And update t/README to describe this test style.
>     
>     Signed-off-by: Li Linchao lilinchao@oschina.cn
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v5
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v5
> Pull-Request: https://github.com/gitgitgadget/git/pull/1269
>
> Range-diff vs v4:
>
>  1:  d0ee9836f23 ! 1:  6cbb4e5fd49 ls-files: update test style
>      @@ t/README: This test harness library does the following things:
>       +      test_cmp expect actual
>       +  '
>       +
>      ++ - Quote or escape the EOF at the head of a here document when
>      ++   there is no variable interpolation in it:
>      ++
>      ++  cmd <<-"EOF"
>      ++  literal here-doc text
>      ++  EOF
>      ++
>      ++  Or:
>      ++
>      ++  cmd <<-\EOF
>      ++  literal here-doc text
>      ++  EOF
>      ++

I do not htink we need to describe two ways to quote at all.  If we
absolutely need to have two examples, then:

+  cmd <<-\EOF
+  literal here-doc text
+  EOF
+
+  but not:
+
+  cmd <<-EOF
+  literal here-doc text
+  EOF

but I'd say the lines are better spent to explain what we have this
particular style guide item for.

        - Quote or escape the EOF delimiter that begins a here-document if
          there is no parameter and other expansion in it, to signal readers
          that they can skim it more casually:

          cmd <<-\EOF
          literal here-document text without any expansion
          EOF

or something.

Other than that, this looks really good.

Thanks.

[PATCH v6] ls-files: update test style

[Li Linchao via GitGitGadget]

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,
and fix some indentions.

Add a new section "recommended style" in t/README to
encourage people to use more modern style in test.

Signed-off-by: Li Linchao <lilinchao@oschina.cn>
---
    ls-files: update test style
    
    Update test style in t/t30[*].sh for uniformity, that's to keep test
    title the same line with helper function itself.
    
    And update t/README to describe this test style.
    
    Signed-off-by: Li Linchao lilinchao@oschina.cn

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/1269

Range-diff vs v5:

 1:  6cbb4e5fd49 ! 1:  8dae5a70e22 ls-files: update test style
     @@ t/README: This test harness library does the following things:
      +      test_cmp expect actual
      +  '
      +
     -+ - Quote or escape the EOF at the head of a here document when
     -+   there is no variable interpolation in it:
     -+
     -+  cmd <<-"EOF"
     -+  literal here-doc text
     -+  EOF
     -+
     -+  Or:
     ++ - Quote or escape the EOF delimiter that begins a here-document if
     ++   there is no parameter and other expansion in it, to signal readers
     ++   that they can skim it more casually:
      +
      +  cmd <<-\EOF
     -+  literal here-doc text
     ++  literal here-document text without any expansion
      +  EOF
      +
      +


 t/README                           | 55 +++++++++++++++++++
 t/t3001-ls-files-others-exclude.sh | 42 +++++++--------
 t/t3002-ls-files-dashpath.sh       | 86 ++++++++++++++++--------------
 t/t3020-ls-files-error-unmatch.sh  | 12 ++---
 t/t3060-ls-files-with-tree.sh      |  8 +--
 5 files changed, 132 insertions(+), 71 deletions(-)

diff --git a/t/README b/t/README
index 309a31133c6..4f9981cf5e3 100644
--- a/t/README
+++ b/t/README
@@ -547,6 +547,61 @@ This test harness library does the following things:
    consistently when command line arguments --verbose (or -v),
    --debug (or -d), and --immediate (or -i) is given.
 
+Recommended style
+-----------------
+Here are some recommented styles when writing test case.
+
+ - Keep test title the same line with test helper function itself.
+
+   Take test_expect_success helper for example, write it like:
+
+  test_expect_success 'test title' '
+      ... test body ...
+  '
+
+   Instead of:
+
+  test_expect_success \
+      'test title' \
+      '... test body ...'
+
+
+ - End the line with a single quote.
+
+ - Indent the body of here-document, and use "<<-" instead of "<<"
+   to strip leading TABs used for indentation:
+
+  test_expect_success 'test something' '
+      cat >expect <<-\EOF &&
+      one
+      two
+      three
+      EOF
+      test_something > actual &&
+      test_cmp expect actual
+  '
+
+   Instead of:
+
+  test_expect_success 'test something' '
+      cat >expect <<\EOF &&
+  one
+  two
+  three
+  EOF
+      test_something > actual &&
+      test_cmp expect actual
+  '
+
+ - Quote or escape the EOF delimiter that begins a here-document if
+   there is no parameter and other expansion in it, to signal readers
+   that they can skim it more casually:
+
+  cmd <<-\EOF
+  literal here-document text without any expansion
+  EOF
+
+
 Do's & don'ts
 -------------
 
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index 48cec4e5f88..e07ac6c6dce 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 &&
-     test_cmp expect output'
+	>output &&
+	test_cmp expect output
+'
 
 # 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'
+	>output &&
+	test_cmp expect output
+'
 
 test_expect_success 'setup skip-worktree gitignore' '
 	git add $allignores &&
@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '
 	rm $allignores
 '
 
-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 &&
-     test_cmp expect output'
+	>output &&
+	test_cmp expect output
+'
 
 test_expect_success !SANITIZE_LEAK 'restore gitignore' '
 	git checkout --ignore-skip-worktree-bits $allignores &&
@@ -283,12 +283,12 @@ test_expect_success 'pattern matches prefix completely' '
 '
 
 test_expect_success 'ls-files with "**" patterns' '
-	cat <<\EOF >expect &&
-a.1
-one/a.1
-one/two/a.1
-three/a.1
-EOF
+	cat <<-\EOF >expect &&
+	a.1
+	one/a.1
+	one/two/a.1
+	three/a.1
+	EOF
 	git ls-files -o -i --exclude "**/a.1" >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh
index 54d22a45dfb..4dd24550eba 100755
--- a/t/t3002-ls-files-dashpath.sh
+++ b/t/t3002-ls-files-dashpath.sh
@@ -16,56 +16,62 @@ filesystem.
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-test_expect_success \
-	setup \
-	'echo frotz >path0 &&
+test_expect_success 'setup' '
+	echo frotz >path0 &&
 	echo frotz >./-foo &&
-	echo frotz >./--'
+	echo frotz >./--
+'
 
-test_expect_success \
-    'git ls-files without path restriction.' \
-    'git ls-files --others >output &&
-     test_cmp output - <<EOF
---
--foo
-output
-path0
-EOF
+test_expect_success 'git ls-files without path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others >output &&
+	cat >expect <<-\EOF &&
+	--
+	-foo
+	output
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction.' \
-    'git ls-files --others path0 >output &&
-	test_cmp output - <<EOF
-path0
-EOF
+test_expect_success 'git ls-files with path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others path0 >output &&
+	cat >expect <<-\EOF &&
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction with --.' \
-    'git ls-files --others -- path0 >output &&
-	test_cmp output - <<EOF
-path0
-EOF
+test_expect_success 'git ls-files with path restriction with --.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- path0 >output &&
+	cat >expect <<-\EOF &&
+	path0
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with path restriction with -- --.' \
-    'git ls-files --others -- -- >output &&
-	test_cmp output - <<EOF
---
-EOF
+test_expect_success 'git ls-files with path restriction with -- --.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- -- >output &&
+	cat >expect <<-\EOF &&
+	--
+	EOF
+	test_cmp output expect
 '
 
-test_expect_success \
-    'git ls-files with no path restriction.' \
-    'git ls-files --others -- >output &&
-	test_cmp output - <<EOF
---
--foo
-output
-path0
-EOF
+test_expect_success 'git ls-files with no path restriction.' '
+	test_when_finished "rm -f expect" &&
+	git ls-files --others -- >output &&
+	cat >expect <<-\EOF &&
+	--
+	-foo
+	output
+	path0
+	EOF
+	test_cmp output expect
+
 '
 
 test_done
diff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh
index 2cbcbc0721b..133593d23c0 100755
--- a/t/t3020-ls-files-error-unmatch.sh
+++ b/t/t3020-ls-files-error-unmatch.sh
@@ -19,12 +19,12 @@ test_expect_success 'setup' '
 	git commit -m "add foo bar"
 '
 
-test_expect_success \
-    'git ls-files --error-unmatch should fail with unmatched path.' \
-    'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'
+test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '
+	test_must_fail git ls-files --error-unmatch foo bar-does-not-match
+'
 
-test_expect_success \
-    'git ls-files --error-unmatch should succeed with matched paths.' \
-    'git ls-files --error-unmatch foo bar'
+test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '
+	git ls-files --error-unmatch foo bar
+'
 
 test_done
diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh
index b257c792a46..52f76f7b57f 100755
--- a/t/t3060-ls-files-with-tree.sh
+++ b/t/t3060-ls-files-with-tree.sh
@@ -10,7 +10,7 @@ a scenario known to trigger a crash with some versions of git.
 '
 . ./test-lib.sh
 
-test_expect_success setup '
+test_expect_success 'setup' '
 
 	# The bug we are exercising requires a fair number of entries
 	# in a sub-directory so that add_index_entry will trigger a
@@ -62,9 +62,9 @@ test_expect_success 'git ls-files --with-tree should succeed from subdir' '
 	)
 '
 
-test_expect_success \
-    'git ls-files --with-tree should add entries from named tree.' \
-    'test_cmp expected output'
+test_expect_success 'git ls-files --with-tree should add entries from named tree.' '
+	test_cmp expected output
+'
 
 test_expect_success 'no duplicates in --with-tree output' '
 	git ls-files --with-tree=HEAD >actual &&

base-commit: e4a4b31577c7419497ac30cebe30d755b97752c5
-- 
gitgitgadget