{"thread":{"id":"31061","subject":"[RFC/PATCH] t3300-*.sh: Fix a TAP parse error","startedAt":"2012-07-21T17:46:57Z","lastAt":"2012-08-19T17:57:37Z","messageCount":13,"participants":["Ramsay Jones","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"195422","messageId":"500AEB11.4050006@ramsay1.demon.co.uk","threadId":"31061","inReplyTo":null,"subject":"[RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-07-21T17:46:57Z","receivedAt":"2012-07-21T17:46:57Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nAt present, running the t3300-*.sh test on cygwin looks like:\n\n    $ cd t\n    $ ./t3300-funny-names.sh\n    ok 1 - setup\n    # passed all 1 test(s)\n    1..1 # SKIP Your filesystem does not allow tabs in filenames\n    $\n\nUnfortunately, this is not valid TAP output, which prove notes\nas follows:\n\n    $ prove --exec sh t3300-funny-names.sh\n    t3300-funny-names.sh .. All 1 subtests passed\n\n    Test Summary Report\n    -------------------\n    t3300-funny-names.sh (Wstat: 0 Tests: 1 Failed: 0)\n      Parse errors: No plan found in TAP output\n    Files=1, Tests=1,  2 wallclock secs ( 0.05 usr  0.00 sys +  \\\n        0.90 cusr  0.49 csys =  1.43 CPU)\n    Result: FAIL\n    $\n\nThis is due to the 'trailing_plan' having a 'skip_directive'\nattached to it. This is not allowed by the TAP grammar, which\nonly allows a 'leading_plan' to be followed by an optional\n'skip_directive'. (see perldoc TAP::Parser::Grammar).\n\nA trailing_plan is one that appears in the TAP output after one or\nmore test status lines (that start 'not '? 'ok ' ...), whereas a\nleading_plan must appear before all test status lines (if any).\n\nIn practice, this means that the test script cannot contain a use\nof the 'skip all' facility:\n\n    skip_all='Some reason to skip *all* tests in this file'\n    test_done\n\nafter having already executed one or more tests with (for example)\n'test_expect_success'. Unfortunately, this is exactly what this\ntest script is doing. The first 'setup' test is actually used to\ndetermine if the test prerequisite is satisfied by the filesystem\n(ie does it allow tabs in filenames?).\n\nIn order to fix the parse errors, place the code to determine the\ntest prerequisite at the top level of the script, rather than as\na parameter to test_expect_success. This allows us to correctly\nuse 'skip_all', thus:\n\n    $ ./t3300-funny-names.sh\n    # passed all 0 test(s)\n    1..0 # SKIP Your filesystem does not allow tabs in filenames\n    $\n\n    $ prove --exec sh t3300-funny-names.sh\n    t3300-funny-names.sh .. skipped: Your filesystem does not \\\n        allow tabs in filenames\n    Files=1, Tests=0,  2 wallclock secs ( 0.02 usr  0.03 sys +  \\\n        0.84 cusr  0.41 csys =  1.29 CPU)\n    Result: NOTESTS\n    $\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n\nHi Jonathan,\n\nThis is an RFC because I suspect some people may prefer the much\nsimpler patch:\n\n    diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh\n    index 1f35e55..2a64385 100755\n    --- a/t/t3300-funny-names.sh\n    +++ b/t/t3300-funny-names.sh\n    @@ -34,7 +34,7 @@ test_expect_success 'setup' '\n     if ! test_have_prereq TABS_IN_FILENAMES\n     then\n     \t# since FAT/NTFS does not allow tabs in filenames, skip this test\n    -\tskip_all='Your filesystem does not allow tabs in filenames'\n    +\tsay '# SKIP Your filesystem does not allow tabs in filenames'\n     \ttest_done\n     fi\n     \n... the output of which looks like:\n\n    $ cd t\n    $ ./t3300-funny-names.sh\n    ok 1 - setup\n    # SKIP Your filesystem does not allow tabs in filenames\n    # passed all 1 test(s)\n    1..1\n    $\n\n    $ prove --exec sh t3300-funny-names.sh\n    t3300-funny-names.sh .. ok\n    All tests successful.\n    Files=1, Tests=1,  1 wallclock secs ( 0.03 usr  0.01 sys +  \\\n        0.85 cusr  0.34 csys =  1.24 CPU)\n    Result: PASS\n    $\n\nNeedless to say, I much prefer the patch below. :-D\n\nATB,\nRamsay Jones\n\n t/t3300-funny-names.sh | 26 +++++++++-----------------\n 1 file changed, 9 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh\nindex 1f35e55..c51674a 100755\n--- a/t/t3300-funny-names.sh\n+++ b/t/t3300-funny-names.sh\n@@ -15,28 +15,20 @@ p0='no-funny'\n p1='tabs\t,\" (dq) and spaces'\n p2='just space'\n \n-test_expect_success 'setup' '\n-\tcat >\"$p0\" <<-\\EOF &&\n-\t1. A quick brown fox jumps over the lazy cat, oops dog.\n-\t2. A quick brown fox jumps over the lazy cat, oops dog.\n-\t3. A quick brown fox jumps over the lazy cat, oops dog.\n-\tEOF\n-\n-\t{ cat \"$p0\" >\"$p1\" || :; } &&\n-\t{ echo \"Foo Bar Baz\" >\"$p2\" || :; } &&\n+cat >\"$p0\" <<\\EOF\n+1. A quick brown fox jumps over the lazy cat, oops dog.\n+2. A quick brown fox jumps over the lazy cat, oops dog.\n+3. A quick brown fox jumps over the lazy cat, oops dog.\n+EOF\n \n-\tif test -f \"$p1\" && cmp \"$p0\" \"$p1\"\n-\tthen\n-\t\ttest_set_prereq TABS_IN_FILENAMES\n-\tfi\n-'\n+cat 2>/dev/null >\"$p1\" \"$p0\"\n+echo 'Foo Bar Baz' >\"$p2\"\n \n-if ! test_have_prereq TABS_IN_FILENAMES\n-then\n+test -f \"$p1\" && cmp \"$p0\" \"$p1\" || {\n \t# since FAT/NTFS does not allow tabs in filenames, skip this test\n \tskip_all='Your filesystem does not allow tabs in filenames'\n \ttest_done\n-fi\n+}\n \n test_expect_success 'setup: populate index and tree' '\n \tgit update-index --add \"$p0\" \"$p2\" &&\n-- \n1.7.11.2\n"},{"id":"195423","messageId":"20120721182049.GL19860@burratino","threadId":"31061","inReplyTo":"500AEB11.4050006@ramsay1.demon.co.uk","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-07-21T18:20:49Z","receivedAt":"2012-07-21T18:20:49Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(cc-ing Ævar, TAP wizard)\nHi,\n\nRamsay Jones wrote:\n\n>     $ ./t3300-funny-names.sh\n>     ok 1 - setup\n>     # passed all 1 test(s)\n>     1..1 # SKIP Your filesystem does not allow tabs in filenames\n>     $\n>\n> Unfortunately, this is not valid TAP output, which prove notes\n> as follows:\n[...]\n>       Parse errors: No plan found in TAP output\n[...]\n> This is an RFC because I suspect some people may prefer the much\n> simpler patch:\n[...]\n>      \t# since FAT/NTFS does not allow tabs in filenames, skip this test\n>     -\tskip_all='Your filesystem does not allow tabs in filenames'\n>     +\tsay '# SKIP Your filesystem does not allow tabs in filenames'\n>      \ttest_done\n[...]\n> ... the output of which looks like:\n[...]\n>     ok 1 - setup\n>     # SKIP Your filesystem does not allow tabs in filenames\n>     # passed all 1 test(s)\n>     1..1\n[..]\n> Needless to say, I much prefer the patch below. :-D\n\nThanks for a nice explanation.  In general I definitely like getting\nrid of these setup tests when possible.  Let's see:\n\n[...]\n> --- a/t/t3300-funny-names.sh\n> +++ b/t/t3300-funny-names.sh\n> @@ -15,28 +15,20 @@ p0='no-funny'\n>  p1='tabs\t,\" (dq) and spaces'\n>  p2='just space'\n>  \n> -test_expect_success 'setup' '\n> -\tcat >\"$p0\" <<-\\EOF &&\n> -\t1. A quick brown fox jumps over the lazy cat, oops dog.\n> -\t2. A quick brown fox jumps over the lazy cat, oops dog.\n> -\t3. A quick brown fox jumps over the lazy cat, oops dog.\n> -\tEOF\n> +cat >\"$p0\" <<\\EOF\n> +1. A quick brown fox jumps over the lazy cat, oops dog.\n> +2. A quick brown fox jumps over the lazy cat, oops dog.\n> +3. A quick brown fox jumps over the lazy cat, oops dog.\n> +EOF\n\nThe problem is that on platforms not supporting funny filenames, it\nwill write a complaint to stderr and because the code is not guarded\nby test_expect_success, that output goes to the terminal.  So I think\nthis is a wrong approach.\n\nWould it make sense to avoid the \"# SKIP\" comment when a test has\nbeen run, like this?\n\ndiff --git i/t/test-lib.sh w/t/test-lib.sh\nindex acda33d1..038f6e9f 100644\n--- i/t/test-lib.sh\n+++ w/t/test-lib.sh\n@@ -354,6 +354,11 @@ test_done () {\n \tcase \"$test_failure\" in\n \t0)\n \t\t# Maybe print SKIP message\n+\t\tif test -n \"$skip_all\" && test \"$test_count\" != 0\n+\t\tthen\n+\t\t\tsay \"# SKIP $skill_all\"\n+\t\t\tskip_all=\n+\t\tfi\n \t\t[ -z \"$skip_all\" ] || skip_all=\" # SKIP $skip_all\"\n \n \t\tif test $test_external_has_tap -eq 0; then\n"},{"id":"195647","messageId":"500EEAAA.8030604@ramsay1.demon.co.uk","threadId":"31061","inReplyTo":"20120721182049.GL19860@burratino","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-07-24T18:34:18Z","receivedAt":"2012-07-24T18:34:18Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jonathan Nieder wrote:\n>> Needless to say, I much prefer the patch below. :-D\n> \n> Thanks for a nice explanation.  In general I definitely like getting\n> rid of these setup tests when possible.  Let's see:\n> \n> [...]\n>> --- a/t/t3300-funny-names.sh\n>> +++ b/t/t3300-funny-names.sh\n>> @@ -15,28 +15,20 @@ p0='no-funny'\n>>  p1='tabs\t,\" (dq) and spaces'\n>>  p2='just space'\n>>  \n>> -test_expect_success 'setup' '\n>> -\tcat >\"$p0\" <<-\\EOF &&\n>> -\t1. A quick brown fox jumps over the lazy cat, oops dog.\n>> -\t2. A quick brown fox jumps over the lazy cat, oops dog.\n>> -\t3. A quick brown fox jumps over the lazy cat, oops dog.\n>> -\tEOF\n>> +cat >\"$p0\" <<\\EOF\n>> +1. A quick brown fox jumps over the lazy cat, oops dog.\n>> +2. A quick brown fox jumps over the lazy cat, oops dog.\n>> +3. A quick brown fox jumps over the lazy cat, oops dog.\n>> +EOF\n> \n> The problem is that on platforms not supporting funny filenames, it\n> will write a complaint to stderr and because the code is not guarded\n> by test_expect_success, that output goes to the terminal.  So I think\n> this is a wrong approach.\n\nHuh? Which platforms are we talking about?\n\nThe only problematic platforms I test on are \"NTFS/bash\" on cygwin and MinGW.\nSince commit 2b843732 (\"Suppress some bash redirection error messages\",\n26-08-2008), I have not noticed any complaints regarding this problem.\nWhat have I missed?\n\nAssuming we are not talking about errors like ENOSPC, EROFS etc., then the\nonly command which would issue a complaint to stderr would be the line\nfollowing the above snippet, thus:\n\n    +cat 2>/dev/null >\"$p1\" \"$p0\"\n\n(note the stderr redirection). This does not output an error to the terminal\nwhen using bash (I think I also tested with dash). However, this does rely\non the shell performing the redirections in the order, left to right, on the\ncommand line. [I had intended to check with POSIX to see if this order was\nmandated or not, but didn't get around to it ...]\n\nHave you found a shell were this does not work?\n\n> Would it make sense to avoid the \"# SKIP\" comment when a test has\n> been run, like this?\n> \n> diff --git i/t/test-lib.sh w/t/test-lib.sh\n> index acda33d1..038f6e9f 100644\n> --- i/t/test-lib.sh\n> +++ w/t/test-lib.sh\n> @@ -354,6 +354,11 @@ test_done () {\n>  \tcase \"$test_failure\" in\n>  \t0)\n>  \t\t# Maybe print SKIP message\n> +\t\tif test -n \"$skip_all\" && test \"$test_count\" != 0\n> +\t\tthen\n> +\t\t\tsay \"# SKIP $skill_all\"\n> +\t\t\tskip_all=\n> +\t\tfi\n>  \t\t[ -z \"$skip_all\" ] || skip_all=\" # SKIP $skip_all\"\n>  \n>  \t\tif test $test_external_has_tap -eq 0; then\n\nNo, I don't think this would be a good direction to go in. This may\nnot be a good idea either, but if you wanted to add a check here, then\nmaybe something like this (totally untested):\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex acda33d..53a2422 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -354,6 +354,9 @@ test_done () {\n \tcase \"$test_failure\" in\n \t0)\n \t\t# Maybe print SKIP message\n+\t\tif test -n \"$skip_all\" && test $test_count -gt 0; then\n+\t\t\terror \"Can't use skip_all after running some tests\"\n+\t\tfi\n \t\t[ -z \"$skip_all\" ] || skip_all=\" # SKIP $skip_all\"\n \n \t\tif test $test_external_has_tap -eq 0; then\n\nDunno! :-D\n\nI will be sending a v2 patch soon.\n\nATB,\nRamsay Jones\n"},{"id":"195648","messageId":"20120724192103.GB5210@burratino","threadId":"31061","inReplyTo":"500EEAAA.8030604@ramsay1.demon.co.uk","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-07-24T19:21:03Z","receivedAt":"2012-07-24T19:21:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nRamsay Jones wrote:\n\n> The only problematic platforms I test on are \"NTFS/bash\" on cygwin and MinGW.\n> Since commit 2b843732 (\"Suppress some bash redirection error messages\",\n> 26-08-2008), I have not noticed any complaints regarding this problem.\n\nYeah, that probably took care of squashing the messages.  Maybe my\nmemory is too long. ;-)\n\n[...]\n> No, I don't think this would be a good direction to go in. This may\n> not be a good idea either, but if you wanted to add a check here, then\n> maybe something like this (totally untested):\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index acda33d..53a2422 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -354,6 +354,9 @@ test_done () {\n>  \tcase \"$test_failure\" in\n>  \t0)\n>  \t\t# Maybe print SKIP message\n> +\t\tif test -n \"$skip_all\" && test $test_count -gt 0; then\n> +\t\t\terror \"Can't use skip_all after running some tests\"\n> +\t\tfi\n>  \t\t[ -z \"$skip_all\" ] || skip_all=\" # SKIP $skip_all\"\n>  \n>  \t\tif test $test_external_has_tap -eq 0; then\n\nI think preventing invalid TAP output like this would be a very good\nthing!  As a start, this patchlet looks good to me, and then I guess\nwe'll have to think more about what happens when a person wants to\nskip_all_remaining after a test has already been run.\n\nCare to format it as a \"git am\"-able patch, or should I?\n\nThanks again,\nJonathan\n"},{"id":"195653","messageId":"7veho1exu6.fsf@alter.siamese.dyndns.org","threadId":"31061","inReplyTo":"500EEAAA.8030604@ramsay1.demon.co.uk","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-24T19:57:37Z","receivedAt":"2012-07-24T19:57:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:\n\n> The only problematic platforms I test on are \"NTFS/bash\" on cygwin and MinGW.\n> Since commit 2b843732 (\"Suppress some bash redirection error messages\",\n> 26-08-2008), I have not noticed any complaints regarding this problem.\n> What have I missed?\n>\n> Assuming we are not talking about errors like ENOSPC, EROFS etc., then the\n> only command which would issue a complaint to stderr would be the line\n> following the above snippet, thus:\n>\n>     +cat 2>/dev/null >\"$p1\" \"$p0\"\n>\n> (note the stderr redirection).\n\nAs I am more worried about longer-term health of the codebase, what\nthe part you moved outside test_expect_* with this patch happens to\ndo _right now_ is of secondary importance, at least from my point of\nview.  The next patch that updates this scirpt may want to run more\ninvolved commands that can fail in different ways.\n\nBeing able to rely on the protection test_expect_* gives us in the\nset-up phase of the test has been very valuable (in addition to\nmaking the result more readable) to us.  Can we keep that property\nin some way while also keeping the ability to skip the remainder of\nthe test script?\n\nObserving that all well-written test scripts we have begin with this\nboilerplate line:\n\n\ttest_expect_success setup '\n\nI wouldn't mind introducing a new helper function test_setup that\nbehaves like test_expect_success but is meant to be used in the\nfirst \"set-up\" phase of the tests in a test script. Perhaps we can\ngive its failure a meaning different than failures in other normal\ntests (e.g. \"even set-up fails, and the remainder of the test is N/A\nhere, hence abort the whole thing\"), and do not count \"test_setup\"\nas part of the test (i.e. do not increment $test_count and friends).\n"},{"id":"195778","messageId":"50103CA6.4050901@ramsay1.demon.co.uk","threadId":"31061","inReplyTo":"20120724192103.GB5210@burratino","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-07-25T18:36:22Z","receivedAt":"2012-07-25T18:36:22Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jonathan Nieder wrote:\n> [...]\n>> No, I don't think this would be a good direction to go in. This may\n>> not be a good idea either, but if you wanted to add a check here, then\n>> maybe something like this (totally untested):\n>>\n>> diff --git a/t/test-lib.sh b/t/test-lib.sh\n>> index acda33d..53a2422 100644\n>> --- a/t/test-lib.sh\n>> +++ b/t/test-lib.sh\n>> @@ -354,6 +354,9 @@ test_done () {\n>>  \tcase \"$test_failure\" in\n>>  \t0)\n>>  \t\t# Maybe print SKIP message\n>> +\t\tif test -n \"$skip_all\" && test $test_count -gt 0; then\n>> +\t\t\terror \"Can't use skip_all after running some tests\"\n>> +\t\tfi\n>>  \t\t[ -z \"$skip_all\" ] || skip_all=\" # SKIP $skip_all\"\n>>  \n>>  \t\tif test $test_external_has_tap -eq 0; then\n> \n> I think preventing invalid TAP output like this would be a very good\n> thing!  As a start, this patchlet looks good to me, and then I guess\n> we'll have to think more about what happens when a person wants to\n> skip_all_remaining after a test has already been run.\n> \n> Care to format it as a \"git am\"-able patch, or should I?\n\nYes, I will happily create a proper (tested) patch and send it to the list.\n\nHowever, given that we are now in the RC period, I probably won't get to it\nimmediately; I need to set aside *at least* one full evening to running the\ntestsuite on cygwin! ;-)\n\nATB,\nRamsay Jones\n"},{"id":"195779","messageId":"501043D9.70604@ramsay1.demon.co.uk","threadId":"31061","inReplyTo":"7veho1exu6.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-07-25T19:07:05Z","receivedAt":"2012-07-25T19:07:05Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n[...]\n> As I am more worried about longer-term health of the codebase, what\n> the part you moved outside test_expect_* with this patch happens to\n> do _right now_ is of secondary importance, at least from my point of\n> view.  The next patch that updates this scirpt may want to run more\n> involved commands that can fail in different ways.\n> \n> Being able to rely on the protection test_expect_* gives us in the\n> set-up phase of the test has been very valuable (in addition to\n> making the result more readable) to us.  Can we keep that property\n> in some way while also keeping the ability to skip the remainder of\n> the test script?\n> \n> Observing that all well-written test scripts we have begin with this\n> boilerplate line:\n> \n> \ttest_expect_success setup '\n> \n> I wouldn't mind introducing a new helper function test_setup that\n> behaves like test_expect_success but is meant to be used in the\n> first \"set-up\" phase of the tests in a test script. Perhaps we can\n> give its failure a meaning different than failures in other normal\n> tests (e.g. \"even set-up fails, and the remainder of the test is N/A\n> here, hence abort the whole thing\"), and do not count \"test_setup\"\n> as part of the test (i.e. do not increment $test_count and friends).\n\nHeh, I did exactly this (except mine was called test_fixture) as part\nof my first (abandoned) attempt to address this problem. :-D\n\nI've attached the patch below, just for discussion.\n\nI didn't test it very much, but it seems to work with a superficial\nworkout:\n\n    $ cd t\n    $ ./t3300-funny-names.sh\n    # passed all 0 test(s)\n    1..0 # SKIP Your filesystem does not allow tabs in filenames\n    $\n\n    $ ./t3300-funny-names.sh -v\n    Initialized empty Git repository in /home/ramsay/git/t/trash directory.t3300-fun\n    ny-names/.git/\n    test fixture:\n            cat >\"$p0\" <<-\\EOF &&\n            1. A quick brown fox jumps over the lazy cat, oops dog.\n            2. A quick brown fox jumps over the lazy cat, oops dog.\n            3. A quick brown fox jumps over the lazy cat, oops dog.\n            EOF\n\n            { cat \"$p0\" >\"$p1\" || :; } &&\n            { echo \"Foo Bar Baz\" >\"$p2\" || :; } &&\n\n            if test -f \"$p1\" && cmp \"$p0\" \"$p1\"\n            then\n                    test_set_prereq TABS_IN_FILENAMES\n            fi\n\n    ./test-lib.sh: line 274: tabs   ,\" (dq) and spaces: No such file or directory\n\n    # passed all 0 test(s)\n    1..0 # SKIP Your filesystem does not allow tabs in filenames\n    $\n\n    $ prove --exec sh t3300-funny-names.sh\n    t3300-funny-names.sh .. skipped: Your filesystem does not allow tabs in filename\n    s\n    Files=1, Tests=0,  1 wallclock secs ( 0.03 usr  0.05 sys +  0.87 cusr  0.41 csys\n     =  1.36 CPU)\n    Result: NOTESTS\n    $\n\nSo why did I abandon this patch? Well, I didn't really; I just decided\nthat I needed *much* more time to polish[1] 'test_fixture'. I wanted to\nfix the \"TAP parse error\" problem before v1.7.12-rc0! :(\n\nHTH\n\nATB,\nRamsay Jones\n\n[1] For example, what should/will happen if someone uses test_must_fail,\ntest_might_fail, etc., within the test_fixture script? Should they simply\nbe banned within a text_fixture?\n\n--- >8 ---\nSubject: [PATCH] test-lib-functions.sh: Add a test_fixture function\n\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n t/t3300-funny-names.sh  |  2 +-\n t/test-lib-functions.sh | 11 +++++++++++\n 2 files changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh\nindex 1f35e55..59331a0 100755\n--- a/t/t3300-funny-names.sh\n+++ b/t/t3300-funny-names.sh\n@@ -15,7 +15,7 @@ p0='no-funny'\n p1='tabs\t,\" (dq) and spaces'\n p2='just space'\n \n-test_expect_success 'setup' '\n+test_fixture '\n \tcat >\"$p0\" <<-\\EOF &&\n \t1. A quick brown fox jumps over the lazy cat, oops dog.\n \t2. A quick brown fox jumps over the lazy cat, oops dog.\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 80daaca..b70c963 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -302,6 +302,17 @@ test_expect_success () {\n \techo >&3 \"\"\n }\n \n+test_fixture () {\n+\ttest \"$#\" = 1 ||\n+\terror \"bug in test script: must be single argument to test_fixture\"\n+\tsay >&3 \"test fixture: $1\"\n+\tif ! test_run_ \"$1\"\n+\tthen\n+\t\terror \"failure in test_fixture code\"\n+\tfi\n+\techo >&3 \"\"\n+}\n+\n # test_external runs external test scripts that provide continuous\n # test output about their progress, and succeeds/fails on\n # zero/non-zero exit code.  It outputs the test output on stdout even\n-- \n1.7.11.2\n"},{"id":"195785","messageId":"20120725205120.GD4732@burratino","threadId":"31061","inReplyTo":"501043D9.70604@ramsay1.demon.co.uk","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-07-25T20:51:20Z","receivedAt":"2012-07-25T20:51:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nRamsay Jones wrote:\n> Junio C Hamano wrote:\n\n>> Observing that all well-written test scripts we have begin with this\n>> boilerplate line:\n>>\n>>\ttest_expect_success setup '\n>>\n>> I wouldn't mind introducing a new helper function test_setup that\n>> behaves like test_expect_success but is meant to be used in the\n>> first \"set-up\" phase of the tests in a test script.\n\nNeat.  This could be used for later set-up tests, too, perhaps with a\nlong-term goal of making non set-up tests independent of each other\n(reorderable and skippable).\n\n[...]\n> [1] For example, what should/will happen if someone uses test_must_fail,\n> test_might_fail, etc., within the test_fixture script? Should they simply\n> be banned within a text_fixture?\n\nWhy wouldn't they act just like they do in test_expect_success blocks?\n\nFWIW I find Junio's test_setup name more self-explanatory.  What\nmnemonic should I be using to remember the _fixture name?\n\nThanks,\nJonathan\n"},{"id":"195798","messageId":"7v1ujzbijs.fsf@alter.siamese.dyndns.org","threadId":"31061","inReplyTo":"20120725205120.GD4732@burratino","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-25T22:08:23Z","receivedAt":"2012-07-25T22:08:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> FWIW I find Junio's test_setup name more self-explanatory.  What\n> mnemonic should I be using to remember the _fixture name?\n\nPrevious exposure to things like Rails?\n"},{"id":"196037","messageId":"50142965.4020801@ramsay1.demon.co.uk","threadId":"31061","inReplyTo":"20120725205120.GD4732@burratino","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-07-28T18:03:17Z","receivedAt":"2012-07-28T18:03:17Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jonathan Nieder wrote:\n> [...]\n>> [1] For example, what should/will happen if someone uses test_must_fail,\n>> test_might_fail, etc., within the test_fixture script? Should they simply\n>> be banned within a text_fixture?\n> \n> Why wouldn't they act just like they do in test_expect_success blocks?\n\nHeh, well they do indeed act just like they do in text_expect_success blocks!\n\nI spent only about 20 minutes writing test_fixture, playing with it, and then\ndeciding to shelve it for now. Again, I wanted a *quick* fix for the TAP\nparse error, so that it would make it into v1.7.12. :(\n\nHaving now spent a further 30 minutes, I can see that I did a better job than\nI thought! :-P\n\nActually, scratch that; rather I should say that Junio and the other authors\nof the test infrastructure did such a good job (particularly with separation\nof concerns), that I lucked into a good implementation.\n\nI still haven't done any serious testing, so if I subsequently find any\nproblems, then the lousy implementation is my fault! ;-)\n\n> FWIW I find Junio's test_setup name more self-explanatory.  What\n> mnemonic should I be using to remember the _fixture name?\n\nI don't have a problem with 'test_setup' either; test-fixture comes from the\nvarious xUnit unit-test libraries. (I think Kent Beck et.al. wrote JUnit first\nand then it was ported to various other languages. eg cppUnit for C++).\n\nBriefly, a test-fixture provides a context or common environment, via code for\ntest setup and teardown, in which to run one or more tests.\n\nHTH\n\nATB,\nRamsay Jones\n"},{"id":"196036","messageId":"50142B98.1030606@ramsay1.demon.co.uk","threadId":"31061","inReplyTo":"7v1ujzbijs.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-07-28T18:12:40Z","receivedAt":"2012-07-28T18:12:40Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n> \n>> FWIW I find Junio's test_setup name more self-explanatory.  What\n>> mnemonic should I be using to remember the _fixture name?\n> \n> Previous exposure to things like Rails?\n\nI did once have a brief look at ruby, but my \"new language to learn\"\nlist was over-subscribed. (I think I went with D) So, I'm not at all\nfamiliar with Rails.\n\nA test-fixture may be used in various xUnit unit-test libraries.\n(eg JUnit, cppUnit, etc)\n\nATB,\nRamsay Jones\n"},{"id":"197151","messageId":"7vk3wyxv6e.fsf@alter.siamese.dyndns.org","threadId":"31061","inReplyTo":"20120725205120.GD4732@burratino","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-16T23:40:25Z","receivedAt":"2012-08-16T23:40:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Ramsay Jones wrote:\n>> Junio C Hamano wrote:\n>\n>>> Observing that all well-written test scripts we have begin with this\n>>> boilerplate line:\n>>>\n>>>\ttest_expect_success setup '\n>>>\n>>> I wouldn't mind introducing a new helper function test_setup that\n>>> behaves like test_expect_success but is meant to be used in the\n>>> first \"set-up\" phase of the tests in a test script.\n>\n> Neat.  This could be used for later set-up tests, too, perhaps with a\n> long-term goal of making non set-up tests independent of each other\n> (reorderable and skippable).\n>\n> [...]\n>> [1] For example, what should/will happen if someone uses test_must_fail,\n>> test_might_fail, etc., within the test_fixture script? Should they simply\n>> be banned within a text_fixture?\n>\n> Why wouldn't they act just like they do in test_expect_success blocks?\n>\n> FWIW I find Junio's test_setup name more self-explanatory.  What\n> mnemonic should I be using to remember the _fixture name?\n\nI see that I was distracted by the \"where does the fixture come\nfrom\" and did not follow through.\n\nI think what it does makes sense (I haven't checked all the\nredirections, though).  Do we want to resurrect the topic?  It needs\na paragraph in the proposed commit log and t/README to explain the\nmotivation and the usage.\n\nThanks.\n"},{"id":"197297","messageId":"50312911.30904@ramsay1.demon.co.uk","threadId":"31061","inReplyTo":"7vk3wyxv6e.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-08-19T17:57:37Z","receivedAt":"2012-08-19T17:57:37Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n>> Ramsay Jones wrote:\n>>> Junio C Hamano wrote:\n>>\n>>>> Observing that all well-written test scripts we have begin with this\n>>>> boilerplate line:\n>>>>\n>>>> \ttest_expect_success setup '\n>>>>\n>>>> I wouldn't mind introducing a new helper function test_setup that\n>>>> behaves like test_expect_success but is meant to be used in the\n>>>> first \"set-up\" phase of the tests in a test script.\n>>\n>> Neat.  This could be used for later set-up tests, too, perhaps with a\n>> long-term goal of making non set-up tests independent of each other\n>> (reorderable and skippable).\n>>\n>> [...]\n>>> [1] For example, what should/will happen if someone uses test_must_fail,\n>>> test_might_fail, etc., within the test_fixture script? Should they simply\n>>> be banned within a text_fixture?\n>>\n>> Why wouldn't they act just like they do in test_expect_success blocks?\n>>\n>> FWIW I find Junio's test_setup name more self-explanatory.  What\n>> mnemonic should I be using to remember the _fixture name?\n> \n> I see that I was distracted by the \"where does the fixture come\n> from\" and did not follow through.\n> \n> I think what it does makes sense (I haven't checked all the\n> redirections, though).  Do we want to resurrect the topic?  It needs\n> a paragraph in the proposed commit log and t/README to explain the\n> motivation and the usage.\n\nYes, this is on my TODO list.\n\nI will name the function 'test_setup' rather than 'test_fixture'.\n\nAlso, the test_fixture function had a single script parameter, since\nI didn't see the point of having a \"title\" like test_expect_success.\nHowever, I'm now in two minds about this; if it were to take a title\nit may be useful to include the title in the error message, if the\ntest contained multiple calls to test_setup. I'm still inclined to\n*not* include a title parameter ...\n\nATB,\nRamsay Jones\n"}]}