{"thread":{"id":"31455","subject":"[PATCH] Prefix shell test output messages with test id","startedAt":"2012-09-06T11:45:21Z","lastAt":"2012-09-06T20:29:38Z","messageCount":4,"participants":["Jan-Marek Glogowski","Thomas Gummerer","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"198432","messageId":"1346931921-5901-1-git-send-email-glogow@fbihome.de","threadId":"31455","inReplyTo":null,"subject":"[PATCH] Prefix shell test output messages with test id","fromName":"Jan-Marek Glogowski","fromEmail":"glogow@fbihome.de","sentAt":"2012-09-06T11:45:21Z","receivedAt":"2012-09-06T11:45:21Z","isPatch":true,"sender":{"key":"glogow@fbihome.de","avatar":null},"body":"This adds the test ID (tXXXX) prefix to the test result message of\nall shell tests.  This is especially useful when doing a parallel\ncheck run, where it's currently quite hard to identify the actual\nfailing test case.\n\nSigned-off-by: Jan-Marek Glogowski <glogow@fbihome.de>\n---\n t/t0000-basic.sh        | 28 ++++++++++++++--------------\n t/test-lib-functions.sh | 11 +++++++----\n t/test-lib.sh           | 10 ++++++----\n 3 Dateien geändert, 27 Zeilen hinzugefügt(+), 22 Zeilen entfernt(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex ccb5435..1bbf5b8 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -58,7 +58,7 @@ test_expect_failure 'pretend we have a known breakage' '\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+\tcat >0000t05-passing-todo.sh <<-EOF &&\n \t#!$SHELL_PATH\n \n \ttest_description='A passing TODO test\n@@ -77,14 +77,14 @@ test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib\n \n \ttest_done\n \tEOF\n-\tchmod +x passing-todo.sh &&\n-\t./passing-todo.sh >out 2>err &&\n+\tchmod +x 0000t05-passing-todo.sh &&\n+\t./0000t05-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> 1..1\n+\t> 0000t05: ok 1 - pretend we have fixed a known breakage # TODO known breakage\n+\t> 0000t05: # fixed 1 known breakage(s)\n+\t> 0000t05: # passed all 1 test(s)\n+\t> 0000t05: 1..1\n \tEOF\n \ttest_cmp expect out)\n \"\n@@ -141,7 +141,7 @@ test_expect_success 'tests clean up even on failures' \"\n \t(\n \tcd failing-cleanup &&\n \n-\tcat >failing-cleanup.sh <<-EOF &&\n+\tcat >0000t12-failing-cleanup.sh <<-EOF &&\n \t#!$SHELL_PATH\n \n \ttest_description='Failing tests with cleanup commands'\n@@ -162,23 +162,23 @@ test_expect_success 'tests clean up even on failures' \"\n \n \tEOF\n \n-\tchmod +x failing-cleanup.sh &&\n-\ttest_must_fail ./failing-cleanup.sh >out 2>err &&\n+\tchmod +x 0000t12-failing-cleanup.sh &&\n+\ttest_must_fail ./0000t12-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-\t> not ok - 1 tests clean up even after a failure\n+\t> 0000t12: 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> 0000t12: 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\n-\t> # failed 2 among 2 test(s)\n-\t> 1..2\n+\t> 0000t12: # failed 2 among 2 test(s)\n+\t> 0000t12: 1..2\n \tEOF\n \ttest_cmp expect out\n \t)\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 9bc57d2..c81ad7f 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -24,6 +24,9 @@\n #\n # In particular, quoting isn't enough, as the path may contain the same quote\n # that we're using.\n+\n+TID=$(basename ${0%%-*})\n+\n test_set_editor () {\n \tFAKE_EDITOR=\"$1\"\n \texport FAKE_EDITOR\n@@ -434,7 +437,7 @@ test_external_without_stderr () {\n test_path_is_file () {\n \tif ! [ -f \"$1\" ]\n \tthen\n-\t\techo \"File $1 doesn't exist. $*\"\n+\t\techo \"$TID: File $1 doesn't exist. $*\"\n \t\tfalse\n \tfi\n }\n@@ -442,7 +445,7 @@ test_path_is_file () {\n test_path_is_dir () {\n \tif ! [ -d \"$1\" ]\n \tthen\n-\t\techo \"Directory $1 doesn't exist. $*\"\n+\t\techo \"$TID: Directory $1 doesn't exist. $*\"\n \t\tfalse\n \tfi\n }\n@@ -450,7 +453,7 @@ test_path_is_dir () {\n test_path_is_missing () {\n \tif [ -e \"$1\" ]\n \tthen\n-\t\techo \"Path exists:\"\n+\t\techo \"$TID: Path exists:\"\n \t\tls -ld \"$1\"\n \t\tif [ $# -ge 1 ]; then\n \t\t\techo \"$*\"\n@@ -476,7 +479,7 @@ test_line_count () {\n \t\terror \"bug in the test script: not 3 parameters to test_line_count\"\n \telif ! test $(wc -l <\"$3\") \"$1\" \"$2\"\n \tthen\n-\t\techo \"test_line_count: line count for $3 !$1 $2\"\n+\t\techo \"$TID: test_line_count: line count for $3 !$1 $2\"\n \t\tcat \"$3\"\n \t\treturn 1\n \tfi\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 78c4286..6fccbe9 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -31,6 +31,8 @@ done,*)\n \t;;\n esac\n \n+TID=$(basename ${0%%-*})\n+\n # Keep the original TERM for say_color\n ORIGINAL_TERM=$TERM\n \n@@ -185,7 +187,7 @@ if test -n \"$color\"; then\n \t\t\t*) test -n \"$quiet\" && return;;\n \t\tesac\n \t\tshift\n-\t\tprintf \"%s\" \"$*\"\n+\t\tprintf \"$TID: %s\" \"$*\"\n \t\ttput sgr0\n \t\techo\n \t\t)\n@@ -194,12 +196,12 @@ else\n \tsay_color() {\n \t\ttest -z \"$1\" && test -n \"$quiet\" && return\n \t\tshift\n-\t\techo \"$*\"\n+\t\techo \"$TID: $*\"\n \t}\n fi\n \n error () {\n-\tsay_color error \"error: $*\"\n+\tsay_color error \"$TID: error: $*\"\n \tGIT_EXIT_OK=t\n \texit 1\n }\n@@ -262,7 +264,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.11.5\n"},{"id":"198437","messageId":"20120906123400.GA25467@tommy-fedora.scientificnet.net","threadId":"31455","inReplyTo":"1346931921-5901-1-git-send-email-glogow@fbihome.de","subject":"Re: [PATCH] Prefix shell test output messages with test id","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2012-09-06T12:34:00Z","receivedAt":"2012-09-06T12:34:00Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 09/06, Jan-Marek Glogowski wrote:\n> This adds the test ID (tXXXX) prefix to the test result message of\n> all shell tests.  This is especially useful when doing a parallel\n> check run, where it's currently quite hard to identify the actual\n> failing test case.\n> \n> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>\n\nThis breaks the TAP output format of the tests, which is needed to run\nthem with prove.  To identify the failing tests more easily when running\nthe tests in parallel, you may want to add GIT_TEST_TARGET = prove to\nyour config.mak.\n\nIf this change is really needed, I think you should add the test-id after\nthe message.\n> ---\n>  t/t0000-basic.sh        | 28 ++++++++++++++--------------\n>  t/test-lib-functions.sh | 11 +++++++----\n>  t/test-lib.sh           | 10 ++++++----\n>  3 Dateien geändert, 27 Zeilen hinzugefügt(+), 22 Zeilen entfernt(-)\n> \n> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n> index ccb5435..1bbf5b8 100755\n> --- a/t/t0000-basic.sh\n> +++ b/t/t0000-basic.sh\n> @@ -58,7 +58,7 @@ test_expect_failure 'pretend we have a known breakage' '\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> +\tcat >0000t05-passing-todo.sh <<-EOF &&\n>  \t#!$SHELL_PATH\n>  \n>  \ttest_description='A passing TODO test\n> @@ -77,14 +77,14 @@ test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib\n>  \n>  \ttest_done\n>  \tEOF\n> -\tchmod +x passing-todo.sh &&\n> -\t./passing-todo.sh >out 2>err &&\n> +\tchmod +x 0000t05-passing-todo.sh &&\n> +\t./0000t05-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> 1..1\n> +\t> 0000t05: ok 1 - pretend we have fixed a known breakage # TODO known breakage\n> +\t> 0000t05: # fixed 1 known breakage(s)\n> +\t> 0000t05: # passed all 1 test(s)\n> +\t> 0000t05: 1..1\n>  \tEOF\n>  \ttest_cmp expect out)\n>  \"\n> @@ -141,7 +141,7 @@ test_expect_success 'tests clean up even on failures' \"\n>  \t(\n>  \tcd failing-cleanup &&\n>  \n> -\tcat >failing-cleanup.sh <<-EOF &&\n> +\tcat >0000t12-failing-cleanup.sh <<-EOF &&\n>  \t#!$SHELL_PATH\n>  \n>  \ttest_description='Failing tests with cleanup commands'\n> @@ -162,23 +162,23 @@ test_expect_success 'tests clean up even on failures' \"\n>  \n>  \tEOF\n>  \n> -\tchmod +x failing-cleanup.sh &&\n> -\ttest_must_fail ./failing-cleanup.sh >out 2>err &&\n> +\tchmod +x 0000t12-failing-cleanup.sh &&\n> +\ttest_must_fail ./0000t12-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> -\t> not ok - 1 tests clean up even after a failure\n> +\t> 0000t12: 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> 0000t12: 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\n> -\t> # failed 2 among 2 test(s)\n> -\t> 1..2\n> +\t> 0000t12: # failed 2 among 2 test(s)\n> +\t> 0000t12: 1..2\n>  \tEOF\n>  \ttest_cmp expect out\n>  \t)\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index 9bc57d2..c81ad7f 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -24,6 +24,9 @@\n>  #\n>  # In particular, quoting isn't enough, as the path may contain the same quote\n>  # that we're using.\n> +\n> +TID=$(basename ${0%%-*})\n> +\n>  test_set_editor () {\n>  \tFAKE_EDITOR=\"$1\"\n>  \texport FAKE_EDITOR\n> @@ -434,7 +437,7 @@ test_external_without_stderr () {\n>  test_path_is_file () {\n>  \tif ! [ -f \"$1\" ]\n>  \tthen\n> -\t\techo \"File $1 doesn't exist. $*\"\n> +\t\techo \"$TID: File $1 doesn't exist. $*\"\n>  \t\tfalse\n>  \tfi\n>  }\n> @@ -442,7 +445,7 @@ test_path_is_file () {\n>  test_path_is_dir () {\n>  \tif ! [ -d \"$1\" ]\n>  \tthen\n> -\t\techo \"Directory $1 doesn't exist. $*\"\n> +\t\techo \"$TID: Directory $1 doesn't exist. $*\"\n>  \t\tfalse\n>  \tfi\n>  }\n> @@ -450,7 +453,7 @@ test_path_is_dir () {\n>  test_path_is_missing () {\n>  \tif [ -e \"$1\" ]\n>  \tthen\n> -\t\techo \"Path exists:\"\n> +\t\techo \"$TID: Path exists:\"\n>  \t\tls -ld \"$1\"\n>  \t\tif [ $# -ge 1 ]; then\n>  \t\t\techo \"$*\"\n> @@ -476,7 +479,7 @@ test_line_count () {\n>  \t\terror \"bug in the test script: not 3 parameters to test_line_count\"\n>  \telif ! test $(wc -l <\"$3\") \"$1\" \"$2\"\n>  \tthen\n> -\t\techo \"test_line_count: line count for $3 !$1 $2\"\n> +\t\techo \"$TID: test_line_count: line count for $3 !$1 $2\"\n>  \t\tcat \"$3\"\n>  \t\treturn 1\n>  \tfi\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 78c4286..6fccbe9 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -31,6 +31,8 @@ done,*)\n>  \t;;\n>  esac\n>  \n> +TID=$(basename ${0%%-*})\n> +\n>  # Keep the original TERM for say_color\n>  ORIGINAL_TERM=$TERM\n>  \n> @@ -185,7 +187,7 @@ if test -n \"$color\"; then\n>  \t\t\t*) test -n \"$quiet\" && return;;\n>  \t\tesac\n>  \t\tshift\n> -\t\tprintf \"%s\" \"$*\"\n> +\t\tprintf \"$TID: %s\" \"$*\"\n>  \t\ttput sgr0\n>  \t\techo\n>  \t\t)\n> @@ -194,12 +196,12 @@ else\n>  \tsay_color() {\n>  \t\ttest -z \"$1\" && test -n \"$quiet\" && return\n>  \t\tshift\n> -\t\techo \"$*\"\n> +\t\techo \"$TID: $*\"\n>  \t}\n>  fi\n>  \n>  error () {\n> -\tsay_color error \"error: $*\"\n> +\tsay_color error \"$TID: error: $*\"\n>  \tGIT_EXIT_OK=t\n>  \texit 1\n>  }\n> @@ -262,7 +264,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> -- \n> 1.7.11.5\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"198452","messageId":"20120906151008.GA7407@sigill.intra.peff.net","threadId":"31455","inReplyTo":"20120906123400.GA25467@tommy-fedora.scientificnet.net","subject":"Re: [PATCH] Prefix shell test output messages with test id","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-06T15:10:08Z","receivedAt":"2012-09-06T15:10:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 06, 2012 at 02:34:00PM +0200, Thomas Gummerer wrote:\n\n> On 09/06, Jan-Marek Glogowski wrote:\n> > This adds the test ID (tXXXX) prefix to the test result message of\n> > all shell tests.  This is especially useful when doing a parallel\n> > check run, where it's currently quite hard to identify the actual\n> > failing test case.\n> > \n> > Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>\n> \n> This breaks the TAP output format of the tests, which is needed to run\n> them with prove.  To identify the failing tests more easily when running\n> the tests in parallel, you may want to add GIT_TEST_TARGET = prove to\n> your config.mak.\n\nYeah, I would second the suggestion to just use \"prove\". However, if for\nsome reason that isn't an option, note that the stock harness stores\nresults in test-results/. You can just grep it like this:\n\n  grep 'failed [^0]' test-results/*\n\nto find out which scripts had failures, which is usually sufficient (if\nyou're running the tests in parallel, you won't have any sensible output\nanyway, so your first step is going to be to rerun the failed script\nwith \"-v\" anyway).\n\nBut really, \"prove\" is much nicer, and I recommend it unless you don't\nhave perl on your system.\n\n-Peff\n"},{"id":"198475","messageId":"504907B2.7060704@fbihome.de","threadId":"31455","inReplyTo":"20120906123400.GA25467@tommy-fedora.scientificnet.net","subject":"Re: [PATCH] Prefix shell test output messages with test id","fromName":"Jan-Marek Glogowski","fromEmail":"glogow@fbihome.de","sentAt":"2012-09-06T20:29:38Z","receivedAt":"2012-09-06T20:29:38Z","isPatch":true,"sender":{"key":"glogow@fbihome.de","avatar":null},"body":"Am 06.09.2012 14:34, schrieb Thomas Gummerer:\n> On 09/06, Jan-Marek Glogowski wrote:\n>> This adds the test ID (tXXXX) prefix to the test result message of\n>> all shell tests.  This is especially useful when doing a parallel\n>> check run, where it's currently quite hard to identify the actual\n>> failing test case.\n>>\n>> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>\n> \n> This breaks the TAP output format of the tests, which is needed to run\n> them with prove.  To identify the failing tests more easily when running\n> the tests in parallel, you may want to add GIT_TEST_TARGET = prove to\n> your config.mak.\n> \n> If this change is really needed, I think you should add the test-id after\n> the message.\n\nWhile grep'ing for prove I found t/README...\n\n>From my point of view the patch can be dropped. I can set GIT_PROVE_OPTS\nto run prove with multiple CPUS. Is there a known (easy) way to\npropagate MAKEFLAGS -j to prove?\n\nAnd I would like to propose a symlink from Documentation/testing =>\n../t/README.\n\nThanks for the quick review.\n"}]}