{"thread":{"id":"24653","subject":"[PATCH 0/3] tests: Better prerequisite handling & documentation","startedAt":"2010-08-06T21:19:22Z","lastAt":"2010-08-08T13:29:22Z","messageCount":8,"participants":["Ævar Arnfjörð Bjarmason","Jonathan Nieder","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"147346","messageId":"1281129565-26124-1-git-send-email-avarab@gmail.com","threadId":"24653","inReplyTo":null,"subject":"[PATCH 0/3] tests: Better prerequisite handling & documentation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-06T21:19:22Z","receivedAt":"2010-08-06T21:19:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"There were some useful nuggets in the \"Tests in Cygwin\" thread from\nMay last year that I've dug out and improved. This series adds support\nfor multiple test prerequisite, and improves the t/README\ndocumentation by adding a \"Prerequisites\" section.\n\nThere's also a small fix to the raw test output.\n\nÆvar Arnfjörð Bjarmason (3):\n  test-lib: Add support for multiple test prerequisites\n  test-lib: Print missing prerequisites in test output\n  t/README: Document the predefined test prerequisites\n\n t/README         |   51 ++++++++++++++++++++++++++++++++++++++++++++++-----\n t/t0000-basic.sh |   17 +++++++++++++++++\n t/test-lib.sh    |   22 +++++++++++++++-------\n 3 files changed, 78 insertions(+), 12 deletions(-)\n"},{"id":"147348","messageId":"1281129565-26124-2-git-send-email-avarab@gmail.com","threadId":"24653","inReplyTo":"1281129565-26124-1-git-send-email-avarab@gmail.com","subject":"[PATCH 1/3] test-lib: Add support for multiple test prerequisites","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-06T21:19:23Z","receivedAt":"2010-08-06T21:19:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the test_have_prereq function in test-lib.sh to support a\ncomma-separated list of prerequisites. This is useful for tests that\nneed e.g. both POSIXPERM and SANITY.\n\nThe implementation was stolen from Junio C Hamano and Johannes Sixt,\nthe tests and documentation were not. See the \"Tests in Cygwin\" thread\nin May 2009 for the originals:\n\n    http://thread.gmane.org/gmane.comp.version-control.git/116729/focus=118385\n    http://thread.gmane.org/gmane.comp.version-control.git/116729/focus=118434\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/README         |    6 ++++++\n t/t0000-basic.sh |   17 +++++++++++++++++\n t/test-lib.sh    |   20 ++++++++++++++------\n 3 files changed, 37 insertions(+), 6 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 0d1183c..d07b67a 100644\n--- a/t/README\n+++ b/t/README\n@@ -350,6 +350,12 @@ library for your script to use.\n \ttest_expect_success TTY 'git --paginate rev-list uses a pager' \\\n \t    ' ... '\n \n+   You can also supply a comma-separated list of prerequisites, in the\n+   rare case where your test depends on more than one:\n+\n+\ttest_expect_success PERL,PYTHON 'yo dawg' \\\n+\t    ' test $(perl -E 'print eval \"1 +\" . qx[python -c \"print 2\"]') == \"4\" '\n+\n  - test_expect_failure [<prereq>] <message> <script>\n \n    This is NOT the opposite of test_expect_success, but is used\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex f2c7336..2887677 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -73,6 +73,23 @@ then\n \texit 1\n fi\n \n+test_set_prereq HAVETHIS\n+haveit=no\n+test_expect_success HAVETHIS,HAVEIT 'test runs if prerequisites are satisfied' '\n+    test_have_prereq HAVEIT &&\n+    test_have_prereq HAVETHIS &&\n+    haveit=yes\n+'\n+donthaveit=yes\n+test_expect_success HAVEIT,DONTHAVEIT 'unmet prerequisites causes test to be skipped' '\n+    donthaveit=no\n+'\n+if test $haveit$donthaveit != yesyes\n+then\n+\tsay \"bug in test framework: multiple prerequisite tags do not work reliably\"\n+\texit 1\n+fi\n+\n clean=no\n test_expect_success 'tests clean up after themselves' '\n     test_when_finished clean=yes\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex e8f21d5..8701923 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -327,12 +327,20 @@ test_set_prereq () {\n satisfied=\" \"\n \n test_have_prereq () {\n-\tcase $satisfied in\n-\t*\" $1 \"*)\n-\t\t: yes, have it ;;\n-\t*)\n-\t\t! : nope ;;\n-\tesac\n+\t# prerequisites can be concatenated with ','\n+\tsave_IFS=$IFS\n+\tIFS=,\n+\tset -- $*\n+\tIFS=$save_IFS\n+\tfor prerequisite\n+\tdo\n+\t\tcase $satisfied in\n+\t\t*\" $prerequisite \"*)\n+\t\t\t: yes, have it ;;\n+\t\t*)\n+\t\t\t! : nope ;;\n+\t\tesac\n+\tdone\n }\n \n # You are not expected to call test_ok_ and test_failure_ directly, use\n-- \n1.7.1\n"},{"id":"147349","messageId":"1281129565-26124-3-git-send-email-avarab@gmail.com","threadId":"24653","inReplyTo":"1281129565-26124-1-git-send-email-avarab@gmail.com","subject":"[PATCH 2/3] test-lib: Print missing prerequisites in test output","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-06T21:19:24Z","receivedAt":"2010-08-06T21:19:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the test output to print needed prerequisites as part of the\nTAP. This makes it easy to see at a glance why a test was\nskipped. Before:\n\n    ok 7 # skip <message>\n    ok 9 # skip <message>\n\nAfter:\n\n    ok 7 # skip <message> (prereqs: DONTHAVEIT)\n    ok 9 # skip <message> (prereqs: HAVEIT,DONTHAVEIT)\n\nThis'll also be useful for smoke testing output, where the developer\nreading the output may not be familiar with the system where tests are\nbeing skipped.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/test-lib.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 8701923..4e0a1c3 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -403,7 +403,7 @@ test_skip () {\n \tcase \"$to_skip\" in\n \tt)\n \t\tsay_color skip >&3 \"skipping test: $@\"\n-\t\tsay_color skip \"ok $test_count # skip $1\"\n+\t\tsay_color skip \"ok $test_count # skip $1 (prereqs: $prereq)\"\n \t\t: true\n \t\t;;\n \t*)\n-- \n1.7.1\n"},{"id":"147347","messageId":"1281129565-26124-4-git-send-email-avarab@gmail.com","threadId":"24653","inReplyTo":"1281129565-26124-1-git-send-email-avarab@gmail.com","subject":"[PATCH 3/3] t/README: Document the predefined test prerequisites","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-06T21:19:25Z","receivedAt":"2010-08-06T21:19:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The README for the test library suggested that you grep the\ntest-lib.sh for test_set_prereq to see what the preset prerequisites\nwere.\n\nRemove that bit, and write a section explaining all the preset\nprerequisites. Most of the text was lifted from from Junio C Hamano\nand Johannes Sixt, See the \"Tests in Cygwin\" thread in May 2009 for\nthe originals:\n\n    http://thread.gmane.org/gmane.comp.version-control.git/116729/focus=118385\n    http://thread.gmane.org/gmane.comp.version-control.git/116729/focus=118434\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/README |   45 ++++++++++++++++++++++++++++++++++++++++-----\n 1 files changed, 40 insertions(+), 5 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex d07b67a..dc07939 100644\n--- a/t/README\n+++ b/t/README\n@@ -410,11 +410,12 @@ library for your script to use.\n  - test_set_prereq SOME_PREREQ\n \n    Set a test prerequisite to be used later with test_have_prereq. The\n-   test-lib will set some prerequisites for you, e.g. PERL and PYTHON\n-   which are derived from ./GIT-BUILD-OPTIONS (grep test_set_prereq\n-   test-lib.sh for more). Others you can set yourself and use later\n-   with either test_have_prereq directly, or the three argument\n-   invocation of test_expect_success and test_expect_failure.\n+   test-lib will set some prerequisites for you, see the\n+   \"Prerequisites\" section below for a full list of these.\n+\n+   Others you can set yourself and use later with either\n+   test_have_prereq directly, or the three argument invocation of\n+   test_expect_success and test_expect_failure.\n \n  - test_have_prereq SOME PREREQ\n \n@@ -487,6 +488,40 @@ library for your script to use.\n \t\t...\n \t'\n \n+Prerequisites\n+-------------\n+\n+These are the prerequisites that the test library predefines with\n+test_have_prereq.\n+\n+See the prereq argument to the test_* functions in the \"Test harness\n+library\" section above and the \"test_have_prereq\" function for how to\n+use these, and \"test_set_prereq\" for how to define your own.\n+\n+ - PERL & PYTHON\n+\n+   Git wasn't compiled with NO_PERL=YesPlease or\n+   NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in\n+   these.\n+\n+ - POSIXPERM\n+\n+   The filesystem supports POSIX style permission bits.\n+\n+ - BSLASHPSPEC\n+\n+   Backslashes in pathspec are not directory separators. This is not\n+   set on Windows. See 6fd1106a for details.\n+\n+ - EXECKEEPSPID\n+\n+   The process retains the same pid across exec(2). See fb9a2bea for\n+   details.\n+\n+ - SYMLINKS\n+\n+   The filesystem we're on supports symbolic links. E.g. a FAT\n+   filesystem doesn't support these. See 704a3143 for details.\n \n Tips for Writing Tests\n ----------------------\n-- \n1.7.1\n"},{"id":"147407","messageId":"20100808015729.GA9284@burratino","threadId":"24653","inReplyTo":"1281129565-26124-2-git-send-email-avarab@gmail.com","subject":"Re: [PATCH 1/3] test-lib: Add support for multiple test prerequisites","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-08T01:57:29Z","receivedAt":"2010-08-08T01:57:29Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> Change the test_have_prereq function in test-lib.sh to support a\n> comma-separated list of prerequisites. This is useful for tests that\n> need e.g. both POSIXPERM and SANITY.\n> \n> The implementation was stolen from Junio C Hamano and Johannes Sixt,\n> the tests and documentation were not.\n\nI think you can sell it better. :)\n\n\tFrom: Johannes Sixt <j6t@kdbg.org>\n\tSubject: test-lib: Allow tests with multiple prerequisites\n\n\tOccasionally an especially taxing test comes around that\n\tmakes multiple assumptions on the system running it.\n\tCurrently the test suite works around that with such hacks as\n\n\t if\n\t\ttest_have_prereq SYMLINKS &&\n\t\ttest_have_prereq POSIXPERM\n\t then\n\t\ttest_set_prereq SYMLINKPERM\n\t fi\n\t test_expect_success SYMLINKPERM 'funny symlink in work tree, un-unlink-able' '\n\t\t...\n\t '\n\n\tor even worse, as in the test that example is based on, leaves\n\tout mention of the “easier” of the two assumptions.\n\n\tMaybe you would be happier to be able to write\n\n\t test_expect_success SYMLINKS,POSIXPERM 'funny symlink' '\n\t\t...\n\t '\n\n\tBased on a patch by Junio.\n\n\t[ab: added tests and documentation]\n\n\tSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\n> +++ b/t/t0000-basic.sh\n> @@ -73,6 +73,23 @@ then\n>  \texit 1\n>  fi\n>  \n> +test_set_prereq HAVETHIS\n> +haveit=no\n> +test_expect_success HAVETHIS,HAVEIT 'test runs if prerequisites are satisfied' '\n> +    test_have_prereq HAVEIT &&\n> +    test_have_prereq HAVETHIS &&\n\nI think the similar code above was just a way to sneak in a sanity\ncheck for test_have_prereq().  I’d leave it out.\n\n> +    haveit=yes\n> +'\n> +donthaveit=yes\n> +test_expect_success HAVEIT,DONTHAVEIT 'unmet prerequisites causes test to be skipped' '\n> +    donthaveit=no\n> +'\n> +if test $haveit$donthaveit != yesyes\n> +then\n> +\tsay \"bug in test framework: multiple prerequisite tags do not work reliably\"\n> +\texit 1\n> +fi\n\nMaybe it would be simpler to squash this in with the other similar checks.\n\n> +++ b/t/test-lib.sh\n> @@ -327,12 +327,20 @@ test_set_prereq () {\n>  satisfied=\" \"\n>  \n>  test_have_prereq () {\n> -\tcase $satisfied in\n> -\t*\" $1 \"*)\n> -\t\t: yes, have it ;;\n> -\t*)\n> -\t\t! : nope ;;\n> -\tesac\n> +\t# prerequisites can be concatenated with ','\n> +\tsave_IFS=$IFS\n> +\tIFS=,\n> +\tset -- $*\n> +\tIFS=$save_IFS\n> +\tfor prerequisite\n> +\tdo\n> +\t\tcase $satisfied in\n> +\t\t*\" $prerequisite \"*)\n> +\t\t\t: yes, have it ;;\n> +\t\t*)\n> +\t\t\t! : nope ;;\n> +\t\tesac\n> +\tdone\n\nDoes that work?\n\nExcept as noted above,\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nThanks.\n---\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 2887677..cdb25af 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -58,35 +58,28 @@ test_expect_failure 'pretend we have fixed a known breakage' '\n     :\n '\n test_set_prereq HAVEIT\n+test_set_prereq ONE\n+test_set_prereq TWO\n haveit=no\n test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '\n     test_have_prereq HAVEIT &&\n     haveit=yes\n '\n+haveboth=no\n+test_expect_success ONE,TWO 'test runs if prerequisites are satisfied' '\n+    haveboth=yes\n+'\n donthaveit=yes\n test_expect_success DONTHAVEIT 'unmet prerequisite causes test to be skipped' '\n     donthaveit=no\n '\n-if test $haveit$donthaveit != yesyes\n-then\n-\tsay \"bug in test framework: prerequisite tags do not work reliably\"\n-\texit 1\n-fi\n-\n-test_set_prereq HAVETHIS\n-haveit=no\n-test_expect_success HAVETHIS,HAVEIT 'test runs if prerequisites are satisfied' '\n-    test_have_prereq HAVEIT &&\n-    test_have_prereq HAVETHIS &&\n-    haveit=yes\n-'\n-donthaveit=yes\n-test_expect_success HAVEIT,DONTHAVEIT 'unmet prerequisites causes test to be skipped' '\n-    donthaveit=no\n+donthaveboth=yes\n+test_expect_success THREE,ONE 'one unmet prerequisite is enough' '\n+    donthaveboth=no\n '\n-if test $haveit$donthaveit != yesyes\n+if test $haveit$donthaveit$haveboth$donthaveboth != yesyesyesyes\n then\n-\tsay \"bug in test framework: multiple prerequisite tags do not work reliably\"\n+\tsay \"bug in test framework: prerequisite tags do not work reliably\"\n \texit 1\n fi\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 8701923..a3b0b22 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -338,7 +338,7 @@ test_have_prereq () {\n \t\t*\" $prerequisite \"*)\n \t\t\t: yes, have it ;;\n \t\t*)\n-\t\t\t! : nope ;;\n+\t\t\treturn 1\n \t\tesac\n \tdone\n }\n-- \n"},{"id":"147408","messageId":"20100808020332.GA11894@burratino","threadId":"24653","inReplyTo":"20100808015729.GA9284@burratino","subject":"Re: [PATCH 1/3] test-lib: Add support for multiple test prerequisites","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-08T02:03:33Z","receivedAt":"2010-08-08T02:03:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -338,7 +338,7 @@ test_have_prereq () {\n>  \t\t*\" $prerequisite \"*)\n>  \t\t\t: yes, have it ;;\n>  \t\t*)\n> -\t\t\t! : nope ;;\n> +\t\t\treturn 1\n\nThe \";;\" is optional for the last arm in a case statement, but\nit is probably simpler to include it.  Sorry about that.\n\n(The only intended effect was only to use “return” that terminates and\npropagates out of the loop instead of “false” that is overridden by a\nlater “true”.)\n"},{"id":"147428","messageId":"201008081115.12444.j6t@kdbg.org","threadId":"24653","inReplyTo":"20100808015729.GA9284@burratino","subject":"Re: [PATCH 1/3] test-lib: Add support for multiple test prerequisites","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-08-08T09:15:12Z","receivedAt":"2010-08-08T09:15:12Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Sonntag, 8. August 2010, Jonathan Nieder wrote:\n> Ævar Arnfjörð Bjarmason wrote:\n> > Change the test_have_prereq function in test-lib.sh to support a\n> > comma-separated list of prerequisites. This is useful for tests that\n> > need e.g. both POSIXPERM and SANITY.\n> >\n> > The implementation was stolen from Junio C Hamano and Johannes Sixt,\n> > the tests and documentation were not.\n>\n> I think you can sell it better. :)\n>\n> \tFrom: Johannes Sixt <j6t@kdbg.org>\n> \tSubject: test-lib: Allow tests with multiple prerequisites\n\nI think the patch has changed sufficiently, particularly in the subtle points, \nthat authorship should not be attributed to me anymore.\n\nThanks for your review!\n\n-- Hannes\n"},{"id":"147440","messageId":"AANLkTikFUH38mzuPo8D-MwJQZdUPEaBUVKgrrfYX=5=p@mail.gmail.com","threadId":"24653","inReplyTo":"20100808015729.GA9284@burratino","subject":"Re: [PATCH 1/3] test-lib: Add support for multiple test prerequisites","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-08T13:29:22Z","receivedAt":"2010-08-08T13:29:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Aug 8, 2010 at 01:57, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> Change the test_have_prereq function in test-lib.sh to support a\n>> comma-separated list of prerequisites. This is useful for tests that\n>> need e.g. both POSIXPERM and SANITY.\n>>\n>> The implementation was stolen from Junio C Hamano and Johannes Sixt,\n>> the tests and documentation were not.\n>\n> I think you can sell it better. :)\n\nAs Johannes points out the patch is different enough that I changed\nthe authorship. Actually I wouldn't modify any patch under someone\nelse's name without getting them to sign off on it.\n\n>> +++ b/t/t0000-basic.sh\n>> @@ -73,6 +73,23 @@ then\n>>       exit 1\n>>  fi\n>>\n>> +test_set_prereq HAVETHIS\n>> +haveit=no\n>> +test_expect_success HAVETHIS,HAVEIT 'test runs if prerequisites are satisfied' '\n>> +    test_have_prereq HAVEIT &&\n>> +    test_have_prereq HAVETHIS &&\n>\n> I think the similar code above was just a way to sneak in a sanity\n> check for test_have_prereq().  I’d leave it out.\n\nIt's a sanity test. If that wasn't there the test might succeed\ne.g. if the implementation was broken and only did a OR on the comma\ndelimited list, not a AND.\n\n>> +    haveit=yes\n>> +'\n>> +donthaveit=yes\n>> +test_expect_success HAVEIT,DONTHAVEIT 'unmet prerequisites causes test to be skipped' '\n>> +    donthaveit=no\n>> +'\n>> +if test $haveit$donthaveit != yesyes\n>> +then\n>> +     say \"bug in test framework: multiple prerequisite tags do not work reliably\"\n>> +     exit 1\n>> +fi\n>\n> Maybe it would be simpler to squash this in with the other similar checks.\n\nI think that's too much complexity for one test, especially in the\nsanity file. I didn't squash it with the existing \"yesyes\" test\nbecause we're testing basic functionality first (one prereq) then the\nfancy stuff (multiple).\n\n>> +++ b/t/test-lib.sh\n>> @@ -327,12 +327,20 @@ test_set_prereq () {\n>>  satisfied=\" \"\n>>\n>>  test_have_prereq () {\n>> -     case $satisfied in\n>> -     *\" $1 \"*)\n>> -             : yes, have it ;;\n>> -     *)\n>> -             ! : nope ;;\n>> -     esac\n>> +     # prerequisites can be concatenated with ','\n>> +     save_IFS=$IFS\n>> +     IFS=,\n>> +     set -- $*\n>> +     IFS=$save_IFS\n>> +     for prerequisite\n>> +     do\n>> +             case $satisfied in\n>> +             *\" $prerequisite \"*)\n>> +                     : yes, have it ;;\n>> +             *)\n>> +                     ! : nope ;;\n>> +             esac\n>> +     done\n>\n> Does that work?\n\nIt passes all the tests :)\n\n> Except as noted above,\n> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n>\n> Thanks.\n\nLikewise.\n"}]}