{"thread":{"id":"31563","subject":"[PATCH] Make test output coloring more intuitive","startedAt":"2012-09-17T11:50:37Z","lastAt":"2012-11-11T02:04:14Z","messageCount":18,"participants":["Adam Spiers","Jeff King","Junio C Hamano","Stefano Lattarini","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"199215","messageId":"1347882637-21766-1-git-send-email-git@adamspiers.org","threadId":"31563","inReplyTo":null,"subject":"[PATCH] Make test output coloring more intuitive","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-17T11:50:37Z","receivedAt":"2012-09-17T11:50:37Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"1. Change the color of individual known breakages from bold green to\n bold yellow. This seems more appropriate when considering the\n universal traffic lights coloring scheme, where green conveys the\n impression that everything's OK, and amber that something's not\n quite right.\n\n2. Likewise, change the color of the summarized total number of known\n breakages from bold red to bold yellow to be less alarmist and more\n consistent with the above.\n\n3. Change color of unexpectedly fixed known breakages to bold red. An\n unexpectedly passing test indicates that the test is wrong or the\n semantics of the code being tested have changed. Either way this\n is an error which is arguably as bad as a failing test, and as such\n is now counted in the totals too.\n\nThe end result of these changes is that:\n\n - red is _only_ used for things which have gone unexpectedly wrong:\n test failures, unexpected test passes, and failures with the\n framework,\n\n - yellow is _only_ used for known breakages, and\n\n - green is _only_ used for things which have gone to plan and\n require no further work to be done.\n\nSigned-off-by: Adam Spiers \n---\n t/t0000-basic.sh | 7 ++++---\n t/test-lib.sh | 13 ++++++++-----\n 2 files changed, 12 insertions(+), 8 deletions(-)\n mode change 100644 => 100755 t/test-lib.sh\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex ae6a3f0..4e111b4 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -81,9 +81,10 @@ test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib\n \t./passing-todo.sh >out 2>err &&\n \t! test -s err &&\n \tsed -e 's/^> //' >expect <<-\\\\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> # fixed 1 known breakage(s); please update test(s)\n+\t> # still have 1 known breakage(s)\n+\t> # passed all remaining 0 test(s)\n \t> 1..1\n \tEOF\n \ttest_cmp expect out)\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nold mode 100644\nnew mode 100755\nindex f8e3733..9907035\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -183,10 +183,12 @@ then\n \t\t\ttput bold; tput setaf 1;; # bold red\n \t\tskip)\n \t\t\ttput bold; tput setaf 2;; # bold green\n+\t\twarn)\n+\t\t\ttput bold; tput setaf 3;; # bold yellow\n \t\tpass)\n \t\t\ttput setaf 2;; # green\n \t\tinfo)\n-\t\t\ttput setaf 3;; # brown\n+\t\t\ttput setaf 3;; # yellow/brown\n \t\t*)\n \t\t\ttest -n \"$quiet\" && return;;\n \t\tesac\n@@ -276,12 +278,13 @@ test_failure_ () {\n \n test_known_broken_ok_ () {\n \ttest_fixed=$(($test_fixed+1))\n-\tsay_color \"\" \"ok $test_count - $@ # TODO known breakage\"\n+\ttest_broken=$(($test_broken+1))\n+\tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n }\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@@ -371,11 +374,11 @@ 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 \"# fixed $test_fixed known breakage(s); please update test(s)\"\n \tfi\n \tif test \"$test_broken\" != 0\n \tthen\n-\t\tsay_color error \"# still have $test_broken known breakage(s)\"\n+\t\tsay_color warn \"# still have $test_broken known breakage(s)\"\n \t\tmsg=\"remaining $(($test_count-$test_broken)) test(s)\"\n \telse\n \t\tmsg=\"$test_count test(s)\"\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199275","messageId":"20120917201119.GB24888@sigill.intra.peff.net","threadId":"31563","inReplyTo":"1347882637-21766-1-git-send-email-git@adamspiers.org","subject":"Re: [PATCH] Make test output coloring more intuitive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-17T20:11:19Z","receivedAt":"2012-09-17T20:11:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 17, 2012 at 12:50:37PM +0100, Adam Spiers wrote:\n\n> The end result of these changes is that:\n> \n> - red is _only_ used for things which have gone unexpectedly wrong:\n> test failures, unexpected test passes, and failures with the\n> framework,\n> \n> - yellow is _only_ used for known breakages, and\n> \n> - green is _only_ used for things which have gone to plan and\n> require no further work to be done.\n\nSounds reasonable, and I think the new output looks nice. I notice that\nskipped tests are still in green. I wonder if they should be in yellow,\ntoo. They may or may not be a problem, but you are failing to run some\nportion of the test suite.\n\n> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n> index ae6a3f0..4e111b4 100755\n> --- a/t/t0000-basic.sh\n> +++ b/t/t0000-basic.sh\n> @@ -81,9 +81,10 @@ test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib\n> \t./passing-todo.sh >out 2>err &&\n> \t! test -s err &&\n> \tsed -e 's/^> //' >expect <<-\\\\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> # fixed 1 known breakage(s); please update test(s)\n> +\t> # still have 1 known breakage(s)\n> +\t> # passed all remaining 0 test(s)\n> \t> 1..1\n> \tEOF\n> \ttest_cmp expect out)\n\nThis hunk is surprising after reading the commit message. It looks like\nyou are also breaking down expect_fail tests by fixed and not fixed.\n\nI think that is probably an OK thing to do (although it might make more\nsense in a separate patch), but are your numbers right? It looks from\nthat count as if there are 2 tests expecting failure (one fixed and one\nstill broken).\n\n-Peff\n"},{"id":"199285","messageId":"7vboh4tluo.fsf@alter.siamese.dyndns.org","threadId":"31563","inReplyTo":"1347882637-21766-1-git-send-email-git@adamspiers.org","subject":"Re: [PATCH] Make test output coloring more intuitive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-17T20:50:39Z","receivedAt":"2012-09-17T20:50:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> 1. Change the color of individual known breakages from bold green to\n> bold yellow. This seems more appropriate when considering the\n> universal traffic lights coloring scheme, where green conveys the\n> impression that everything's OK, and amber that something's not\n> quite right.\n>\n> 2. Likewise, change the color of the summarized total number of known\n> breakages from bold red to bold yellow to be less alarmist and more\n> consistent with the above.\n>\n> 3. Change color of unexpectedly fixed known breakages to bold red. An\n> unexpectedly passing test indicates that the test is wrong or the\n> semantics of the code being tested have changed. Either way this\n> is an error which is arguably as bad as a failing test, and as such\n> is now counted in the totals too.\n\nI agree with Peff's comments.\n\nThe point #3 above wants to be a separate patch; we may even want to\nconsider a follow-up change to add an option to make a \"test that is\nexpected to fail did not fail\" case a failure.\n\n> test_known_broken_ok_ () {\n> \ttest_fixed=$(($test_fixed+1))\n> -\tsay_color \"\" \"ok $test_count - $@ # TODO known breakage\"\n> +\ttest_broken=$(($test_broken+1))\n> +\tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n> }\n\nAlso I wonder if this is still a \"TODO\". \"# TODO fixed known breakage\",\nmeaning that it is something that must be looked at by whoever happened\nto have fixed the known breakage by accident, might be a better wording.\n"},{"id":"199413","messageId":"20120918212122.GA2567@atlantic.linksys.moosehall","threadId":"31563","inReplyTo":"20120917201119.GB24888@sigill.intra.peff.net","subject":"Re: [PATCH] Make test output coloring more intuitive","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-18T21:21:22Z","receivedAt":"2012-09-18T21:21:22Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Mon, Sep 17, 2012 at 04:11:19PM -0400, Jeff King wrote:\n> On Mon, Sep 17, 2012 at 12:50:37PM +0100, Adam Spiers wrote:\n> \n> > The end result of these changes is that:\n> > \n> > - red is _only_ used for things which have gone unexpectedly wrong:\n> > test failures, unexpected test passes, and failures with the\n> > framework,\n> > \n> > - yellow is _only_ used for known breakages, and\n> > \n> > - green is _only_ used for things which have gone to plan and\n> > require no further work to be done.\n> \n> Sounds reasonable, and I think the new output looks nice. I notice that\n> skipped tests are still in green. I wonder if they should be in yellow,\n> too. They may or may not be a problem, but you are failing to run some\n> portion of the test suite.\n\nFair point, I'll reroll the series and change skipped tests to yellow\n(non-bold, to distinguish from known breakages which are bold yellow).\n\n> > diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n> > index ae6a3f0..4e111b4 100755\n> > --- a/t/t0000-basic.sh\n> > +++ b/t/t0000-basic.sh\n> > @@ -81,9 +81,10 @@ test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib\n> > \t./passing-todo.sh >out 2>err &&\n> > \t! test -s err &&\n> > \tsed -e 's/^> //' >expect <<-\\\\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> # fixed 1 known breakage(s); please update test(s)\n> > +\t> # still have 1 known breakage(s)\n> > +\t> # passed all remaining 0 test(s)\n> > \t> 1..1\n> > \tEOF\n> > \ttest_cmp expect out)\n> \n> This hunk is surprising after reading the commit message. It looks like\n> you are also breaking down expect_fail tests by fixed and not fixed.\n\nCorrect.\n\n> I think that is probably an OK thing to do (although it might make more\n> sense in a separate patch), but are your numbers right?\n\nThey are right (at least as I intended), but I agree it's a bit\nconfusing.\n\n> It looks from that count as if there are 2 tests expecting failure\n> (one fixed and one still broken).\n\nIt's actually one test which is both fixed *and* in some sense broken.\nThe confusion arises from the ambiguity of the word \"broken\", which\ncould mean either \"failed as expected\" or \"expected to fail but\ndidn't\". Previously it was just the former, but my patch changed it\nto encompass both cases. The motivation behind this was to avoid the\n\n # passed all $count test(s)\n\nsummary message which is overly comforting when one or more tests were\nexpected to fail but didn't. However perhaps it's cleaner to keep the\ncounter buckets separated. I'll try to come up with a better\nsolution.\n"},{"id":"199415","messageId":"20120918213617.GB2567@atlantic.linksys.moosehall","threadId":"31563","inReplyTo":"7vboh4tluo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Make test output coloring more intuitive","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-18T21:36:17Z","receivedAt":"2012-09-18T21:36:17Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Mon, Sep 17, 2012 at 01:50:39PM -0700, Junio C Hamano wrote:\n> Adam Spiers writes:\n> \n> > 1. Change the color of individual known breakages from bold green to\n> > bold yellow. This seems more appropriate when considering the\n> > universal traffic lights coloring scheme, where green conveys the\n> > impression that everything's OK, and amber that something's not\n> > quite right.\n> >\n> > 2. Likewise, change the color of the summarized total number of known\n> > breakages from bold red to bold yellow to be less alarmist and more\n> > consistent with the above.\n> >\n> > 3. Change color of unexpectedly fixed known breakages to bold red. An\n> > unexpectedly passing test indicates that the test is wrong or the\n> > semantics of the code being tested have changed. Either way this\n> > is an error which is arguably as bad as a failing test, and as such\n> > is now counted in the totals too.\n> \n> I agree with Peff's comments.\n> \n> The point #3 above wants to be a separate patch;\n\nOK, re-rolling this.\n\n> we may even want to consider a follow-up change to add an option to\n> make a \"test that is expected to fail did not fail\" case a failure.\n\nSounds like a nice idea.\n\n> > test_known_broken_ok_ () {\n> > \ttest_fixed=$(($test_fixed+1))\n> > -\tsay_color \"\" \"ok $test_count - $@ # TODO known breakage\"\n> > +\ttest_broken=$(($test_broken+1))\n> > +\tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n> > }\n> \n> Also I wonder if this is still a \"TODO\".\n\nHah, I should trust my first instincts more; my first version of the\npatch dropped the \"TODO\", but then I put it back in because I thought\npeople would object :-)\n\n> \"# TODO fixed known breakage\", meaning that it is something that\n> must be looked at by whoever happened to have fixed the known\n> breakage by accident, might be a better wording.\n\nI would challenge the assumption that the test passed because someone\ndeliberately fixed the code it was testing. Whilst this is a probable\nscenario (e.g. if they forgot to adjust the test to expect success\nrather than failure), it's not the only one. Tests can be brittle and\ndepend on external factors which can change unexpectedly. For\nexample, I noticed that the final 'general options plus command' test\nin t9902-completion.sh is currently dependent on the contents of the\ncurrent $PATH. This is because I have a shell-script called\n`git-check-email' on mine, so `check-email' becomes an extra\nsubcommand completion possibility for the prefix `check'. Almost\nevery other developer running this test would not encounter the same\nfailure. It's not a huge stretch to imagine a similar corner case\nwhich could unexpectedly cause a test to pass when failure was\nexpected.\n\nTherefore I would suggest a wording which avoids implying that\nsomething was deliberately fixed. That was my reasoning behind\nintroducing the word \"vanished\" to the output.\n"},{"id":"199418","messageId":"20120918215929.GA30346@sigill.intra.peff.net","threadId":"31563","inReplyTo":"20120918213617.GB2567@atlantic.linksys.moosehall","subject":"Re: [PATCH] Make test output coloring more intuitive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-18T21:59:29Z","receivedAt":"2012-09-18T21:59:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 18, 2012 at 10:36:17PM +0100, Adam Spiers wrote:\n\n> > > +\tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n> > > }\n> > \n> > Also I wonder if this is still a \"TODO\".\n> \n> Hah, I should trust my first instincts more; my first version of the\n> patch dropped the \"TODO\", but then I put it back in because I thought\n> people would object :-)\n\nTODO is a special token[1] respected by TAP harnesses like \"prove\". I'm\nnot sure what practical impact it has, but it should probably remain.\n\n-Peff\n\n[1] http://testanything.org/wiki/index.php/TAP_specification#TODO_tests\n"},{"id":"199419","messageId":"CAOkDyE_oMz9ZHuHTBH3_Q_75qvzqiNcfFnt6orcj1dJ5uDsWtA@mail.gmail.com","threadId":"31563","inReplyTo":"20120918215929.GA30346@sigill.intra.peff.net","subject":"Re: [PATCH] Make test output coloring more intuitive","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-18T22:14:07Z","receivedAt":"2012-09-18T22:14:07Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Tue, Sep 18, 2012 at 10:59 PM, Jeff King wrote:\n> TODO is a special token[1] respected by TAP harnesses like \"prove\". I'm\n> not sure what practical impact it has, but it should probably remain.\n>\n> -Peff\n>\n> [1] http://testanything.org/wiki/index.php/TAP_specification#TODO_tests\n\nThanks, I didn't know that. For reasons I just stated elsewhere in\nthis thread, I don't _completely_ agree with the specification where\nit says:\n\n \"Should a todo test point begin succeeding, the harness should\n report it as a bonus. This indicates that whatever you were\n supposed to do has been done and you should promote this to a\n normal test point.\"\n\nHowever those semantics are well-defined enough to warrant keeping the\n\"TODO\" as you suggest.\n"},{"id":"199525","messageId":"505A24EC.1000203@gmail.com","threadId":"31563","inReplyTo":"20120917201119.GB24888@sigill.intra.peff.net","subject":"Re: [PATCH] Make test output coloring more intuitive","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-09-19T20:02:52Z","receivedAt":"2012-09-19T20:02:52Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 09/17/2012 10:11 PM, Jeff King wrote:\n> On Mon, Sep 17, 2012 at 12:50:37PM +0100, Adam Spiers wrote:\n> \n>> The end result of these changes is that:\n>>\n>> - red is _only_ used for things which have gone unexpectedly wrong:\n>> test failures, unexpected test passes, and failures with the\n>> framework,\n>>\n>> - yellow is _only_ used for known breakages, and\n>>\n>> - green is _only_ used for things which have gone to plan and\n>> require no further work to be done.\n> \n> Sounds reasonable, and I think the new output looks nice. I notice that\n> skipped tests are still in green. I wonder if they should be in yellow,\n> too.\n>\nWhat about blue instead? This would keep the colouring scheme more\nconsistent with the one used by prove:\n \nby autotest:\n \nand by the Automake-generated test harness:\n \n\nJust my 2 cents,\n Stefano\n"},{"id":"199527","messageId":"20120919201201.GD19246@atlantic.linksys.moosehall","threadId":"31563","inReplyTo":"505A24EC.1000203@gmail.com","subject":"Re: [PATCH] Make test output coloring more intuitive","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T20:12:01Z","receivedAt":"2012-09-19T20:12:01Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Wed, Sep 19, 2012 at 10:02:52PM +0200, Stefano Lattarini wrote:\n> On 09/17/2012 10:11 PM, Jeff King wrote:\n> > On Mon, Sep 17, 2012 at 12:50:37PM +0100, Adam Spiers wrote:\n> > \n> >> The end result of these changes is that:\n> >>\n> >> - red is _only_ used for things which have gone unexpectedly wrong:\n> >> test failures, unexpected test passes, and failures with the\n> >> framework,\n> >>\n> >> - yellow is _only_ used for known breakages, and\n> >>\n> >> - green is _only_ used for things which have gone to plan and\n> >> require no further work to be done.\n> > \n> > Sounds reasonable, and I think the new output looks nice. I notice that\n> > skipped tests are still in green. I wonder if they should be in yellow,\n> > too.\n> >\n> What about blue instead? This would keep the colouring scheme more\n> consistent with the one used by prove:\n> \n> by autotest:\n> \n> and by the Automake-generated test harness:\n> \n\nSounds good to me! Blue is the conventional color for informational\nsigns, so seems like a natural fit for skipped tests. Not sure\nwhether it should be bold or not though? I'll wait for a more solid\ngroup consensus before re-rolling yet another patch :-)\n"},{"id":"199528","messageId":"20120919201326.GA23016@sigill.intra.peff.net","threadId":"31563","inReplyTo":"20120919201201.GD19246@atlantic.linksys.moosehall","subject":"Re: [PATCH] Make test output coloring more intuitive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-19T20:13:26Z","receivedAt":"2012-09-19T20:13:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 19, 2012 at 09:12:01PM +0100, Adam Spiers wrote:\n\n> > > Sounds reasonable, and I think the new output looks nice. I notice that\n> > > skipped tests are still in green. I wonder if they should be in yellow,\n> > > too.\n> > >\n> > What about blue instead? This would keep the colouring scheme more\n> > consistent with the one used by prove:\n> > \n> > by autotest:\n> > \n> > and by the Automake-generated test harness:\n> > \n> \n> Sounds good to me! Blue is the conventional color for informational\n> signs, so seems like a natural fit for skipped tests. Not sure\n> whether it should be bold or not though? I'll wait for a more solid\n> group consensus before re-rolling yet another patch :-)\n\nBlue would be fine with me. No strong opinion on bold or not (my gut is\nthat most things should not be bold unless there is a good reason, but\nthat is just a feeling).\n\n-Peff\n"},{"id":"199531","messageId":"1348086263-27555-1-git-send-email-git@adamspiers.org","threadId":"31563","inReplyTo":"20120919201326.GA23016@sigill.intra.peff.net","subject":"[PATCH v4 3/6] Color skipped tests blue","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-19T20:24:23Z","receivedAt":"2012-09-19T20:24:23Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Skipped tests indicate incomplete test coverage. Whilst this is\nnot a test failure or other error, it's still not complete\nsuccess, so according to the universal traffic lights coloring\nscheme, yellow/brown seems more suitable than green. However,\nit's more informational than cautionary, so instead we use blue\nwhich is a universal color for information signs.\n\nSigned-off-by: Adam Spiers \n---\n t/test-lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 5293830..78c88c2 100755\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -182,13 +182,13 @@ then\n \t\terror)\n \t\t\ttput bold; tput setaf 1;; # bold red\n \t\tskip)\n-\t\t\ttput bold; tput setaf 2;; # bold green\n+\t\t\ttput setaf 4;; # blue\n \t\twarn)\n \t\t\ttput bold; tput setaf 3;; # bold yellow\n \t\tpass)\n \t\t\ttput setaf 2;; # green\n \t\tinfo)\n-\t\t\ttput setaf 3;; # brown\n+\t\t\ttput setaf 3;; # yellow/brown\n \t\t*)\n \t\t\ttest -n \"$quiet\" && return;;\n \t\tesac\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199570","messageId":"505AAE34.2020408@viscovery.net","threadId":"31563","inReplyTo":"1348086263-27555-1-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v4 3/6] Color skipped tests blue","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-09-20T05:48:36Z","receivedAt":"2012-09-20T05:48:36Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 9/19/2012 22:24, schrieb Adam Spiers:\n> \t\tskip)\n> -\t\t\ttput bold; tput setaf 2;; # bold green\n> +\t\t\ttput setaf 4;; # blue\n\nIt's unreadable on black background. Keep it bold; that works on both\nblack and white background.\n\n-- Hannes\n"},{"id":"199577","messageId":"CAOkDyE8uu4DWqKKAQTEcBCjmWXJfNueyJQuvJhAUaqnsihpkUg@mail.gmail.com","threadId":"31563","inReplyTo":"505AAE34.2020408@viscovery.net","subject":"Re: [PATCH v4 3/6] Color skipped tests blue","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-20T09:04:36Z","receivedAt":"2012-09-20T09:04:36Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Thu, Sep 20, 2012 at 6:48 AM, Johannes Sixt wrote:\n> Am 9/19/2012 22:24, schrieb Adam Spiers:\n>> skip)\n>> - tput bold; tput setaf 2;; # bold green\n>> + tput setaf 4;; # blue\n>\n> It's unreadable on black background. Keep it bold; that works on both\n> black and white background.\n\nWhilst my preference aligns with yours in this particular case, we are\nnow on a slippery slope, since these days terminal colors are\ninfinitely configurable, and some heretics even choose backgrounds\nwhich are neither black nor white ;-) I don't want to trigger a long\ndiscussion or end up spamming the list with lots of different color\nscheme patches in an attempt to please everyone. So bold blue will be\nmy last version of this patch.\n"},{"id":"199578","messageId":"1348132118-23139-1-git-send-email-git@adamspiers.org","threadId":"31563","inReplyTo":"505AAE34.2020408@viscovery.net","subject":"[PATCH v5 3/3] Color skipped tests bold blue","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-09-20T09:08:38Z","receivedAt":"2012-09-20T09:08:38Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Skipped tests indicate incomplete test coverage. Whilst this is\nnot a test failure or other error, it's still not complete\nsuccess, so according to the universal traffic lights coloring\nscheme, yellow/brown seems more suitable than green. However,\nit's more informational than cautionary, so instead we use blue\nwhich is a universal color for information signs. Bold blue\nshould work better on both black and white backgrounds than\nnormal blue.\n\nSigned-off-by: Adam Spiers \n---\n t/test-lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 5293830..5ef87d4 100755\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -182,13 +182,13 @@ then\n \t\terror)\n \t\t\ttput bold; tput setaf 1;; # bold red\n \t\tskip)\n-\t\t\ttput bold; tput setaf 2;; # bold green\n+\t\t\ttput bold; tput setaf 4;; # bold blue\n \t\twarn)\n \t\t\ttput bold; tput setaf 3;; # bold yellow\n \t\tpass)\n \t\t\ttput setaf 2;; # green\n \t\tinfo)\n-\t\t\ttput setaf 3;; # brown\n+\t\t\ttput setaf 3;; # yellow/brown\n \t\t*)\n \t\t\ttest -n \"$quiet\" && return;;\n \t\tesac\n-- \n1.7.12.147.g6d168f4\n"},{"id":"199579","messageId":"505AEB1B.7010702@gmail.com","threadId":"31563","inReplyTo":"1348132118-23139-1-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v5 3/3] Color skipped tests bold blue","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-09-20T10:08:27Z","receivedAt":"2012-09-20T10:08:27Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"Hi Adam.\n\nOn 09/20/2012 11:08 AM, Adam Spiers wrote:\n> Skipped tests indicate incomplete test coverage. Whilst this is\n> not a test failure or other error, it's still not complete\n> success, so according to the universal traffic lights coloring\n> scheme, yellow/brown seems more suitable than green. However,\n> it's more informational than cautionary, so instead we use blue\n> which is a universal color for information signs. Bold blue\n> should work better on both black and white backgrounds than\n> normal blue.\n>\nA very minor nit (feel free to ignore it): IMHO, it should be nice\nto state explicitly in the commit message that blue is already used\nby other testsuite-related software to highlight skipped tests; you\ncan report the examples of at least Automake, Autotest and prove --\nand extra kudos if you find further examples ;-)\n\nThanks,\n Stefano\n"},{"id":"199585","messageId":"7va9wkd5u6.fsf@alter.siamese.dyndns.org","threadId":"31563","inReplyTo":"505AEB1B.7010702@gmail.com","subject":"Re: [PATCH v5 3/3] Color skipped tests bold blue","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-20T16:20:01Z","receivedAt":"2012-09-20T16:20:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefano Lattarini writes:\n\n> Hi Adam.\n>\n> On 09/20/2012 11:08 AM, Adam Spiers wrote:\n>> Skipped tests indicate incomplete test coverage. Whilst this is\n>> not a test failure or other error, it's still not complete\n>> success, so according to the universal traffic lights coloring\n>> scheme, yellow/brown seems more suitable than green. However,\n>> it's more informational than cautionary, so instead we use blue\n>> which is a universal color for information signs. Bold blue\n>> should work better on both black and white backgrounds than\n>> normal blue.\n>>\n> A very minor nit (feel free to ignore it): IMHO, it should be nice\n> to state explicitly in the commit message that blue is already used\n> by other testsuite-related software to highlight skipped tests; you\n> can report the examples of at least Automake, Autotest and prove --\n> and extra kudos if you find further examples ;-)\n\nThanks. Will tweak the proposed log message to include the above\nwhen queuing the updated patch.\n"},{"id":"199646","messageId":"20120921061325.GA15867@sigill.intra.peff.net","threadId":"31563","inReplyTo":"1348086263-27555-1-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v4 3/6] Color skipped tests blue","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-21T06:13:25Z","receivedAt":"2012-09-21T06:13:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 19, 2012 at 09:24:23PM +0100, Adam Spiers wrote:\n\n> t/test-lib.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 5293830..78c88c2 100755\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -182,13 +182,13 @@ then\n> \t\terror)\n> \t\t\ttput bold; tput setaf 1;; # bold red\n> \t\tskip)\n> -\t\t\ttput bold; tput setaf 2;; # bold green\n> +\t\t\ttput setaf 4;; # blue\n> \t\twarn)\n> \t\t\ttput bold; tput setaf 3;; # bold yellow\n> \t\tpass)\n> \t\t\ttput setaf 2;; # green\n> \t\tinfo)\n> -\t\t\ttput setaf 3;; # brown\n> +\t\t\ttput setaf 3;; # yellow/brown\n\nI happened to be running a test script with \"-v\" earlier today, and I\nnoticed that the \"expecting success...\" dump of the test contents is\nalso yellow. By your new rules, shouldn't it be blue?\n\nI think it is matching the \"info\" type, which from the discussion should\nbe blue, no?\n\nMaybe it is just my terminal. I see it is labeled as \"brown\" here, but\nit looks very yellow (and I am using the stock xterm colors. According\nto:\n\n https://en.wikipedia.org/wiki/ANSI_colors\n\nIt looks it really is brown on some platforms. I'm not sure if it is\nworth worrying about. I don't really want to get into configurable\ncolors just for the test-suite output.\n\n-Peff\n"},{"id":"202798","messageId":"20121111020355.GA11565@gmail.com","threadId":"31563","inReplyTo":"20120921061325.GA15867@sigill.intra.peff.net","subject":"Re: [PATCH v4 3/6] Color skipped tests blue","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-11-11T02:04:14Z","receivedAt":"2012-11-11T02:04:14Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Sep 21, 2012 at 02:13:25AM -0400, Jeff King wrote:\n> On Wed, Sep 19, 2012 at 09:24:23PM +0100, Adam Spiers wrote:\n> \n> > t/test-lib.sh | 4 ++--\n> > 1 file changed, 2 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/t/test-lib.sh b/t/test-lib.sh\n> > index 5293830..78c88c2 100755\n> > --- a/t/test-lib.sh\n> > +++ b/t/test-lib.sh\n> > @@ -182,13 +182,13 @@ then\n> > \t\terror)\n> > \t\t\ttput bold; tput setaf 1;; # bold red\n> > \t\tskip)\n> > -\t\t\ttput bold; tput setaf 2;; # bold green\n> > +\t\t\ttput setaf 4;; # blue\n> > \t\twarn)\n> > \t\t\ttput bold; tput setaf 3;; # bold yellow\n> > \t\tpass)\n> > \t\t\ttput setaf 2;; # green\n> > \t\tinfo)\n> > -\t\t\ttput setaf 3;; # brown\n> > +\t\t\ttput setaf 3;; # yellow/brown\n> \n> I happened to be running a test script with \"-v\" earlier today, and I\n> noticed that the \"expecting success...\" dump of the test contents is\n> also yellow. By your new rules, shouldn't it be blue?\n> \n> I think it is matching the \"info\" type, which from the discussion should\n> be blue, no?\n\nIt uses the \"default\" colour:\n\n say >&3 \"expecting success: $2\"\n\nwhere say is defined:\n\n say () {\n say_color info \"$*\"\n }\n\nMany other messages are output in this default colour too, and I never\nproposed to change it. The only time in the discussion where blue was\nassociated with \"info\" was in this sentence I wrote in the\ncommit message for the patch altering the colour of \"skip\" messages:\n\n \"However, it's more informational than cautionary, so instead we\n use blue which is a universal color for information signs.\"\n\nWhilst it could also be applied to \"info\", I don't think it would be a\ngood idea to have the \"skip\" and \"info\" colours *both* as bold blue.\nIt seems to me more important that the \"skip\" messages should visually\nstand out more than \"info\", since they are rarer and a more notable\nlevel of information than the latter (especially if --verbose is\nused). Additionally, yellow is already somewhat overloaded (yellow\nfor \"info\" and bold yellow for \"warn\"). Therefore I would suggest\nchanging \"info\" to perhaps bold white or bold cyan. Or \"skip\" could\nbe magenta and \"info\" blue. But now we are heading down a slippery\nslope; it'll be near impossible to please everyone. Any final\nthoughts?\n\n> Maybe it is just my terminal. I see it is labeled as \"brown\" here, but\n> it looks very yellow (and I am using the stock xterm colors. According\n> to:\n> \n> https://en.wikipedia.org/wiki/ANSI_colors\n> \n> It looks it really is brown on some platforms.\n\nYes, it can be.\n\n> I'm not sure if it is\n> worth worrying about. I don't really want to get into configurable\n> colors just for the test-suite output.\n\nAgreed. There is no indisputably correct combination. However, I\nthink that, modulo a tweak for the above, we are definitely in the\nright ball park. The main thing is that the traffic light colour\nscheme is adhered to, and that different types of message are clearly\nvisually separated, with more important ones standing out more than\nless important ones.\n"}]}