{"thread":{"id":"32424","subject":"[PATCH v7 1/7] tests: test number comes first in 'not ok $count - $message'","startedAt":"2012-12-21T03:12:31Z","lastAt":"2012-12-21T16:59:52Z","messageCount":13,"participants":["Junio C Hamano","Jeff King","Stefano Lattarini","Adam Spiers"],"isPatch":true,"patchVersion":7,"patchTotal":7},"messages":[{"id":"205309","messageId":"1356059558-23479-1-git-send-email-gitster@pobox.com","threadId":"32424","inReplyTo":"CAOkDyE9tDYRYzojzNnjWsT7UygxMAurHqLSDGA66_LMPD2Wmnw@mail.gmail.com","subject":"[PATCH v7 0/7] coloring test output after traffic signal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-21T03:12:31Z","receivedAt":"2012-12-21T03:12:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"To conclude the bikeshedding discussion we had today, here is what I\nqueued by squashing stuff into relevant patches, so that people can\neyeball the result for the last time.\n\nAdam Spiers (7):\n  tests: test number comes first in 'not ok $count - $message'\n  tests: paint known breakages in yellow\n  tests: paint skipped tests in blue\n  tests: change info messages from yellow/brown to cyan\n  tests: refactor mechanics of testing in a sub test-lib\n  tests: test the test framework more thoroughly\n  tests: paint unexpectedly fixed known breakages in bold red\n\n t/t0000-basic.sh | 214 ++++++++++++++++++++++++++++++++++++++++++-------------\n t/test-lib.sh    |  29 +++++---\n 2 files changed, 184 insertions(+), 59 deletions(-)\n\n-- \n1.8.1.rc2.225.g8d36ab4\n"},{"id":"205308","messageId":"1356059558-23479-2-git-send-email-gitster@pobox.com","threadId":"32424","inReplyTo":"1356059558-23479-1-git-send-email-gitster@pobox.com","subject":"[PATCH v7 1/7] tests: test number comes first in 'not ok $count - $message'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-21T03:12:32Z","receivedAt":"2012-12-21T03:12:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Adam Spiers <git@adamspiers.org>\n\nThe old output to say \"not ok - 1 messsage\" was working by accident\nonly because the test numbers are optional in TAP.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t0000-basic.sh | 4 ++--\n t/test-lib.sh    | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\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\nindex f8e3733..03b86b8 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -268,7 +268,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.8.1.rc2.225.g8d36ab4\n"},{"id":"205310","messageId":"1356059558-23479-3-git-send-email-gitster@pobox.com","threadId":"32424","inReplyTo":"1356059558-23479-1-git-send-email-gitster@pobox.com","subject":"[PATCH v7 2/7] tests: paint known breakages in yellow","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-21T03:12:33Z","receivedAt":"2012-12-21T03:12:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Adam Spiers <git@adamspiers.org>\n\nYellow seems a more appropriate color than bold green when\nconsidering the universal traffic lights coloring scheme, where\ngreen conveys 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 the same yellow to be less alarmist and\nmore consistent with the above.\n\nAn earlier version of this patch used bold yellow but because these\nare all long-known failures, reminding them to developers in bold\nover and over does not help encouraging them to take a look at them\nvery much.  This iteration paints them in plain yellow instead to\nmake them less distracting.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\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 03b86b8..72aafd0 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 setaf 3;; # brown/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.8.1.rc2.225.g8d36ab4\n"},{"id":"205312","messageId":"1356059558-23479-4-git-send-email-gitster@pobox.com","threadId":"32424","inReplyTo":"1356059558-23479-1-git-send-email-gitster@pobox.com","subject":"[PATCH v7 3/7] tests: paint skipped tests in blue","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-21T03:12:34Z","receivedAt":"2012-12-21T03:12:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Adam Spiers <git@adamspiers.org>\n\nSkipped tests indicate incomplete test coverage.  Whilst this is not a\ntest failure or other error, it's still not a complete success.\n\nOther testsuite related software like automake, autotest and prove\nseem to use blue for skipped tests, so let's follow suit.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/test-lib.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 72aafd0..f32df80 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -182,7 +182,7 @@ 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 4;; # blue\n \t\twarn)\n \t\t\ttput setaf 3;; # brown/yellow\n \t\tpass)\n-- \n1.8.1.rc2.225.g8d36ab4\n"},{"id":"205313","messageId":"1356059558-23479-5-git-send-email-gitster@pobox.com","threadId":"32424","inReplyTo":"1356059558-23479-1-git-send-email-gitster@pobox.com","subject":"[PATCH v7 4/7] tests: change info messages from yellow/brown to cyan","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-21T03:12:35Z","receivedAt":"2012-12-21T03:12:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Adam Spiers <git@adamspiers.org>\n\nNow that we've adopted a \"traffic lights\" coloring scheme, yellow is\nused for warning messages, so we need to re-color info messages to\nsomething less alarmist.  Blue is a universal color for informational\nmessages; however we are using that for skipped tests in order to\nalign with the color schemes of other test suites.  Therefore we use\ncyan which is also blue-ish, but visually distinct from blue.\n\nThis was suggested on the list a while ago and no-one raised any\nobjections:\n\n    http://thread.gmane.org/gmane.comp.version-control.git/205675/focus=205966\n\nAn earlier iteration of this patch used bold cyan, but the point of\nthis change is to make them less alarming; let's drop the boldness.\n\nAlso paint the message to report skipping the whole thing via\nGIT_SKIP_TESTS mechanism in the same color as the \"info\" color\nthat is used on the final summary line for the entire script.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/test-lib.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex f32df80..8b75c9a 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -186,9 +186,9 @@ then\n \t\twarn)\n \t\t\ttput setaf 3;; # brown/yellow\n \t\tpass)\n-\t\t\ttput setaf 2;;            # green\n+\t\t\ttput setaf 2;; # green\n \t\tinfo)\n-\t\t\ttput setaf 3;;            # brown\n+\t\t\ttput setaf 6;; # cyan\n \t\t*)\n \t\t\ttest -n \"$quiet\" && return;;\n \t\tesac\n@@ -584,7 +584,7 @@ for skp in $GIT_SKIP_TESTS\n do\n \tcase \"$this_test\" in\n \t$skp)\n-\t\tsay_color skip >&3 \"skipping test $this_test altogether\"\n+\t\tsay_color info >&3 \"skipping test $this_test altogether\"\n \t\tskip_all=\"skip all tests in $this_test\"\n \t\ttest_done\n \tesac\n-- \n1.8.1.rc2.225.g8d36ab4\n"},{"id":"205311","messageId":"1356059558-23479-6-git-send-email-gitster@pobox.com","threadId":"32424","inReplyTo":"1356059558-23479-1-git-send-email-gitster@pobox.com","subject":"[PATCH v7 5/7] tests: refactor mechanics of testing in a sub test-lib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-21T03:12:36Z","receivedAt":"2012-12-21T03:12:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Adam Spiers <git@adamspiers.org>\n\nThis 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>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t0000-basic.sh | 85 ++++++++++++++++++++++++++------------------------------\n 1 file changed, 40 insertions(+), 45 deletions(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex c6b42de..d0f46e8 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -55,39 +55,53 @@ 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-\t#!$SHELL_PATH\n-\n-\ttest_description='A passing TODO test\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+run_sub_test_lib_test () {\n+\tname=\"$1\" descr=\"$2\" # stdin is the body of the test code\n+\tmkdir \"$name\" &&\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-\ttest_expect_failure 'pretend we have fixed a known breakage' '\n-\t\t:\n-\t'\n+check_sub_test_lib_test () {\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+\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 +151,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 +162,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 +177,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.8.1.rc2.225.g8d36ab4\n"},{"id":"205315","messageId":"1356059558-23479-7-git-send-email-gitster@pobox.com","threadId":"32424","inReplyTo":"1356059558-23479-1-git-send-email-gitster@pobox.com","subject":"[PATCH v7 6/7] tests: test the test framework more thoroughly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-21T03:12:37Z","receivedAt":"2012-12-21T03:12:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Adam Spiers <git@adamspiers.org>\n\nAdd 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 to avoid disrupting the metrics for\nthe parent tests.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t0000-basic.sh | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 105 insertions(+)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex d0f46e8..384b0ae 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -89,6 +89,56 @@ check_sub_test_lib_test () {\n \t)\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\n+\tdo\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@@ -102,6 +152,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.8.1.rc2.225.g8d36ab4\n"},{"id":"205314","messageId":"1356059558-23479-8-git-send-email-gitster@pobox.com","threadId":"32424","inReplyTo":"1356059558-23479-1-git-send-email-gitster@pobox.com","subject":"[PATCH v7 7/7] tests: paint unexpectedly fixed known breakages in bold red","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-21T03:12:38Z","receivedAt":"2012-12-21T03:12:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Adam Spiers <git@adamspiers.org>\n\nChange 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 is\nan error which is potentially as bad as a failing test, and as such is\nno longer portrayed as a pass in the output.\n\nSigned-off-by: Adam Spiers <git@adamspiers.org>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\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 384b0ae..8973d2c 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -145,13 +145,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@@ -199,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 8b75c9a..30a0937 100644\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.8.1.rc2.225.g8d36ab4\n"},{"id":"205321","messageId":"20121221081521.GA22193@sigill.intra.peff.net","threadId":"32424","inReplyTo":"1356059558-23479-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v7 0/7] coloring test output after traffic signal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-21T08:15:21Z","receivedAt":"2012-12-21T08:15:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 20, 2012 at 07:12:31PM -0800, Junio C Hamano wrote:\n\n> To conclude the bikeshedding discussion we had today, here is what I\n> queued by squashing stuff into relevant patches, so that people can\n> eyeball the result for the last time.\n\nThanks, this looks OK to me.\n\nAnd thank you, Adam, for your patience. Seven iterations of color\nbikeshedding is more than should be asked of anyone. :)\n\n-Peff\n"},{"id":"205324","messageId":"50D4230F.9080502@gmail.com","threadId":"32424","inReplyTo":"1356059558-23479-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v7 2/7] tests: paint known breakages in yellow","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-12-21T08:51:27Z","receivedAt":"2012-12-21T08:51:27Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 12/21/2012 04:12 AM, Junio C Hamano wrote:\n> From: Adam Spiers <git@adamspiers.org>\n> \n> Yellow seems a more appropriate color than bold green when\n> considering the universal traffic lights coloring scheme, where\n> green conveys the impression that everything's OK, and amber that\n> something's not quite right.\n>\nHere are few more details about the behaviour of other testing\ntools, in case you want to squash them in the commit message for\nfuture references:\n\n1. Automake (at least up to 1.13) and Autotest (at least up to the\n   2.69 Autoconf release) use \"bold\" green for reporting expected\n   failures.\n\n2. On the other hand, the 'prove' utility (as of TAP::Harness v3.23\n   and Perl v5.14.2) use yellow (not bold) for the same purpose.\n\nRegards,\n  Stefano\n"},{"id":"205325","messageId":"CAOkDyE8ac0jCutt-hP-p-Bk+PqVda0diXkOANrPbMpT5XTy2GA@mail.gmail.com","threadId":"32424","inReplyTo":"20121221081521.GA22193@sigill.intra.peff.net","subject":"Re: [PATCH v7 0/7] coloring test output after traffic signal","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-21T10:00:09Z","receivedAt":"2012-12-21T10:00:09Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Dec 21, 2012 at 8:15 AM, Jeff King <peff@peff.net> wrote:\n> On Thu, Dec 20, 2012 at 07:12:31PM -0800, Junio C Hamano wrote:\n>> To conclude the bikeshedding discussion we had today, here is what I\n>> queued by squashing stuff into relevant patches, so that people can\n>> eyeball the result for the last time.\n\nGreat, thanks a lot Junio.\n\n> Thanks, this looks OK to me.\n\nTo me too.\n\n> And thank you, Adam, for your patience. Seven iterations of color\n> bikeshedding is more than should be asked of anyone. :)\n\n;-)\n"},{"id":"205338","messageId":"7vd2y32ys0.fsf@alter.siamese.dyndns.org","threadId":"32424","inReplyTo":"50D4230F.9080502@gmail.com","subject":"Re: [PATCH v7 2/7] tests: paint known breakages in yellow","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-21T15:46:55Z","receivedAt":"2012-12-21T15:46:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefano Lattarini <stefano.lattarini@gmail.com> writes:\n\n> On 12/21/2012 04:12 AM, Junio C Hamano wrote:\n>> From: Adam Spiers <git@adamspiers.org>\n>> \n>> Yellow seems a more appropriate color than bold green when\n>> considering the universal traffic lights coloring scheme, where\n>> green conveys the impression that everything's OK, and amber that\n>> something's not quite right.\n>>\n> Here are few more details about the behaviour of other testing\n> tools, in case you want to squash them in the commit message for\n> future references:\n>\n> 1. Automake (at least up to 1.13) and Autotest (at least up to the\n>    2.69 Autoconf release) use \"bold\" green for reporting expected\n>    failures.\n>\n> 2. On the other hand, the 'prove' utility (as of TAP::Harness v3.23\n>    and Perl v5.14.2) use yellow (not bold) for the same purpose.\n\nNice to know, thanks.\n\nI re-read the above three times, trying to see how to add it to the\nlog message, but having hard time phrasing it.\n\nThe only thing the additional knowledge adds seems to be to give\nrationale for the old choice of \"bold green\"---it was not chosen\nfrom thin-air but can be viewed as following the automake/autotest\nscheme, and other systems cannot agree on what color to pick for\nthis purpose.\n\nI do not see a need to justify why we chose differently from\nautomake/autotest; we could say something like:\n\n    Yellow seems a more appropriate color than bold green when\n    considering the universal traffic lights coloring scheme, where\n    green conveys the impression that everything's OK, and amber that\n    something's not quite right.  This is in line with what 'prove'\n    uses, but different from 'automake/autotest' do.\n\nbut we are not in the business of choosing which is more correct\nbetween prove and automake/autotest, and I do not see how it adds\nmuch value to tell readers that color choices are not universally\nagreed upon across various test software suites---that's kind of\nknown, isn't it?\n\nSo...\n"},{"id":"205347","messageId":"50D49588.60305@gmail.com","threadId":"32424","inReplyTo":"7vd2y32ys0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v7 2/7] tests: paint known breakages in yellow","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-12-21T16:59:52Z","receivedAt":"2012-12-21T16:59:52Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 12/21/2012 04:46 PM, Junio C Hamano wrote:\n>\n> [SNIP]\n>\n> The only thing the additional knowledge adds seems to be to give\n> rationale for the old choice of \"bold green\"---it was not chosen\n> from thin-air but can be viewed as following the automake/autotest\n> scheme, and other systems cannot agree on what color to pick for\n> this purpose.\n> \n> I do not see a need to justify why we chose differently from\n> automake/autotest; we could say something like:\n> \n>     Yellow seems a more appropriate color than bold green when\n>     considering the universal traffic lights coloring scheme, where\n>     green conveys the impression that everything's OK, and amber that\n>     something's not quite right.  This is in line with what 'prove'\n>     uses, but different from 'automake/autotest' do.\n> \n> but we are not in the business of choosing which is more correct\n> between prove and automake/autotest, and I do not see how it adds\n> much value to tell readers that color choices are not universally\n> agreed upon across various test software suites---that's kind of\n> known, isn't it?\n> \n> So...\n>\nThat is fine with me, I just pointed it out because I suspected not\neverybody was aware of all these details.  If you decide they don't\nmatter, it's perfectly OK -- but at least now it's an informed\nchoice ;-)\n\nThanks,\n  Stefano\n"}]}