{"thread":{"id":"24363","subject":"[PATCH] t3000 (ls-files -o): modernize style","startedAt":"2010-07-11T04:20:25Z","lastAt":"2010-07-11T15:14:32Z","messageCount":4,"participants":["Jonathan Nieder","Jakub Narebski","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"145292","messageId":"20100711042025.GA7424@burratino","threadId":"24363","inReplyTo":null,"subject":"[PATCH] t3000 (ls-files -o): modernize style","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-11T04:20:25Z","receivedAt":"2010-07-11T04:20:25Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"This script is part of the second batch of tests, from the same day\nthe test infrastructure was added to git.  Update it to use a more\nmodern style in the spirit of v1.6.4-rc0~45^2~2 (2009-05-22).\nIn particular:\n\n - Put setup code inside test assertions, to avoid unexpected\n   breakages and avoid stray output without -v (as t/README\n   recommends); and\n\n - Put the test title on the same line as the \"test_expect_success\",\n   and end the line with a single-quote to begin the body of the test\n   which is one multi-line string.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n t/t3000-ls-files-others.sh |   99 +++++++++++++++++++++-----------------------\n 1 files changed, 47 insertions(+), 52 deletions(-)\n\ndiff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh\nindex 86291e8..2eec011 100755\n--- a/t/t3000-ls-files-others.sh\n+++ b/t/t3000-ls-files-others.sh\n@@ -17,57 +17,52 @@ filesystem.\n '\n . ./test-lib.sh\n \n-date >path0\n-if test_have_prereq SYMLINKS\n-then\n-\tln -s xyzzy path1\n-else\n-\tdate > path1\n-fi\n-mkdir path2 path3 path4\n-date >path2/file2\n-date >path2-junk\n-date >path3/file3\n-date >path3-junk\n-git update-index --add path3-junk path3/file3\n-\n+test_expect_success 'setup ' '\n+\tdate >path0 &&\n+\tif test_have_prereq SYMLINKS\n+\tthen\n+\t\tln -s xyzzy path1\n+\telse\n+\t\tdate >path1\n+\tfi &&\n+\tmkdir path2 path3 path4 &&\n+\tdate >path2/file2 &&\n+\tdate >path2-junk &&\n+\tdate >path3/file3 &&\n+\tdate >path3-junk &&\n+\tgit update-index --add path3-junk path3/file3\n+'\n+\n-cat >expected1 <<EOF\n-expected1\n-expected2\n-expected3\n-output\n-path0\n-path1\n-path2-junk\n-path2/file2\n-EOF\n-sed -e 's|path2/file2|path2/|' <expected1 >expected2\n-cat <expected2 >expected3\n-echo path4/ >>expected2\n-\n+test_expect_success 'setup: expected output' '\n+\tcat >expected1 <<-\\EOF &&\n+\texpected1\n+\texpected2\n+\texpected3\n+\toutput\n+\tpath0\n+\tpath1\n+\tpath2-junk\n+\tpath2/file2\n+\tEOF\n+\n+\tsed -e \"s|path2/file2|path2/|\" <expected1 >expected2 &&\n+\tcp expected2 expected3 &&\n+\techo path4/ >>expected2\n+'\n+\n-test_expect_success \\\n-    'git ls-files --others to show output.' \\\n-    'git ls-files --others >output'\n-\n-test_expect_success \\\n-    'git ls-files --others should pick up symlinks.' \\\n-    'test_cmp expected1 output'\n-\n+test_expect_success 'ls-files --others' '\n+\tgit ls-files --others >output &&\n+\ttest_cmp expected1 output\n+'\n+\n-test_expect_success \\\n-    'git ls-files --others --directory to show output.' \\\n-    'git ls-files --others --directory >output'\n-\n-\n-test_expect_success \\\n-    'git ls-files --others --directory should not get confused.' \\\n-    'test_cmp expected2 output'\n-\n+test_expect_success 'ls-files --others --directory' '\n+\tgit ls-files --others --directory >output &&\n+\ttest_cmp expected2 output\n+'\n+\n-test_expect_success \\\n-    'git ls-files --others --directory --no-empty-directory to show output.' \\\n-    'git ls-files --others --directory --no-empty-directory >output'\n-\n-test_expect_success \\\n-    '--no-empty-directory hides empty directory' \\\n-    'test_cmp expected3 output'\n+test_expect_success '--no-empty-directory hides empty directory' '\n+\tgit ls-files --others --directory --no-empty-directory >output &&\n+\ttest_cmp expected3 output\n+'\n \n test_done\n-- \n1.7.2.rc2\n"},{"id":"145318","messageId":"m3d3uul8pd.fsf@localhost.localdomain","threadId":"24363","inReplyTo":"20100711042025.GA7424@burratino","subject":"Re: [PATCH] t3000 (ls-files -o): modernize style","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-07-11T08:33:33Z","receivedAt":"2010-07-11T08:33:33Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> This script is part of the second batch of tests, from the same day\n> the test infrastructure was added to git.  Update it to use a more\n> modern style in the spirit of v1.6.4-rc0~45^2~2 (2009-05-22).\n> In particular:\n> \n>  - Put setup code inside test assertions, to avoid unexpected\n>    breakages and avoid stray output without -v (as t/README\n>    recommends); and\n\nO.K., but...\n \n> -cat >expected1 <<EOF\n> -expected1\n> -expected2\n> -expected3\n> -output\n> -path0\n> -path1\n> -path2-junk\n> -path2/file2\n> -EOF\n> -sed -e 's|path2/file2|path2/|' <expected1 >expected2\n> -cat <expected2 >expected3\n> -echo path4/ >>expected2\n> -\n> +test_expect_success 'setup: expected output' '\n> +\tcat >expected1 <<-\\EOF &&\n> +\texpected1\n> +\texpected2\n> +\texpected3\n> +\toutput\n> +\tpath0\n> +\tpath1\n> +\tpath2-junk\n> +\tpath2/file2\n> +\tEOF\n> +\n> +\tsed -e \"s|path2/file2|path2/|\" <expected1 >expected2 &&\n> +\tcp expected2 expected3 &&\n> +\techo path4/ >>expected2\n> +'\n\n... I don't think it is really needed.  There isn't anything there\nthat can fail, and there isn't anything there that produces any\noutput.\n\nBTW. do all sane shells that can be used to run test suite support\n'<<-' here-doc redirection operators?  Is it in POSIX?\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"145320","messageId":"AANLkTinR4hnLkIbPxxD2f4k6PLpVwALpxKIQHLOvT2hr@mail.gmail.com","threadId":"24363","inReplyTo":"m3d3uul8pd.fsf@localhost.localdomain","subject":"Re: [PATCH] t3000 (ls-files -o): modernize style","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-11T09:10:34Z","receivedAt":"2010-07-11T09:10:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Jul 11, 2010 at 08:33, Jakub Narebski <jnareb@gmail.com> wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n>\n>> This script is part of the second batch of tests, from the same day\n>> the test infrastructure was added to git.  Update it to use a more\n>> modern style in the spirit of v1.6.4-rc0~45^2~2 (2009-05-22).\n>> In particular:\n\nOn the patch:\n\n    Acked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\n>>  - Put setup code inside test assertions, to avoid unexpected\n>>    breakages and avoid stray output without -v (as t/README\n>>    recommends); and\n>\n> O.K., but...\n>\n>> -cat >expected1 <<EOF\n>> -expected1\n>> -expected2\n>> -expected3\n>> -output\n>> -path0\n>> -path1\n>> -path2-junk\n>> -path2/file2\n>> -EOF\n>> -sed -e 's|path2/file2|path2/|' <expected1 >expected2\n>> -cat <expected2 >expected3\n>> -echo path4/ >>expected2\n>> -\n>> +test_expect_success 'setup: expected output' '\n>> +     cat >expected1 <<-\\EOF &&\n>> +     expected1\n>> +     expected2\n>> +     expected3\n>> +     output\n>> +     path0\n>> +     path1\n>> +     path2-junk\n>> +     path2/file2\n>> +     EOF\n>> +\n>> +     sed -e \"s|path2/file2|path2/|\" <expected1 >expected2 &&\n>> +     cp expected2 expected3 &&\n>> +     echo path4/ >>expected2\n>> +'\n>\n> ... I don't think it is really needed.  There isn't anything there\n> that can fail, and there isn't anything there that produces any\n> output.\n\nThe disk could fill up. In any case I like the test style of using\nsetup tests instead of stray setup code inside the tests themselves.\n\n> BTW. do all sane shells that can be used to run test suite support\n> '<<-' here-doc redirection operators?  Is it in POSIX?\n\nI don't know. But it's POSIX enough that nobody's complained about it\nthus far, it's used in a lot of existing code:\n\n    $ ack -l '<<-EOF'\n    t/t1012-read-tree-df.sh\n    t/t7005-editor.sh\n    t/t3507-cherry-pick-conflict.sh\n    t/t7401-submodule-summary.sh\n    t/t7201-co.sh\n    t/t4132-apply-removal.sh\n    t/t4041-diff-submodule-option.sh\n    contrib/hooks/post-receive-email\n"},{"id":"145330","messageId":"20100711151432.GA1781@burratino","threadId":"24363","inReplyTo":"m3d3uul8pd.fsf@localhost.localdomain","subject":"Re: [PATCH] t3000 (ls-files -o): modernize style","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-11T15:14:32Z","receivedAt":"2010-07-11T15:14:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jakub Narebski wrote:\n\n> ... I don't think it is really needed.  There isn't anything there\n> that can fail, and there isn't anything there that produces any\n> output.\n\nI should clarify.  I wrote this patch as preparation to add some new\ntests to the same script.  The new tests are not cooked yet, so I just\nsent the trivial part; there certainly is no urgent reason to apply it\nsoon (though no reason I see not to apply it either).\n\nA better explanation for the rationale would be \"follow the modern\nstyle to make the script more readable and set a good example for\nfuture tests\".\n\n> BTW. do all sane shells that can be used to run test suite support\n> '<<-' here-doc redirection operators?  Is it in POSIX?\n\nYes[1] and yes[2].\n\nThanks for looking it over.\nJonathan\n\n[1] $ git grep -l -F -e '<<-EO' -e '<<-\\EO' origin/maint -- t | wc -l\n    13\n[2] http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_07_04\n"}]}