{"thread":{"id":"49906","subject":"[PATCH 1/5] t/README: make the 'Skipping tests' section less confusing","startedAt":"2018-11-27T22:54:54Z","lastAt":"2018-12-04T14:46:23Z","messageCount":7,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"364168","messageId":"20181127225445.30045-1-avarab@gmail.com","threadId":"49906","inReplyTo":null,"subject":"[PATCH 1/5] t/README: make the 'Skipping tests' section less confusing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-27T22:54:41Z","receivedAt":"2018-11-27T22:54:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"I added this section in b5500d16cd (\"t/README: Add a section about\nskipping tests\", 2010-07-02), but apparently didn't notice that there\nwas an existing \"Skipping Tests\" section added in\nfbd458a3f6 (\"t/README: Add 'Skipping Tests' section below 'Running\nTests'\", 2008-06-20).\n\nThen since 20873f45e7 (\"t/README: Document the do's and don'ts of\ntests\", 2010-07-02) and 0445e6f0a1 (\"test-lib: '--run' to run only\nspecific tests\", 2014-04-30) we've started to refer to \"Skipping\ntests\" or \"Skipping Tests\" sections in prose elsewhere.\n\nLet's clean up this confusion by renaming the section, and while we're\nat it improve the example. Usually we don't use the PERL prerequisite,\nand we should accurately describe why you'd want to use prerequisites,\nas opposed to GIT_SKIP_TESTS. So let's say \"Tests that depend[...]\"\ninstead of \"If you need to skip tests[...]\" here.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/README | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 28711cc508..b6ec28f634 100644\n--- a/t/README\n+++ b/t/README\n@@ -494,7 +494,7 @@ And here are the \"don'ts:\"\n \n    The harness will catch this as a programming error of the test.\n    Use test_done instead if you need to stop the tests early (see\n-   \"Skipping tests\" below).\n+   \"Using test prerequisites\" below).\n \n  - Don't use '! git cmd' when you want to make sure the git command\n    exits with failure in a controlled way by calling \"die()\".  Instead,\n@@ -587,28 +587,28 @@ And here are the \"don'ts:\"\n    it'll complain if anything is amiss.\n \n \n-Skipping tests\n---------------\n+Using test prerequisites\n+------------------------\n \n-If you need to skip tests you should do so by using the three-arg form\n-of the test_* functions (see the \"Test harness library\" section\n-below), e.g.:\n+Tests that depend on something which may not be present on the system\n+should use the three-arg form of the test_* functions (see the \"Test\n+harness library\" section below), e.g.:\n \n-    test_expect_success PERL 'I need Perl' '\n-        perl -e \"hlagh() if unf_unf()\"\n+    test_expect_success SYMLINKS 'setup' '\n+        ln -s target link\n     '\n \n The advantage of skipping tests like this is that platforms that don't\n-have the PERL and other optional dependencies get an indication of how\n+have the SYMLINKS and other optional dependencies get an indication of how\n many tests they're missing.\n \n If the test code is too hairy for that (i.e. does a lot of setup work\n outside test assertions) you can also skip all remaining tests by\n setting skip_all and immediately call test_done:\n \n-\tif ! test_have_prereq PERL\n+\tif ! test_have_prereq SYMLINKS\n \tthen\n-\t    skip_all='skipping perl interface tests, perl not available'\n+\t    skip_all=\"skipping symlink tests, the filesystem doesn't support it\"\n \t    test_done\n \tfi\n \n-- \n2.20.0.rc1.379.g1dd7ef354c\n\n"},{"id":"364169","messageId":"20181127225445.30045-3-avarab@gmail.com","threadId":"49906","inReplyTo":"20181127225445.30045-1-avarab@gmail.com","subject":"[PATCH 3/5] t/README: re-flow a paragraph","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-27T22:54:43Z","receivedAt":"2018-11-27T22:54:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"An earlier change changed this paragraph to make the first line quite\nshort as to produce a more minimal diff. Let's re-flow it. There's no\nchanges here if diffed with --word-diff.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/README | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 3139f4330a..c03b268813 100644\n--- a/t/README\n+++ b/t/README\n@@ -216,11 +216,10 @@ Or tests matching a glob:\n \n     $ GIT_SKIP_TESTS='t[0-4]??? t91?? t9200.8' make\n \n-The value of the environment variable is a\n-SP separated list of patterns that tells which tests to skip,\n-and either can match the \"t[0-9]{4}\" part to skip the whole\n-test, or t[0-9]{4} followed by \".$number\" to say which\n-particular test to skip.\n+The value of the environment variable is a SP separated list of\n+patterns that tells which tests to skip, and either can match the\n+\"t[0-9]{4}\" part to skip the whole test, or t[0-9]{4} followed by\n+\".$number\" to say which particular test to skip.\n \n For an individual test suite --run could be used to specify that\n only some tests should be run or that some tests should be\n-- \n2.20.0.rc1.379.g1dd7ef354c\n\n"},{"id":"364170","messageId":"20181127225445.30045-4-avarab@gmail.com","threadId":"49906","inReplyTo":"20181127225445.30045-1-avarab@gmail.com","subject":"[PATCH 4/5] test-lib: add more exhaustive GIT_SKIP_TESTS testing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-27T22:54:44Z","receivedAt":"2018-11-27T22:54:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a test for the when GIT_SKIP_TESTS is used to skip entire test\nfiles. Support for this was added back in 04ece59399 (\"GIT_SKIP_TESTS:\nallow users to omit tests that are known to break\", 2006-12-28), but\nnever tested for.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/t0000-basic.sh | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex b6566003dd..b87a8f18c2 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -393,6 +393,23 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' \"\n \t)\n \"\n \n+test_expect_success 'GIT_SKIP_TESTS entire file' \"\n+\t(\n+\t\tGIT_SKIP_TESTS='git' && export GIT_SKIP_TESTS &&\n+\t\trun_sub_test_lib_test git-skip-tests-entire-file \\\n+\t\t\t'GIT_SKIP_TESTS' <<-\\\\EOF &&\n+\t\tfor i in 1 2 3\n+\t\tdo\n+\t\t\ttest_expect_success \\\"passing test #\\$i\\\" 'true'\n+\t\tdone\n+\t\ttest_done\n+\t\tEOF\n+\t\tcheck_sub_test_lib_test git-skip-tests-entire-file <<-\\\\EOF\n+\t\t1..0 # SKIP skip all tests in git\n+\t\tEOF\n+\t)\n+\"\n+\n test_expect_success '--run basic' \"\n \trun_sub_test_lib_test run-basic \\\n \t\t'--run basic' --run='1 3 5' <<-\\\\EOF &&\n-- \n2.20.0.rc1.379.g1dd7ef354c\n\n"},{"id":"364171","messageId":"20181127225445.30045-5-avarab@gmail.com","threadId":"49906","inReplyTo":"20181127225445.30045-1-avarab@gmail.com","subject":"[PATCH 5/5] test-lib: add support for GIT_TODO_TESTS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-27T22:54:45Z","receivedAt":"2018-11-27T22:55:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As noted in the updated t/README documentation being added here\nsetting this new GIT_TODO_TESTS variable is usually better than\nGIT_SKIP_TESTS.\n\nMy use-case for this is to get feedback from the CI infrastructure[1]\nabout which tests are passing due to fixes that have trickled into\ngit.git.\n\nWith the GIT_SKIP_TESTS variable this use-case is painful, you need to\ndo an occasional manual run without GIT_SKIP_TESTS set. It's much\nbetter to use GIT_TODO_TESTS and get a report of passing TODO tests\nfrom prove(1) at the bottom of the test output. Once those passing\nTODO tests have trickled down to 'master' the relevant glob (set for\nall of master/next/pu) can be removed.\n\nAs seen in the \"GIT_TODO_TESTS mixed failure\" test the lack of\ninteraction with existing tests marked as TODO by the test suite\nitself is intentional. There's no need to print out something saying\nthey matched GIT_TODO_TESTS if they're already TODO tests.\n\n1. https://public-inbox.org/git/875zwm15k2.fsf@evledraar.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n ci/lib-travisci.sh |  2 +-\n t/README           | 18 +++++++++--\n t/t0000-basic.sh   | 81 +++++++++++++++++++++++++++++++++++++++-------\n t/test-lib.sh      | 31 +++++++++++++++---\n 4 files changed, 112 insertions(+), 20 deletions(-)\n\ndiff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh\nindex 69dff4d1ec..ad8290bfdb 100755\n--- a/ci/lib-travisci.sh\n+++ b/ci/lib-travisci.sh\n@@ -121,7 +121,7 @@ osx-clang|osx-gcc)\n \t# t9810 occasionally fails on Travis CI OS X\n \t# t9816 occasionally fails with \"TAP out of sequence errors\" on\n \t# Travis CI OS X\n-\texport GIT_SKIP_TESTS=\"t9810 t9816\"\n+\texport GIT_TODO_TESTS=\"t9810 t9816\"\n \t;;\n GIT_TEST_GETTEXT_POISON)\n \texport GIT_TEST_GETTEXT_POISON=YesPlease\ndiff --git a/t/README b/t/README\nindex c03b268813..922b4fb3bf 100644\n--- a/t/README\n+++ b/t/README\n@@ -207,8 +207,19 @@ ideally be reported as bugs and fixed, or guarded by a prerequisite\n (see \"Using test prerequisites\" below). But until then they can be\n skipped.\n \n-To skip tests, set the GIT_SKIP_TESTS variable. Individual tests can\n-be skipped:\n+To skip tests, set either the GIT_SKIP_TESTS or GIT_TODO_TESTS\n+variables. The difference is that with SKIP the tests won't be run at\n+all, whereas they will be run with TODO, but in success or failure\n+annotated as a TODO test.\n+\n+It's usually preferrable to use TODO, since the test suite will report\n+those tests that unexpectedly succeed, which may indicate that\n+whatever bug broke the test in the past has been fixed, and the test\n+should be un-TODO'd. There's no such feedback loop with\n+GIT_SKIP_TESTS.\n+\n+The GIT_SKIP_TESTS and GIT_TODO_TESTS take the same values. Individual\n+tests can be skipped:\n \n     $ GIT_SKIP_TESTS=t9200.8 sh ./t9200-git-cvsexport-commit.sh\n \n@@ -223,7 +234,8 @@ patterns that tells which tests to skip, and either can match the\n \n For an individual test suite --run could be used to specify that\n only some tests should be run or that some tests should be\n-excluded from a run.\n+excluded from a run. The --run option is a shorthand for setting\n+a GIT_SKIP_TESTS pattern.\n \n The argument for --run is a list of individual test numbers or\n ranges with an optional negation prefix that define what tests in\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex b87a8f18c2..34c9c5c2a3 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -324,9 +324,10 @@ test_expect_success 'test --verbose-only' '\n \tEOF\n '\n \n-test_expect_success 'GIT_SKIP_TESTS' \"\n+test_expect_success 'GIT_{SKIP,TODO}_TESTS' \"\n \t(\n \t\tGIT_SKIP_TESTS='git.2' && export GIT_SKIP_TESTS &&\n+\t\tGIT_TODO_TESTS='git.3' && export GIT_TODO_TESTS &&\n \t\trun_sub_test_lib_test git-skip-tests-basic \\\n \t\t\t'GIT_SKIP_TESTS' <<-\\\\EOF &&\n \t\tfor i in 1 2 3\n@@ -338,16 +339,17 @@ test_expect_success 'GIT_SKIP_TESTS' \"\n \t\tcheck_sub_test_lib_test git-skip-tests-basic <<-\\\\EOF\n \t\t> ok 1 - passing test #1\n \t\t> ok 2 # skip passing test #2 (GIT_SKIP_TESTS)\n-\t\t> ok 3 - passing test #3\n-\t\t> # passed all 3 test(s)\n+\t\t> ok 3 - passing test #3 # TODO (GIT_TODO_TESTS)\n+\t\t> # passed all 3 test(s) (including 1/0 ok/failing TODO test(s))\n \t\t> 1..3\n \t\tEOF\n \t)\n \"\n \n-test_expect_success 'GIT_SKIP_TESTS several tests' \"\n+test_expect_success 'GIT_{SKIP,TODO}_TESTS several tests' \"\n \t(\n \t\tGIT_SKIP_TESTS='git.2 git.5' && export GIT_SKIP_TESTS &&\n+\t\tGIT_TODO_TESTS='git.3 git.6' && export GIT_TODO_TESTS &&\n \t\trun_sub_test_lib_test git-skip-tests-several \\\n \t\t\t'GIT_SKIP_TESTS several tests' <<-\\\\EOF &&\n \t\tfor i in 1 2 3 4 5 6\n@@ -359,22 +361,25 @@ test_expect_success 'GIT_SKIP_TESTS several tests' \"\n \t\tcheck_sub_test_lib_test git-skip-tests-several <<-\\\\EOF\n \t\t> ok 1 - passing test #1\n \t\t> ok 2 # skip passing test #2 (GIT_SKIP_TESTS)\n-\t\t> ok 3 - passing test #3\n+\t\t> ok 3 - passing test #3 # TODO (GIT_TODO_TESTS)\n \t\t> ok 4 - passing test #4\n \t\t> ok 5 # skip passing test #5 (GIT_SKIP_TESTS)\n-\t\t> ok 6 - passing test #6\n-\t\t> # passed all 6 test(s)\n+\t\t> ok 6 - passing test #6 # TODO (GIT_TODO_TESTS)\n+\t\t> # passed all 6 test(s) (including 2/0 ok/failing TODO test(s))\n \t\t> 1..6\n \t\tEOF\n \t)\n \"\n \n-test_expect_success 'GIT_SKIP_TESTS sh pattern' \"\n+test_expect_success 'GIT_{SKIP,TODO}_TESTS sh pattern' \"\n \t(\n \t\tGIT_SKIP_TESTS='git.[2-5]' && export GIT_SKIP_TESTS &&\n+\t\t# Overlap between SKIP and TODO. No attempt is made to\n+\t\t# be smart about it.\n+\t\tGIT_TODO_TESTS='git.[4-8]' && export GIT_TODO_TESTS &&\n \t\trun_sub_test_lib_test git-skip-tests-sh-pattern \\\n \t\t\t'GIT_SKIP_TESTS sh pattern' <<-\\\\EOF &&\n-\t\tfor i in 1 2 3 4 5 6\n+\t\tfor i in 1 2 3 4 5 6 7 8 9\n \t\tdo\n \t\t\ttest_expect_success \\\"passing test #\\$i\\\" 'true'\n \t\tdone\n@@ -386,9 +391,12 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' \"\n \t\t> ok 3 # skip passing test #3 (GIT_SKIP_TESTS)\n \t\t> ok 4 # skip passing test #4 (GIT_SKIP_TESTS)\n \t\t> ok 5 # skip passing test #5 (GIT_SKIP_TESTS)\n-\t\t> ok 6 - passing test #6\n-\t\t> # passed all 6 test(s)\n-\t\t> 1..6\n+\t\t> ok 6 - passing test #6 # TODO (GIT_TODO_TESTS)\n+\t\t> ok 7 - passing test #7 # TODO (GIT_TODO_TESTS)\n+\t\t> ok 8 - passing test #8 # TODO (GIT_TODO_TESTS)\n+\t\t> ok 9 - passing test #9\n+\t\t> # passed all 9 test(s) (including 3/0 ok/failing TODO test(s))\n+\t\t> 1..9\n \t\tEOF\n \t)\n \"\n@@ -410,6 +418,55 @@ test_expect_success 'GIT_SKIP_TESTS entire file' \"\n \t)\n \"\n \n+test_expect_success 'GIT_TODO_TESTS entire file' \"\n+\t(\n+\t\tGIT_TODO_TESTS='git' && export GIT_TODO_TESTS &&\n+\t\trun_sub_test_lib_test git-todo-tests-entire-file \\\n+\t\t\t'GIT_TODO_TESTS' <<-\\\\EOF &&\n+\t\tfor i in 1 2 3\n+\t\tdo\n+\t\t\ttest_expect_success \\\"failing test #\\$i\\\" 'fail'\n+\t\tdone\n+\t\ttest_done\n+\t\tEOF\n+\t\tcheck_sub_test_lib_test git-todo-tests-entire-file <<-\\\\EOF\n+\t\tnot ok 1 - failing test #1 # TODO (GIT_TODO_TESTS)\n+\t\t#\tfail\n+\t\tnot ok 2 - failing test #2 # TODO (GIT_TODO_TESTS)\n+\t\t#\tfail\n+\t\tnot ok 3 - failing test #3 # TODO (GIT_TODO_TESTS)\n+\t\t#\tfail\n+\t\t# passed all 3 test(s) (including 0/3 ok/failing TODO test(s))\n+\t\t1..3\n+\t\tEOF\n+\t)\n+\"\n+\n+test_expect_success 'GIT_TODO_TESTS mixed failure' \"\n+\t(\n+\t\tGIT_TODO_TESTS='git' && export GIT_TODO_TESTS &&\n+\t\trun_sub_test_lib_test git-todo-tests-mixed \\\n+\t\t\t'GIT_TODO_TESTS' <<-\\\\EOF &&\n+\t\ttest_expect_success \\\"success\\\" 'true'\n+\t\ttest_expect_success \\\"success\\\" 'false'\n+\t\ttest_expect_failure \\\"success\\\" 'true'\n+\t\ttest_expect_failure \\\"success\\\" 'false'\n+\t\ttest_done\n+\t\tEOF\n+\t\tcheck_sub_test_lib_test git-todo-tests-mixed <<-\\\\EOF\n+\t\tok 1 - success # TODO (GIT_TODO_TESTS)\n+\t\tnot ok 2 - success # TODO (GIT_TODO_TESTS)\n+\t\t#\tfalse\n+\t\tok 3 - success # TODO known breakage vanished\n+\t\tnot ok 4 - success # TODO known breakage\n+\t\t# 1 known breakage(s) vanished; please update test(s)\n+\t\t# still have 1 known breakage(s)\n+\t\t# passed all remaining 2 test(s) (including 1/1 ok/failing TODO test(s))\n+\t\t1..4\n+\t\tEOF\n+\t)\n+\"\n+\n test_expect_success '--run basic' \"\n \trun_sub_test_lib_test run-basic \\\n \t\t'--run basic' --run='1 3 5' <<-\\\\EOF &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 6c6c0af7a1..ccd316e7bd 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -456,6 +456,8 @@ test_count=0\n test_fixed=0\n test_broken=0\n test_success=0\n+test_todo_success=0\n+test_todo_failure=0\n \n test_external_has_tap=0\n \n@@ -482,13 +484,29 @@ trap 'exit $?' INT\n # the test_expect_* functions instead.\n \n test_ok_ () {\n+\ttodo_blurb=\n+\tif match_pattern_list $this_test.$test_count $GIT_TODO_TESTS ||\n+\t\t    match_pattern_list $this_test $GIT_TODO_TESTS\n+\tthen\n+\t\ttodo_blurb=' # TODO (GIT_TODO_TESTS)'\n+\t\ttest_todo_success=$(($test_todo_success + 1))\n+\tfi\n \ttest_success=$(($test_success + 1))\n-\tsay_color \"\" \"ok $test_count - $@\"\n+\tsay_color \"\" \"ok $test_count - $@$todo_blurb\"\n }\n \n test_failure_ () {\n-\ttest_failure=$(($test_failure + 1))\n-\tsay_color error \"not ok $test_count - $1\"\n+\ttodo_blurb=\n+\tif match_pattern_list $this_test.$test_count $GIT_TODO_TESTS ||\n+\t\t    match_pattern_list $this_test $GIT_TODO_TESTS\n+\tthen\n+\t\ttest_success=$(($test_success + 1))\n+\t\ttest_todo_failure=$(($test_todo_failure + 1))\n+\t\ttodo_blurb=' # TODO (GIT_TODO_TESTS)'\n+\telse\n+\t\ttest_failure=$(($test_failure + 1))\n+\tfi\n+\tsay_color error \"not ok $test_count - $1$todo_blurb\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n@@ -845,13 +863,18 @@ test_done () {\n \t\ttest_remaining=$test_count\n \t\tmsg=\"$test_count test(s)\"\n \tfi\n+\tmsg_todo=\n+\tif test \"$test_todo_success\" != 0 || test \"$test_todo_failure\" != 0\n+\tthen\n+\t\tmsg_todo=\" (including $test_todo_success/$test_todo_failure ok/failing TODO test(s))\"\n+\tfi\n \tcase \"$test_failure\" in\n \t0)\n \t\tif test $test_external_has_tap -eq 0\n \t\tthen\n \t\t\tif test $test_remaining -gt 0\n \t\t\tthen\n-\t\t\t\tsay_color pass \"# passed all $msg\"\n+\t\t\t\tsay_color pass \"# passed all $msg$msg_todo\"\n \t\t\tfi\n \n \t\t\t# Maybe print SKIP message\n-- \n2.20.0.rc1.379.g1dd7ef354c\n\n"},{"id":"364172","messageId":"20181127225445.30045-2-avarab@gmail.com","threadId":"49906","inReplyTo":"20181127225445.30045-1-avarab@gmail.com","subject":"[PATCH 2/5] t/README: modernize description of GIT_SKIP_TESTS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-27T22:54:42Z","receivedAt":"2018-11-27T22:55:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When the GIT_SKIP_TESTS documentation was added in\nfbd458a3f6 (\"t/README: Add 'Skipping Tests' section below 'Running\nTests'\", 2008-06-20) there was no way to declare test prerequisites,\nthat came later in a7bb394037 (\"test-lib: Infrastructure to test and\ncheck for prerequisites\", 2009-03-01).\n\nThe docs were newer updated, and have been saying that you might want\nto use GIT_SKIP_TESTS for a use-case which we'd never use them for,\nskipping tests because 'unzip' isn't there. For that we'd use the\nUNZIP prerequisite added in 552a26c8c0 (\"Use prerequisites to skip\ntests that need unzip\", 2009-03-16). Fix the docs accordingly.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/README | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex b6ec28f634..3139f4330a 100644\n--- a/t/README\n+++ b/t/README\n@@ -202,20 +202,21 @@ GIT_TEST_EXEC_PATH defaults to `$GIT_TEST_INSTALLED/git --exec-path`.\n Skipping Tests\n --------------\n \n-In some environments, certain tests have no way of succeeding\n-due to platform limitation, such as lack of 'unzip' program, or\n-filesystem that do not allow arbitrary sequence of non-NUL bytes\n-as pathnames.\n+Certain tests may fail intermittently or entirely. These should\n+ideally be reported as bugs and fixed, or guarded by a prerequisite\n+(see \"Using test prerequisites\" below). But until then they can be\n+skipped.\n \n-You should be able to say something like\n+To skip tests, set the GIT_SKIP_TESTS variable. Individual tests can\n+be skipped:\n \n     $ GIT_SKIP_TESTS=t9200.8 sh ./t9200-git-cvsexport-commit.sh\n \n-and even:\n+Or tests matching a glob:\n \n     $ GIT_SKIP_TESTS='t[0-4]??? t91?? t9200.8' make\n \n-to omit such tests.  The value of the environment variable is a\n+The value of the environment variable is a\n SP separated list of patterns that tells which tests to skip,\n and either can match the \"t[0-9]{4}\" part to skip the whole\n test, or t[0-9]{4} followed by \".$number\" to say which\n-- \n2.20.0.rc1.379.g1dd7ef354c\n\n"},{"id":"364293","messageId":"xmqqk1kwo04u.fsf@gitster-ct.c.googlers.com","threadId":"49906","inReplyTo":"20181127225445.30045-5-avarab@gmail.com","subject":"Re: [PATCH 5/5] test-lib: add support for GIT_TODO_TESTS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-29T06:42:25Z","receivedAt":"2018-11-29T06:42:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> -To skip tests, set the GIT_SKIP_TESTS variable. Individual tests can\n> -be skipped:\n> +To skip tests, set either the GIT_SKIP_TESTS or GIT_TODO_TESTS\n> +variables. The difference is that with SKIP the tests won't be run at\n> +all, whereas they will be run with TODO, but in success or failure\n> +annotated as a TODO test.\n\nIt is entirely unclear what \"They will be run with TODO\" means.\n\nI know what you want to achieve from the code change; instead of\njust skipping, you want to cause as much side effect the skipped\ntest piece wants to make until it fails and dies, without taking\nthe remainder of the test down.  And I kind of agree that sometimes\nsuch a mode would be very useful (i.e. when you do not want to\nbother separating such a failing test properly into the setup part\nthat would affect later tests and the one-thing-it-wants-to-test\npart that can now be safely skipped).  From the cursory look, the\ncode change in this patch is sensible realization of that idea.\n\nHaving said all that, I won't picking this up until next month ;-) I\nreally want to see that everybody is concentrating on making sure\nthat 2.20 is solid before playing with shiny new toys.\n\nThanks.\n\n"},{"id":"364557","messageId":"20181204144615.GH30222@szeder.dev","threadId":"49906","inReplyTo":"20181127225445.30045-5-avarab@gmail.com","subject":"Re: [PATCH 5/5] test-lib: add support for GIT_TODO_TESTS","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-12-04T14:46:15Z","receivedAt":"2018-12-04T14:46:23Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Nov 27, 2018 at 11:54:45PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> As noted in the updated t/README documentation being added here\n> setting this new GIT_TODO_TESTS variable is usually better than\n> GIT_SKIP_TESTS.\n\nI don't see why this is \"usually better\".  I get how it can help your\nparticular use-case described below, but that doesn't mean that it's\n\"usually better\".\n\n> My use-case for this is to get feedback from the CI infrastructure[1]\n> about which tests are passing due to fixes that have trickled into\n> git.git.\n> \n> With the GIT_SKIP_TESTS variable this use-case is painful, you need to\n> do an occasional manual run without GIT_SKIP_TESTS set. It's much\n> better to use GIT_TODO_TESTS and get a report of passing TODO tests\n> from prove(1) at the bottom of the test output. Once those passing\n> TODO tests have trickled down to 'master' the relevant glob (set for\n> all of master/next/pu) can be removed.\n\nNeither from the commit message nor from the documentation is it clear\nto me what the result of 'make test' will be when a test listed in\nGIT_TODO_TESTS fails.\n\n> As seen in the \"GIT_TODO_TESTS mixed failure\" test the lack of\n> interaction with existing tests marked as TODO by the test suite\n> itself is intentional. There's no need to print out something saying\n> they matched GIT_TODO_TESTS if they're already TODO tests.\n> \n> 1. https://public-inbox.org/git/875zwm15k2.fsf@evledraar.gmail.com/\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>  ci/lib-travisci.sh |  2 +-\n>  t/README           | 18 +++++++++--\n>  t/t0000-basic.sh   | 81 +++++++++++++++++++++++++++++++++++++++-------\n>  t/test-lib.sh      | 31 +++++++++++++++---\n>  4 files changed, 112 insertions(+), 20 deletions(-)\n> \n> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh\n> index 69dff4d1ec..ad8290bfdb 100755\n> --- a/ci/lib-travisci.sh\n> +++ b/ci/lib-travisci.sh\n> @@ -121,7 +121,7 @@ osx-clang|osx-gcc)\n>  \t# t9810 occasionally fails on Travis CI OS X\n>  \t# t9816 occasionally fails with \"TAP out of sequence errors\" on\n>  \t# Travis CI OS X\n> -\texport GIT_SKIP_TESTS=\"t9810 t9816\"\n> +\texport GIT_TODO_TESTS=\"t9810 t9816\"\n\nThis change is not mentioned in the commit message.\n\nAs noted in the hunk's context, these test scripts are not skipped\nbecause they don't work on OSX at all, but because they are flaky.\nConsequently, reporting them as \"maybe should be un-TODO'd\" when they\nhappen to succeed is pointless and will just lead to confusion, so\nthis seems to be a case when GIT_TODO_TESTS is actually worse than\nGIT_SKIP_TESTS.\n\nIf a failing test in GIT_TODO_TESTS makes the whole 'make test' fail,\nthen this should be most definitely left as GIT_SKIP_TESTS.\n\n>  \t;;\n>  GIT_TEST_GETTEXT_POISON)\n>  \texport GIT_TEST_GETTEXT_POISON=YesPlease\n> diff --git a/t/README b/t/README\n> index c03b268813..922b4fb3bf 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -207,8 +207,19 @@ ideally be reported as bugs and fixed, or guarded by a prerequisite\n>  (see \"Using test prerequisites\" below). But until then they can be\n>  skipped.\n>  \n> -To skip tests, set the GIT_SKIP_TESTS variable. Individual tests can\n> -be skipped:\n> +To skip tests, set either the GIT_SKIP_TESTS or GIT_TODO_TESTS\n> +variables. The difference is that with SKIP the tests won't be run at\n> +all, whereas they will be run with TODO, but in success or failure\n> +annotated as a TODO test.\n\nThis is confusing.  \"To skip\" a test means that the test is not run at\nall.  Now, if GIT_TODO_TESTS were to run the listed tests, then it\ndefinitely won't skip them, so this sentence contradicts the previous\none.\n\nWhat does \"annotated as a TODO test\" mean?  Something similar to how\n'test_expect_failure' works?\n\n> +It's usually preferrable to use TODO, since the test suite will report\n> +those tests that unexpectedly succeed, which may indicate that\n> +whatever bug broke the test in the past has been fixed, and the test\n> +should be un-TODO'd. There's no such feedback loop with\n> +GIT_SKIP_TESTS.\n> +\n> +The GIT_SKIP_TESTS and GIT_TODO_TESTS take the same values. Individual\n> +tests can be skipped:\n>  \n>      $ GIT_SKIP_TESTS=t9200.8 sh ./t9200-git-cvsexport-commit.sh\n>  \n> @@ -223,7 +234,8 @@ patterns that tells which tests to skip, and either can match the\n>  \n>  For an individual test suite --run could be used to specify that\n>  only some tests should be run or that some tests should be\n> -excluded from a run.\n> +excluded from a run. The --run option is a shorthand for setting\n> +a GIT_SKIP_TESTS pattern.\n>  \n>  The argument for --run is a list of individual test numbers or\n>  ranges with an optional negation prefix that define what tests in\n"}]}