{"thread":{"id":"58057","subject":"[PATCH] ls-files: update test style","startedAt":"2022-06-23T08:47:06Z","lastAt":"2022-07-03T15:49:19Z","messageCount":14,"participants":["Li Linchao via GitGitGadget","Ævar Arnfjörð Bjarmason","Junio C Hamano","lilinchao@oschina.cn"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"457764","messageId":"pull.1269.git.1655974015414.gitgitgadget@gmail.com","threadId":"58057","inReplyTo":null,"subject":"[PATCH] ls-files: update test style","fromName":"Li Linchao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-23T08:46:55Z","receivedAt":"2022-06-23T08:47:06Z","isPatch":true,"sender":{"key":"name:Li Linchao","avatar":null},"body":"From: Li Linchao \n\nUpdate test style in t/t30[*].sh for uniformity, that's to\nkeep test title the same line with helper function itself.\n\nAnd update t/README to describe this test style.\n\nSigned-off-by: Li Linchao \n---\n ls-files: update test style\n \n Update test style in t/t30[*].sh for uniformity, that's to keep test\n title the same line with helper function itself.\n \n And update t/README to describe this test style.\n \n Signed-off-by: Li Linchao lilinchao@oschina.cn\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1269\n\n t/README | 15 ++++++++++++++\n t/t3001-ls-files-others-exclude.sh | 24 +++++++++++-----------\n t/t3002-ls-files-dashpath.sh | 33 +++++++++++++-----------------\n t/t3020-ls-files-error-unmatch.sh | 12 +++++------\n t/t3060-ls-files-with-tree.sh | 6 +++---\n 5 files changed, 50 insertions(+), 40 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 309a31133c6..70205fba41b 100644\n--- a/t/README\n+++ b/t/README\n@@ -560,6 +560,21 @@ Here are the \"do's:\"\n Even code that isn't a test per se, but merely some setup code\n should be inside a test assertion.\n \n+ - Keep test title the same line with test helper function itself,\n+ and end the line with a single quote.\n+\n+ Take test_expect_success helper for example, write it like:\n+\n+ test_expect_success 'test title to describe this test case' '\n+ # test body\n+ '\n+\n+ Instead of:\n+\n+ test_expect_success \\\n+ 'test title to describe this test case' \\\n+ '# test body'\n+\n - Chain your test assertions\n \n Write test code like this:\ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex 48cec4e5f88..76361b92336 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -67,26 +67,26 @@ echo '!*.2\n \n allignores='.gitignore one/.gitignore one/two/.gitignore'\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n >output &&\n- test_cmp expect output'\n+ test_cmp expect output\n+'\n \n # Test \\r\\n (MSDOS-like systems)\n printf '*.1\\r\\n/*.3\\r\\n!*.6\\r\\n' >.gitignore\n \n-test_expect_success \\\n- 'git ls-files --others with \\r\\n line endings.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with \\r\\n line endings.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n >output &&\n- test_cmp expect output'\n+ test_cmp expect output\n+'\n \n test_expect_success 'setup skip-worktree gitignore' '\n \tgit add $allignores &&\n@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '\n \trm $allignores\n '\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n >output &&\n- test_cmp expect output'\n+ test_cmp expect output\n+'\n \n test_expect_success !SANITIZE_LEAK 'restore gitignore' '\n \tgit checkout --ignore-skip-worktree-bits $allignores &&\ndiff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh\nindex 54d22a45dfb..adbe96fa2df 100755\n--- a/t/t3002-ls-files-dashpath.sh\n+++ b/t/t3002-ls-files-dashpath.sh\n@@ -16,15 +16,14 @@ filesystem.\n TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n-test_expect_success \\\n-\tsetup \\\n-\t'echo frotz >path0 &&\n+test_expect_success 'setup' '\n+ echo frotz >path0 &&\n \techo frotz >./-foo &&\n-\techo frotz >./--'\n+\techo frotz >./--\n+'\n \n-test_expect_success \\\n- 'git ls-files without path restriction.' \\\n- 'git ls-files --others >output &&\n+test_expect_success 'git ls-files without path restriction.' '\n+ git ls-files --others >output &&\n test_cmp output - <output &&\n+test_expect_success 'git ls-files with path restriction.' '\n+ git ls-files --others path0 >output &&\n \ttest_cmp output - <output &&\n+test_expect_success 'git ls-files with path restriction with --.' '\n+ git ls-files --others -- path0 >output &&\n \ttest_cmp output - <output &&\n+test_expect_success 'git ls-files with path restriction with -- --.' '\n+ git ls-files --others -- -- >output &&\n \ttest_cmp output - <output &&\n+test_expect_success 'git ls-files with no path restriction.' '\n+ git ls-files --others -- >output &&\n \ttest_cmp output - <actual &&\n\nbase-commit: 5b71c59bc3b9365075e2a175aa7b6f2b0c84ce44\n-- \ngitgitgadget\n"},{"id":"457771","messageId":"220623.86wnd7k5un.gmgdl@evledraar.gmail.com","threadId":"58057","inReplyTo":"pull.1269.git.1655974015414.gitgitgadget@gmail.com","subject":"Re: [PATCH] ls-files: update test style","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-23T10:50:01Z","receivedAt":"2022-06-23T10:54:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jun 23 2022, Li Linchao via GitGitGadget wrote:\n\n> From: Li Linchao \n>\n> Update test style in t/t30[*].sh for uniformity, that's to\n> keep test title the same line with helper function itself.\n\nWe have a few of these sorts of old style tests, and it's good to update\nthem.\n\n> Write test code like this:\n> diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\n> index 48cec4e5f88..76361b92336 100755\n> --- a/t/t3001-ls-files-others-exclude.sh\n> +++ b/t/t3001-ls-files-others-exclude.sh\n> @@ -67,26 +67,26 @@ echo '!*.2\n> \n> allignores='.gitignore one/.gitignore one/two/.gitignore'\n> \n> -test_expect_success \\\n> - 'git ls-files --others with various exclude options.' \\\n> - 'git ls-files --others \\\n> +test_expect_success 'git ls-files --others with various exclude options.' '\n> +\tgit ls-files --others \\\n> --exclude=\\*.6 \\\n> --exclude-per-directory=.gitignore \\\n> --exclude-from=.git/ignore \\\n> >output &&\n\nThis though really stops too short, here we end up with:\n\n\tgit-ls-files --others \\\n\t<7 spaces>--exclude [...]\n\n> - test_cmp expect output'\n> + test_cmp expect output\n\nAnd you've space-indented this test_cmp, presumably the below has the\nsame issues (I didn't check in detail)\n\nInstead the argument lists should be indented, and the rest\nshould be TAB indented.\n\n> +'\n> \n> # Test \\r\\n (MSDOS-like systems)\n> printf '*.1\\r\\n/*.3\\r\\n!*.6\\r\\n' >.gitignore\n> \n> -test_expect_success \\\n> - 'git ls-files --others with \\r\\n line endings.' \\\n> - 'git ls-files --others \\\n> +test_expect_success 'git ls-files --others with \\r\\n line endings.' '\n> +\tgit ls-files --others \\\n> --exclude=\\*.6 \\\n> --exclude-per-directory=.gitignore \\\n> --exclude-from=.git/ignore \\\n> >output &&\n> - test_cmp expect output'\n> + test_cmp expect output\n> +'\n\nAside from the above I think it's also worth incorporating all the\n\"printf\", \"echo\", \"cat\" etc. that we do into the \"test_expect_success\"\nthemselves, and if they're needed by more than one test perhaps make\nthem a \"setup\" helper function (which would test_when_finished \"rm -f\n.gitignore\" clean up after itself).\n\nThat's obviously bigger than some whitespace changes, so we could punt\non it for now, but as we're looking at this anyway we could convert\nfully to a more modern style in a follow-up commit...\n\n> -test_expect_success \\\n> - 'git ls-files with path restriction with --.' \\\n> - 'git ls-files --others -- path0 >output &&\n> +test_expect_success 'git ls-files with path restriction with --.' '\n> + git ls-files --others -- path0 >output &&\n> \ttest_cmp output - < path0\n> EOF\n> '\n\nOn the topic of leaving things on the table: here we could use \"<<-EOF\"\n(or actually better \"<<-\\EOF\") instead, and indent the here-doc, as we\nusually do.\n"},{"id":"457805","messageId":"xmqq5ykr1f37.fsf@gitster.g","threadId":"58057","inReplyTo":"pull.1269.git.1655974015414.gitgitgadget@gmail.com","subject":"Re: [PATCH] ls-files: update test style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-23T17:09:48Z","receivedAt":"2022-06-23T17:41:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Li Linchao via GitGitGadget\" writes:\n\n> diff --git a/t/README b/t/README\n> index 309a31133c6..70205fba41b 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -560,6 +560,21 @@ Here are the \"do's:\"\n> Even code that isn't a test per se, but merely some setup code\n> should be inside a test assertion.\n> \n> + - Keep test title the same line with test helper function itself,\n> + and end the line with a single quote.\n> +\n> + Take test_expect_success helper for example, write it like:\n> +\n> + test_expect_success 'test title to describe this test case' '\n> + # test body\n> + '\n\nIf you want to show the pretty layout, then the test body should\nbe shown indented, i.e.\n\n\ttest_expect_success 'title' '\n\t\t... test body ...\n\t'\n\nBut I am not sure if this belongs to the existing \"Do's and don'ts\"\nsection, which lists tips that matter for correctness.\n\nThis new one certainly encouraged as a more modern style, but is not\nabout correctness at all.\n\nA separate \"recommended style\" section might make sense, but there\nwill be a lot more entries, like when to quote and not to quote EOF\nmarker for here-document, indenting the body of here-document, etc.\n"},{"id":"457826","messageId":"2022062412565939884621@oschina.cn","threadId":"58057","inReplyTo":"220623.86wnd7k5un.gmgdl@evledraar.gmail.com","subject":"Re: Re: [PATCH] ls-files: update test style","fromName":"lilinchao@oschina.cn","fromEmail":"lilinchao@oschina.cn","sentAt":"2022-06-24T04:57:59Z","receivedAt":"2022-06-24T04:58:37Z","isPatch":true,"sender":{"key":"lilinchao@oschina.cn","avatar":null},"body":">\n>On Thu, Jun 23 2022, Li Linchao via GitGitGadget wrote:\n>\n>> From: Li Linchao \n>>\n>> Update test style in t/t30[*].sh for uniformity, that's to\n>> keep test title the same line with helper function itself.\n>\n>We have a few of these sorts of old style tests, and it's good to update\n>them. \nYes. Currently there are at least 400+ old style tests :). It not easy for me\nto fix them all at once with some magic regex expressions, so I'm not going\nto update them all in one patch. But I think, first of all, we can explicitly\ndocument which test style we prefer first.\n>\n>>     Write test code like this:\n>> diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\n>> index 48cec4e5f88..76361b92336 100755\n>> --- a/t/t3001-ls-files-others-exclude.sh\n>> +++ b/t/t3001-ls-files-others-exclude.sh\n>> @@ -67,26 +67,26 @@ echo '!*.2\n>> \n>>  allignores='.gitignore one/.gitignore one/two/.gitignore'\n>> \n>> -test_expect_success \\\n>> -    'git ls-files --others with various exclude options.' \\\n>> -    'git ls-files --others \\\n>> +test_expect_success 'git ls-files --others with various exclude options.' '\n>> +\tgit ls-files --others \\\n>>         --exclude=\\*.6 \\\n>>         --exclude-per-directory=.gitignore \\\n>>         --exclude-from=.git/ignore \\\n>>         >output &&\n>\n>This though really stops too short, here we end up with:\n>\n>\tgit-ls-files --others \\\n>\t<7 spaces>--exclude [...]\n> \nOK.\n>> -     test_cmp expect output'\n>> +     test_cmp expect output\n>\n>And you've space-indented this test_cmp, presumably the below has the\n>same issues (I didn't check in detail) \n>\n>Instead the argument lists should be indented, and the rest\n>should be TAB indented. \nOK.\n>\n>> +'\n>> \n>>  # Test \\r\\n (MSDOS-like systems)\n>>  printf '*.1\\r\\n/*.3\\r\\n!*.6\\r\\n' >.gitignore\n>> \n>> -test_expect_success \\\n>> -    'git ls-files --others with \\r\\n line endings.' \\\n>> -    'git ls-files --others \\\n>> +test_expect_success 'git ls-files --others with \\r\\n line endings.' '\n>> +\tgit ls-files --others \\\n>>         --exclude=\\*.6 \\\n>>         --exclude-per-directory=.gitignore \\\n>>         --exclude-from=.git/ignore \\\n>>         >output &&\n>> -     test_cmp expect output'\n>> +     test_cmp expect output\n>> +'\n>\n>Aside from the above I think it's also worth incorporating all the\n>\"printf\", \"echo\", \"cat\" etc. that we do into the \"test_expect_success\"\n>themselves, and if they're needed by more than one test perhaps make\n>them a \"setup\" helper function (which would test_when_finished \"rm -f\n>.gitignore\" clean up after itself). \nYes, make sense.\n>\n>That's obviously bigger than some whitespace changes, so we could punt\n>on it for now, but as we're looking at this anyway we could convert\n>fully to a more modern style in a follow-up commit...\n>\n>> -test_expect_success \\\n>> -    'git ls-files with path restriction with --.' \\\n>> -    'git ls-files --others -- path0 >output &&\n>> +test_expect_success 'git ls-files with path restriction with --.' '\n>> +    git ls-files --others -- path0 >output &&\n>>  test_cmp output - <>  path0\n>>  EOF\n>>  '\n>\n>On the topic of leaving things on the table: here we could use \"<<-EOF\"\n>(or actually better \"<<-\\EOF\") instead, and indent the here-doc, as we\n>usually do. \nOK, will do.\n"},{"id":"457827","messageId":"2022062413010539388025@oschina.cn","threadId":"58057","inReplyTo":"xmqq5ykr1f37.fsf@gitster.g","subject":"Re: Re: [PATCH] ls-files: update test style","fromName":"lilinchao@oschina.cn","fromEmail":"lilinchao@oschina.cn","sentAt":"2022-06-24T05:05:00Z","receivedAt":"2022-06-24T05:05:18Z","isPatch":true,"sender":{"key":"lilinchao@oschina.cn","avatar":null},"body":"\n>\"Li Linchao via GitGitGadget\" writes:\n>\n>> diff --git a/t/README b/t/README\n>> index 309a31133c6..70205fba41b 100644\n>> --- a/t/README\n>> +++ b/t/README\n>> @@ -560,6 +560,21 @@ Here are the \"do's:\"\n>>     Even code that isn't a test per se, but merely some setup code\n>>     should be inside a test assertion.\n>> \n>> + - Keep test title the same line with test helper function itself,\n>> +   and end the line with a single quote.\n>> +\n>> +   Take test_expect_success helper for example, write it like:\n>> +\n>> +  test_expect_success 'test title to describe this test case' '\n>> +  # test body\n>> +  '\n>\n>If you want to show the pretty layout, then the test body should\n>be shown indented, i.e.\n>\n>\ttest_expect_success 'title' '\n>\t... test body ...\n>\t'\nOK.\n>\n>But I am not sure if this belongs to the existing \"Do's and don'ts\"\n>section, which lists tips that matter for correctness.\n>\n>This new one certainly encouraged as a more modern style, but is not\n>about correctness at all.\nOK, I will remove it from \"Do's and don'ts\" section.\n>\n>A separate \"recommended style\" section might make sense, but there\n>will be a lot more entries, like when to quote and not to quote EOF\n>marker for here-document, indenting the body of here-document, etc.\nYes, a \"recommended style\" section needed.\n\n"},{"id":"458003","messageId":"pull.1269.v2.git.1656407664694.gitgitgadget@gmail.com","threadId":"58057","inReplyTo":"pull.1269.git.1655974015414.gitgitgadget@gmail.com","subject":"[PATCH v2] ls-files: update test style","fromName":"Li Linchao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-28T09:14:24Z","receivedAt":"2022-06-28T09:14:34Z","isPatch":true,"sender":{"key":"name:Li Linchao","avatar":null},"body":"From: Li Linchao \n\nUpdate test style in t/t30[*].sh for uniformity, that's to\nkeep test title the same line with helper function itself.\n\nAnd update t/README to describe this test style.\n\nSigned-off-by: Li Linchao \n---\n ls-files: update test style\n \n Update test style in t/t30[*].sh for uniformity, that's to keep test\n title the same line with helper function itself.\n \n And update t/README to describe this test style.\n \n Signed-off-by: Li Linchao lilinchao@oschina.cn\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1269\n\nRange-diff vs v1:\n\n 1: e18a6172cff ! 1: 775c17499d5 ls-files: update test style\n @@ Commit message\n Signed-off-by: Li Linchao \n \n ## t/README ##\n -@@ t/README: Here are the \"do's:\"\n - Even code that isn't a test per se, but merely some setup code\n - should be inside a test assertion.\n +@@ t/README: This test harness library does the following things:\n + consistently when command line arguments --verbose (or -v),\n + --debug (or -d), and --immediate (or -i) is given.\n \n -+ - Keep test title the same line with test helper function itself,\n -+ and end the line with a single quote.\n ++Recommended style\n ++-----------------\n ++Here are some recommented style that you should follow when you write some test cases.\n ++\n ++ - Keep test title the same line with test helper function itself.\n +\n + Take test_expect_success helper for example, write it like:\n +\n -+ test_expect_success 'test title to describe this test case' '\n -+ # test body\n ++ test_expect_success 'test title' '\n ++ ... test body ...\n + '\n +\n + Instead of:\n +\n + test_expect_success \\\n -+ 'test title to describe this test case' \\\n -+ '# test body'\n ++ 'test title' \\\n ++ '... test body ...'\n ++\n ++\n ++ - End the line with a single quote.\n ++\n ++ - Indent the body of here-document, and cut off prefix TAB by using\n ++ \"<<-\":\n +\n - - Chain your test assertions\n ++ test_expect_success 'test something' '\n ++ cat >expect <<-\\EOF &&\n ++ one\n ++ two\n ++ three\n ++ EOF\n ++ test_something > actual &&\n ++ test_cmp expect actual\n ++ '\n ++\n ++ Instead of:\n ++\n ++ test_expect_success 'test something' '\n ++ cat >expect <<\\EOF &&\n ++ one\n ++ two\n ++ three\n ++ EOF\n ++ test_something > actual &&\n ++ test_cmp expect actual\n ++ '\n ++\n ++\n + Do's & don'ts\n + -------------\n \n - Write test code like this:\n \n ## t/t3001-ls-files-others-exclude.sh ##\n @@ t/t3001-ls-files-others-exclude.sh: echo '!*.2\n @@ t/t3001-ls-files-others-exclude.sh: echo '!*.2\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n - >output &&\n +- >output &&\n - test_cmp expect output'\n -+ test_cmp expect output\n ++\t>output &&\n ++\ttest_cmp expect output\n +'\n \n # Test \\r\\n (MSDOS-like systems)\n @@ t/t3001-ls-files-others-exclude.sh: echo '!*.2\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n - >output &&\n +- >output &&\n - test_cmp expect output'\n -+ test_cmp expect output\n ++\t>output &&\n ++\ttest_cmp expect output\n +'\n \n test_expect_success 'setup skip-worktree gitignore' '\n @@ t/t3001-ls-files-others-exclude.sh: test_expect_success 'setup skip-worktree git\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n - >output &&\n +- >output &&\n - test_cmp expect output'\n -+ test_cmp expect output\n ++\t>output &&\n ++\ttest_cmp expect output\n +'\n \n test_expect_success !SANITIZE_LEAK 'restore gitignore' '\n \tgit checkout --ignore-skip-worktree-bits $allignores &&\n +@@ t/t3001-ls-files-others-exclude.sh: test_expect_success 'pattern matches prefix completely' '\n + '\n + \n + test_expect_success 'ls-files with \"**\" patterns' '\n +-\tcat <<\\EOF >expect &&\n +-a.1\n +-one/a.1\n +-one/two/a.1\n +-three/a.1\n +-EOF\n ++\tcat <<-\\EOF >expect &&\n ++\ta.1\n ++\tone/a.1\n ++\tone/two/a.1\n ++\tthree/a.1\n ++\tEOF\n + \tgit ls-files -o -i --exclude \"**/a.1\" >actual &&\n + \ttest_cmp expect actual\n + '\n \n ## t/t3002-ls-files-dashpath.sh ##\n @@ t/t3002-ls-files-dashpath.sh: filesystem.\n @@ t/t3002-ls-files-dashpath.sh: filesystem.\n -\tsetup \\\n -\t'echo frotz >path0 &&\n +test_expect_success 'setup' '\n -+ echo frotz >path0 &&\n ++\techo frotz >path0 &&\n \techo frotz >./-foo &&\n -\techo frotz >./--'\n +\techo frotz >./--\n @@ t/t3002-ls-files-dashpath.sh: filesystem.\n -test_expect_success \\\n - 'git ls-files without path restriction.' \\\n - 'git ls-files --others >output &&\n +- test_cmp output - <output &&\n - test_cmp output - <output &&\n ++\tcat >expect <<-\\EOF &&\n ++\t--\n ++\t-foo\n ++\toutput\n ++\tpath0\n ++\tEOF\n ++\ttest_cmp output expect\n '\n \n -test_expect_success \\\n - 'git ls-files with path restriction.' \\\n - 'git ls-files --others path0 >output &&\n +-\ttest_cmp output - <output &&\n - \ttest_cmp output - <output &&\n ++\tcat >expect <<-\\EOF &&\n ++\tpath0\n ++\tEOF\n ++\ttest_cmp output expect\n '\n \n -test_expect_success \\\n - 'git ls-files with path restriction with --.' \\\n - 'git ls-files --others -- path0 >output &&\n +-\ttest_cmp output - <output &&\n - \ttest_cmp output - <output &&\n ++\tcat >expect <<-\\EOF &&\n ++\tpath0\n ++\tEOF\n ++\ttest_cmp output expect\n '\n \n -test_expect_success \\\n - 'git ls-files with path restriction with -- --.' \\\n - 'git ls-files --others -- -- >output &&\n +-\ttest_cmp output - <output &&\n - \ttest_cmp output - <output &&\n ++\tcat >expect <<-\\EOF &&\n ++\t--\n ++\tEOF\n ++\ttest_cmp output expect\n '\n \n -test_expect_success \\\n - 'git ls-files with no path restriction.' \\\n - 'git ls-files --others -- >output &&\n +-\ttest_cmp output - <output &&\n - \ttest_cmp output - <output &&\n ++\tcat >expect <<-\\EOF &&\n ++\t--\n ++\t-foo\n ++\toutput\n ++\tpath0\n ++\tEOF\n ++\ttest_cmp output expect\n ++\n + '\n + \n + test_done\n +\n + ## t/t3007-ls-files-recurse-submodules.sh ##\n +@@ t/t3007-ls-files-recurse-submodules.sh: test_expect_success '--recurse-submodules does not support --error-unmatch' '\n + \n + test_incompatible_with_recurse_submodules () {\n + \ttest_expect_success \"--recurse-submodules and $1 are incompatible\" \"\n +-\t\ttest_must_fail git ls-files --recurse-submodules $1 2>actual &&\n +-\t\ttest_i18ngrep 'unsupported mode' actual\n ++\ttest_must_fail git ls-files --recurse-submodules $1 2>actual &&\n ++\ttest_i18ngrep 'unsupported mode' actual\n + \t\"\n + }\n + \n \n ## t/t3020-ls-files-error-unmatch.sh ##\n @@ t/t3020-ls-files-error-unmatch.sh: test_expect_success 'setup' '\n @@ t/t3020-ls-files-error-unmatch.sh: test_expect_success 'setup' '\n - 'git ls-files --error-unmatch should fail with unmatched path.' \\\n - 'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'\n +test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '\n -+ test_must_fail git ls-files --error-unmatch foo bar-does-not-match\n ++\ttest_must_fail git ls-files --error-unmatch foo bar-does-not-match\n +'\n \n -test_expect_success \\\n - 'git ls-files --error-unmatch should succeed with matched paths.' \\\n - 'git ls-files --error-unmatch foo bar'\n +test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '\n -+ git ls-files --error-unmatch foo bar\n ++\tgit ls-files --error-unmatch foo bar\n +'\n \n test_done\n \n ## t/t3060-ls-files-with-tree.sh ##\n +@@ t/t3060-ls-files-with-tree.sh: a scenario known to trigger a crash with some versions of git.\n + '\n + . ./test-lib.sh\n + \n +-test_expect_success setup '\n ++test_expect_success 'setup' '\n + \n + \t# The bug we are exercising requires a fair number of entries\n + \t# in a sub-directory so that add_index_entry will trigger a\n @@ t/t3060-ls-files-with-tree.sh: test_expect_success 'git ls-files --with-tree should succeed from subdir' '\n \t)\n '\n\n\n t/README | 47 ++++++++++++++\n t/t3001-ls-files-others-exclude.sh | 42 ++++++-------\n t/t3002-ls-files-dashpath.sh | 86 ++++++++++++++------------\n t/t3007-ls-files-recurse-submodules.sh | 4 +-\n t/t3020-ls-files-error-unmatch.sh | 12 ++--\n t/t3060-ls-files-with-tree.sh | 8 +--\n 6 files changed, 126 insertions(+), 73 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 309a31133c6..845af1883c3 100644\n--- a/t/README\n+++ b/t/README\n@@ -547,6 +547,53 @@ This test harness library does the following things:\n consistently when command line arguments --verbose (or -v),\n --debug (or -d), and --immediate (or -i) is given.\n \n+Recommended style\n+-----------------\n+Here are some recommented style that you should follow when you write some test cases.\n+\n+ - Keep test title the same line with test helper function itself.\n+\n+ Take test_expect_success helper for example, write it like:\n+\n+ test_expect_success 'test title' '\n+ ... test body ...\n+ '\n+\n+ Instead of:\n+\n+ test_expect_success \\\n+ 'test title' \\\n+ '... test body ...'\n+\n+\n+ - End the line with a single quote.\n+\n+ - Indent the body of here-document, and cut off prefix TAB by using\n+ \"<<-\":\n+\n+ test_expect_success 'test something' '\n+ cat >expect <<-\\EOF &&\n+ one\n+ two\n+ three\n+ EOF\n+ test_something > actual &&\n+ test_cmp expect actual\n+ '\n+\n+ Instead of:\n+\n+ test_expect_success 'test something' '\n+ cat >expect <<\\EOF &&\n+ one\n+ two\n+ three\n+ EOF\n+ test_something > actual &&\n+ test_cmp expect actual\n+ '\n+\n+\n Do's & don'ts\n -------------\n \ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex 48cec4e5f88..e07ac6c6dce 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -67,26 +67,26 @@ echo '!*.2\n \n allignores='.gitignore one/.gitignore one/two/.gitignore'\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n # Test \\r\\n (MSDOS-like systems)\n printf '*.1\\r\\n/*.3\\r\\n!*.6\\r\\n' >.gitignore\n \n-test_expect_success \\\n- 'git ls-files --others with \\r\\n line endings.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with \\r\\n line endings.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n test_expect_success 'setup skip-worktree gitignore' '\n \tgit add $allignores &&\n@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '\n \trm $allignores\n '\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n test_expect_success !SANITIZE_LEAK 'restore gitignore' '\n \tgit checkout --ignore-skip-worktree-bits $allignores &&\n@@ -283,12 +283,12 @@ test_expect_success 'pattern matches prefix completely' '\n '\n \n test_expect_success 'ls-files with \"**\" patterns' '\n-\tcat <<\\EOF >expect &&\n-a.1\n-one/a.1\n-one/two/a.1\n-three/a.1\n-EOF\n+\tcat <<-\\EOF >expect &&\n+\ta.1\n+\tone/a.1\n+\tone/two/a.1\n+\tthree/a.1\n+\tEOF\n \tgit ls-files -o -i --exclude \"**/a.1\" >actual &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh\nindex 54d22a45dfb..4dd24550eba 100755\n--- a/t/t3002-ls-files-dashpath.sh\n+++ b/t/t3002-ls-files-dashpath.sh\n@@ -16,56 +16,62 @@ filesystem.\n TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n-test_expect_success \\\n-\tsetup \\\n-\t'echo frotz >path0 &&\n+test_expect_success 'setup' '\n+\techo frotz >path0 &&\n \techo frotz >./-foo &&\n-\techo frotz >./--'\n+\techo frotz >./--\n+'\n \n-test_expect_success \\\n- 'git ls-files without path restriction.' \\\n- 'git ls-files --others >output &&\n- test_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\t-foo\n+\toutput\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction.' \\\n- 'git ls-files --others path0 >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction with --.' \\\n- 'git ls-files --others -- path0 >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction with -- --.' \\\n- 'git ls-files --others -- -- >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with no path restriction.' \\\n- 'git ls-files --others -- >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\t-foo\n+\toutput\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n+\n '\n \n test_done\ndiff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh\nindex dd7770e85de..2a057e987fe 100755\n--- a/t/t3007-ls-files-recurse-submodules.sh\n+++ b/t/t3007-ls-files-recurse-submodules.sh\n@@ -301,8 +301,8 @@ test_expect_success '--recurse-submodules does not support --error-unmatch' '\n \n test_incompatible_with_recurse_submodules () {\n \ttest_expect_success \"--recurse-submodules and $1 are incompatible\" \"\n-\t\ttest_must_fail git ls-files --recurse-submodules $1 2>actual &&\n-\t\ttest_i18ngrep 'unsupported mode' actual\n+\ttest_must_fail git ls-files --recurse-submodules $1 2>actual &&\n+\ttest_i18ngrep 'unsupported mode' actual\n \t\"\n }\n \ndiff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh\nindex 2cbcbc0721b..133593d23c0 100755\n--- a/t/t3020-ls-files-error-unmatch.sh\n+++ b/t/t3020-ls-files-error-unmatch.sh\n@@ -19,12 +19,12 @@ test_expect_success 'setup' '\n \tgit commit -m \"add foo bar\"\n '\n \n-test_expect_success \\\n- 'git ls-files --error-unmatch should fail with unmatched path.' \\\n- 'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'\n+test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '\n+\ttest_must_fail git ls-files --error-unmatch foo bar-does-not-match\n+'\n \n-test_expect_success \\\n- 'git ls-files --error-unmatch should succeed with matched paths.' \\\n- 'git ls-files --error-unmatch foo bar'\n+test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '\n+\tgit ls-files --error-unmatch foo bar\n+'\n \n test_done\ndiff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh\nindex b257c792a46..52f76f7b57f 100755\n--- a/t/t3060-ls-files-with-tree.sh\n+++ b/t/t3060-ls-files-with-tree.sh\n@@ -10,7 +10,7 @@ a scenario known to trigger a crash with some versions of git.\n '\n . ./test-lib.sh\n \n-test_expect_success setup '\n+test_expect_success 'setup' '\n \n \t# The bug we are exercising requires a fair number of entries\n \t# in a sub-directory so that add_index_entry will trigger a\n@@ -62,9 +62,9 @@ test_expect_success 'git ls-files --with-tree should succeed from subdir' '\n \t)\n '\n \n-test_expect_success \\\n- 'git ls-files --with-tree should add entries from named tree.' \\\n- 'test_cmp expected output'\n+test_expect_success 'git ls-files --with-tree should add entries from named tree.' '\n+\ttest_cmp expected output\n+'\n \n test_expect_success 'no duplicates in --with-tree output' '\n \tgit ls-files --with-tree=HEAD >actual &&\n\nbase-commit: e4a4b31577c7419497ac30cebe30d755b97752c5\n-- \ngitgitgadget\n"},{"id":"458005","messageId":"pull.1269.v3.git.1656409884091.gitgitgadget@gmail.com","threadId":"58057","inReplyTo":"pull.1269.v2.git.1656407664694.gitgitgadget@gmail.com","subject":"[PATCH v3] ls-files: update test style","fromName":"Li Linchao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-28T09:51:23Z","receivedAt":"2022-06-28T09:51:33Z","isPatch":true,"sender":{"key":"name:Li Linchao","avatar":null},"body":"From: Li Linchao \n\nUpdate test style in t/t30[*].sh for uniformity, that's to\nkeep test title the same line with helper function itself,\nand fix some indentions.\n\nAdd a new section \"recommended style\" in t/README to\nencourage people to use more modern style in test.\n\nSigned-off-by: Li Linchao \n---\n ls-files: update test style\n \n Update test style in t/t30[*].sh for uniformity, that's to keep test\n title the same line with helper function itself.\n \n And update t/README to describe this test style.\n \n Signed-off-by: Li Linchao lilinchao@oschina.cn\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1269\n\nRange-diff vs v2:\n\n 1: 775c17499d5 ! 1: b3d80bd9cd5 ls-files: update test style\n @@ Commit message\n ls-files: update test style\n \n Update test style in t/t30[*].sh for uniformity, that's to\n - keep test title the same line with helper function itself.\n + keep test title the same line with helper function itself,\n + and fix some indentions.\n \n - And update t/README to describe this test style.\n + Add a new section \"recommended style\" in t/README to\n + encourage people to use more modern style in test.\n \n Signed-off-by: Li Linchao \n \n @@ t/README: This test harness library does the following things:\n \n +Recommended style\n +-----------------\n -+Here are some recommented style that you should follow when you write some test cases.\n ++Here are some recommented styles when writing test case.\n +\n + - Keep test title the same line with test helper function itself.\n +\n @@ t/README: This test harness library does the following things:\n +\n + - End the line with a single quote.\n +\n -+ - Indent the body of here-document, and cut off prefix TAB by using\n -+ \"<<-\":\n ++ - Indent the body of here-document, and use \"<< -\" instead of \"<<\" to strip prefix TAB:\n +\n + test_expect_success 'test something' '\n + cat >expect <<-\\EOF &&\n @@ t/t3002-ls-files-dashpath.sh: filesystem.\n \n test_done\n \n - ## t/t3007-ls-files-recurse-submodules.sh ##\n -@@ t/t3007-ls-files-recurse-submodules.sh: test_expect_success '--recurse-submodules does not support --error-unmatch' '\n - \n - test_incompatible_with_recurse_submodules () {\n - \ttest_expect_success \"--recurse-submodules and $1 are incompatible\" \"\n --\t\ttest_must_fail git ls-files --recurse-submodules $1 2>actual &&\n --\t\ttest_i18ngrep 'unsupported mode' actual\n -+\ttest_must_fail git ls-files --recurse-submodules $1 2>actual &&\n -+\ttest_i18ngrep 'unsupported mode' actual\n - \t\"\n - }\n - \n -\n ## t/t3020-ls-files-error-unmatch.sh ##\n @@ t/t3020-ls-files-error-unmatch.sh: test_expect_success 'setup' '\n \tgit commit -m \"add foo bar\"\n\n\n t/README | 46 ++++++++++++++++\n t/t3001-ls-files-others-exclude.sh | 42 +++++++--------\n t/t3002-ls-files-dashpath.sh | 86 ++++++++++++++++--------------\n t/t3020-ls-files-error-unmatch.sh | 12 ++---\n t/t3060-ls-files-with-tree.sh | 8 +--\n 5 files changed, 123 insertions(+), 71 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 309a31133c6..5e0539412b4 100644\n--- a/t/README\n+++ b/t/README\n@@ -547,6 +547,52 @@ This test harness library does the following things:\n consistently when command line arguments --verbose (or -v),\n --debug (or -d), and --immediate (or -i) is given.\n \n+Recommended style\n+-----------------\n+Here are some recommented styles when writing test case.\n+\n+ - Keep test title the same line with test helper function itself.\n+\n+ Take test_expect_success helper for example, write it like:\n+\n+ test_expect_success 'test title' '\n+ ... test body ...\n+ '\n+\n+ Instead of:\n+\n+ test_expect_success \\\n+ 'test title' \\\n+ '... test body ...'\n+\n+\n+ - End the line with a single quote.\n+\n+ - Indent the body of here-document, and use \"<< -\" instead of \"<<\" to strip prefix TAB:\n+\n+ test_expect_success 'test something' '\n+ cat >expect <<-\\EOF &&\n+ one\n+ two\n+ three\n+ EOF\n+ test_something > actual &&\n+ test_cmp expect actual\n+ '\n+\n+ Instead of:\n+\n+ test_expect_success 'test something' '\n+ cat >expect <<\\EOF &&\n+ one\n+ two\n+ three\n+ EOF\n+ test_something > actual &&\n+ test_cmp expect actual\n+ '\n+\n+\n Do's & don'ts\n -------------\n \ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex 48cec4e5f88..e07ac6c6dce 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -67,26 +67,26 @@ echo '!*.2\n \n allignores='.gitignore one/.gitignore one/two/.gitignore'\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n # Test \\r\\n (MSDOS-like systems)\n printf '*.1\\r\\n/*.3\\r\\n!*.6\\r\\n' >.gitignore\n \n-test_expect_success \\\n- 'git ls-files --others with \\r\\n line endings.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with \\r\\n line endings.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n test_expect_success 'setup skip-worktree gitignore' '\n \tgit add $allignores &&\n@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '\n \trm $allignores\n '\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n test_expect_success !SANITIZE_LEAK 'restore gitignore' '\n \tgit checkout --ignore-skip-worktree-bits $allignores &&\n@@ -283,12 +283,12 @@ test_expect_success 'pattern matches prefix completely' '\n '\n \n test_expect_success 'ls-files with \"**\" patterns' '\n-\tcat <<\\EOF >expect &&\n-a.1\n-one/a.1\n-one/two/a.1\n-three/a.1\n-EOF\n+\tcat <<-\\EOF >expect &&\n+\ta.1\n+\tone/a.1\n+\tone/two/a.1\n+\tthree/a.1\n+\tEOF\n \tgit ls-files -o -i --exclude \"**/a.1\" >actual &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh\nindex 54d22a45dfb..4dd24550eba 100755\n--- a/t/t3002-ls-files-dashpath.sh\n+++ b/t/t3002-ls-files-dashpath.sh\n@@ -16,56 +16,62 @@ filesystem.\n TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n-test_expect_success \\\n-\tsetup \\\n-\t'echo frotz >path0 &&\n+test_expect_success 'setup' '\n+\techo frotz >path0 &&\n \techo frotz >./-foo &&\n-\techo frotz >./--'\n+\techo frotz >./--\n+'\n \n-test_expect_success \\\n- 'git ls-files without path restriction.' \\\n- 'git ls-files --others >output &&\n- test_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\t-foo\n+\toutput\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction.' \\\n- 'git ls-files --others path0 >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction with --.' \\\n- 'git ls-files --others -- path0 >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction with -- --.' \\\n- 'git ls-files --others -- -- >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with no path restriction.' \\\n- 'git ls-files --others -- >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\t-foo\n+\toutput\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n+\n '\n \n test_done\ndiff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh\nindex 2cbcbc0721b..133593d23c0 100755\n--- a/t/t3020-ls-files-error-unmatch.sh\n+++ b/t/t3020-ls-files-error-unmatch.sh\n@@ -19,12 +19,12 @@ test_expect_success 'setup' '\n \tgit commit -m \"add foo bar\"\n '\n \n-test_expect_success \\\n- 'git ls-files --error-unmatch should fail with unmatched path.' \\\n- 'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'\n+test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '\n+\ttest_must_fail git ls-files --error-unmatch foo bar-does-not-match\n+'\n \n-test_expect_success \\\n- 'git ls-files --error-unmatch should succeed with matched paths.' \\\n- 'git ls-files --error-unmatch foo bar'\n+test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '\n+\tgit ls-files --error-unmatch foo bar\n+'\n \n test_done\ndiff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh\nindex b257c792a46..52f76f7b57f 100755\n--- a/t/t3060-ls-files-with-tree.sh\n+++ b/t/t3060-ls-files-with-tree.sh\n@@ -10,7 +10,7 @@ a scenario known to trigger a crash with some versions of git.\n '\n . ./test-lib.sh\n \n-test_expect_success setup '\n+test_expect_success 'setup' '\n \n \t# The bug we are exercising requires a fair number of entries\n \t# in a sub-directory so that add_index_entry will trigger a\n@@ -62,9 +62,9 @@ test_expect_success 'git ls-files --with-tree should succeed from subdir' '\n \t)\n '\n \n-test_expect_success \\\n- 'git ls-files --with-tree should add entries from named tree.' \\\n- 'test_cmp expected output'\n+test_expect_success 'git ls-files --with-tree should add entries from named tree.' '\n+\ttest_cmp expected output\n+'\n \n test_expect_success 'no duplicates in --with-tree output' '\n \tgit ls-files --with-tree=HEAD >actual &&\n\nbase-commit: e4a4b31577c7419497ac30cebe30d755b97752c5\n-- \ngitgitgadget\n"},{"id":"458066","messageId":"xmqqzghwim3m.fsf@gitster.g","threadId":"58057","inReplyTo":"pull.1269.v3.git.1656409884091.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] ls-files: update test style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-28T20:12:13Z","receivedAt":"2022-06-28T20:16:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Li Linchao via GitGitGadget\" writes:\n\n> diff --git a/t/README b/t/README\n> index 309a31133c6..5e0539412b4 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -547,6 +547,52 @@ This test harness library does the following things:\n> consistently when command line arguments --verbose (or -v),\n> --debug (or -d), and --immediate (or -i) is given.\n> \n> +Recommended style\n> +-----------------\n> +Here are some recommented styles when writing test case.\n> +\n> + - Keep test title the same line with test helper function itself.\n> +\n> + Take test_expect_success helper for example, write it like:\n> +\n> + test_expect_success 'test title' '\n> + ... test body ...\n> + '\n\nIndent the body further to the right?\n\n> + - Indent the body of here-document, and use \"<< -\" instead of \"<<\" to strip prefix TAB:\n\nOverly long line.\n\nDid you mean to have a space between \"<<\" and \"-\"?\n\n\"prefix TAB\" -> \"leading TABs used for indentation\" (plural is the\nimportant part)? \n\nMention end of here-document marker should by default be quoted,\nunless the body needs $variable_interpolation?\n\n> + test_expect_success 'test something' '\n> + cat >expect <<-\\EOF &&\n> + one\n> + two\n> + three\n> + EOF\n> + test_something > actual &&\n> + test_cmp expect actual\n> + '\n> +\n> + Instead of:\n> +\n> + test_expect_success 'test something' '\n> + cat >expect <<\\EOF &&\n> + one\n> + two\n> + three\n> + EOF\n> + test_something > actual &&\n> + test_cmp expect actual\n> + '\n\n\nThanks.\n"},{"id":"458082","messageId":"2022062915111229306214@oschina.cn","threadId":"58057","inReplyTo":"xmqqzghwim3m.fsf@gitster.g","subject":"Re: Re: [PATCH v3] ls-files: update test style","fromName":"lilinchao@oschina.cn","fromEmail":"lilinchao@oschina.cn","sentAt":"2022-06-29T07:12:13Z","receivedAt":"2022-06-29T07:12:32Z","isPatch":true,"sender":{"key":"lilinchao@oschina.cn","avatar":null},"body":"\n>\"Li Linchao via GitGitGadget\" writes:\n>\n>> diff --git a/t/README b/t/README\n>> index 309a31133c6..5e0539412b4 100644\n>> --- a/t/README\n>> +++ b/t/README\n>> @@ -547,6 +547,52 @@ This test harness library does the following things:\n>>     consistently when command line arguments --verbose (or -v),\n>>     --debug (or -d), and --immediate (or -i) is given.\n>> \n>> +Recommended style\n>> +-----------------\n>> +Here are some recommented styles when writing test case.\n>> +\n>> + - Keep test title the same line with test helper function itself.\n>> +\n>> +   Take test_expect_success helper for example, write it like:\n>> +\n>> +  test_expect_success 'test title' '\n>> +  ... test body ...\n>> +  '\n>\n>Indent the body further to the right?\n>\n>> + - Indent the body of here-document, and use \"<< -\" instead of \"<<\" to strip prefix TAB:\n>\n>Overly long line.\n>\n>Did you mean to have a space between \"<<\" and \"-\"? \nOps, I'll fix it. This came from an extension in VS code which is to preview AsciiDoc,\nit mistakenly render \"<<-\" into \"<←\", so I put a space between \"<<\" and \"-\".\n>\n>\"prefix TAB\" -> \"leading TABs used for indentation\" (plural is the\n>important part)?  \n>\n>Mention end of here-document marker should by default be quoted,\n>unless the body needs $variable_interpolation? \nSorry, I don't get it. I don't see many of ending \"EOF\" are quoted in our tests.\n>\n>> +  test_expect_success 'test something' '\n>> +      cat >expect <<-\\EOF &&\n>> +      one\n>> +      two\n>> +      three\n>> +      EOF\n>> +      test_something > actual &&\n>> +      test_cmp expect actual\n>> +  '\n>> +\n>> +   Instead of:\n>> +\n>> +  test_expect_success 'test something' '\n>> +      cat >expect <<\\EOF &&\n>> +  one\n>> +  two\n>> +  three\n>> +  EOF\n>> +      test_something > actual &&\n>> +      test_cmp expect actual\n>> +  '\n>\n>\n>Thanks."},{"id":"458159","messageId":"pull.1269.v4.git.1656568797328.gitgitgadget@gmail.com","threadId":"58057","inReplyTo":"pull.1269.v3.git.1656409884091.gitgitgadget@gmail.com","subject":"[PATCH v4] ls-files: update test style","fromName":"Li Linchao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-30T05:59:57Z","receivedAt":"2022-06-30T06:00:08Z","isPatch":true,"sender":{"key":"name:Li Linchao","avatar":null},"body":"From: Li Linchao \n\nUpdate test style in t/t30[*].sh for uniformity, that's to\nkeep test title the same line with helper function itself,\nand fix some indentions.\n\nAdd a new section \"recommended style\" in t/README to\nencourage people to use more modern style in test.\n\nSigned-off-by: Li Linchao \n---\n ls-files: update test style\n \n Update test style in t/t30[*].sh for uniformity, that's to keep test\n title the same line with helper function itself.\n \n And update t/README to describe this test style.\n \n Signed-off-by: Li Linchao lilinchao@oschina.cn\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/1269\n\nRange-diff vs v3:\n\n 1: b3d80bd9cd5 ! 1: d0ee9836f23 ls-files: update test style\n @@ t/README: This test harness library does the following things:\n + Take test_expect_success helper for example, write it like:\n +\n + test_expect_success 'test title' '\n -+ ... test body ...\n ++ ... test body ...\n + '\n +\n + Instead of:\n +\n + test_expect_success \\\n -+ 'test title' \\\n -+ '... test body ...'\n ++ 'test title' \\\n ++ '... test body ...'\n +\n +\n + - End the line with a single quote.\n +\n -+ - Indent the body of here-document, and use \"<< -\" instead of \"<<\" to strip prefix TAB:\n ++ - Indent the body of here-document, and use \"<<-\" instead of \"<<\"\n ++ to strip leading TABs used for indentation:\n +\n + test_expect_success 'test something' '\n + cat >expect <<-\\EOF &&\n\n\n t/README | 47 ++++++++++++++++\n t/t3001-ls-files-others-exclude.sh | 42 +++++++--------\n t/t3002-ls-files-dashpath.sh | 86 ++++++++++++++++--------------\n t/t3020-ls-files-error-unmatch.sh | 12 ++---\n t/t3060-ls-files-with-tree.sh | 8 +--\n 5 files changed, 124 insertions(+), 71 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 309a31133c6..f662471338d 100644\n--- a/t/README\n+++ b/t/README\n@@ -547,6 +547,53 @@ This test harness library does the following things:\n consistently when command line arguments --verbose (or -v),\n --debug (or -d), and --immediate (or -i) is given.\n \n+Recommended style\n+-----------------\n+Here are some recommented styles when writing test case.\n+\n+ - Keep test title the same line with test helper function itself.\n+\n+ Take test_expect_success helper for example, write it like:\n+\n+ test_expect_success 'test title' '\n+ ... test body ...\n+ '\n+\n+ Instead of:\n+\n+ test_expect_success \\\n+ 'test title' \\\n+ '... test body ...'\n+\n+\n+ - End the line with a single quote.\n+\n+ - Indent the body of here-document, and use \"<<-\" instead of \"<<\"\n+ to strip leading TABs used for indentation:\n+\n+ test_expect_success 'test something' '\n+ cat >expect <<-\\EOF &&\n+ one\n+ two\n+ three\n+ EOF\n+ test_something > actual &&\n+ test_cmp expect actual\n+ '\n+\n+ Instead of:\n+\n+ test_expect_success 'test something' '\n+ cat >expect <<\\EOF &&\n+ one\n+ two\n+ three\n+ EOF\n+ test_something > actual &&\n+ test_cmp expect actual\n+ '\n+\n+\n Do's & don'ts\n -------------\n \ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex 48cec4e5f88..e07ac6c6dce 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -67,26 +67,26 @@ echo '!*.2\n \n allignores='.gitignore one/.gitignore one/two/.gitignore'\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n # Test \\r\\n (MSDOS-like systems)\n printf '*.1\\r\\n/*.3\\r\\n!*.6\\r\\n' >.gitignore\n \n-test_expect_success \\\n- 'git ls-files --others with \\r\\n line endings.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with \\r\\n line endings.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n test_expect_success 'setup skip-worktree gitignore' '\n \tgit add $allignores &&\n@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '\n \trm $allignores\n '\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n test_expect_success !SANITIZE_LEAK 'restore gitignore' '\n \tgit checkout --ignore-skip-worktree-bits $allignores &&\n@@ -283,12 +283,12 @@ test_expect_success 'pattern matches prefix completely' '\n '\n \n test_expect_success 'ls-files with \"**\" patterns' '\n-\tcat <<\\EOF >expect &&\n-a.1\n-one/a.1\n-one/two/a.1\n-three/a.1\n-EOF\n+\tcat <<-\\EOF >expect &&\n+\ta.1\n+\tone/a.1\n+\tone/two/a.1\n+\tthree/a.1\n+\tEOF\n \tgit ls-files -o -i --exclude \"**/a.1\" >actual &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh\nindex 54d22a45dfb..4dd24550eba 100755\n--- a/t/t3002-ls-files-dashpath.sh\n+++ b/t/t3002-ls-files-dashpath.sh\n@@ -16,56 +16,62 @@ filesystem.\n TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n-test_expect_success \\\n-\tsetup \\\n-\t'echo frotz >path0 &&\n+test_expect_success 'setup' '\n+\techo frotz >path0 &&\n \techo frotz >./-foo &&\n-\techo frotz >./--'\n+\techo frotz >./--\n+'\n \n-test_expect_success \\\n- 'git ls-files without path restriction.' \\\n- 'git ls-files --others >output &&\n- test_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\t-foo\n+\toutput\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction.' \\\n- 'git ls-files --others path0 >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction with --.' \\\n- 'git ls-files --others -- path0 >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction with -- --.' \\\n- 'git ls-files --others -- -- >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with no path restriction.' \\\n- 'git ls-files --others -- >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\t-foo\n+\toutput\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n+\n '\n \n test_done\ndiff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh\nindex 2cbcbc0721b..133593d23c0 100755\n--- a/t/t3020-ls-files-error-unmatch.sh\n+++ b/t/t3020-ls-files-error-unmatch.sh\n@@ -19,12 +19,12 @@ test_expect_success 'setup' '\n \tgit commit -m \"add foo bar\"\n '\n \n-test_expect_success \\\n- 'git ls-files --error-unmatch should fail with unmatched path.' \\\n- 'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'\n+test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '\n+\ttest_must_fail git ls-files --error-unmatch foo bar-does-not-match\n+'\n \n-test_expect_success \\\n- 'git ls-files --error-unmatch should succeed with matched paths.' \\\n- 'git ls-files --error-unmatch foo bar'\n+test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '\n+\tgit ls-files --error-unmatch foo bar\n+'\n \n test_done\ndiff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh\nindex b257c792a46..52f76f7b57f 100755\n--- a/t/t3060-ls-files-with-tree.sh\n+++ b/t/t3060-ls-files-with-tree.sh\n@@ -10,7 +10,7 @@ a scenario known to trigger a crash with some versions of git.\n '\n . ./test-lib.sh\n \n-test_expect_success setup '\n+test_expect_success 'setup' '\n \n \t# The bug we are exercising requires a fair number of entries\n \t# in a sub-directory so that add_index_entry will trigger a\n@@ -62,9 +62,9 @@ test_expect_success 'git ls-files --with-tree should succeed from subdir' '\n \t)\n '\n \n-test_expect_success \\\n- 'git ls-files --with-tree should add entries from named tree.' \\\n- 'test_cmp expected output'\n+test_expect_success 'git ls-files --with-tree should add entries from named tree.' '\n+\ttest_cmp expected output\n+'\n \n test_expect_success 'no duplicates in --with-tree output' '\n \tgit ls-files --with-tree=HEAD >actual &&\n\nbase-commit: e4a4b31577c7419497ac30cebe30d755b97752c5\n-- \ngitgitgadget\n"},{"id":"458199","messageId":"xmqq1qv6gn9q.fsf@gitster.g","threadId":"58057","inReplyTo":"2022062915111229306214@oschina.cn","subject":"Re: [PATCH v3] ls-files: update test style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-30T15:54:25Z","receivedAt":"2022-06-30T15:54:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"lilinchao@oschina.cn\" writes:\n\n>>Mention end of here-document marker should by default be quoted,\n>>unless the body needs $variable_interpolation? \n> Sorry, I don't get it. I don't see many of ending \"EOF\" are quoted in our tests.\n\nWe encourage to quote the end marker when there is no interpolation\nin the here-document, so that ones that do not quote can stand out\nand grab attention by developers who are reading the tests, i.e.\n\n\tcmd <<-EOF\n\t$variable interpolated\n\tEOF\n\nThe fact that EOF is not quoted serves as a signal to readers that\nthey may want to inspect the here-doc text carefully with the\nvariable interpolation in mind. If the EOF marker is quoted,\n\n\tcmd <<-\\EOF\n\tliteral here text\n\tEOF\n\neven if there are many lines in the here-document, the readers may\nnot have to be so careful---what is used is what they see (modulo\nremoving the leading indentation tabs).\n\n\n"},{"id":"458378","messageId":"pull.1269.v5.git.1656673435357.gitgitgadget@gmail.com","threadId":"58057","inReplyTo":"pull.1269.v4.git.1656568797328.gitgitgadget@gmail.com","subject":"[PATCH v5] ls-files: update test style","fromName":"Li Linchao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-01T11:03:55Z","receivedAt":"2022-07-01T11:04:04Z","isPatch":true,"sender":{"key":"name:Li Linchao","avatar":null},"body":"From: Li Linchao \n\nUpdate test style in t/t30[*].sh for uniformity, that's to\nkeep test title the same line with helper function itself,\nand fix some indentions.\n\nAdd a new section \"recommended style\" in t/README to\nencourage people to use more modern style in test.\n\nSigned-off-by: Li Linchao \n---\n ls-files: update test style\n \n Update test style in t/t30[*].sh for uniformity, that's to keep test\n title the same line with helper function itself.\n \n And update t/README to describe this test style.\n \n Signed-off-by: Li Linchao lilinchao@oschina.cn\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v5\nPull-Request: https://github.com/gitgitgadget/git/pull/1269\n\nRange-diff vs v4:\n\n 1: d0ee9836f23 ! 1: 6cbb4e5fd49 ls-files: update test style\n @@ t/README: This test harness library does the following things:\n + test_cmp expect actual\n + '\n +\n ++ - Quote or escape the EOF at the head of a here document when\n ++ there is no variable interpolation in it:\n ++\n ++ cmd <<-\"EOF\"\n ++ literal here-doc text\n ++ EOF\n ++\n ++ Or:\n ++\n ++ cmd <<-\\EOF\n ++ literal here-doc text\n ++ EOF\n ++\n +\n Do's & don'ts\n -------------\n\n\n t/README | 60 +++++++++++++++++++++\n t/t3001-ls-files-others-exclude.sh | 42 +++++++--------\n t/t3002-ls-files-dashpath.sh | 86 ++++++++++++++++--------------\n t/t3020-ls-files-error-unmatch.sh | 12 ++---\n t/t3060-ls-files-with-tree.sh | 8 +--\n 5 files changed, 137 insertions(+), 71 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 309a31133c6..7d63697fb52 100644\n--- a/t/README\n+++ b/t/README\n@@ -547,6 +547,66 @@ This test harness library does the following things:\n consistently when command line arguments --verbose (or -v),\n --debug (or -d), and --immediate (or -i) is given.\n \n+Recommended style\n+-----------------\n+Here are some recommented styles when writing test case.\n+\n+ - Keep test title the same line with test helper function itself.\n+\n+ Take test_expect_success helper for example, write it like:\n+\n+ test_expect_success 'test title' '\n+ ... test body ...\n+ '\n+\n+ Instead of:\n+\n+ test_expect_success \\\n+ 'test title' \\\n+ '... test body ...'\n+\n+\n+ - End the line with a single quote.\n+\n+ - Indent the body of here-document, and use \"<<-\" instead of \"<<\"\n+ to strip leading TABs used for indentation:\n+\n+ test_expect_success 'test something' '\n+ cat >expect <<-\\EOF &&\n+ one\n+ two\n+ three\n+ EOF\n+ test_something > actual &&\n+ test_cmp expect actual\n+ '\n+\n+ Instead of:\n+\n+ test_expect_success 'test something' '\n+ cat >expect <<\\EOF &&\n+ one\n+ two\n+ three\n+ EOF\n+ test_something > actual &&\n+ test_cmp expect actual\n+ '\n+\n+ - Quote or escape the EOF at the head of a here document when\n+ there is no variable interpolation in it:\n+\n+ cmd <<-\"EOF\"\n+ literal here-doc text\n+ EOF\n+\n+ Or:\n+\n+ cmd <<-\\EOF\n+ literal here-doc text\n+ EOF\n+\n+\n Do's & don'ts\n -------------\n \ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex 48cec4e5f88..e07ac6c6dce 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -67,26 +67,26 @@ echo '!*.2\n \n allignores='.gitignore one/.gitignore one/two/.gitignore'\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n # Test \\r\\n (MSDOS-like systems)\n printf '*.1\\r\\n/*.3\\r\\n!*.6\\r\\n' >.gitignore\n \n-test_expect_success \\\n- 'git ls-files --others with \\r\\n line endings.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with \\r\\n line endings.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n test_expect_success 'setup skip-worktree gitignore' '\n \tgit add $allignores &&\n@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '\n \trm $allignores\n '\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n test_expect_success !SANITIZE_LEAK 'restore gitignore' '\n \tgit checkout --ignore-skip-worktree-bits $allignores &&\n@@ -283,12 +283,12 @@ test_expect_success 'pattern matches prefix completely' '\n '\n \n test_expect_success 'ls-files with \"**\" patterns' '\n-\tcat <<\\EOF >expect &&\n-a.1\n-one/a.1\n-one/two/a.1\n-three/a.1\n-EOF\n+\tcat <<-\\EOF >expect &&\n+\ta.1\n+\tone/a.1\n+\tone/two/a.1\n+\tthree/a.1\n+\tEOF\n \tgit ls-files -o -i --exclude \"**/a.1\" >actual &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh\nindex 54d22a45dfb..4dd24550eba 100755\n--- a/t/t3002-ls-files-dashpath.sh\n+++ b/t/t3002-ls-files-dashpath.sh\n@@ -16,56 +16,62 @@ filesystem.\n TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n-test_expect_success \\\n-\tsetup \\\n-\t'echo frotz >path0 &&\n+test_expect_success 'setup' '\n+\techo frotz >path0 &&\n \techo frotz >./-foo &&\n-\techo frotz >./--'\n+\techo frotz >./--\n+'\n \n-test_expect_success \\\n- 'git ls-files without path restriction.' \\\n- 'git ls-files --others >output &&\n- test_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\t-foo\n+\toutput\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction.' \\\n- 'git ls-files --others path0 >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction with --.' \\\n- 'git ls-files --others -- path0 >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction with -- --.' \\\n- 'git ls-files --others -- -- >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with no path restriction.' \\\n- 'git ls-files --others -- >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\t-foo\n+\toutput\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n+\n '\n \n test_done\ndiff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh\nindex 2cbcbc0721b..133593d23c0 100755\n--- a/t/t3020-ls-files-error-unmatch.sh\n+++ b/t/t3020-ls-files-error-unmatch.sh\n@@ -19,12 +19,12 @@ test_expect_success 'setup' '\n \tgit commit -m \"add foo bar\"\n '\n \n-test_expect_success \\\n- 'git ls-files --error-unmatch should fail with unmatched path.' \\\n- 'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'\n+test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '\n+\ttest_must_fail git ls-files --error-unmatch foo bar-does-not-match\n+'\n \n-test_expect_success \\\n- 'git ls-files --error-unmatch should succeed with matched paths.' \\\n- 'git ls-files --error-unmatch foo bar'\n+test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '\n+\tgit ls-files --error-unmatch foo bar\n+'\n \n test_done\ndiff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh\nindex b257c792a46..52f76f7b57f 100755\n--- a/t/t3060-ls-files-with-tree.sh\n+++ b/t/t3060-ls-files-with-tree.sh\n@@ -10,7 +10,7 @@ a scenario known to trigger a crash with some versions of git.\n '\n . ./test-lib.sh\n \n-test_expect_success setup '\n+test_expect_success 'setup' '\n \n \t# The bug we are exercising requires a fair number of entries\n \t# in a sub-directory so that add_index_entry will trigger a\n@@ -62,9 +62,9 @@ test_expect_success 'git ls-files --with-tree should succeed from subdir' '\n \t)\n '\n \n-test_expect_success \\\n- 'git ls-files --with-tree should add entries from named tree.' \\\n- 'test_cmp expected output'\n+test_expect_success 'git ls-files --with-tree should add entries from named tree.' '\n+\ttest_cmp expected output\n+'\n \n test_expect_success 'no duplicates in --with-tree output' '\n \tgit ls-files --with-tree=HEAD >actual &&\n\nbase-commit: e4a4b31577c7419497ac30cebe30d755b97752c5\n-- \ngitgitgadget\n"},{"id":"458422","messageId":"xmqqbku87bgo.fsf@gitster.g","threadId":"58057","inReplyTo":"pull.1269.v5.git.1656673435357.gitgitgadget@gmail.com","subject":"Re: [PATCH v5] ls-files: update test style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-01T21:46:31Z","receivedAt":"2022-07-01T21:46:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Li Linchao via GitGitGadget\" writes:\n\n> From: Li Linchao \n>\n> Update test style in t/t30[*].sh for uniformity, that's to\n> keep test title the same line with helper function itself,\n> and fix some indentions.\n>\n> Add a new section \"recommended style\" in t/README to\n> encourage people to use more modern style in test.\n>\n> Signed-off-by: Li Linchao \n> ---\n> ls-files: update test style\n> \n> Update test style in t/t30[*].sh for uniformity, that's to keep test\n> title the same line with helper function itself.\n> \n> And update t/README to describe this test style.\n> \n> Signed-off-by: Li Linchao lilinchao@oschina.cn\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v5\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v5\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1269\n>\n> Range-diff vs v4:\n>\n> 1: d0ee9836f23 ! 1: 6cbb4e5fd49 ls-files: update test style\n> @@ t/README: This test harness library does the following things:\n> + test_cmp expect actual\n> + '\n> +\n> ++ - Quote or escape the EOF at the head of a here document when\n> ++ there is no variable interpolation in it:\n> ++\n> ++ cmd <<-\"EOF\"\n> ++ literal here-doc text\n> ++ EOF\n> ++\n> ++ Or:\n> ++\n> ++ cmd <<-\\EOF\n> ++ literal here-doc text\n> ++ EOF\n> ++\n\nI do not htink we need to describe two ways to quote at all. If we\nabsolutely need to have two examples, then:\n\n+ cmd <<-\\EOF\n+ literal here-doc text\n+ EOF\n+\n+ but not:\n+\n+ cmd <<-EOF\n+ literal here-doc text\n+ EOF\n\nbut I'd say the lines are better spent to explain what we have this\nparticular style guide item for.\n\n - Quote or escape the EOF delimiter that begins a here-document if\n there is no parameter and other expansion in it, to signal readers\n that they can skim it more casually:\n\n cmd <<-\\EOF\n literal here-document text without any expansion\n EOF\n\nor something.\n\nOther than that, this looks really good.\n\nThanks.\n"},{"id":"458433","messageId":"pull.1269.v6.git.1656863349926.gitgitgadget@gmail.com","threadId":"58057","inReplyTo":"pull.1269.v5.git.1656673435357.gitgitgadget@gmail.com","subject":"[PATCH v6] ls-files: update test style","fromName":"Li Linchao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-03T15:49:09Z","receivedAt":"2022-07-03T15:49:19Z","isPatch":true,"sender":{"key":"name:Li Linchao","avatar":null},"body":"From: Li Linchao \n\nUpdate test style in t/t30[*].sh for uniformity, that's to\nkeep test title the same line with helper function itself,\nand fix some indentions.\n\nAdd a new section \"recommended style\" in t/README to\nencourage people to use more modern style in test.\n\nSigned-off-by: Li Linchao \n---\n ls-files: update test style\n \n Update test style in t/t30[*].sh for uniformity, that's to keep test\n title the same line with helper function itself.\n \n And update t/README to describe this test style.\n \n Signed-off-by: Li Linchao lilinchao@oschina.cn\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1269%2FCactusinhand%2Fllc%2Ffix-test-title-style-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1269/Cactusinhand/llc/fix-test-title-style-v6\nPull-Request: https://github.com/gitgitgadget/git/pull/1269\n\nRange-diff vs v5:\n\n 1: 6cbb4e5fd49 ! 1: 8dae5a70e22 ls-files: update test style\n @@ t/README: This test harness library does the following things:\n + test_cmp expect actual\n + '\n +\n -+ - Quote or escape the EOF at the head of a here document when\n -+ there is no variable interpolation in it:\n -+\n -+ cmd <<-\"EOF\"\n -+ literal here-doc text\n -+ EOF\n -+\n -+ Or:\n ++ - Quote or escape the EOF delimiter that begins a here-document if\n ++ there is no parameter and other expansion in it, to signal readers\n ++ that they can skim it more casually:\n +\n + cmd <<-\\EOF\n -+ literal here-doc text\n ++ literal here-document text without any expansion\n + EOF\n +\n +\n\n\n t/README | 55 +++++++++++++++++++\n t/t3001-ls-files-others-exclude.sh | 42 +++++++--------\n t/t3002-ls-files-dashpath.sh | 86 ++++++++++++++++--------------\n t/t3020-ls-files-error-unmatch.sh | 12 ++---\n t/t3060-ls-files-with-tree.sh | 8 +--\n 5 files changed, 132 insertions(+), 71 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 309a31133c6..4f9981cf5e3 100644\n--- a/t/README\n+++ b/t/README\n@@ -547,6 +547,61 @@ This test harness library does the following things:\n consistently when command line arguments --verbose (or -v),\n --debug (or -d), and --immediate (or -i) is given.\n \n+Recommended style\n+-----------------\n+Here are some recommented styles when writing test case.\n+\n+ - Keep test title the same line with test helper function itself.\n+\n+ Take test_expect_success helper for example, write it like:\n+\n+ test_expect_success 'test title' '\n+ ... test body ...\n+ '\n+\n+ Instead of:\n+\n+ test_expect_success \\\n+ 'test title' \\\n+ '... test body ...'\n+\n+\n+ - End the line with a single quote.\n+\n+ - Indent the body of here-document, and use \"<<-\" instead of \"<<\"\n+ to strip leading TABs used for indentation:\n+\n+ test_expect_success 'test something' '\n+ cat >expect <<-\\EOF &&\n+ one\n+ two\n+ three\n+ EOF\n+ test_something > actual &&\n+ test_cmp expect actual\n+ '\n+\n+ Instead of:\n+\n+ test_expect_success 'test something' '\n+ cat >expect <<\\EOF &&\n+ one\n+ two\n+ three\n+ EOF\n+ test_something > actual &&\n+ test_cmp expect actual\n+ '\n+\n+ - Quote or escape the EOF delimiter that begins a here-document if\n+ there is no parameter and other expansion in it, to signal readers\n+ that they can skim it more casually:\n+\n+ cmd <<-\\EOF\n+ literal here-document text without any expansion\n+ EOF\n+\n+\n Do's & don'ts\n -------------\n \ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex 48cec4e5f88..e07ac6c6dce 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -67,26 +67,26 @@ echo '!*.2\n \n allignores='.gitignore one/.gitignore one/two/.gitignore'\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n # Test \\r\\n (MSDOS-like systems)\n printf '*.1\\r\\n/*.3\\r\\n!*.6\\r\\n' >.gitignore\n \n-test_expect_success \\\n- 'git ls-files --others with \\r\\n line endings.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with \\r\\n line endings.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n test_expect_success 'setup skip-worktree gitignore' '\n \tgit add $allignores &&\n@@ -94,14 +94,14 @@ test_expect_success 'setup skip-worktree gitignore' '\n \trm $allignores\n '\n \n-test_expect_success \\\n- 'git ls-files --others with various exclude options.' \\\n- 'git ls-files --others \\\n+test_expect_success 'git ls-files --others with various exclude options.' '\n+\tgit ls-files --others \\\n --exclude=\\*.6 \\\n --exclude-per-directory=.gitignore \\\n --exclude-from=.git/ignore \\\n- >output &&\n- test_cmp expect output'\n+\t>output &&\n+\ttest_cmp expect output\n+'\n \n test_expect_success !SANITIZE_LEAK 'restore gitignore' '\n \tgit checkout --ignore-skip-worktree-bits $allignores &&\n@@ -283,12 +283,12 @@ test_expect_success 'pattern matches prefix completely' '\n '\n \n test_expect_success 'ls-files with \"**\" patterns' '\n-\tcat <<\\EOF >expect &&\n-a.1\n-one/a.1\n-one/two/a.1\n-three/a.1\n-EOF\n+\tcat <<-\\EOF >expect &&\n+\ta.1\n+\tone/a.1\n+\tone/two/a.1\n+\tthree/a.1\n+\tEOF\n \tgit ls-files -o -i --exclude \"**/a.1\" >actual &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t3002-ls-files-dashpath.sh b/t/t3002-ls-files-dashpath.sh\nindex 54d22a45dfb..4dd24550eba 100755\n--- a/t/t3002-ls-files-dashpath.sh\n+++ b/t/t3002-ls-files-dashpath.sh\n@@ -16,56 +16,62 @@ filesystem.\n TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n-test_expect_success \\\n-\tsetup \\\n-\t'echo frotz >path0 &&\n+test_expect_success 'setup' '\n+\techo frotz >path0 &&\n \techo frotz >./-foo &&\n-\techo frotz >./--'\n+\techo frotz >./--\n+'\n \n-test_expect_success \\\n- 'git ls-files without path restriction.' \\\n- 'git ls-files --others >output &&\n- test_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\t-foo\n+\toutput\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction.' \\\n- 'git ls-files --others path0 >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction with --.' \\\n- 'git ls-files --others -- path0 >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with path restriction with -- --.' \\\n- 'git ls-files --others -- -- >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\tEOF\n+\ttest_cmp output expect\n '\n \n-test_expect_success \\\n- 'git ls-files with no path restriction.' \\\n- 'git ls-files --others -- >output &&\n-\ttest_cmp output - <output &&\n+\tcat >expect <<-\\EOF &&\n+\t--\n+\t-foo\n+\toutput\n+\tpath0\n+\tEOF\n+\ttest_cmp output expect\n+\n '\n \n test_done\ndiff --git a/t/t3020-ls-files-error-unmatch.sh b/t/t3020-ls-files-error-unmatch.sh\nindex 2cbcbc0721b..133593d23c0 100755\n--- a/t/t3020-ls-files-error-unmatch.sh\n+++ b/t/t3020-ls-files-error-unmatch.sh\n@@ -19,12 +19,12 @@ test_expect_success 'setup' '\n \tgit commit -m \"add foo bar\"\n '\n \n-test_expect_success \\\n- 'git ls-files --error-unmatch should fail with unmatched path.' \\\n- 'test_must_fail git ls-files --error-unmatch foo bar-does-not-match'\n+test_expect_success 'git ls-files --error-unmatch should fail with unmatched path.' '\n+\ttest_must_fail git ls-files --error-unmatch foo bar-does-not-match\n+'\n \n-test_expect_success \\\n- 'git ls-files --error-unmatch should succeed with matched paths.' \\\n- 'git ls-files --error-unmatch foo bar'\n+test_expect_success 'git ls-files --error-unmatch should succeed with matched paths.' '\n+\tgit ls-files --error-unmatch foo bar\n+'\n \n test_done\ndiff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh\nindex b257c792a46..52f76f7b57f 100755\n--- a/t/t3060-ls-files-with-tree.sh\n+++ b/t/t3060-ls-files-with-tree.sh\n@@ -10,7 +10,7 @@ a scenario known to trigger a crash with some versions of git.\n '\n . ./test-lib.sh\n \n-test_expect_success setup '\n+test_expect_success 'setup' '\n \n \t# The bug we are exercising requires a fair number of entries\n \t# in a sub-directory so that add_index_entry will trigger a\n@@ -62,9 +62,9 @@ test_expect_success 'git ls-files --with-tree should succeed from subdir' '\n \t)\n '\n \n-test_expect_success \\\n- 'git ls-files --with-tree should add entries from named tree.' \\\n- 'test_cmp expected output'\n+test_expect_success 'git ls-files --with-tree should add entries from named tree.' '\n+\ttest_cmp expected output\n+'\n \n test_expect_success 'no duplicates in --with-tree output' '\n \tgit ls-files --with-tree=HEAD >actual &&\n\nbase-commit: e4a4b31577c7419497ac30cebe30d755b97752c5\n-- \ngitgitgadget\n"}]}