# [PATCH v7 1/7] tests: test number comes first in 'not ok $count - $message' 13 messages from 2012-12-21 to 2012-12-21. Participants: Junio C Hamano, Jeff King, Stefano Lattarini, Adam Spiers. Thread: https://gitlist.dev/t/32424 ## Junio C Hamano, 2012-12-21 03:12 Subject: [PATCH v7 0/7] coloring test output after traffic signal Message-ID: <1356059558-23479-1-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1356059558-23479-1-git-send-email-gitster%40pobox.com In-Reply-To: ``` To conclude the bikeshedding discussion we had today, here is what I queued by squashing stuff into relevant patches, so that people can eyeball the result for the last time. Adam Spiers (7): tests: test number comes first in 'not ok $count - $message' tests: paint known breakages in yellow tests: paint skipped tests in blue tests: change info messages from yellow/brown to cyan tests: refactor mechanics of testing in a sub test-lib tests: test the test framework more thoroughly tests: paint unexpectedly fixed known breakages in bold red t/t0000-basic.sh | 214 ++++++++++++++++++++++++++++++++++++++++++------------- t/test-lib.sh | 29 +++++--- 2 files changed, 184 insertions(+), 59 deletions(-) -- 1.8.1.rc2.225.g8d36ab4 ``` ## Junio C Hamano, 2012-12-21 03:12 Subject: [PATCH v7 1/7] tests: test number comes first in 'not ok $count - $message' Message-ID: <1356059558-23479-2-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1356059558-23479-2-git-send-email-gitster%40pobox.com In-Reply-To: <1356059558-23479-1-git-send-email-gitster@pobox.com> ``` From: Adam Spiers The old output to say "not ok - 1 messsage" was working by accident only because the test numbers are optional in TAP. Signed-off-by: Adam Spiers Signed-off-by: Junio C Hamano --- t/t0000-basic.sh | 4 ++-- t/test-lib.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index ae6a3f0..c6b42de 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -167,13 +167,13 @@ test_expect_success 'tests clean up even on failures' " ! test -s err && ! test -f \"trash directory.failing-cleanup/clean-after-failure\" && sed -e 's/Z$//' -e 's/^> //' >expect <<-\\EOF && - > not ok - 1 tests clean up even after a failure + > not ok 1 - tests clean up even after a failure > # Z > # touch clean-after-failure && > # test_when_finished rm clean-after-failure && > # (exit 1) > # Z - > not ok - 2 failure to clean up causes the test to fail + > not ok 2 - failure to clean up causes the test to fail > # Z > # test_when_finished \"(exit 2)\" > # Z diff --git a/t/test-lib.sh b/t/test-lib.sh index f8e3733..03b86b8 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -268,7 +268,7 @@ test_ok_ () { test_failure_ () { test_failure=$(($test_failure + 1)) - say_color error "not ok - $test_count $1" + say_color error "not ok $test_count - $1" shift echo "$@" | sed -e 's/^/# /' test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; } -- 1.8.1.rc2.225.g8d36ab4 ``` ## Junio C Hamano, 2012-12-21 03:12 Subject: [PATCH v7 2/7] tests: paint known breakages in yellow Message-ID: <1356059558-23479-3-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1356059558-23479-3-git-send-email-gitster%40pobox.com In-Reply-To: <1356059558-23479-1-git-send-email-gitster@pobox.com> ``` From: Adam Spiers Yellow seems a more appropriate color than bold green when considering the universal traffic lights coloring scheme, where green conveys the impression that everything's OK, and amber that something's not quite right. Likewise, change the color of the summarized total number of known breakages from bold red to the same yellow to be less alarmist and more consistent with the above. An earlier version of this patch used bold yellow but because these are all long-known failures, reminding them to developers in bold over and over does not help encouraging them to take a look at them very much. This iteration paints them in plain yellow instead to make them less distracting. Signed-off-by: Adam Spiers Signed-off-by: Junio C Hamano --- t/test-lib.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 03b86b8..72aafd0 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -183,6 +183,8 @@ then tput bold; tput setaf 1;; # bold red skip) tput bold; tput setaf 2;; # bold green + warn) + tput setaf 3;; # brown/yellow pass) tput setaf 2;; # green info) @@ -281,7 +283,7 @@ test_known_broken_ok_ () { test_known_broken_failure_ () { test_broken=$(($test_broken+1)) - say_color skip "not ok $test_count - $@ # TODO known breakage" + say_color warn "not ok $test_count - $@ # TODO known breakage" } test_debug () { @@ -375,7 +377,7 @@ test_done () { fi if test "$test_broken" != 0 then - say_color error "# still have $test_broken known breakage(s)" + say_color warn "# still have $test_broken known breakage(s)" msg="remaining $(($test_count-$test_broken)) test(s)" else msg="$test_count test(s)" -- 1.8.1.rc2.225.g8d36ab4 ``` ## Junio C Hamano, 2012-12-21 03:12 Subject: [PATCH v7 3/7] tests: paint skipped tests in blue Message-ID: <1356059558-23479-4-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1356059558-23479-4-git-send-email-gitster%40pobox.com In-Reply-To: <1356059558-23479-1-git-send-email-gitster@pobox.com> ``` From: Adam Spiers Skipped tests indicate incomplete test coverage. Whilst this is not a test failure or other error, it's still not a complete success. Other testsuite related software like automake, autotest and prove seem to use blue for skipped tests, so let's follow suit. Signed-off-by: Adam Spiers Signed-off-by: Junio C Hamano --- t/test-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 72aafd0..f32df80 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -182,7 +182,7 @@ then error) tput bold; tput setaf 1;; # bold red skip) - tput bold; tput setaf 2;; # bold green + tput setaf 4;; # blue warn) tput setaf 3;; # brown/yellow pass) -- 1.8.1.rc2.225.g8d36ab4 ``` ## Junio C Hamano, 2012-12-21 03:12 Subject: [PATCH v7 4/7] tests: change info messages from yellow/brown to cyan Message-ID: <1356059558-23479-5-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1356059558-23479-5-git-send-email-gitster%40pobox.com In-Reply-To: <1356059558-23479-1-git-send-email-gitster@pobox.com> ``` From: Adam Spiers Now that we've adopted a "traffic lights" coloring scheme, yellow is used for warning messages, so we need to re-color info messages to something less alarmist. Blue is a universal color for informational messages; however we are using that for skipped tests in order to align with the color schemes of other test suites. Therefore we use cyan which is also blue-ish, but visually distinct from blue. This was suggested on the list a while ago and no-one raised any objections: http://thread.gmane.org/gmane.comp.version-control.git/205675/focus=205966 An earlier iteration of this patch used bold cyan, but the point of this change is to make them less alarming; let's drop the boldness. Also paint the message to report skipping the whole thing via GIT_SKIP_TESTS mechanism in the same color as the "info" color that is used on the final summary line for the entire script. Signed-off-by: Adam Spiers Signed-off-by: Junio C Hamano --- t/test-lib.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index f32df80..8b75c9a 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -186,9 +186,9 @@ then warn) tput setaf 3;; # brown/yellow pass) - tput setaf 2;; # green + tput setaf 2;; # green info) - tput setaf 3;; # brown + tput setaf 6;; # cyan *) test -n "$quiet" && return;; esac @@ -584,7 +584,7 @@ for skp in $GIT_SKIP_TESTS do case "$this_test" in $skp) - say_color skip >&3 "skipping test $this_test altogether" + say_color info >&3 "skipping test $this_test altogether" skip_all="skip all tests in $this_test" test_done esac -- 1.8.1.rc2.225.g8d36ab4 ``` ## Junio C Hamano, 2012-12-21 03:12 Subject: [PATCH v7 5/7] tests: refactor mechanics of testing in a sub test-lib Message-ID: <1356059558-23479-6-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1356059558-23479-6-git-send-email-gitster%40pobox.com In-Reply-To: <1356059558-23479-1-git-send-email-gitster@pobox.com> ``` From: Adam Spiers This will allow us to test the test framework more thoroughly without disrupting the top-level test metrics. Signed-off-by: Adam Spiers Signed-off-by: Junio C Hamano --- t/t0000-basic.sh | 85 ++++++++++++++++++++++++++------------------------------ 1 file changed, 40 insertions(+), 45 deletions(-) diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index c6b42de..d0f46e8 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -55,39 +55,53 @@ test_expect_failure 'pretend we have a known breakage' ' false ' -test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib)' " - mkdir passing-todo && - (cd passing-todo && - cat >passing-todo.sh <<-EOF && - #!$SHELL_PATH - - test_description='A passing TODO test - - This is run in a sub test-lib so that we do not get incorrect - passing metrics - ' - - # Point to the t/test-lib.sh, which isn't in ../ as usual - TEST_DIRECTORY=\"$TEST_DIRECTORY\" - . \"\$TEST_DIRECTORY\"/test-lib.sh +run_sub_test_lib_test () { + name="$1" descr="$2" # stdin is the body of the test code + mkdir "$name" && + ( + cd "$name" && + cat >"$name.sh" <<-EOF && + #!$SHELL_PATH + + test_description='$descr (run in sub test-lib) + + This is run in a sub test-lib so that we do not get incorrect + passing metrics + ' + + # Point to the t/test-lib.sh, which isn't in ../ as usual + . "\$TEST_DIRECTORY"/test-lib.sh + EOF + cat >>"$name.sh" && + chmod +x "$name.sh" && + export TEST_DIRECTORY && + ./"$name.sh" >out 2>err + ) +} - test_expect_failure 'pretend we have fixed a known breakage' ' - : - ' +check_sub_test_lib_test () { + name="$1" # stdin is the expected output from the test + ( + cd "$name" && + ! test -s err && + sed -e 's/^> //' -e 's/Z$//' >expect && + test_cmp expect out + ) +} +test_expect_success 'pretend we have fixed a known breakage' " + run_sub_test_lib_test passing-todo 'A passing TODO test' <<-\\EOF && + test_expect_failure 'pretend we have fixed a known breakage' 'true' test_done EOF - chmod +x passing-todo.sh && - ./passing-todo.sh >out 2>err && - ! test -s err && - sed -e 's/^> //' >expect <<-\\EOF && + check_sub_test_lib_test passing-todo <<-\\EOF > ok 1 - pretend we have fixed a known breakage # TODO known breakage > # fixed 1 known breakage(s) > # passed all 1 test(s) > 1..1 EOF - test_cmp expect out) " + test_set_prereq HAVEIT haveit=no test_expect_success HAVEIT 'test runs if prerequisite is satisfied' ' @@ -137,19 +151,8 @@ then fi test_expect_success 'tests clean up even on failures' " - mkdir failing-cleanup && - ( - cd failing-cleanup && - - cat >failing-cleanup.sh <<-EOF && - #!$SHELL_PATH - - test_description='Failing tests with cleanup commands' - - # Point to the t/test-lib.sh, which isn't in ../ as usual - TEST_DIRECTORY=\"$TEST_DIRECTORY\" - . \"\$TEST_DIRECTORY\"/test-lib.sh - + test_must_fail run_sub_test_lib_test \ + failing-cleanup 'Failing tests with cleanup commands' <<-\\EOF && test_expect_success 'tests clean up even after a failure' ' touch clean-after-failure && test_when_finished rm clean-after-failure && @@ -159,14 +162,8 @@ test_expect_success 'tests clean up even on failures' " test_when_finished \"(exit 2)\" ' test_done - EOF - - chmod +x failing-cleanup.sh && - test_must_fail ./failing-cleanup.sh >out 2>err && - ! test -s err && - ! test -f \"trash directory.failing-cleanup/clean-after-failure\" && - sed -e 's/Z$//' -e 's/^> //' >expect <<-\\EOF && + check_sub_test_lib_test failing-cleanup <<-\\EOF > not ok 1 - tests clean up even after a failure > # Z > # touch clean-after-failure && @@ -180,8 +177,6 @@ test_expect_success 'tests clean up even on failures' " > # failed 2 among 2 test(s) > 1..2 EOF - test_cmp expect out - ) " ################################################################ -- 1.8.1.rc2.225.g8d36ab4 ``` ## Junio C Hamano, 2012-12-21 03:12 Subject: [PATCH v7 6/7] tests: test the test framework more thoroughly Message-ID: <1356059558-23479-7-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1356059558-23479-7-git-send-email-gitster%40pobox.com In-Reply-To: <1356059558-23479-1-git-send-email-gitster@pobox.com> ``` From: Adam Spiers Add 5 new full test suite runs each with a different number of passing/failing/broken/fixed tests, in order to ensure that the correct exit code and output are generated in each case. As before, these are run in a subdirectory to avoid disrupting the metrics for the parent tests. Signed-off-by: Adam Spiers Signed-off-by: Junio C Hamano --- t/t0000-basic.sh | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index d0f46e8..384b0ae 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -89,6 +89,56 @@ check_sub_test_lib_test () { ) } +test_expect_success 'pretend we have a fully passing test suite' " + run_sub_test_lib_test full-pass '3 passing tests' <<-\\EOF && + for i in 1 2 3 + do + test_expect_success \"passing test #\$i\" 'true' + done + test_done + EOF + check_sub_test_lib_test full-pass <<-\\EOF + > ok 1 - passing test #1 + > ok 2 - passing test #2 + > ok 3 - passing test #3 + > # passed all 3 test(s) + > 1..3 + EOF +" + +test_expect_success 'pretend we have a partially passing test suite' " + test_must_fail run_sub_test_lib_test \ + partial-pass '2/3 tests passing' <<-\\EOF && + test_expect_success 'passing test #1' 'true' + test_expect_success 'failing test #2' 'false' + test_expect_success 'passing test #3' 'true' + test_done + EOF + check_sub_test_lib_test partial-pass <<-\\EOF + > ok 1 - passing test #1 + > not ok 2 - failing test #2 + # false + > ok 3 - passing test #3 + > # failed 1 among 3 test(s) + > 1..3 + EOF +" + +test_expect_success 'pretend we have a known breakage' " + run_sub_test_lib_test failing-todo 'A failing TODO test' <<-\\EOF && + test_expect_success 'passing test' 'true' + test_expect_failure 'pretend we have a known breakage' 'false' + test_done + EOF + check_sub_test_lib_test failing-todo <<-\\EOF + > ok 1 - passing test + > not ok 2 - pretend we have a known breakage # TODO known breakage + > # still have 1 known breakage(s) + > # passed all remaining 1 test(s) + > 1..2 + EOF +" + test_expect_success 'pretend we have fixed a known breakage' " run_sub_test_lib_test passing-todo 'A passing TODO test' <<-\\EOF && test_expect_failure 'pretend we have fixed a known breakage' 'true' @@ -102,6 +152,61 @@ test_expect_success 'pretend we have fixed a known breakage' " EOF " +test_expect_success 'pretend we have a pass, fail, and known breakage' " + test_must_fail run_sub_test_lib_test \ + mixed-results1 'mixed results #1' <<-\\EOF && + test_expect_success 'passing test' 'true' + test_expect_success 'failing test' 'false' + test_expect_failure 'pretend we have a known breakage' 'false' + test_done + EOF + check_sub_test_lib_test mixed-results1 <<-\\EOF + > ok 1 - passing test + > not ok 2 - failing test + > # false + > not ok 3 - pretend we have a known breakage # TODO known breakage + > # still have 1 known breakage(s) + > # failed 1 among remaining 2 test(s) + > 1..3 + EOF +" + +test_expect_success 'pretend we have a mix of all possible results' " + test_must_fail run_sub_test_lib_test \ + mixed-results2 'mixed results #2' <<-\\EOF && + test_expect_success 'passing test' 'true' + test_expect_success 'passing test' 'true' + test_expect_success 'passing test' 'true' + test_expect_success 'passing test' 'true' + test_expect_success 'failing test' 'false' + test_expect_success 'failing test' 'false' + test_expect_success 'failing test' 'false' + test_expect_failure 'pretend we have a known breakage' 'false' + test_expect_failure 'pretend we have a known breakage' 'false' + test_expect_failure 'pretend we have fixed a known breakage' 'true' + test_done + EOF + check_sub_test_lib_test mixed-results2 <<-\\EOF + > ok 1 - passing test + > ok 2 - passing test + > ok 3 - passing test + > ok 4 - passing test + > not ok 5 - failing test + > # false + > not ok 6 - failing test + > # false + > not ok 7 - failing test + > # false + > not ok 8 - pretend we have a known breakage # TODO known breakage + > not ok 9 - pretend we have a known breakage # TODO known breakage + > ok 10 - pretend we have fixed a known breakage # TODO known breakage + > # fixed 1 known breakage(s) + > # still have 2 known breakage(s) + > # failed 3 among remaining 8 test(s) + > 1..10 + EOF +" + test_set_prereq HAVEIT haveit=no test_expect_success HAVEIT 'test runs if prerequisite is satisfied' ' -- 1.8.1.rc2.225.g8d36ab4 ``` ## Junio C Hamano, 2012-12-21 03:12 Subject: [PATCH v7 7/7] tests: paint unexpectedly fixed known breakages in bold red Message-ID: <1356059558-23479-8-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1356059558-23479-8-git-send-email-gitster%40pobox.com In-Reply-To: <1356059558-23479-1-git-send-email-gitster@pobox.com> ``` From: Adam Spiers Change color of unexpectedly fixed known breakages to bold red. An unexpectedly passing test indicates that the test code is somehow broken or out of sync with the code it is testing. Either way this is an error which is potentially as bad as a failing test, and as such is no longer portrayed as a pass in the output. Signed-off-by: Adam Spiers Signed-off-by: Junio C Hamano --- t/t0000-basic.sh | 30 ++++++++++++++++++++++++------ t/test-lib.sh | 13 +++++++++---- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index 384b0ae..8973d2c 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -145,13 +145,31 @@ test_expect_success 'pretend we have fixed a known breakage' " test_done EOF check_sub_test_lib_test passing-todo <<-\\EOF - > ok 1 - pretend we have fixed a known breakage # TODO known breakage - > # fixed 1 known breakage(s) - > # passed all 1 test(s) + > ok 1 - pretend we have fixed a known breakage # TODO known breakage vanished + > # 1 known breakage(s) vanished; please update test(s) > 1..1 EOF " +test_expect_success 'pretend we have fixed one of two known breakages (run in sub test-lib)' " + run_sub_test_lib_test partially-passing-todos \ + '2 TODO tests, one passing' <<-\\EOF && + test_expect_failure 'pretend we have a known breakage' 'false' + test_expect_success 'pretend we have a passing test' 'true' + test_expect_failure 'pretend we have fixed another known breakage' 'true' + test_done + EOF + check_sub_test_lib_test partially-passing-todos <<-\\EOF + > not ok 1 - pretend we have a known breakage # TODO known breakage + > ok 2 - pretend we have a passing test + > ok 3 - pretend we have fixed another known breakage # TODO known breakage vanished + > # 1 known breakage(s) vanished; please update test(s) + > # still have 1 known breakage(s) + > # passed all remaining 1 test(s) + > 1..3 + EOF +" + test_expect_success 'pretend we have a pass, fail, and known breakage' " test_must_fail run_sub_test_lib_test \ mixed-results1 'mixed results #1' <<-\\EOF && @@ -199,10 +217,10 @@ test_expect_success 'pretend we have a mix of all possible results' " > # false > not ok 8 - pretend we have a known breakage # TODO known breakage > not ok 9 - pretend we have a known breakage # TODO known breakage - > ok 10 - pretend we have fixed a known breakage # TODO known breakage - > # fixed 1 known breakage(s) + > ok 10 - pretend we have fixed a known breakage # TODO known breakage vanished + > # 1 known breakage(s) vanished; please update test(s) > # still have 2 known breakage(s) - > # failed 3 among remaining 8 test(s) + > # failed 3 among remaining 7 test(s) > 1..10 EOF " diff --git a/t/test-lib.sh b/t/test-lib.sh index 8b75c9a..30a0937 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -278,7 +278,7 @@ test_failure_ () { test_known_broken_ok_ () { test_fixed=$(($test_fixed+1)) - say_color "" "ok $test_count - $@ # TODO known breakage" + say_color error "ok $test_count - $@ # TODO known breakage vanished" } test_known_broken_failure_ () { @@ -373,13 +373,18 @@ test_done () { if test "$test_fixed" != 0 then - say_color pass "# fixed $test_fixed known breakage(s)" + say_color error "# $test_fixed known breakage(s) vanished; please update test(s)" fi if test "$test_broken" != 0 then say_color warn "# still have $test_broken known breakage(s)" - msg="remaining $(($test_count-$test_broken)) test(s)" + fi + if test "$test_broken" != 0 || test "$test_fixed" != 0 + then + test_remaining=$(( $test_count - $test_broken - $test_fixed )) + msg="remaining $test_remaining test(s)" else + test_remaining=$test_count msg="$test_count test(s)" fi case "$test_failure" in @@ -393,7 +398,7 @@ test_done () { if test $test_external_has_tap -eq 0 then - if test $test_count -gt 0 + if test $test_remaining -gt 0 then say_color pass "# passed all $msg" fi -- 1.8.1.rc2.225.g8d36ab4 ``` ## Jeff King, 2012-12-21 08:15 Subject: Re: [PATCH v7 0/7] coloring test output after traffic signal Message-ID: <20121221081521.GA22193@sigill.intra.peff.net> URL: https://gitlist.dev/e/20121221081521.GA22193%40sigill.intra.peff.net In-Reply-To: <1356059558-23479-1-git-send-email-gitster@pobox.com> ``` On Thu, Dec 20, 2012 at 07:12:31PM -0800, Junio C Hamano wrote: > To conclude the bikeshedding discussion we had today, here is what I > queued by squashing stuff into relevant patches, so that people can > eyeball the result for the last time. Thanks, this looks OK to me. And thank you, Adam, for your patience. Seven iterations of color bikeshedding is more than should be asked of anyone. :) -Peff ``` ## Stefano Lattarini, 2012-12-21 08:51 Subject: Re: [PATCH v7 2/7] tests: paint known breakages in yellow Message-ID: <50D4230F.9080502@gmail.com> URL: https://gitlist.dev/e/50D4230F.9080502%40gmail.com In-Reply-To: <1356059558-23479-3-git-send-email-gitster@pobox.com> ``` On 12/21/2012 04:12 AM, Junio C Hamano wrote: > From: Adam Spiers > > Yellow seems a more appropriate color than bold green when > considering the universal traffic lights coloring scheme, where > green conveys the impression that everything's OK, and amber that > something's not quite right. > Here are few more details about the behaviour of other testing tools, in case you want to squash them in the commit message for future references: 1. Automake (at least up to 1.13) and Autotest (at least up to the 2.69 Autoconf release) use "bold" green for reporting expected failures. 2. On the other hand, the 'prove' utility (as of TAP::Harness v3.23 and Perl v5.14.2) use yellow (not bold) for the same purpose. Regards, Stefano ``` ## Adam Spiers, 2012-12-21 10:00 Subject: Re: [PATCH v7 0/7] coloring test output after traffic signal Message-ID: URL: https://gitlist.dev/e/CAOkDyE8ac0jCutt-hP-p-Bk%2BPqVda0diXkOANrPbMpT5XTy2GA%40mail.gmail.com In-Reply-To: <20121221081521.GA22193@sigill.intra.peff.net> ``` On Fri, Dec 21, 2012 at 8:15 AM, Jeff King wrote: > On Thu, Dec 20, 2012 at 07:12:31PM -0800, Junio C Hamano wrote: >> To conclude the bikeshedding discussion we had today, here is what I >> queued by squashing stuff into relevant patches, so that people can >> eyeball the result for the last time. Great, thanks a lot Junio. > Thanks, this looks OK to me. To me too. > And thank you, Adam, for your patience. Seven iterations of color > bikeshedding is more than should be asked of anyone. :) ;-) ``` ## Junio C Hamano, 2012-12-21 15:46 Subject: Re: [PATCH v7 2/7] tests: paint known breakages in yellow Message-ID: <7vd2y32ys0.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd2y32ys0.fsf%40alter.siamese.dyndns.org In-Reply-To: <50D4230F.9080502@gmail.com> ``` Stefano Lattarini writes: > On 12/21/2012 04:12 AM, Junio C Hamano wrote: >> From: Adam Spiers >> >> Yellow seems a more appropriate color than bold green when >> considering the universal traffic lights coloring scheme, where >> green conveys the impression that everything's OK, and amber that >> something's not quite right. >> > Here are few more details about the behaviour of other testing > tools, in case you want to squash them in the commit message for > future references: > > 1. Automake (at least up to 1.13) and Autotest (at least up to the > 2.69 Autoconf release) use "bold" green for reporting expected > failures. > > 2. On the other hand, the 'prove' utility (as of TAP::Harness v3.23 > and Perl v5.14.2) use yellow (not bold) for the same purpose. Nice to know, thanks. I re-read the above three times, trying to see how to add it to the log message, but having hard time phrasing it. The only thing the additional knowledge adds seems to be to give rationale for the old choice of "bold green"---it was not chosen from thin-air but can be viewed as following the automake/autotest scheme, and other systems cannot agree on what color to pick for this purpose. I do not see a need to justify why we chose differently from automake/autotest; we could say something like: Yellow seems a more appropriate color than bold green when considering the universal traffic lights coloring scheme, where green conveys the impression that everything's OK, and amber that something's not quite right. This is in line with what 'prove' uses, but different from 'automake/autotest' do. but we are not in the business of choosing which is more correct between prove and automake/autotest, and I do not see how it adds much value to tell readers that color choices are not universally agreed upon across various test software suites---that's kind of known, isn't it? So... ``` ## Stefano Lattarini, 2012-12-21 16:59 Subject: Re: [PATCH v7 2/7] tests: paint known breakages in yellow Message-ID: <50D49588.60305@gmail.com> URL: https://gitlist.dev/e/50D49588.60305%40gmail.com In-Reply-To: <7vd2y32ys0.fsf@alter.siamese.dyndns.org> ``` On 12/21/2012 04:46 PM, Junio C Hamano wrote: > > [SNIP] > > The only thing the additional knowledge adds seems to be to give > rationale for the old choice of "bold green"---it was not chosen > from thin-air but can be viewed as following the automake/autotest > scheme, and other systems cannot agree on what color to pick for > this purpose. > > I do not see a need to justify why we chose differently from > automake/autotest; we could say something like: > > Yellow seems a more appropriate color than bold green when > considering the universal traffic lights coloring scheme, where > green conveys the impression that everything's OK, and amber that > something's not quite right. This is in line with what 'prove' > uses, but different from 'automake/autotest' do. > > but we are not in the business of choosing which is more correct > between prove and automake/autotest, and I do not see how it adds > much value to tell readers that color choices are not universally > agreed upon across various test software suites---that's kind of > known, isn't it? > > So... > That is fine with me, I just pointed it out because I suspected not everybody was aware of all these details. If you decide they don't matter, it's perfectly OK -- but at least now it's an informed choice ;-) Thanks, Stefano ```