{"thread":{"id":"31608","subject":"[PATCH v2 1/6] Change the color of individual known breakages","startedAt":"2012-09-19T17:15:09Z","lastAt":"2012-09-20T21:13:28Z","messageCount":19,"participants":["Adam Spiers","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":6},"messages":[{"id":"199485","messageId":"1348074915-19985-1-git-send-email-git@adamspiers.org","threadId":"31608","inReplyTo":"20120918213617.GB2567@atlantic.linksys.moosehall","subject":"[PATCH v2 0/6] make test output coloring more intuitive","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T17:15:09Z","receivedAt":"2012-09-19T17:15:09Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"This series of commits attempts to make test output coloring\nmore intuitive, so that:\n\n  - red is _only_ used for things which have gone unexpectedly wrong:\n    test failures, unexpected test passes, and failures with the\n    framework,\n\n  - yellow is _only_ used for known breakages and skipped tests, and\n\n  - green is _only_ used for things which have gone to plan and\n    require no further work to be done.\n\nAdam Spiers (6):\n  Change the color of individual known breakages\n  Make 'not ok $count - $message' consistent with 'ok $count -\n    $message'\n  Color skipped tests the same as informational messages\n  Refactor mechanics of testing in a sub test-lib\n  Test the test framework more thoroughly\n  Treat unexpectedly fixed known breakages more seriously\n\n t/t0000-basic.sh | 179 ++++++++++++++++++++++++++++++++++++++++++++++++-------\n t/test-lib.sh    |  25 +++++---\n 2 files changed, 174 insertions(+), 30 deletions(-)\n mode change 100644 => 100755 t/test-lib.sh\n\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199484","messageId":"1348074915-19985-2-git-send-email-git@adamspiers.org","threadId":"31608","inReplyTo":"1348074915-19985-1-git-send-email-git@adamspiers.org","subject":"[PATCH v2 1/6] Change the color of individual known breakages","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T17:15:10Z","receivedAt":"2012-09-19T17:15:10Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Bold yellow seems a more appropriate color than bold green when\nconsidering the universal traffic lights coloring scheme, where green\nconveys the impression that everything's OK, and amber that\nsomething's not quite right.\n\nLikewise, change the color of the summarized total number of known\nbreakages from bold red to bold yellow to be less alarmist and more\nconsistent with the above.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\n---\n t/test-lib.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex f8e3733..426820e 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -183,6 +183,8 @@ then\n \t\t\ttput bold; tput setaf 1;; # bold red\n \t\tskip)\n \t\t\ttput bold; tput setaf 2;; # bold green\n+\t\twarn)\n+\t\t\ttput bold; tput setaf 3;; # bold yellow\n \t\tpass)\n \t\t\ttput setaf 2;;            # green\n \t\tinfo)\n@@ -281,7 +283,7 @@ test_known_broken_ok_ () {\n \n test_known_broken_failure_ () {\n \ttest_broken=$(($test_broken+1))\n-\tsay_color skip \"not ok $test_count - $@ # TODO known breakage\"\n+\tsay_color warn \"not ok $test_count - $@ # TODO known breakage\"\n }\n \n test_debug () {\n@@ -375,7 +377,7 @@ test_done () {\n \tfi\n \tif test \"$test_broken\" != 0\n \tthen\n-\t\tsay_color error \"# still have $test_broken known breakage(s)\"\n+\t\tsay_color warn \"# still have $test_broken known breakage(s)\"\n \t\tmsg=\"remaining $(($test_count-$test_broken)) test(s)\"\n \telse\n \t\tmsg=\"$test_count test(s)\"\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199486","messageId":"1348074915-19985-3-git-send-email-git@adamspiers.org","threadId":"31608","inReplyTo":"1348074915-19985-1-git-send-email-git@adamspiers.org","subject":"[PATCH v2 2/6] Make 'not ok $count - $message' consistent with 'ok $count - $message'","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T17:15:11Z","receivedAt":"2012-09-19T17:15:11Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Signed-off-by: Adam Spiers <git@adamspiers.org>\n---\n t/t0000-basic.sh | 4 ++--\n t/test-lib.sh    | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n mode change 100644 => 100755 t/test-lib.sh\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex ae6a3f0..c6b42de 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -167,13 +167,13 @@ test_expect_success 'tests clean up even on failures' \"\n \t! test -s err &&\n \t! test -f \\\"trash directory.failing-cleanup/clean-after-failure\\\" &&\n \tsed -e 's/Z$//' -e 's/^> //' >expect <<-\\\\EOF &&\n-\t> not ok - 1 tests clean up even after a failure\n+\t> not ok 1 - tests clean up even after a failure\n \t> #\tZ\n \t> #\ttouch clean-after-failure &&\n \t> #\ttest_when_finished rm clean-after-failure &&\n \t> #\t(exit 1)\n \t> #\tZ\n-\t> not ok - 2 failure to clean up causes the test to fail\n+\t> not ok 2 - failure to clean up causes the test to fail\n \t> #\tZ\n \t> #\ttest_when_finished \\\"(exit 2)\\\"\n \t> #\tZ\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nold mode 100644\nnew mode 100755\nindex 426820e..5293830\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -270,7 +270,7 @@ test_ok_ () {\n \n test_failure_ () {\n \ttest_failure=$(($test_failure + 1))\n-\tsay_color error \"not ok - $test_count $1\"\n+\tsay_color error \"not ok $test_count - $1\"\n \tshift\n \techo \"$@\" | sed -e 's/^/#\t/'\n \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199487","messageId":"1348074915-19985-4-git-send-email-git@adamspiers.org","threadId":"31608","inReplyTo":"1348074915-19985-1-git-send-email-git@adamspiers.org","subject":"[PATCH v2 3/6] Color skipped tests the same as informational messages","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T17:15:12Z","receivedAt":"2012-09-19T17:15:12Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Skipped tests indicate incomplete test coverage.  Whilst this is\nnot a test failure or other error, it's still not complete\nsuccess, so according to the universal traffic lights coloring\nscheme, yellow/brown seems more suitable than green.  However,\nit's more informational than cautionary, so we leave it non-bold\nin order to contrast with warnings.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\n---\n t/test-lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 5293830..7028ba8 100755\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -182,13 +182,13 @@ then\n \t\terror)\n \t\t\ttput bold; tput setaf 1;; # bold red\n \t\tskip)\n-\t\t\ttput bold; tput setaf 2;; # bold green\n+\t\t\ttput setaf 3;;            # yellow/brown\n \t\twarn)\n \t\t\ttput bold; tput setaf 3;; # bold yellow\n \t\tpass)\n \t\t\ttput setaf 2;;            # green\n \t\tinfo)\n-\t\t\ttput setaf 3;;            # brown\n+\t\t\ttput setaf 3;;            # yellow/brown\n \t\t*)\n \t\t\ttest -n \"$quiet\" && return;;\n \t\tesac\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199489","messageId":"1348074915-19985-5-git-send-email-git@adamspiers.org","threadId":"31608","inReplyTo":"1348074915-19985-1-git-send-email-git@adamspiers.org","subject":"[PATCH v2 4/6] Refactor mechanics of testing in a sub test-lib","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T17:15:13Z","receivedAt":"2012-09-19T17:15:13Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"This will allow us to test the test framework more thoroughly\nwithout disrupting the top-level test metrics.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\n---\n t/t0000-basic.sh | 44 +++++++++++++++++++++++++++-----------------\n 1 file changed, 27 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex c6b42de..662cd2f 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -55,39 +55,49 @@ test_expect_failure 'pretend we have a known breakage' '\n \tfalse\n '\n \n-test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib)' \"\n-\tmkdir passing-todo &&\n-\t(cd passing-todo &&\n-\tcat >passing-todo.sh <<-EOF &&\n+run_sub_test_lib_test () {\n+\tname=\"$1\" descr=\"$2\" # stdin is body of test code\n+\tmkdir $name &&\n+\t(cd $name &&\n+\tcat >$name.sh <<-EOF &&\n \t#!$SHELL_PATH\n \n-\ttest_description='A passing TODO test\n+\ttest_description='$descr (run in sub test-lib)\n \n \tThis is run in a sub test-lib so that we do not get incorrect\n \tpassing metrics\n \t'\n \n \t# Point to the t/test-lib.sh, which isn't in ../ as usual\n-\tTEST_DIRECTORY=\\\"$TEST_DIRECTORY\\\"\n-\t. \\\"\\$TEST_DIRECTORY\\\"/test-lib.sh\n-\n-\ttest_expect_failure 'pretend we have fixed a known breakage' '\n-\t\t:\n-\t'\n+\tTEST_DIRECTORY=\"$TEST_DIRECTORY\"\n+\t. \"\\$TEST_DIRECTORY\"/test-lib.sh\n+\tEOF\n+\tcat >>$name.sh &&\n+\tchmod +x $name.sh &&\n+\t./$name.sh >out 2>err)\n+}\n+\n+check_sub_test_lib_test () {\n+\tname=\"$1\" # stdin is test's expected stdout\n+\t(cd $name &&\n+\t! test -s err &&\n+\tsed -e 's/^> //' >expect &&\n+\ttest_cmp expect out)\n+}\n \n+test_expect_success 'pretend we have fixed a known breakage' \"\n+\trun_sub_test_lib_test passing-todo 'A passing TODO test' <<-EOF &&\n+\ttest_expect_failure 'pretend we have fixed a known breakage' 'true'\n \ttest_done\n \tEOF\n-\tchmod +x passing-todo.sh &&\n-\t./passing-todo.sh >out 2>err &&\n-\t! test -s err &&\n-\tsed -e 's/^> //' >expect <<-\\\\EOF &&\n+\tcheck_sub_test_lib_test passing-todo <<-EOF\n \t> ok 1 - pretend we have fixed a known breakage # TODO known breakage\n \t> # fixed 1 known breakage(s)\n \t> # passed all 1 test(s)\n \t> 1..1\n \tEOF\n-\ttest_cmp expect out)\n \"\n+\n test_set_prereq HAVEIT\n haveit=no\n test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '\n@@ -166,7 +176,7 @@ test_expect_success 'tests clean up even on failures' \"\n \ttest_must_fail ./failing-cleanup.sh >out 2>err &&\n \t! test -s err &&\n \t! test -f \\\"trash directory.failing-cleanup/clean-after-failure\\\" &&\n-\tsed -e 's/Z$//' -e 's/^> //' >expect <<-\\\\EOF &&\n+\tsed -e 's/Z$//' -e 's/^> //' >expect <<-EOF &&\n \t> not ok 1 - tests clean up even after a failure\n \t> #\tZ\n \t> #\ttouch clean-after-failure &&\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199490","messageId":"1348074915-19985-6-git-send-email-git@adamspiers.org","threadId":"31608","inReplyTo":"1348074915-19985-1-git-send-email-git@adamspiers.org","subject":"[PATCH v2 5/6] Test the test framework more thoroughly","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T17:15:14Z","receivedAt":"2012-09-19T17:15:14Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Add 5 new full test suite runs each with a different number of\npassing/failing/broken/fixed tests, in order to ensure that the\ncorrect exit code and output are generated in each case.  As before,\nthese are run in a subdirectory in order to disrupt the metrics for\nthe parent tests.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\n---\n t/t0000-basic.sh | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 110 insertions(+)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 662cd2f..644cc2c 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -77,6 +77,15 @@ run_sub_test_lib_test () {\n \t./$name.sh >out 2>err)\n }\n \n+run_sub_test_lib_test_expecting_failures () {\n+\tif run_sub_test_lib_test \"$@\"; then\n+\t\techo 'sub test-lib run should have failed'\n+\t\treturn 1\n+\telse\n+\t\treturn 0\n+\tfi\n+}\n+\n check_sub_test_lib_test () {\n \tname=\"$1\" # stdin is test's expected stdout\n \t(cd $name &&\n@@ -85,6 +94,54 @@ check_sub_test_lib_test () {\n \ttest_cmp expect out)\n }\n \n+test_expect_success 'pretend we have a fully passing test suite' \"\n+\trun_sub_test_lib_test full-pass '3 passing tests' <<-EOF &&\n+\tfor i in 1 2 3; do\n+\t\ttest_expect_success \\\"passing test #\\\\\\$i\\\" 'true'\n+\tdone\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test full-pass <<-EOF\n+\t> ok 1 - passing test #1\n+\t> ok 2 - passing test #2\n+\t> ok 3 - passing test #3\n+\t> # passed all 3 test(s)\n+\t> 1..3\n+\tEOF\n+\"\n+\n+test_expect_success 'pretend we have a partially passing test suite' \"\n+\trun_sub_test_lib_test_expecting_failures partial-pass '2/3 tests passing' <<-EOF &&\n+\ttest_expect_success 'passing test #1' 'true'\n+\ttest_expect_success 'failing test #2' 'false'\n+\ttest_expect_success 'passing test #3' 'true'\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test partial-pass <<-EOF\n+\t> ok 1 - passing test #1\n+\t> not ok 2 - failing test #2\n+\t#\tfalse\n+\t> ok 3 - passing test #3\n+\t> # failed 1 among 3 test(s)\n+\t> 1..3\n+\tEOF\n+\"\n+\n+test_expect_success 'pretend we have a known breakage' \"\n+\trun_sub_test_lib_test failing-todo 'A failing TODO test' <<-EOF &&\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_failure 'pretend we have a known breakage' 'false'\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test failing-todo <<-EOF\n+\t> ok 1 - passing test\n+\t> not ok 2 - pretend we have a known breakage # TODO known breakage\n+\t> # still have 1 known breakage(s)\n+\t> # passed all remaining 1 test(s)\n+\t> 1..2\n+\tEOF\n+\"\n+\n test_expect_success 'pretend we have fixed a known breakage' \"\n \trun_sub_test_lib_test passing-todo 'A passing TODO test' <<-EOF &&\n \ttest_expect_failure 'pretend we have fixed a known breakage' 'true'\n@@ -98,6 +155,59 @@ test_expect_success 'pretend we have fixed a known breakage' \"\n \tEOF\n \"\n \n+test_expect_success 'pretend we have a pass, fail, and known breakage' \"\n+\trun_sub_test_lib_test_expecting_failures mixed-results1 'mixed results #1' <<-EOF &&\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_success 'failing test' 'false'\n+\ttest_expect_failure 'pretend we have a known breakage' 'false'\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test mixed-results1 <<-EOF\n+\t> ok 1 - passing test\n+\t> not ok 2 - failing test\n+\t> #\tfalse\n+\t> not ok 3 - pretend we have a known breakage # TODO known breakage\n+\t> # still have 1 known breakage(s)\n+\t> # failed 1 among remaining 2 test(s)\n+\t> 1..3\n+\tEOF\n+\"\n+\n+test_expect_success 'pretend we have a mix of all possible results' \"\n+\trun_sub_test_lib_test_expecting_failures mixed-results2 'mixed results #2' <<-EOF &&\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_success 'failing test' 'false'\n+\ttest_expect_success 'failing test' 'false'\n+\ttest_expect_success 'failing test' 'false'\n+\ttest_expect_failure 'pretend we have a known breakage' 'false'\n+\ttest_expect_failure 'pretend we have a known breakage' 'false'\n+\ttest_expect_failure 'pretend we have fixed a known breakage' 'true'\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test mixed-results2 <<-EOF\n+\t> ok 1 - passing test\n+\t> ok 2 - passing test\n+\t> ok 3 - passing test\n+\t> ok 4 - passing test\n+\t> not ok 5 - failing test\n+\t> #\tfalse\n+\t> not ok 6 - failing test\n+\t> #\tfalse\n+\t> not ok 7 - failing test\n+\t> #\tfalse\n+\t> not ok 8 - pretend we have a known breakage # TODO known breakage\n+\t> not ok 9 - pretend we have a known breakage # TODO known breakage\n+\t> ok 10 - pretend we have fixed a known breakage # TODO known breakage\n+\t> # fixed 1 known breakage(s)\n+\t> # still have 2 known breakage(s)\n+\t> # failed 3 among remaining 8 test(s)\n+\t> 1..10\n+\tEOF\n+\"\n+\n test_set_prereq HAVEIT\n haveit=no\n test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199488","messageId":"1348074915-19985-7-git-send-email-git@adamspiers.org","threadId":"31608","inReplyTo":"1348074915-19985-1-git-send-email-git@adamspiers.org","subject":"[PATCH v2 6/6] Treat unexpectedly fixed known breakages more seriously","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T17:15:15Z","receivedAt":"2012-09-19T17:15:15Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Change color of unexpectedly fixed known breakages to bold red.  An\nunexpectedly passing test indicates that the test code is somehow\nbroken or out of sync with the code it is testing.  Either way this\nis an error which is potentially as bad as a failing test, and as\nsuch is no longer portrayed as a pass in the output.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\n---\n t/t0000-basic.sh | 29 +++++++++++++++++++++++------\n t/test-lib.sh    | 13 +++++++++----\n 2 files changed, 32 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 644cc2c..459d0c7 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -148,13 +148,30 @@ test_expect_success 'pretend we have fixed a known breakage' \"\n \ttest_done\n \tEOF\n \tcheck_sub_test_lib_test passing-todo <<-EOF\n-\t> ok 1 - pretend we have fixed a known breakage # TODO known breakage\n-\t> # fixed 1 known breakage(s)\n-\t> # passed all 1 test(s)\n+\t> ok 1 - pretend we have fixed a known breakage # TODO known breakage vanished\n+\t> # 1 known breakage(s) vanished; please update test(s)\n \t> 1..1\n \tEOF\n \"\n \n+test_expect_success 'pretend we have fixed one of two known breakages (run in sub test-lib)' \"\n+\trun_sub_test_lib_test partially-passing-todos '2 TODO tests, one passing' <<-EOF &&\n+\ttest_expect_failure 'pretend we have a known breakage' 'false'\n+\ttest_expect_success 'pretend we have a passing test' 'true'\n+\ttest_expect_failure 'pretend we have fixed another known breakage' 'true'\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test partially-passing-todos <<-EOF\n+\t> not ok 1 - pretend we have a known breakage # TODO known breakage\n+\t> ok 2 - pretend we have a passing test\n+\t> ok 3 - pretend we have fixed another known breakage # TODO known breakage vanished\n+\t> # 1 known breakage(s) vanished; please update test(s)\n+\t> # still have 1 known breakage(s)\n+\t> # passed all remaining 1 test(s)\n+\t> 1..3\n+\tEOF\n+\"\n+\n test_expect_success 'pretend we have a pass, fail, and known breakage' \"\n \trun_sub_test_lib_test_expecting_failures mixed-results1 'mixed results #1' <<-EOF &&\n \ttest_expect_success 'passing test' 'true'\n@@ -200,10 +217,10 @@ test_expect_success 'pretend we have a mix of all possible results' \"\n \t> #\tfalse\n \t> not ok 8 - pretend we have a known breakage # TODO known breakage\n \t> not ok 9 - pretend we have a known breakage # TODO known breakage\n-\t> ok 10 - pretend we have fixed a known breakage # TODO known breakage\n-\t> # fixed 1 known breakage(s)\n+\t> ok 10 - pretend we have fixed a known breakage # TODO known breakage vanished\n+\t> # 1 known breakage(s) vanished; please update test(s)\n \t> # still have 2 known breakage(s)\n-\t> # failed 3 among remaining 8 test(s)\n+\t> # failed 3 among remaining 7 test(s)\n \t> 1..10\n \tEOF\n \"\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 7028ba8..b403e85 100755\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -278,7 +278,7 @@ test_failure_ () {\n \n test_known_broken_ok_ () {\n \ttest_fixed=$(($test_fixed+1))\n-\tsay_color \"\" \"ok $test_count - $@ # TODO known breakage\"\n+\tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n }\n \n test_known_broken_failure_ () {\n@@ -373,13 +373,18 @@ test_done () {\n \n \tif test \"$test_fixed\" != 0\n \tthen\n-\t\tsay_color pass \"# fixed $test_fixed known breakage(s)\"\n+\t\tsay_color error \"# $test_fixed known breakage(s) vanished; please update test(s)\"\n \tfi\n \tif test \"$test_broken\" != 0\n \tthen\n \t\tsay_color warn \"# still have $test_broken known breakage(s)\"\n-\t\tmsg=\"remaining $(($test_count-$test_broken)) test(s)\"\n+\tfi\n+\tif test \"$test_broken\" != 0 || test \"$test_fixed\" != 0\n+\tthen\n+\t\ttest_remaining=$(( $test_count - $test_broken - $test_fixed ))\n+\t\tmsg=\"remaining $test_remaining test(s)\"\n \telse\n+\t\ttest_remaining=$test_count\n \t\tmsg=\"$test_count test(s)\"\n \tfi\n \tcase \"$test_failure\" in\n@@ -393,7 +398,7 @@ test_done () {\n \n \t\tif test $test_external_has_tap -eq 0\n \t\tthen\n-\t\t\tif test $test_count -gt 0\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\tfi\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199497","messageId":"20120919175036.GB11699@sigill.intra.peff.net","threadId":"31608","inReplyTo":"1348074915-19985-3-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v2 2/6] Make 'not ok $count - $message' consistent with 'ok $count - $message'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-19T17:50:36Z","receivedAt":"2012-09-19T17:50:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 19, 2012 at 06:15:11PM +0100, Adam Spiers wrote:\n\n>  test_failure_ () {\n>  \ttest_failure=$(($test_failure + 1))\n> -\tsay_color error \"not ok - $test_count $1\"\n> +\tsay_color error \"not ok $test_count - $1\"\n\nInteresting. I wondered what TAP had to say about this, and in fact we\nwere doing it wrong before. However, since the test numbers are optional\nin TAP, a harness is supposed to keep its own counter when we fail to\nprovide a number.\n\nSo it happened to work, but this change in fact makes us more\nTAP-compliant. Good.\n\n-Peff\n"},{"id":"199499","messageId":"20120919175655.GC11699@sigill.intra.peff.net","threadId":"31608","inReplyTo":"1348074915-19985-5-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v2 4/6] Refactor mechanics of testing in a sub test-lib","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-19T17:56:55Z","receivedAt":"2012-09-19T17:56:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 19, 2012 at 06:15:13PM +0100, Adam Spiers wrote:\n\n> This will allow us to test the test framework more thoroughly\n> without disrupting the top-level test metrics.\n\nI see this is prep for the next patch, and the parts pulling out the\ntest-runs into functions make sense. But this hunk confuses me:\n\n> @@ -166,7 +176,7 @@ test_expect_success 'tests clean up even on failures' \"\n>  \ttest_must_fail ./failing-cleanup.sh >out 2>err &&\n>  \t! test -s err &&\n>  \t! test -f \\\"trash directory.failing-cleanup/clean-after-failure\\\" &&\n> -\tsed -e 's/Z$//' -e 's/^> //' >expect <<-\\\\EOF &&\n> +\tsed -e 's/Z$//' -e 's/^> //' >expect <<-EOF &&\n>  \t> not ok 1 - tests clean up even after a failure\n>  \t> #\tZ\n>  \t> #\ttouch clean-after-failure &&\n\nIs it just that you are dropping the '\\' in all of the here-docs because\nthey are not needed? I think our usual style is not to interpolate, and\nto do so only when we explicitly want it, which can prevent accidental\nerrors due to missing quoting.\n\nAlso, why is this one not converted into a check_sub... invocation?\n\n-Peff\n"},{"id":"199502","messageId":"20120919180500.GD11699@sigill.intra.peff.net","threadId":"31608","inReplyTo":"1348074915-19985-1-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v2 0/6] make test output coloring more intuitive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-19T18:05:00Z","receivedAt":"2012-09-19T18:05:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 19, 2012 at 06:15:09PM +0100, Adam Spiers wrote:\n\n> This series of commits attempts to make test output coloring\n> more intuitive, so that:\n> \n>   - red is _only_ used for things which have gone unexpectedly wrong:\n>     test failures, unexpected test passes, and failures with the\n>     framework,\n> \n>   - yellow is _only_ used for known breakages and skipped tests, and\n> \n>   - green is _only_ used for things which have gone to plan and\n>     require no further work to be done.\n\nThanks, I like this much better than the original (and it's much easier\nto review broken apart like this).\n\nI raised a few minor questions in the refactoring patch, but other than\nthat (and assuming your answers are what I expect, I do not care enough\nabout them to block the series), it looks very good.\n\nThe new \"a passing expect_failure is a breakage\" is a good thing. When\nit's unexpected, it will help call attention to it and let us figure out\nearly what changed. And when it is expected (because you are fixing the\nbreakage), it is an easy way to remind you to update the tests. :)\n\nThanks for working on it.\n\n-Peff\n"},{"id":"199509","messageId":"20120919184406.GC19246@atlantic.linksys.moosehall","threadId":"31608","inReplyTo":"20120919175655.GC11699@sigill.intra.peff.net","subject":"Re: [PATCH v2 4/6] Refactor mechanics of testing in a sub test-lib","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T18:44:06Z","receivedAt":"2012-09-19T18:44:06Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Wed, Sep 19, 2012 at 01:56:55PM -0400, Jeff King wrote:\n> On Wed, Sep 19, 2012 at 06:15:13PM +0100, Adam Spiers wrote:\n> \n> > This will allow us to test the test framework more thoroughly\n> > without disrupting the top-level test metrics.\n> \n> I see this is prep for the next patch, and the parts pulling out the\n> test-runs into functions make sense. But this hunk confuses me:\n> \n> > @@ -166,7 +176,7 @@ test_expect_success 'tests clean up even on failures' \"\n> >  \ttest_must_fail ./failing-cleanup.sh >out 2>err &&\n> >  \t! test -s err &&\n> >  \t! test -f \\\"trash directory.failing-cleanup/clean-after-failure\\\" &&\n> > -\tsed -e 's/Z$//' -e 's/^> //' >expect <<-\\\\EOF &&\n> > +\tsed -e 's/Z$//' -e 's/^> //' >expect <<-EOF &&\n> >  \t> not ok 1 - tests clean up even after a failure\n> >  \t> #\tZ\n> >  \t> #\ttouch clean-after-failure &&\n> \n> Is it just that you are dropping the '\\' in all of the here-docs because\n> they are not needed?\n\nHmm, I think I previously misunderstood the point of the \\\\ due to\nnever seeing that syntax before (since my Perl background taught me to\nwrite <<'EOF' instead).  I noticed that the tests all passed without\nit, and mistakenly assumed it had become unnecessary due to the\nrefactoring.\n\n> I think our usual style is not to interpolate, and\n> to do so only when we explicitly want it, which can prevent accidental\n> errors due to missing quoting.\n\nRight, that makes sense.  I'd vote to put it back in then.\n\n> Also, why is this one not converted into a check_sub... invocation?\n\nBecause it was much further down in that file so I didn't notice it\nduring the refactoring ;-)  I've also noticed I can use test_must_fail\ninstead of introducing run_sub_test_lib_test_expecting_failures.\n\nSo I'll have to re-roll 4--6 again.  Presumably I can just reply to\n[PATCH v2 4/6] with modified v3 versions without having to resend\nthe first three in the series, which haven't changed.\n"},{"id":"199511","messageId":"1348080591-16430-1-git-send-email-git@adamspiers.org","threadId":"31608","inReplyTo":"20120919184406.GC19246@atlantic.linksys.moosehall","subject":"[PATCH v3 4/6] Refactor mechanics of testing in a sub test-lib","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T18:49:49Z","receivedAt":"2012-09-19T18:49:49Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"This will allow us to test the test framework more thoroughly\nwithout disrupting the top-level test metrics.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\n---\n t/t0000-basic.sh | 67 ++++++++++++++++++++++++--------------------------------\n 1 file changed, 29 insertions(+), 38 deletions(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex c6b42de..029e3bd 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -55,39 +55,49 @@ test_expect_failure 'pretend we have a known breakage' '\n \tfalse\n '\n \n-test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib)' \"\n-\tmkdir passing-todo &&\n-\t(cd passing-todo &&\n-\tcat >passing-todo.sh <<-EOF &&\n+run_sub_test_lib_test () {\n+\tname=\"$1\" descr=\"$2\" # stdin is body of test code\n+\tmkdir $name &&\n+\t(cd $name &&\n+\tcat >$name.sh <<-EOF &&\n \t#!$SHELL_PATH\n \n-\ttest_description='A passing TODO test\n+\ttest_description='$descr (run in sub test-lib)\n \n \tThis is run in a sub test-lib so that we do not get incorrect\n \tpassing metrics\n \t'\n \n \t# Point to the t/test-lib.sh, which isn't in ../ as usual\n-\tTEST_DIRECTORY=\\\"$TEST_DIRECTORY\\\"\n-\t. \\\"\\$TEST_DIRECTORY\\\"/test-lib.sh\n-\n-\ttest_expect_failure 'pretend we have fixed a known breakage' '\n-\t\t:\n-\t'\n+\tTEST_DIRECTORY=\"$TEST_DIRECTORY\"\n+\t. \"\\$TEST_DIRECTORY\"/test-lib.sh\n+\tEOF\n+\tcat >>$name.sh &&\n+\tchmod +x $name.sh &&\n+\t./$name.sh >out 2>err)\n+}\n+\n+check_sub_test_lib_test () {\n+\tname=\"$1\" # stdin is test's expected stdout\n+\t(cd $name &&\n+\t! test -s err &&\n+\tsed -e 's/^> //' -e 's/Z$//' >expect &&\n+\ttest_cmp expect out)\n+}\n \n+test_expect_success 'pretend we have fixed a known breakage' \"\n+\trun_sub_test_lib_test passing-todo 'A passing TODO test' <<-\\\\EOF &&\n+\ttest_expect_failure 'pretend we have fixed a known breakage' 'true'\n \ttest_done\n \tEOF\n-\tchmod +x passing-todo.sh &&\n-\t./passing-todo.sh >out 2>err &&\n-\t! test -s err &&\n-\tsed -e 's/^> //' >expect <<-\\\\EOF &&\n+\tcheck_sub_test_lib_test passing-todo <<-\\\\EOF\n \t> ok 1 - pretend we have fixed a known breakage # TODO known breakage\n \t> # fixed 1 known breakage(s)\n \t> # passed all 1 test(s)\n \t> 1..1\n \tEOF\n-\ttest_cmp expect out)\n \"\n+\n test_set_prereq HAVEIT\n haveit=no\n test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '\n@@ -137,19 +147,8 @@ then\n fi\n \n test_expect_success 'tests clean up even on failures' \"\n-\tmkdir failing-cleanup &&\n-\t(\n-\tcd failing-cleanup &&\n-\n-\tcat >failing-cleanup.sh <<-EOF &&\n-\t#!$SHELL_PATH\n-\n-\ttest_description='Failing tests with cleanup commands'\n-\n-\t# Point to the t/test-lib.sh, which isn't in ../ as usual\n-\tTEST_DIRECTORY=\\\"$TEST_DIRECTORY\\\"\n-\t. \\\"\\$TEST_DIRECTORY\\\"/test-lib.sh\n-\n+\ttest_must_fail run_sub_test_lib_test \\\n+\t\tfailing-cleanup 'Failing tests with cleanup commands' <<-\\\\EOF &&\n \ttest_expect_success 'tests clean up even after a failure' '\n \t\ttouch clean-after-failure &&\n \t\ttest_when_finished rm clean-after-failure &&\n@@ -159,14 +158,8 @@ test_expect_success 'tests clean up even on failures' \"\n \t\ttest_when_finished \\\"(exit 2)\\\"\n \t'\n \ttest_done\n-\n \tEOF\n-\n-\tchmod +x failing-cleanup.sh &&\n-\ttest_must_fail ./failing-cleanup.sh >out 2>err &&\n-\t! test -s err &&\n-\t! test -f \\\"trash directory.failing-cleanup/clean-after-failure\\\" &&\n-\tsed -e 's/Z$//' -e 's/^> //' >expect <<-\\\\EOF &&\n+\tcheck_sub_test_lib_test failing-cleanup <<-\\\\EOF\n \t> not ok 1 - tests clean up even after a failure\n \t> #\tZ\n \t> #\ttouch clean-after-failure &&\n@@ -180,8 +173,6 @@ test_expect_success 'tests clean up even on failures' \"\n \t> # failed 2 among 2 test(s)\n \t> 1..2\n \tEOF\n-\ttest_cmp expect out\n-\t)\n \"\n \n ################################################################\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199510","messageId":"1348080591-16430-2-git-send-email-git@adamspiers.org","threadId":"31608","inReplyTo":"1348080591-16430-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 5/6] Test the test framework more thoroughly","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T18:49:50Z","receivedAt":"2012-09-19T18:49:50Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Add 5 new full test suite runs each with a different number of\npassing/failing/broken/fixed tests, in order to ensure that the\ncorrect exit code and output are generated in each case.  As before,\nthese are run in a subdirectory in order to disrupt the metrics for\nthe parent tests.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\n---\n t/t0000-basic.sh | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 104 insertions(+)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 029e3bd..65f578f 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -85,6 +85,55 @@ check_sub_test_lib_test () {\n \ttest_cmp expect out)\n }\n \n+test_expect_success 'pretend we have a fully passing test suite' \"\n+\trun_sub_test_lib_test full-pass '3 passing tests' <<-\\\\EOF &&\n+\tfor i in 1 2 3; do\n+\t\ttest_expect_success \\\"passing test #\\$i\\\" 'true'\n+\tdone\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test full-pass <<-\\\\EOF\n+\t> ok 1 - passing test #1\n+\t> ok 2 - passing test #2\n+\t> ok 3 - passing test #3\n+\t> # passed all 3 test(s)\n+\t> 1..3\n+\tEOF\n+\"\n+\n+test_expect_success 'pretend we have a partially passing test suite' \"\n+\ttest_must_fail run_sub_test_lib_test \\\n+\t\tpartial-pass '2/3 tests passing' <<-\\\\EOF &&\n+\ttest_expect_success 'passing test #1' 'true'\n+\ttest_expect_success 'failing test #2' 'false'\n+\ttest_expect_success 'passing test #3' 'true'\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test partial-pass <<-\\\\EOF\n+\t> ok 1 - passing test #1\n+\t> not ok 2 - failing test #2\n+\t#\tfalse\n+\t> ok 3 - passing test #3\n+\t> # failed 1 among 3 test(s)\n+\t> 1..3\n+\tEOF\n+\"\n+\n+test_expect_success 'pretend we have a known breakage' \"\n+\trun_sub_test_lib_test failing-todo 'A failing TODO test' <<-\\\\EOF &&\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_failure 'pretend we have a known breakage' 'false'\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test failing-todo <<-\\\\EOF\n+\t> ok 1 - passing test\n+\t> not ok 2 - pretend we have a known breakage # TODO known breakage\n+\t> # still have 1 known breakage(s)\n+\t> # passed all remaining 1 test(s)\n+\t> 1..2\n+\tEOF\n+\"\n+\n test_expect_success 'pretend we have fixed a known breakage' \"\n \trun_sub_test_lib_test passing-todo 'A passing TODO test' <<-\\\\EOF &&\n \ttest_expect_failure 'pretend we have fixed a known breakage' 'true'\n@@ -98,6 +147,61 @@ test_expect_success 'pretend we have fixed a known breakage' \"\n \tEOF\n \"\n \n+test_expect_success 'pretend we have a pass, fail, and known breakage' \"\n+\ttest_must_fail run_sub_test_lib_test \\\n+\t\tmixed-results1 'mixed results #1' <<-\\\\EOF &&\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_success 'failing test' 'false'\n+\ttest_expect_failure 'pretend we have a known breakage' 'false'\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test mixed-results1 <<-\\\\EOF\n+\t> ok 1 - passing test\n+\t> not ok 2 - failing test\n+\t> #\tfalse\n+\t> not ok 3 - pretend we have a known breakage # TODO known breakage\n+\t> # still have 1 known breakage(s)\n+\t> # failed 1 among remaining 2 test(s)\n+\t> 1..3\n+\tEOF\n+\"\n+\n+test_expect_success 'pretend we have a mix of all possible results' \"\n+\ttest_must_fail run_sub_test_lib_test \\\n+\t\tmixed-results2 'mixed results #2' <<-\\\\EOF &&\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_success 'passing test' 'true'\n+\ttest_expect_success 'failing test' 'false'\n+\ttest_expect_success 'failing test' 'false'\n+\ttest_expect_success 'failing test' 'false'\n+\ttest_expect_failure 'pretend we have a known breakage' 'false'\n+\ttest_expect_failure 'pretend we have a known breakage' 'false'\n+\ttest_expect_failure 'pretend we have fixed a known breakage' 'true'\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test mixed-results2 <<-\\\\EOF\n+\t> ok 1 - passing test\n+\t> ok 2 - passing test\n+\t> ok 3 - passing test\n+\t> ok 4 - passing test\n+\t> not ok 5 - failing test\n+\t> #\tfalse\n+\t> not ok 6 - failing test\n+\t> #\tfalse\n+\t> not ok 7 - failing test\n+\t> #\tfalse\n+\t> not ok 8 - pretend we have a known breakage # TODO known breakage\n+\t> not ok 9 - pretend we have a known breakage # TODO known breakage\n+\t> ok 10 - pretend we have fixed a known breakage # TODO known breakage\n+\t> # fixed 1 known breakage(s)\n+\t> # still have 2 known breakage(s)\n+\t> # failed 3 among remaining 8 test(s)\n+\t> 1..10\n+\tEOF\n+\"\n+\n test_set_prereq HAVEIT\n haveit=no\n test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199512","messageId":"1348080591-16430-3-git-send-email-git@adamspiers.org","threadId":"31608","inReplyTo":"1348080591-16430-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 6/6] Treat unexpectedly fixed known breakages more seriously","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T18:49:51Z","receivedAt":"2012-09-19T18:49:51Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Change color of unexpectedly fixed known breakages to bold red.  An\nunexpectedly passing test indicates that the test code is somehow\nbroken or out of sync with the code it is testing.  Either way this\nis an error which is potentially as bad as a failing test, and as\nsuch is no longer portrayed as a pass in the output.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\n---\n t/t0000-basic.sh | 30 ++++++++++++++++++++++++------\n t/test-lib.sh    | 13 +++++++++----\n 2 files changed, 33 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 65f578f..ed44f7d 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -140,13 +140,31 @@ test_expect_success 'pretend we have fixed a known breakage' \"\n \ttest_done\n \tEOF\n \tcheck_sub_test_lib_test passing-todo <<-\\\\EOF\n-\t> ok 1 - pretend we have fixed a known breakage # TODO known breakage\n-\t> # fixed 1 known breakage(s)\n-\t> # passed all 1 test(s)\n+\t> ok 1 - pretend we have fixed a known breakage # TODO known breakage vanished\n+\t> # 1 known breakage(s) vanished; please update test(s)\n \t> 1..1\n \tEOF\n \"\n \n+test_expect_success 'pretend we have fixed one of two known breakages (run in sub test-lib)' \"\n+\trun_sub_test_lib_test partially-passing-todos \\\n+\t\t'2 TODO tests, one passing' <<-\\\\EOF &&\n+\ttest_expect_failure 'pretend we have a known breakage' 'false'\n+\ttest_expect_success 'pretend we have a passing test' 'true'\n+\ttest_expect_failure 'pretend we have fixed another known breakage' 'true'\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test partially-passing-todos <<-\\\\EOF\n+\t> not ok 1 - pretend we have a known breakage # TODO known breakage\n+\t> ok 2 - pretend we have a passing test\n+\t> ok 3 - pretend we have fixed another known breakage # TODO known breakage vanished\n+\t> # 1 known breakage(s) vanished; please update test(s)\n+\t> # still have 1 known breakage(s)\n+\t> # passed all remaining 1 test(s)\n+\t> 1..3\n+\tEOF\n+\"\n+\n test_expect_success 'pretend we have a pass, fail, and known breakage' \"\n \ttest_must_fail run_sub_test_lib_test \\\n \t\tmixed-results1 'mixed results #1' <<-\\\\EOF &&\n@@ -194,10 +212,10 @@ test_expect_success 'pretend we have a mix of all possible results' \"\n \t> #\tfalse\n \t> not ok 8 - pretend we have a known breakage # TODO known breakage\n \t> not ok 9 - pretend we have a known breakage # TODO known breakage\n-\t> ok 10 - pretend we have fixed a known breakage # TODO known breakage\n-\t> # fixed 1 known breakage(s)\n+\t> ok 10 - pretend we have fixed a known breakage # TODO known breakage vanished\n+\t> # 1 known breakage(s) vanished; please update test(s)\n \t> # still have 2 known breakage(s)\n-\t> # failed 3 among remaining 8 test(s)\n+\t> # failed 3 among remaining 7 test(s)\n \t> 1..10\n \tEOF\n \"\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 7028ba8..b403e85 100755\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -278,7 +278,7 @@ test_failure_ () {\n \n test_known_broken_ok_ () {\n \ttest_fixed=$(($test_fixed+1))\n-\tsay_color \"\" \"ok $test_count - $@ # TODO known breakage\"\n+\tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n }\n \n test_known_broken_failure_ () {\n@@ -373,13 +373,18 @@ test_done () {\n \n \tif test \"$test_fixed\" != 0\n \tthen\n-\t\tsay_color pass \"# fixed $test_fixed known breakage(s)\"\n+\t\tsay_color error \"# $test_fixed known breakage(s) vanished; please update test(s)\"\n \tfi\n \tif test \"$test_broken\" != 0\n \tthen\n \t\tsay_color warn \"# still have $test_broken known breakage(s)\"\n-\t\tmsg=\"remaining $(($test_count-$test_broken)) test(s)\"\n+\tfi\n+\tif test \"$test_broken\" != 0 || test \"$test_fixed\" != 0\n+\tthen\n+\t\ttest_remaining=$(( $test_count - $test_broken - $test_fixed ))\n+\t\tmsg=\"remaining $test_remaining test(s)\"\n \telse\n+\t\ttest_remaining=$test_count\n \t\tmsg=\"$test_count test(s)\"\n \tfi\n \tcase \"$test_failure\" in\n@@ -393,7 +398,7 @@ test_done () {\n \n \t\tif test $test_external_has_tap -eq 0\n \t\tthen\n-\t\t\tif test $test_count -gt 0\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\tfi\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199519","messageId":"20120919193708.GA21950@sigill.intra.peff.net","threadId":"31608","inReplyTo":"20120919184406.GC19246@atlantic.linksys.moosehall","subject":"Re: [PATCH v2 4/6] Refactor mechanics of testing in a sub test-lib","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-19T19:37:08Z","receivedAt":"2012-09-19T19:37:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 19, 2012 at 07:44:06PM +0100, Adam Spiers wrote:\n\n> > Is it just that you are dropping the '\\' in all of the here-docs because\n> > they are not needed?\n> \n> Hmm, I think I previously misunderstood the point of the \\\\ due to\n> never seeing that syntax before (since my Perl background taught me to\n> write <<'EOF' instead).  I noticed that the tests all passed without\n> it, and mistakenly assumed it had become unnecessary due to the\n> refactoring.\n\nOK. You can write 'EOF' in the shell, too, but we tend not to in this\nproject (and you can write \\EOF in perl, but I agree that it is much\nless common in perl code I have seen).\n\nLooking at it again, it is actually quite subtle what is going on. We\nwrap the outer test_expect_* calls in double-quotes so that the inner\nones can use single-quotes easily. But that means that technically the\ncontents of the here-doc _are_ interpolated. But not at test run-time,\nbut rather at the call to test_expect_*. And that is why we nee to use\n\"\\\\\" instead of \"\\\". So I think anybody trying to tweak these tests\nusing shell metacharacters is in for a surprise either way. I'm not sure\nit is worth worrying about, though, as handling it would probably make\nthe existing tests less readable.\n\n> > Also, why is this one not converted into a check_sub... invocation?\n> \n> Because it was much further down in that file so I didn't notice it\n> during the refactoring ;-)\n\nOK. :)\n\n> I've also noticed I can use test_must_fail instead of introducing\n> run_sub_test_lib_test_expecting_failures.\n\nGood catch. I didn't notice that, but it definitely makes sense to reuse\nit.\n\n> \n> So I'll have to re-roll 4--6 again.  Presumably I can just reply to\n> [PATCH v2 4/6] with modified v3 versions without having to resend\n> the first three in the series, which haven't changed.\n\nIt all looks sane to me. Thanks again.\n\n-Peff\n"},{"id":"199529","messageId":"20120919201558.GE19246@atlantic.linksys.moosehall","threadId":"31608","inReplyTo":"20120919193708.GA21950@sigill.intra.peff.net","subject":"Re: [PATCH v2 4/6] Refactor mechanics of testing in a sub test-lib","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T20:15:58Z","receivedAt":"2012-09-19T20:15:58Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Wed, Sep 19, 2012 at 03:37:08PM -0400, Jeff King wrote:\n> Looking at it again, it is actually quite subtle what is going on. We\n> wrap the outer test_expect_* calls in double-quotes so that the inner\n> ones can use single-quotes easily. But that means that technically the\n> contents of the here-doc _are_ interpolated. But not at test run-time,\n> but rather at the call to test_expect_*. And that is why we nee to use\n> \"\\\\\" instead of \"\\\". So I think anybody trying to tweak these tests\n> using shell metacharacters is in for a surprise either way.\n\nActually I already did that in one place:\n\n    test_expect_success 'pretend we have a fully passing test suite' \"\n            run_sub_test_lib_test full-pass '3 passing tests' <<-\\\\EOF &&\n            for i in 1 2 3; do\n                    test_expect_success \\\"passing test #\\$i\\\" 'true'\n            done\n            test_done\n            EOF\n    [...]\n    \"\n\nWithout the \\\\ preceeding the EOF, it needed to be:\n\n                    test_expect_success \\\"passing test #\\\\\\$i\\\" 'true'\n\n> I'm not sure it is worth worrying about, though, as handling it\n> would probably make the existing tests less readable.\n\nYeah, the for loop is perhaps slightly overkill :-)\n\n> It all looks sane to me. Thanks again.\n\nThanks for your (unnervingly) thorough reviews ;-)\n"},{"id":"199548","messageId":"7vmx0leg62.fsf@alter.siamese.dyndns.org","threadId":"31608","inReplyTo":"1348074915-19985-3-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v2 2/6] Make 'not ok $count - $message' consistent with 'ok $count - $message'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-19T23:39:17Z","receivedAt":"2012-09-19T23:39:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers <git@adamspiers.org> writes:\n\n> Signed-off-by: Adam Spiers <git@adamspiers.org>\n> ---\n>  t/t0000-basic.sh | 4 ++--\n>  t/test-lib.sh    | 2 +-\n>  2 files changed, 3 insertions(+), 3 deletions(-)\n\n>  mode change 100644 => 100755 t/test-lib.sh\n\nPeff might have already pointed out, but this is wrong.  Will queue\nwith an obvious tweak.\n\nThanks.\n"},{"id":"199551","messageId":"CAOkDyE9oYzxoKGC8q4=whGo7R0krKtRjFoGqLMQu__nJOPd4Cg@mail.gmail.com","threadId":"31608","inReplyTo":"7vmx0leg62.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 2/6] Make 'not ok $count - $message' consistent with 'ok $count - $message'","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T23:45:26Z","receivedAt":"2012-09-19T23:45:26Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Thu, Sep 20, 2012 at 12:39 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Adam Spiers <git@adamspiers.org> writes:\n>\n>> Signed-off-by: Adam Spiers <git@adamspiers.org>\n>> ---\n>>  t/t0000-basic.sh | 4 ++--\n>>  t/test-lib.sh    | 2 +-\n>>  2 files changed, 3 insertions(+), 3 deletions(-)\n>\n>>  mode change 100644 => 100755 t/test-lib.sh\n>\n> Peff might have already pointed out, but this is wrong.  Will queue\n> with an obvious tweak.\n\nOops, good catch.  That's a bug in my emacs config, I suspect.\nThanks for the tweak.  You guys have eagle eyes!\n"},{"id":"199627","messageId":"7vhaqs9z47.fsf@alter.siamese.dyndns.org","threadId":"31608","inReplyTo":"1348080591-16430-1-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v3 4/6] Refactor mechanics of testing in a sub test-lib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-20T21:13:28Z","receivedAt":"2012-09-20T21:13:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers <git@adamspiers.org> writes:\n\n> This will allow us to test the test framework more thoroughly\n> without disrupting the top-level test metrics.\n>\n> Signed-off-by: Adam Spiers <git@adamspiers.org>\n> ---\n>  t/t0000-basic.sh | 67 ++++++++++++++++++++++++--------------------------------\n>  1 file changed, 29 insertions(+), 38 deletions(-)\n>\n> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n> index c6b42de..029e3bd 100755\n> --- a/t/t0000-basic.sh\n> +++ b/t/t0000-basic.sh\n> @@ -55,39 +55,49 @@ test_expect_failure 'pretend we have a known breakage' '\n>  \tfalse\n>  '\n>  \n> -test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib)' \"\n> -\tmkdir passing-todo &&\n> -\t(cd passing-todo &&\n> -\tcat >passing-todo.sh <<-EOF &&\n> +run_sub_test_lib_test () {\n> +\tname=\"$1\" descr=\"$2\" # stdin is body of test code\n> +\tmkdir $name &&\n> +\t(cd $name &&\n> +\tcat >$name.sh <<-EOF &&\n>  \t#!$SHELL_PATH\n>  \n> -\ttest_description='A passing TODO test\n> +\ttest_description='$descr (run in sub test-lib)\n>  \n>  \tThis is run in a sub test-lib so that we do not get incorrect\n>  \tpassing metrics\n>  \t'\n>  \n>  \t# Point to the t/test-lib.sh, which isn't in ../ as usual\n> -\tTEST_DIRECTORY=\\\"$TEST_DIRECTORY\\\"\n> -\t. \\\"\\$TEST_DIRECTORY\\\"/test-lib.sh\n> -\n> -\ttest_expect_failure 'pretend we have fixed a known breakage' '\n> -\t\t:\n> -\t'\n> +\tTEST_DIRECTORY=\"$TEST_DIRECTORY\"\n> +\t. \"\\$TEST_DIRECTORY\"/test-lib.sh\n> +\tEOF\n\nThe quoting of $TEST_DIRECTORY in the assignment does not look\ncorrect (imagine a path with a double quote in it).\n\nRemoving the assignment and instead exporting TEST_DIRECTORY before\ncalling name.sh may be a reasonable fix, than trying to quotemeta\nthe value of $TEST_DIRECTORY here.\n\nI'll re-queue this series in 'pu' with fixes and retitles; please\neyeball them before submitting a reroll.\n\n  b465316 tests: paint unexpectedly fixed known breakages in bold red\n  7214717 tests: test the test framework more thoroughly\n  03c772a [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up\n  99fe0af tests: refactor mechanics of testing in a sub test-lib\n  6af90bf tests: paint skipped tests in bold blue\n  0b87581 tests: test number comes first in 'not ok $count - $message'\n  1c55079 tests: paint known breakages in bold yellow\n\nThe third one from the tip looks like the following, to re-indent to\nmake it readable and then minimally fix the quoting.\n\nThanks.\n\n t/t0000-basic.sh | 50 +++++++++++++++++++++++++++-----------------------\n 1 file changed, 27 insertions(+), 23 deletions(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex ee78e68..c3345a9 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -56,33 +56,37 @@ test_expect_failure 'pretend we have a known breakage' '\n '\n \n run_sub_test_lib_test () {\n-\tname=\"$1\" descr=\"$2\" # stdin is body of test code\n+\tname=\"$1\" descr=\"$2\" # stdin is the body of the test code\n \tmkdir $name &&\n-\t(cd $name &&\n-\tcat >$name.sh <<-EOF &&\n-\t#!$SHELL_PATH\n-\n-\ttest_description='$descr (run in sub test-lib)\n-\n-\tThis is run in a sub test-lib so that we do not get incorrect\n-\tpassing metrics\n-\t'\n-\n-\t# Point to the t/test-lib.sh, which isn't in ../ as usual\n-\tTEST_DIRECTORY=\"$TEST_DIRECTORY\"\n-\t. \"\\$TEST_DIRECTORY\"/test-lib.sh\n-\tEOF\n-\tcat >>$name.sh &&\n-\tchmod +x $name.sh &&\n-\t./$name.sh >out 2>err)\n+\t(\n+\t\tcd $name &&\n+\t\tcat >$name.sh <<-EOF &&\n+\t\t#!$SHELL_PATH\n+\n+\t\ttest_description='$descr (run in sub test-lib)\n+\n+\t\tThis is run in a sub test-lib so that we do not get incorrect\n+\t\tpassing metrics\n+\t\t'\n+\n+\t\t# Point to the t/test-lib.sh, which isn't in ../ as usual\n+\t\t. \"\\$TEST_DIRECTORY\"/test-lib.sh\n+\t\tEOF\n+\t\tcat >>$name.sh &&\n+\t\tchmod +x $name.sh &&\n+\t\texport TEST_DIRECTORY &&\n+\t\t./$name.sh >out 2>err\n+\t)\n }\n \n check_sub_test_lib_test () {\n-\tname=\"$1\" # stdin is expected output from the test\n-\t(cd $name &&\n-\t! test -s err &&\n-\tsed -e 's/^> //' -e 's/Z$//' >expect &&\n-\ttest_cmp expect out)\n+\tname=\"$1\" # stdin is the expected output from the test\n+\t(\n+\t\tcd $name &&\n+\t\t! test -s err &&\n+\t\tsed -e 's/^> //' -e 's/Z$//' >expect &&\n+\t\ttest_cmp expect out\n+\t)\n }\n \n test_expect_success 'pretend we have fixed a known breakage' \"\n-- \n1.7.12.1.389.g3dff30b\n"}]}