{"thread":{"id":"32357","subject":"[PATCH v6 5/7] tests: refactor mechanics of testing in a sub test-lib","startedAt":"2012-12-16T18:28:08Z","lastAt":"2012-12-20T23:28:57Z","messageCount":18,"participants":["Adam Spiers","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":6,"patchTotal":7},"messages":[{"id":"204976","messageId":"1355682495-22382-1-git-send-email-git@adamspiers.org","threadId":"32357","inReplyTo":null,"subject":"[PATCH v6 0/7] make test output coloring more intuitive","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-16T18:28:08Z","receivedAt":"2012-12-16T18:28:08Z","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,\n\n - green is only used for things which have gone to plan and\n require no further work to be done,\n\n - blue is only used for skipped tests, and\n\n - cyan is used for other informational messages.\n\nSince unexpected test passes are no longer treated as passes, the\nsummary lines displayed at the end of a test run have enough different\npossible outputs to warrant them being covered in the test framework's\nself-tests. Therefore this series also refactors and extends the\nself-tests.\n\nAdam Spiers (7):\n tests: test number comes first in 'not ok $count - $message'\n tests: paint known breakages in bold yellow\n tests: paint skipped tests in bold blue\n tests: change info messages from yellow/brown to bold 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 | 211 ++++++++++++++++++++++++++++++++++++++++++-------------\n t/test-lib.sh | 25 ++++---\n 2 files changed, 180 insertions(+), 56 deletions(-)\n\n-- \n1.7.12.1.396.g53b3ea9\n"},{"id":"204975","messageId":"1355682495-22382-2-git-send-email-git@adamspiers.org","threadId":"32357","inReplyTo":"1355682495-22382-1-git-send-email-git@adamspiers.org","subject":"[PATCH v6 1/7] tests: test number comes first in 'not ok $count - $message'","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-16T18:28:09Z","receivedAt":"2012-12-16T18:28:09Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"The 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 \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 562cf41..46ccda3 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -189,13 +189,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 f50f834..d0b236f 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -298,7 +298,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.1.396.g53b3ea9\n"},{"id":"204970","messageId":"1355682495-22382-3-git-send-email-git@adamspiers.org","threadId":"32357","inReplyTo":"1355682495-22382-1-git-send-email-git@adamspiers.org","subject":"[PATCH v6 2/7] tests: paint known breakages in bold yellow","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-16T18:28:10Z","receivedAt":"2012-12-16T18:28: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\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 bold yellow to be less alarmist and more\nconsistent with the above.\n\nSigned-off-by: Adam Spiers \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 d0b236f..710f051 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -213,6 +213,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 brown/yellow\n \t\tpass)\n \t\t\ttput setaf 2;; # green\n \t\tinfo)\n@@ -311,7 +313,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@@ -408,7 +410,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.1.396.g53b3ea9\n"},{"id":"204972","messageId":"1355682495-22382-4-git-send-email-git@adamspiers.org","threadId":"32357","inReplyTo":"1355682495-22382-1-git-send-email-git@adamspiers.org","subject":"[PATCH v6 3/7] tests: paint skipped tests in bold blue","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-16T18:28:11Z","receivedAt":"2012-12-16T18:28:11Z","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 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 \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 710f051..220b172 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -212,7 +212,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 bold; tput setaf 4;; # bold blue\n \t\twarn)\n \t\t\ttput bold; tput setaf 3;; # bold brown/yellow\n \t\tpass)\n-- \n1.7.12.1.396.g53b3ea9\n"},{"id":"204971","messageId":"1355682495-22382-5-git-send-email-git@adamspiers.org","threadId":"32357","inReplyTo":"1355682495-22382-1-git-send-email-git@adamspiers.org","subject":"[PATCH v6 4/7] tests: change info messages from yellow/brown to bold cyan","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-16T18:28:12Z","receivedAt":"2012-12-16T18:28:12Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Now 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\nbold cyan which is also blue-ish, but visually distinct from bold\nblue. This was suggested on the list a while ago and no-one raised\nany objections:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/205675/focus=205966\n\nSigned-off-by: Adam Spiers \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 220b172..5d9d0fc 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -218,7 +218,7 @@ then\n \t\tpass)\n \t\t\ttput setaf 2;; # green\n \t\tinfo)\n-\t\t\ttput setaf 3;; # brown\n+\t\t\ttput bold; tput setaf 6;; # bold cyan\n \t\t*)\n \t\t\ttest -n \"$quiet\" && return;;\n \t\tesac\n-- \n1.7.12.1.396.g53b3ea9\n"},{"id":"204969","messageId":"1355682495-22382-6-git-send-email-git@adamspiers.org","threadId":"32357","inReplyTo":"1355682495-22382-1-git-send-email-git@adamspiers.org","subject":"[PATCH v6 5/7] tests: refactor mechanics of testing in a sub test-lib","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-16T18:28:13Z","receivedAt":"2012-12-16T18:28: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 \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 46ccda3..fc5200f 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -45,39 +45,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@@ -159,19 +173,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@@ -181,14 +184,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@@ -202,8 +199,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.1.396.g53b3ea9\n"},{"id":"204974","messageId":"1355682495-22382-7-git-send-email-git@adamspiers.org","threadId":"32357","inReplyTo":"1355682495-22382-1-git-send-email-git@adamspiers.org","subject":"[PATCH v6 6/7] tests: test the test framework more thoroughly","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-16T18:28:14Z","receivedAt":"2012-12-16T18:28: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 to avoid disrupting the metrics for\nthe parent tests.\n\nSigned-off-by: Adam Spiers \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 fc5200f..5c1dde0 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -79,6 +79,55 @@ 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; 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@@ -92,6 +141,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.1.396.g53b3ea9\n"},{"id":"204973","messageId":"1355682495-22382-8-git-send-email-git@adamspiers.org","threadId":"32357","inReplyTo":"1355682495-22382-1-git-send-email-git@adamspiers.org","subject":"[PATCH v6 7/7] tests: paint unexpectedly fixed known breakages in bold red","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-16T18:28:15Z","receivedAt":"2012-12-16T18:28: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 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 \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 5c1dde0..bd6127f 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -134,13 +134,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@@ -188,10 +206,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 5d9d0fc..b1acdfc 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -308,7 +308,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@@ -406,13 +406,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@@ -426,7 +431,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.1.396.g53b3ea9\n"},{"id":"204978","messageId":"7v8v8xrfnp.fsf@alter.siamese.dyndns.org","threadId":"32357","inReplyTo":"1355682495-22382-1-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v6 0/7] make test output coloring more intuitive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-16T18:54:50Z","receivedAt":"2012-12-16T18:54:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> This series of commits attempts to make test output coloring\n> more intuitive,...\n\nThanks; I understand that this is to replace the previous one\nb465316 (tests: paint unexpectedly fixed known breakages in bold\nred, 2012-09-19)---am I correct?\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,\n>\n> - green is only used for things which have gone to plan and\n> require no further work to be done,\n>\n> - blue is only used for skipped tests, and\n>\n> - cyan is used for other informational messages.\n\nOK.\n\n> Since unexpected test passes are no longer treated as passes, the\n> summary lines displayed at the end of a test run have enough different\n> possible outputs to warrant them being covered in the test framework's\n> self-tests. Therefore this series also refactors and extends the\n> self-tests.\n>\n> Adam Spiers (7):\n> tests: test number comes first in 'not ok $count - $message'\n> tests: paint known breakages in bold yellow\n> tests: paint skipped tests in bold blue\n> tests: change info messages from yellow/brown to bold 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 | 211 ++++++++++++++++++++++++++++++++++++++++++-------------\n> t/test-lib.sh | 25 ++++---\n> 2 files changed, 180 insertions(+), 56 deletions(-)\n\nWill take a look; thanks.\n"},{"id":"204979","messageId":"CAOkDyE9B_HfUZmqNqO35mtjTvdihBTiW=uOV2oEQgLUw1xyf=A@mail.gmail.com","threadId":"32357","inReplyTo":"7v8v8xrfnp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v6 0/7] make test output coloring more intuitive","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-16T19:01:56Z","receivedAt":"2012-12-16T19:01:56Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano wrote:\n> Adam Spiers writes:\n>\n>> This series of commits attempts to make test output coloring\n>> more intuitive,...\n>\n> Thanks; I understand that this is to replace the previous one\n> b465316 (tests: paint unexpectedly fixed known breakages in bold\n> red, 2012-09-19)---am I correct?\n\nCorrect. AFAICS I have incorporated all feedback raised in previous\nreviews.\n\n> Will take a look; thanks.\n\nThanks. Sorry again for the delay. I'm now (finally) resuming work\non as/check-ignore.\n"},{"id":"204992","messageId":"7vsj75pp7l.fsf@alter.siamese.dyndns.org","threadId":"32357","inReplyTo":"CAOkDyE9B_HfUZmqNqO35mtjTvdihBTiW=uOV2oEQgLUw1xyf=A@mail.gmail.com","subject":"Re: [PATCH v6 0/7] make test output coloring more intuitive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-16T23:11:26Z","receivedAt":"2012-12-16T23:11:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano wrote:\n>> Adam Spiers writes:\n>>\n>>> This series of commits attempts to make test output coloring\n>>> more intuitive,...\n>>\n>> Thanks; I understand that this is to replace the previous one\n>> b465316 (tests: paint unexpectedly fixed known breakages in bold\n>> red, 2012-09-19)---am I correct?\n>\n> Correct. AFAICS I have incorporated all feedback raised in previous\n> reviews.\n\nSeemed clean from a cursory look. Will replace. Thanks.\n"},{"id":"205262","messageId":"20121220153411.GA1497@sigill.intra.peff.net","threadId":"32357","inReplyTo":"CAOkDyE9B_HfUZmqNqO35mtjTvdihBTiW=uOV2oEQgLUw1xyf=A@mail.gmail.com","subject":"Re: [PATCH v6 0/7] make test output coloring more intuitive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-20T15:34:11Z","receivedAt":"2012-12-20T15:34:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 16, 2012 at 07:01:56PM +0000, Adam Spiers wrote:\n\n> On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano wrote:\n> > Adam Spiers writes:\n> >\n> >> This series of commits attempts to make test output coloring\n> >> more intuitive,...\n> >\n> > Thanks; I understand that this is to replace the previous one\n> > b465316 (tests: paint unexpectedly fixed known breakages in bold\n> > red, 2012-09-19)---am I correct?\n> \n> Correct. AFAICS I have incorporated all feedback raised in previous\n> reviews.\n> \n> > Will take a look; thanks.\n> \n> Thanks. Sorry again for the delay. I'm now (finally) resuming work\n> on as/check-ignore.\n\nI eyeballed the test output of \"pu\". I do think this resolves all of the\nissues brought up before, and I really hate to bikeshed on the colors at\nthis point, but I find that bold cyan a bit hard on the eyes when\nrunning with \"-v\" (where most of the output is in that color, as it\ndumps the shell for each test). Is there any reason not to tone it down\na bit like:\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 256f1c6..31f59af 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -227,7 +227,7 @@ then\n \t\tpass)\n \t\t\ttput setaf 2;; # green\n \t\tinfo)\n-\t\t\ttput bold; tput setaf 6;; # bold cyan\n+\t\t\ttput setaf 6;; # cyan\n \t\t*)\n \t\t\ttest -n \"$quiet\" && return;;\n \t\tesac\n\n-Peff\n"},{"id":"205263","messageId":"CAOkDyE9y6JvNKTCBoJqu47Hn-3axfjZPUdBhf4bOEfSP-9Q84A@mail.gmail.com","threadId":"32357","inReplyTo":"20121220153411.GA1497@sigill.intra.peff.net","subject":"Re: [PATCH v6 0/7] make test output coloring more intuitive","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-20T15:44:53Z","receivedAt":"2012-12-20T15:44:53Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Thu, Dec 20, 2012 at 3:34 PM, Jeff King wrote:\n> On Sun, Dec 16, 2012 at 07:01:56PM +0000, Adam Spiers wrote:\n>> On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano wrote:\n>> > Adam Spiers writes:\n>> >> This series of commits attempts to make test output coloring\n>> >> more intuitive,...\n>> >\n>> > Thanks; I understand that this is to replace the previous one\n>> > b465316 (tests: paint unexpectedly fixed known breakages in bold\n>> > red, 2012-09-19)---am I correct?\n>>\n>> Correct. AFAICS I have incorporated all feedback raised in previous\n>> reviews.\n>>\n>> > Will take a look; thanks.\n>>\n>> Thanks. Sorry again for the delay. I'm now (finally) resuming work\n>> on as/check-ignore.\n>\n> I eyeballed the test output of \"pu\". I do think this resolves all of the\n> issues brought up before, and I really hate to bikeshed on the colors at\n> this point, but I find that bold cyan a bit hard on the eyes when\n> running with \"-v\" (where most of the output is in that color, as it\n> dumps the shell for each test). Is there any reason not to tone it down\n> a bit like:\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 256f1c6..31f59af 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -227,7 +227,7 @@ then\n> pass)\n> tput setaf 2;; # green\n> info)\n> - tput bold; tput setaf 6;; # bold cyan\n> + tput setaf 6;; # cyan\n> *)\n> test -n \"$quiet\" && return;;\n> esac\n>\n> -Peff\n\nGood point, I forgot to check what it looked like with -v. Since this\nseries is already on v6, is there a more lightweight way of addressing\nthis tiny tweak than sending v7?\n"},{"id":"205268","messageId":"20121220161110.GA10605@sigill.intra.peff.net","threadId":"32357","inReplyTo":"CAOkDyE9y6JvNKTCBoJqu47Hn-3axfjZPUdBhf4bOEfSP-9Q84A@mail.gmail.com","subject":"Re: [PATCH v6 0/7] make test output coloring more intuitive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-20T16:11:10Z","receivedAt":"2012-12-20T16:11:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 20, 2012 at 03:44:53PM +0000, Adam Spiers wrote:\n\n> > diff --git a/t/test-lib.sh b/t/test-lib.sh\n> > index 256f1c6..31f59af 100644\n> > --- a/t/test-lib.sh\n> > +++ b/t/test-lib.sh\n> > @@ -227,7 +227,7 @@ then\n> > pass)\n> > tput setaf 2;; # green\n> > info)\n> > - tput bold; tput setaf 6;; # bold cyan\n> > + tput setaf 6;; # cyan\n> > *)\n> > test -n \"$quiet\" && return;;\n> > esac\n> >\n> \n> Good point, I forgot to check what it looked like with -v. Since this\n> series is already on v6, is there a more lightweight way of addressing\n> this tiny tweak than sending v7?\n\nIt is ultimately up to Junio, but I suspect he would be OK if you just\nreposted patch 4/7 with the above squashed. Or even just said \"I like\nthis, please squash it into patch 4 (change info messages from\nyellow/brown to bold cyan).\n\nAs an aside, it made me wonder how hard/useful it would be to color the\nsnippets even more. Doing this:\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex f9ccbf2..3d44a94 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -364,7 +364,12 @@ test_expect_success () {\n \texport test_prereq\n \tif ! test_skip \"$@\"\n \tthen\n-\t\tsay >&3 \"expecting success: $2\"\n+\t\tif test -z \"$GIT_TEST_HIGHLIGHT\"; then\n+\t\t\tsay >&3 \"expecting success: $2\"\n+\t\telse\n+\t\t\tsay >&3 \"expecting success:\"\n+\t\t\techo \"$2\" | eval \"$GIT_TEST_HIGHLIGHT\"\n+\t\tfi\n \t\tif test_run_ \"$2\"\n \t\tthen\n \t\t\ttest_ok_ \"$1\"\n\nproduces highlighted snippets with:\n\n GIT_TEST_HIGHLIGHT='highlight -S sh -O ansi'\n\nor\n\n GIT_TEST_HIGHLIGHT='pygmentize -l sh'\n\ndepending on what you have installed on your system. I'm not convinced\nit actually adds anything, but it's a fun toy. A real patch would\nprobably turn it off in non-verbose mode, as invoking the highlighter\n(especially pygmentize) repeatedly is somewhat expensive.\n\n-Peff\n"},{"id":"205275","messageId":"CAOkDyE-yfFQxxgsumRB8N1zXN2LTq89=pArU-85Z+3Oyruiwxg@mail.gmail.com","threadId":"32357","inReplyTo":"20121220161110.GA10605@sigill.intra.peff.net","subject":"Re: [PATCH v6 0/7] make test output coloring more intuitive","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-20T18:08:15Z","receivedAt":"2012-12-20T18:08:15Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Thu, Dec 20, 2012 at 4:11 PM, Jeff King wrote:\n> On Thu, Dec 20, 2012 at 03:44:53PM +0000, Adam Spiers wrote:\n>> > diff --git a/t/test-lib.sh b/t/test-lib.sh\n>> > index 256f1c6..31f59af 100644\n>> > --- a/t/test-lib.sh\n>> > +++ b/t/test-lib.sh\n>> > @@ -227,7 +227,7 @@ then\n>> > pass)\n>> > tput setaf 2;; # green\n>> > info)\n>> > - tput bold; tput setaf 6;; # bold cyan\n>> > + tput setaf 6;; # cyan\n>> > *)\n>> > test -n \"$quiet\" && return;;\n>> > esac\n>> >\n>>\n>> Good point, I forgot to check what it looked like with -v. Since this\n>> series is already on v6, is there a more lightweight way of addressing\n>> this tiny tweak than sending v7?\n>\n> It is ultimately up to Junio, but I suspect he would be OK if you just\n> reposted patch 4/7 with the above squashed.\n\nI'll do that if Junio is OK with that.\n\n> Or even just said \"I like\n> this, please squash it into patch 4 (change info messages from\n> yellow/brown to bold cyan).\n\nYes, I'm OK with this way too :) Of course \"bold\" would need to be dropped\nfrom the commit message.\n"},{"id":"205284","messageId":"7vy5gs4jiy.fsf@alter.siamese.dyndns.org","threadId":"32357","inReplyTo":"20121220161110.GA10605@sigill.intra.peff.net","subject":"Re: [PATCH v6 0/7] make test output coloring more intuitive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-20T19:21:09Z","receivedAt":"2012-12-20T19:21:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> Good point, I forgot to check what it looked like with -v. Since this\n>> series is already on v6, is there a more lightweight way of addressing\n>> this tiny tweak than sending v7?\n>\n> It is ultimately up to Junio, but I suspect he would be OK if you just\n> reposted patch 4/7 with the above squashed. Or even just said \"I like\n> this, please squash it into patch 4 (change info messages from\n> yellow/brown to bold cyan).\n\nSurely; as long as the series is not in 'next', the change to be\nsquashed is not too big and it is not too much work (and in this\ncase it certainly is not).\n\nI actually wonder if \"skipped test in bold blue\" and \"known breakage\nin bold yellow\" should also lose the boldness. Errors and warnings\nin bold are good, but I would say the degree of need for attention\nare more like this:\n\n\terror (failed tests - you should look into it)\n skip (skipped - perhaps you need more packages?)\n warn (expected failure - you may want to look into fixing it someday)\n\tinfo\n pass\n\nThe \"expected_failure\" cases painted in \"warn\" are all long-known\nfailures; I do not think reminding about them in \"bold\" over and\nover will help encouraging the developers take a look at them.\n\nThe \"skipped\" cases fall into two categories. Either you already\nknow you choose to not to care (e.g. I do not expect to use git-p4\nand decided not to install p4 anywhere, so I may have t98?? on\nGIT_SKIP_TESTS environment) or you haven't reached that point on a\nnew system and haven't realized that you didn't install a package\nneeded to run tests you care about (e.g. cvsserver tests would not\nrun without Perl interface to SQLite). For the former, the bold\noutput is merely distracting; for the latter, bold _might_ help in\nthis case.\n\nAt least, I think\n\n\tGIT_SKIP_TESTS=t98?? sh t9800-git-p4-basic.sh -v\n\nshould paint \"skipping test t9800 altogether\" (emitted with \"-v) and\nthe last line \"1..0 # SKIP skip all tests in t9800\" both in the same\n\"info\" color.\n\nHow about going further to reduce \"bold\" a bit more, like this?\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex aaf013e..2bbb81d 100644\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 4;; # bold blue\n+\t\t\ttput setaf 4;; # bold blue\n \t\twarn)\n-\t\t\ttput bold; tput setaf 3;; # bold brown/yellow\n+\t\t\ttput setaf 3;; # bold brown/yellow\n \t\tpass)\n \t\t\ttput setaf 2;; # green\n \t\tinfo)\n-\t\t\ttput bold; tput setaf 6;; # bold cyan\n+\t\t\ttput setaf 6;; # bold cyan\n \t\t*)\n \t\t\ttest -n \"$quiet\" && return;;\n \t\tesac\n@@ -589,7 +589,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"},{"id":"205285","messageId":"20121220195010.GA21785@sigill.intra.peff.net","threadId":"32357","inReplyTo":"7vy5gs4jiy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v6 0/7] make test output coloring more intuitive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-20T19:50:10Z","receivedAt":"2012-12-20T19:50:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 20, 2012 at 11:21:09AM -0800, Junio C Hamano wrote:\n\n> The \"expected_failure\" cases painted in \"warn\" are all long-known\n> failures; I do not think reminding about them in \"bold\" over and\n> over will help encouraging the developers take a look at them.\n> \n> The \"skipped\" cases fall into two categories. Either you already\n> know you choose to not to care (e.g. I do not expect to use git-p4\n> and decided not to install p4 anywhere, so I may have t98?? on\n> GIT_SKIP_TESTS environment) or you haven't reached that point on a\n> new system and haven't realized that you didn't install a package\n> needed to run tests you care about (e.g. cvsserver tests would not\n> run without Perl interface to SQLite). For the former, the bold\n> output is merely distracting; for the latter, bold _might_ help in\n> this case.\n> \n> At least, I think\n> \n> \tGIT_SKIP_TESTS=t98?? sh t9800-git-p4-basic.sh -v\n> \n> should paint \"skipping test t9800 altogether\" (emitted with \"-v) and\n> the last line \"1..0 # SKIP skip all tests in t9800\" both in the same\n> \"info\" color.\n> \n> How about going further to reduce \"bold\" a bit more, like this?\n\nYeah, I think it is a little easier on the eyes while maintaining the\nintended color scheme.\n\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index aaf013e..2bbb81d 100644\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 4;; # bold blue\n> +\t\t\ttput setaf 4;; # bold blue\n\nOn my xterm, at least, this is actually the difference between light\nblue\" and dark blue, not bold and not-bold. I think it is OK, though to\nbe honest, having seen the \"skip all\" messages in cyan (e.g., running\nt9800), I think just printing skip messages in cyan looks best. But it\nis not that big a deal to me, and we are well into bikeshed territory, I\nthink, so that will be my last word on the subject.\n\n-Peff\n"},{"id":"205301","messageId":"CAOkDyE9tDYRYzojzNnjWsT7UygxMAurHqLSDGA66_LMPD2Wmnw@mail.gmail.com","threadId":"32357","inReplyTo":"7vy5gs4jiy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v6 0/7] make test output coloring more intuitive","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-20T23:28:57Z","receivedAt":"2012-12-20T23:28:57Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Thu, Dec 20, 2012 at 7:21 PM, Junio C Hamano wrote:\n> Jeff King writes:\n>>> Good point, I forgot to check what it looked like with -v. Since this\n>>> series is already on v6, is there a more lightweight way of addressing\n>>> this tiny tweak than sending v7?\n>>\n>> It is ultimately up to Junio, but I suspect he would be OK if you just\n>> reposted patch 4/7 with the above squashed. Or even just said \"I like\n>> this, please squash it into patch 4 (change info messages from\n>> yellow/brown to bold cyan).\n>\n> Surely; as long as the series is not in 'next', the change to be\n> squashed is not too big and it is not too much work (and in this\n> case it certainly is not).\n\nOK.\n\n> I actually wonder if \"skipped test in bold blue\" and \"known breakage\n> in bold yellow\" should also lose the boldness. Errors and warnings\n> in bold are good, but I would say the degree of need for attention\n> are more like this:\n>\n> error (failed tests - you should look into it)\n> skip (skipped - perhaps you need more packages?)\n> warn (expected failure - you may want to look into fixing it someday)\n> info\n> pass\n>\n> The \"expected_failure\" cases painted in \"warn\" are all long-known\n> failures; I do not think reminding about them in \"bold\" over and\n> over will help encouraging the developers take a look at them.\n\nAs Peff already noted, on many (most?) X terminals \"bold\" colours are\njust brighter colours, rather than a heavier typeface. How bold they\nlook is therefore dependent on the colour scheme used by that\nterminal.\n\n> The \"skipped\" cases fall into two categories. Either you already\n> know you choose to not to care (e.g. I do not expect to use git-p4\n> and decided not to install p4 anywhere, so I may have t98?? on\n> GIT_SKIP_TESTS environment) or you haven't reached that point on a\n> new system and haven't realized that you didn't install a package\n> needed to run tests you care about (e.g. cvsserver tests would not\n> run without Perl interface to SQLite). For the former, the bold\n> output is merely distracting; for the latter, bold _might_ help in\n> this case.\n\nVery good point.\n\n> At least, I think\n>\n> GIT_SKIP_TESTS=t98?? sh t9800-git-p4-basic.sh -v\n>\n> should paint \"skipping test t9800 altogether\" (emitted with \"-v) and\n> the last line \"1..0 # SKIP skip all tests in t9800\" both in the same\n> \"info\" color.\n>\n> How about going further to reduce \"bold\" a bit more, like this?\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index aaf013e..2bbb81d 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -182,13 +182,13 @@ then\n> error)\n> tput bold; tput setaf 1;; # bold red\n> skip)\n> - tput bold; tput setaf 4;; # bold blue\n> + tput setaf 4;; # bold blue\n\nThe comment still says \"bold\".\n\n> warn)\n> - tput bold; tput setaf 3;; # bold brown/yellow\n> + tput setaf 3;; # bold brown/yellow\n\nDitto here ...\n\n> pass)\n> tput setaf 2;; # green\n> info)\n> - tput bold; tput setaf 6;; # bold cyan\n> + tput setaf 6;; # bold cyan\n\n... and here.\n\n> *)\n> test -n \"$quiet\" && return;;\n> esac\n> @@ -589,7 +589,7 @@ for skp in $GIT_SKIP_TESTS\n> do\n> case \"$this_test\" in\n> $skp)\n> - say_color skip >&3 \"skipping test $this_test altogether\"\n> + say_color info >&3 \"skipping test $this_test altogether\"\n> skip_all=\"skip all tests in $this_test\"\n> test_done\n> esac\n\nYes, I like this last hunk especially.\n\nI have no objection in principle to a reduction in boldness.\n\nHowever, I am beginning to get disheartened that at this rate, this\nseries will never land. I already submitted v4 of the series which\nalready had non-bold blue. I then received feedback indicating that\nbold blue would be more suitable, so despite alarm bells beginning to\nring in my head, I submitted v5 with bold blue, declaring that that\nwould be my last version:\n\n http://article.gmane.org/gmane.comp.version-control.git/206042\n\nA further concern about \"info\" messages not being blue prompted me\nto attempt to canvass more opinions:\n\n http://article.gmane.org/gmane.comp.version-control.git/209321\n\nI received none, so submitted v6 based on my best judgement. Now we\nare talking about a potential v7 going *back* to non-bold blue. I can\nsubmit v7 if you think it's worth it, but would that really be the end\nof the discussion? It's clear from the above that colour scheme\ndesign by committee is about as good an idea as asking a bunch of kids\nto reach consensus on their favourite colour ;-)\n\nSo if possible I'd be very happy for Junio to simply make an executive\ndecision (I don't care which way, as long as it fits the traffic\nlights scheme and uses distinct hues of blue/cyan for the different\ncategories of skip/info messages), tweak the latest v6 series\naccordingly, and then push so that we can all go back to more pressing\nthings ;-)\n\nHopefully that is a reasonable way forward?\n\nThanks,\nAdam\n"}]}