threads / patch / 58057

patchls-files: update test style

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

## tl;dr

14 messages between Jun 23, 2022 and Jul 3, 2022. Diffs are folded; open one to read it.

replies: 13people: 4as markdown or json

Li Linchao via GitGitGadget· Jun 23, 2022, 08:46 UTC · lore
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(-)
Show changes to 5 files +50 −40

t/README, t/t3001-ls-files-others-exclude.sh, t/t3002-ls-files-dashpath.sh, t/t3020-ls-files-error-unmatch.sh, t/t3060-ls-files-with-tree.sh

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
Ævar Arnfjörð Bjarmason· Jun 23, 2022, 10:50 UTC · re: Li Linchao via GitGitGadget · lore

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

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.

Show 18 quoted lines
>     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.

Show 17 quoted lines
> +'
>  
>  # 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...

Show 9 quoted lines
> -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.

lilinchao@oschina.cn· Jun 24, 2022, 04:57 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Show 10 quoted lines
>
>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.

Show 25 quoted lines
>
>>     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.
Show 8 quoted lines
>> -     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.
Show 24 quoted lines
>
>> +'
>> 
>>  # 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.
Show 18 quoted lines
>
>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.
Junio C Hamano· Jun 23, 2022, 17:09 UTC · re: Li Linchao via GitGitGadget · lore

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

"Li Linchao via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 16 quoted lines
> 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.

lilinchao@oschina.cn· Jun 24, 2022, 05:05 UTC · re: Junio C Hamano · lore

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

Show 25 quoted lines
>"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.
Show 6 quoted lines
>
>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.
Li Linchao via GitGitGadget· Jun 28, 2022, 09:14 UTC · re: Li Linchao via GitGitGadget · lore

[PATCH v2] ls-files: update test style

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(-)
Show changes to 6 files +126 −70

t/README, t/t3001-ls-files-others-exclude.sh, t/t3002-ls-files-dashpath.sh, t/t3007-ls-files-recurse-submodules.sh, t/t3020-ls-files-error-unmatch.sh, t/t3060-ls-files-with-tree.sh

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
Li Linchao via GitGitGadget· Jun 28, 2022, 09:51 UTC · re: Li Linchao via GitGitGadget · lore

[PATCH v3] ls-files: update test style

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(-)
Show changes to 5 files +123 −68

t/README, t/t3001-ls-files-others-exclude.sh, t/t3002-ls-files-dashpath.sh, t/t3020-ls-files-error-unmatch.sh, t/t3060-ls-files-with-tree.sh

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
Junio C Hamano· Jun 28, 2022, 20:12 UTC · re: Li Linchao via GitGitGadget · lore

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

"Li Linchao via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 19 quoted lines
> 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?

Show 21 quoted lines
> +  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.
lilinchao@oschina.cn· Jun 29, 2022, 07:12 UTC · re: Junio C Hamano · lore

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

Show 29 quoted lines
>"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 "-".

Show 6 quoted lines
>
>"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.
Show 25 quoted lines
>
>> +  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.
Junio C Hamano· Jun 30, 2022, 15:54 UTC · re: lilinchao@oschina.cn · lore

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

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

Li Linchao via GitGitGadget· Jun 30, 2022, 05:59 UTC · re: Li Linchao via GitGitGadget · lore

[PATCH v4] ls-files: update test style

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(-)
Show changes to 5 files +124 −68

t/README, t/t3001-ls-files-others-exclude.sh, t/t3002-ls-files-dashpath.sh, t/t3020-ls-files-error-unmatch.sh, t/t3060-ls-files-with-tree.sh

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
Li Linchao via GitGitGadget· Jul 1, 2022, 11:03 UTC · re: Li Linchao via GitGitGadget · lore

[PATCH v5] ls-files: update test style

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(-)
Show changes to 5 files +137 −68

t/README, t/t3001-ls-files-others-exclude.sh, t/t3002-ls-files-dashpath.sh, t/t3020-ls-files-error-unmatch.sh, t/t3060-ls-files-with-tree.sh

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
Junio C Hamano· Jul 1, 2022, 21:46 UTC · re: Li Linchao via GitGitGadget · lore

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

"Li Linchao via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 44 quoted lines
> 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.
Li Linchao via GitGitGadget· Jul 3, 2022, 15:49 UTC · re: Li Linchao via GitGitGadget · lore

[PATCH v6] ls-files: update test style

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(-)
Show changes to 5 files +132 −68

t/README, t/t3001-ls-files-others-exclude.sh, t/t3002-ls-files-dashpath.sh, t/t3020-ls-files-error-unmatch.sh, t/t3060-ls-files-with-tree.sh

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

← back to recent threads