{"thread":{"id":"33843","subject":"[PATCH 0/6] --valgrind improvements","startedAt":"2013-05-16T20:50:11Z","lastAt":"2013-06-18T18:21:56Z","messageCount":44,"participants":["Thomas Rast","Elia Pinto","Junio C Hamano","Phil Hord","Johannes Sixt","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"217641","messageId":"cover.1368736093.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":null,"subject":"[PATCH 0/6] --valgrind improvements","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-16T20:50:11Z","receivedAt":"2013-05-16T20:50:11Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Peff and me discussed improving the usability of --valgrind testing.\nIn particular, two ideas that came up were options for running only a\nsubset of the tests in a file under --valgrind, and for running a\nsingle test script under valgrind while exploiting parallelism.\n\nSo here's a little series. It goes like this:\n\n test-lib: enable MALLOC_* for the actual tests\n\nFix for an unrelated bug that I came across.\n\n test-lib: refactor $GIT_SKIP_TESTS matching\n test-lib: verbose mode for only tests matching a pattern\n test-lib: valgrind for only tests matching a pattern\n\nAn option --valgrind-only= that lets you run only the\nsubtest matching under valgrind.\n\n test-lib: allow prefixing a custom string before \"ok N\" etc.\n test-lib: support running tests under valgrind in parallel\n\nAn option --valgrind-parallel= to run instances in parallel,\neach of which runs every -th test under valgrind, staggered so that\nthey cover everything. It's a bit of a hack, and thus RFC, but gives\ndecent results. On my 2-core laptop I measured a just over 2x\nspeedup. On a 6-core it starts falling off because of the extra\n(non-valgrind) runs, resulting in a 4.8x speedup.\n\nOne open issue with the last patch that currently eludes me: if I\ncombine --valgrind-parallel with any --valgrind=*, there are lots of\nerrors as (apparently) the valgrind wrapper setups race against each\nother. However, without any --valgrind=* (thus defaulting to\n'memcheck') this doesn't happen.\n\n\nThomas Rast (6):\n test-lib: enable MALLOC_* for the actual tests\n test-lib: refactor $GIT_SKIP_TESTS matching\n test-lib: verbose mode for only tests matching a pattern\n test-lib: valgrind for only tests matching a pattern\n test-lib: allow prefixing a custom string before \"ok N\" etc.\n test-lib: support running tests under valgrind in parallel\n\n t/README | 10 +++\n t/test-lib.sh | 175 ++++++++++++++++++++++++++++++++++++++++---------\n t/valgrind/valgrind.sh | 3 +\n 3 files changed, 156 insertions(+), 32 deletions(-)\n\n-- \n1.8.3.rc2.393.g8636c0b\n"},{"id":"217642","messageId":"043f7ee12630ae267bcde88e92a7dfacbf41e730.1368736093.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1368736093.git.trast@inf.ethz.ch","subject":"[PATCH 1/6] test-lib: enable MALLOC_* for the actual tests","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-16T20:50:12Z","receivedAt":"2013-05-16T20:50:12Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"1b3185f (MALLOC_CHECK: various clean-ups, 2012-09-14) moved around the\nMALLOC_CHECK_ and MALLOC_PERTURB_ assignments, intending to limit\ntheir effect to only the test runs. However, they were actually\nenabled only during test cleanup. Call setup/teardown_malloc_check\nalso around the evaluation of the actual test snippet.\n\nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex ca6bdef..229f5f7 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -337,8 +337,10 @@ test_eval_ () {\n test_run_ () {\n \ttest_cleanup=:\n \texpecting_failure=$2\n+\tsetup_malloc_check\n \ttest_eval_ \"$1\"\n \teval_ret=$?\n+\tteardown_malloc_check\n \n \tif test -z \"$immediate\" || test $eval_ret = 0 || test -n \"$expecting_failure\"\n \tthen\n-- \n1.8.3.rc2.393.g8636c0b\n"},{"id":"217643","messageId":"9b7e6a882a21b7e1734ec6fd90d3cf0208bb68f1.1368736093.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1368736093.git.trast@inf.ethz.ch","subject":"[PATCH 2/6] test-lib: refactor $GIT_SKIP_TESTS matching","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-16T20:50:13Z","receivedAt":"2013-05-16T20:50:13Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"It's already used twice, and we will have more of the same kind of\nmatching in a minute.\n\nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 41 ++++++++++++++++++++++++-----------------\n 1 file changed, 24 insertions(+), 17 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 229f5f7..c5a80d1 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -328,6 +328,20 @@ test_debug () {\n \ttest \"$debug\" = \"\" || eval \"$1\"\n }\n \n+match_pattern_list () {\n+\targ=\"$1\"\n+\tshift\n+\ttest -z \"$*\" && return 1\n+\tfor pat in $@\n+\tdo\n+\t\tcase \"$arg\" in\n+\t\t$pat)\n+\t\t\treturn 0\n+\t\tesac\n+\tdone\n+\treturn 1\n+}\n+\n test_eval_ () {\n \t# This is a separate function because some tests use\n \t# \"return\" to end a test_expect_success block early.\n@@ -358,14 +372,10 @@ test_run_ () {\n test_skip () {\n \ttest_count=$(($test_count+1))\n \tto_skip=\n-\tfor skp in $GIT_SKIP_TESTS\n-\tdo\n-\t\tcase $this_test.$test_count in\n-\t\t$skp)\n-\t\t\tto_skip=t\n-\t\t\tbreak\n-\t\tesac\n-\tdone\n+\tif match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS\n+\tthen\n+\t\tto_skip=t\n+\tfi\n \tif test -z \"$to_skip\" && test -n \"$test_prereq\" &&\n \t ! test_have_prereq \"$test_prereq\"\n \tthen\n@@ -630,15 +640,12 @@ cd -P \"$TRASH_DIRECTORY\" || exit 1\n \n this_test=${0##*/}\n this_test=${this_test%%-*}\n-for skp in $GIT_SKIP_TESTS\n-do\n-\tcase \"$this_test\" in\n-\t$skp)\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-done\n+if match_pattern_list \"$this_test\" $GIT_SKIP_TESTS\n+then\n+\tsay_color info >&3 \"skipping test $this_test altogether\"\n+\tskip_all=\"skip all tests in $this_test\"\n+\ttest_done\n+fi\n \n # Provide an implementation of the 'yes' utility\n yes () {\n-- \n1.8.3.rc2.393.g8636c0b\n"},{"id":"217645","messageId":"10e5eacece7fc9657c312b5c51747d23ae9ef40b.1368736093.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1368736093.git.trast@inf.ethz.ch","subject":"[PATCH 3/6] test-lib: verbose mode for only tests matching a pattern","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-16T20:50:14Z","receivedAt":"2013-05-16T20:50:14Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"With the new --verbose-only= option, one can enable --verbose\nat a per-test granularity. The pattern is matched against the test\nnumber, e.g.\n\n ./t0000-basic.sh --verbose-only='2[0-2]'\n\nto see only the full output of test 20-22, while showing the rest in the\none-liner format.\n\nThis is arguably not *too* useful on its own, but makes the next patch\neasier to follow.\n\nSigned-off-by: Thomas Rast \n---\n t/README | 5 +++++\n t/test-lib.sh | 29 +++++++++++++++++++++++++----\n 2 files changed, 30 insertions(+), 4 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex e669bb3..9c8f9b1 100644\n--- a/t/README\n+++ b/t/README\n@@ -76,6 +76,11 @@ appropriately before running \"make\".\n \tcommand being run and their output if any are also\n \toutput.\n \n+--verbose-only=::\n+\tLike --verbose, but the effect is limited to tests with\n+\tnumbers matching . The number matched against is\n+\tsimply the running count of the test within the file.\n+\n --debug::\n \tThis may help the person who is developing a new test.\n \tIt causes the command defined with test_debug to run.\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex c5a80d1..01e7445 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -184,6 +184,9 @@ do\n \t\thelp=t; shift ;;\n \t-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)\n \t\tverbose=t; shift ;;\n+\t--verbose-only=*)\n+\t\tverbose_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t-q|--q|--qu|--qui|--quie|--quiet)\n \t\t# Ignore --quiet under a TAP::Harness. Saying how many tests\n \t\t# passed without the ok/not ok details is always an error.\n@@ -342,6 +345,24 @@ match_pattern_list () {\n \treturn 1\n }\n \n+toggle_verbose () {\n+\ttest -z \"$verbose_only\" && return\n+\tif match_pattern_list $test_count $verbose_only\n+\tthen\n+\t\texec 4>&2 3>&1\n+\telse\n+\t\texec 4>/dev/null 3>/dev/null\n+\tfi\n+}\n+\n+setup_test_eval () {\n+\tsetup_malloc_check\n+\ttoggle_verbose\n+}\n+teardown_test_eval () {\n+\tteardown_malloc_check\n+}\n+\n test_eval_ () {\n \t# This is a separate function because some tests use\n \t# \"return\" to end a test_expect_success block early.\n@@ -351,16 +372,16 @@ test_eval_ () {\n test_run_ () {\n \ttest_cleanup=:\n \texpecting_failure=$2\n-\tsetup_malloc_check\n+\tsetup_test_eval\n \ttest_eval_ \"$1\"\n \teval_ret=$?\n-\tteardown_malloc_check\n+\tteardown_test_eval\n \n \tif test -z \"$immediate\" || test $eval_ret = 0 || test -n \"$expecting_failure\"\n \tthen\n-\t\tsetup_malloc_check\n+\t\tsetup_test_eval\n \t\ttest_eval_ \"$test_cleanup\"\n-\t\tteardown_malloc_check\n+\t\tteardown_test_eval\n \tfi\n \tif test \"$verbose\" = \"t\" && test -n \"$HARNESS_ACTIVE\"\n \tthen\n-- \n1.8.3.rc2.393.g8636c0b\n"},{"id":"217644","messageId":"0be45a574ec5f527ef1f3cc50f7acba2f025a2a3.1368736093.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1368736093.git.trast@inf.ethz.ch","subject":"[PATCH 4/6] test-lib: valgrind for only tests matching a pattern","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-16T20:50:15Z","receivedAt":"2013-05-16T20:50:15Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"With the new --valgrind-only= option, one can enable\n--valgrind at a per-test granularity, exactly analogous to\n--verbose-only from the previous commit.\n\nThe options are wired such that --valgrind implies --verbose (as\nbefore), but --valgrind-only= implies\n--verbose-only= unless --verbose is also in effect.\n\nSigned-off-by: Thomas Rast \n---\n t/README | 5 +++++\n t/test-lib.sh | 30 +++++++++++++++++++++++++++++-\n t/valgrind/valgrind.sh | 3 +++\n 3 files changed, 37 insertions(+), 1 deletion(-)\n\ndiff --git a/t/README b/t/README\nindex 9c8f9b1..bffd484 100644\n--- a/t/README\n+++ b/t/README\n@@ -126,6 +126,11 @@ appropriately before running \"make\".\n \tthe 't/valgrind/' directory and use the commands under\n \t't/valgrind/bin/'.\n \n+--valgrind-only=::\n+\tLike --valgrind, but the effect is limited to tests with\n+\tnumbers matching . The number matched against is\n+\tsimply the running count of the test within the file.\n+\n --tee::\n \tIn addition to printing the test output to the terminal,\n \twrite it to files named 't/test-results/$TEST_NAME.out'.\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 01e7445..9ae7c7b 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -201,6 +201,9 @@ do\n \t--valgrind=*)\n \t\tvalgrind=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n \t\tshift ;;\n+\t--valgrind-only=*)\n+\t\tvalgrind_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t--tee)\n \t\tshift ;; # was handled already\n \t--root=*)\n@@ -211,7 +214,14 @@ do\n \tesac\n done\n \n-test -n \"$valgrind\" && verbose=t\n+if test -n \"$valgrind_only\"\n+then\n+\ttest -z \"$valgrind\" && valgrind=memcheck\n+\ttest -z \"$verbose\" && verbose_only=\"$valgrind_only\"\n+elif test -n \"$valgrind\"\n+then\n+\tverbose=t\n+fi\n \n if test -n \"$color\"\n then\n@@ -355,8 +365,23 @@ toggle_verbose () {\n \tfi\n }\n \n+toggle_valgrind () {\n+\ttest -z \"$GIT_VALGRIND\" && return\n+\tif test -z \"$valgrind_only\"\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=t\n+\t\treturn\n+\tfi\n+\tGIT_VALGRIND_ENABLED=\n+\tif match_pattern_list $test_count $valgrind_only\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=t\n+\tfi\n+}\n+\n setup_test_eval () {\n \tsetup_malloc_check\n+\ttoggle_valgrind\n \ttoggle_verbose\n }\n teardown_test_eval () {\n@@ -571,6 +596,9 @@ then\n \texport GIT_VALGRIND\n \tGIT_VALGRIND_MODE=\"$valgrind\"\n \texport GIT_VALGRIND_MODE\n+\tGIT_VALGRIND_ENABLED=t\n+\ttest -n \"$valgrind_only\" && GIT_VALGRIND_ENABLED=\n+\texport GIT_VALGRIND_ENABLED\n elif test -n \"$GIT_TEST_INSTALLED\"\n then\n \tGIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path) ||\ndiff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh\nindex 6b87c91..4215303 100755\n--- a/t/valgrind/valgrind.sh\n+++ b/t/valgrind/valgrind.sh\n@@ -4,6 +4,9 @@ base=$(basename \"$0\")\n \n TOOL_OPTIONS='--leak-check=no'\n \n+test -z \"$GIT_VALGRIND_ENABLED\" &&\n+exec \"$GIT_VALGRIND\"/../../\"$base\" \"$@\"\n+\n case \"$GIT_VALGRIND_MODE\" in\n memcheck-fast)\n \t;;\n-- \n1.8.3.rc2.393.g8636c0b\n"},{"id":"217647","messageId":"f440021d75345b1242e54f47697c3d2ac9593e99.1368736093.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1368736093.git.trast@inf.ethz.ch","subject":"[PATCH 5/6] test-lib: allow prefixing a custom string before \"ok N\" etc.","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-16T20:50:16Z","receivedAt":"2013-05-16T20:50:16Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"This is not really meant for external use, but allows the next commit\nto neatly distinguish between sub-tests and the main run.\n\nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 27 +++++++++++++++------------\n 1 file changed, 15 insertions(+), 12 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 9ae7c7b..55fa749 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -209,6 +209,9 @@ do\n \t--root=*)\n \t\troot=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n \t\tshift ;;\n+\t--statusprefix=*)\n+\t\tstatusprefix=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t*)\n \t\techo \"error: unknown test option '$1'\" >&2; exit 1 ;;\n \tesac\n@@ -316,12 +319,12 @@ trap 'die' EXIT\n \n test_ok_ () {\n \ttest_success=$(($test_success + 1))\n-\tsay_color \"\" \"ok $test_count - $@\"\n+\tsay_color \"\" \"${statusprefix}ok $test_count - $@\"\n }\n \n test_failure_ () {\n \ttest_failure=$(($test_failure + 1))\n-\tsay_color error \"not ok $test_count - $1\"\n+\tsay_color error \"${statusprefix}not ok $test_count - $1\"\n \tshift\n \techo \"$@\" | sed -e 's/^/#\t/'\n \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n@@ -329,12 +332,12 @@ test_failure_ () {\n \n test_known_broken_ok_ () {\n \ttest_fixed=$(($test_fixed+1))\n-\tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n+\tsay_color error \"${statusprefix}ok $test_count - $@ # TODO known breakage vanished\"\n }\n \n test_known_broken_failure_ () {\n \ttest_broken=$(($test_broken+1))\n-\tsay_color warn \"not ok $test_count - $@ # TODO known breakage\"\n+\tsay_color warn \"${statusprefix}not ok $test_count - $@ # TODO known breakage\"\n }\n \n test_debug () {\n@@ -435,8 +438,8 @@ test_skip () {\n \t\t\tof_prereq=\" of $test_prereq\"\n \t\tfi\n \n-\t\tsay_color skip >&3 \"skipping test: $@\"\n-\t\tsay_color skip \"ok $test_count # skip $1 (missing $missing_prereq${of_prereq})\"\n+\t\tsay_color skip >&3 \"${statusprefix}skipping test: $@\"\n+\t\tsay_color skip \"${statusprefix}ok $test_count # skip $1 (missing $missing_prereq${of_prereq})\"\n \t\t: true\n \t\t;;\n \t*)\n@@ -472,11 +475,11 @@ test_done () {\n \n \tif test \"$test_fixed\" != 0\n \tthen\n-\t\tsay_color error \"# $test_fixed known breakage(s) vanished; please update test(s)\"\n+\t\tsay_color error \"${statusprefix}# $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\tsay_color warn \"${statusprefix}# still have $test_broken known breakage(s)\"\n \tfi\n \tif test \"$test_broken\" != 0 || test \"$test_fixed\" != 0\n \tthen\n@@ -499,9 +502,9 @@ test_done () {\n \t\tthen\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\t\tsay_color pass \"${statusprefix}# passed all $msg\"\n \t\t\tfi\n-\t\t\tsay \"1..$test_count$skip_all\"\n+\t\t\tsay \"${statusprefix}1..$test_count$skip_all\"\n \t\tfi\n \n \t\ttest -d \"$remove_trash\" &&\n@@ -515,8 +518,8 @@ test_done () {\n \t*)\n \t\tif test $test_external_has_tap -eq 0\n \t\tthen\n-\t\t\tsay_color error \"# failed $test_failure among $msg\"\n-\t\t\tsay \"1..$test_count\"\n+\t\t\tsay_color error \"${statusprefix}# failed $test_failure among $msg\"\n+\t\t\tsay \"${statusprefix}1..$test_count\"\n \t\tfi\n \n \t\texit 1 ;;\n-- \n1.8.3.rc2.393.g8636c0b\n"},{"id":"217646","messageId":"1d22b4b7885ff6b27c0837608aeaacd20e36ed94.1368736093.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1368736093.git.trast@inf.ethz.ch","subject":"[RFC PATCH 6/6] test-lib: support running tests under valgrind in parallel","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-16T20:50:17Z","receivedAt":"2013-05-16T20:50:17Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"With the new --valgrind-parallel= option, we support running the\ntests in a single test script under valgrind in parallel using 'n'\nprocesses.\n\nThis really follows the dumbest approach possible, as follows:\n\n* We spawn the test script 'n' times, using a throw-away\n TEST_OUTPUT_DIRECTORY. Each of the instances is given options that\n ensures that it only runs every n-th test under valgrind, but\n together they cover the entire range.\n\n* We add up the numbers from the individual tests, and provide the\n usual output.\n\nThis is really a gross hack at this point, and should be improved. In\nparticular we should keep the actual outputs somewhere more easily\ndiscoverable, and summarize them to the user.\n\nNevertheless, this is already workable and gives a speedup of more\nthan 2 on a dual-core (hyperthreaded) machine, using n=4. This is\nexpected since the overhead of valgrind is so big (on the order of 20x\nunder good conditions, and a large startup overhead at every git\ninvocation) that redundantly running the non-valgrind tests in between\nis not that expensive.\n\nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 55 insertions(+), 5 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 55fa749..b4e81bc 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -204,6 +204,15 @@ do\n \t--valgrind-only=*)\n \t\tvalgrind_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n \t\tshift ;;\n+\t--valgrind-parallel=*)\n+\t\tvalgrind_parallel=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n+\t--valgrind-only-stride=*)\n+\t\tvalgrind_only_stride=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n+\t--valgrind-only-offset=*)\n+\t\tvalgrind_only_offset=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t--tee)\n \t\tshift ;; # was handled already\n \t--root=*)\n@@ -217,7 +226,7 @@ do\n \tesac\n done\n \n-if test -n \"$valgrind_only\"\n+if test -n \"$valgrind_only\" || test -n \"$valgrind_only_stride\"\n then\n \ttest -z \"$valgrind\" && valgrind=memcheck\n \ttest -z \"$verbose\" && verbose_only=\"$valgrind_only\"\n@@ -359,8 +368,10 @@ match_pattern_list () {\n }\n \n toggle_verbose () {\n-\ttest -z \"$verbose_only\" && return\n-\tif match_pattern_list $test_count $verbose_only\n+\ttest -z \"$verbose_only\" && test -z \"$valgrind_only_stride\" && return\n+\tif match_pattern_list $test_count $verbose_only ||\n+\t\t{ test -n \"$valgrind_only_stride\" &&\n+\t\texpr $test_count \"%\" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null; }\n \tthen\n \t\texec 4>&2 3>&1\n \telse\n@@ -370,7 +381,7 @@ toggle_verbose () {\n \n toggle_valgrind () {\n \ttest -z \"$GIT_VALGRIND\" && return\n-\tif test -z \"$valgrind_only\"\n+\tif test -z \"$valgrind_only\" && test -z \"$valgrind_only_stride\"\n \tthen\n \t\tGIT_VALGRIND_ENABLED=t\n \t\treturn\n@@ -379,6 +390,10 @@ toggle_valgrind () {\n \tif match_pattern_list $test_count $valgrind_only\n \tthen\n \t\tGIT_VALGRIND_ENABLED=t\n+\telif test -n \"$valgrind_only_stride\" &&\n+\t\texpr $test_count \"%\" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=t\n \tfi\n }\n \n@@ -600,7 +615,10 @@ then\n \tGIT_VALGRIND_MODE=\"$valgrind\"\n \texport GIT_VALGRIND_MODE\n \tGIT_VALGRIND_ENABLED=t\n-\ttest -n \"$valgrind_only\" && GIT_VALGRIND_ENABLED=\n+\tif test -n \"$valgrind_only\" || test -n \"$valgrind_only_stride\"\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=\n+\tfi\n \texport GIT_VALGRIND_ENABLED\n elif test -n \"$GIT_TEST_INSTALLED\"\n then\n@@ -686,6 +704,38 @@ then\n else\n \tmkdir -p \"$TRASH_DIRECTORY\"\n fi\n+\n+if test -n \"$valgrind_parallel\"\n+then\n+\tfor i in $(test_seq 1 $valgrind_parallel)\n+\tdo\n+\t\troot=\"$TRASH_DIRECTORY/vgparallel-$i\"\n+\t\tmkdir \"$root\"\n+\t\tTEST_OUTPUT_DIRECTORY=\"$root\" \\\n+\t\t\t${SHELL_PATH} \"$0\" \\\n+\t\t\t--root=\"$root\" --statusprefix=\"[$i] \" \\\n+\t\t\t--valgrind=\"$valgrind\" \\\n+\t\t\t--valgrind-only-stride=\"$valgrind_parallel\" \\\n+\t\t\t--valgrind-only-offset=\"$i\" &\n+\t\tpids=\"$pids $!\"\n+\tdone\n+\ttrap \"kill $pids\" INT TERM HUP\n+\twait $pids\n+\ttrap - INT TERM HUP\n+\tfor i in $(test_seq 1 $valgrind_parallel)\n+\tdo\n+\t\troot=\"$TRASH_DIRECTORY/vgparallel-$i\"\n+\t\teval \"$(cat \"$root/test-results/$(basename \"$0\" .sh)\"-*.counts |\n+\t\t\tsed 's/^\\([a-z][a-z]*\\) \\([0-9][0-9]*\\)/inner_\\1=\\2/')\"\n+\t\ttest_count=$(expr $test_count + $inner_total)\n+\t\ttest_success=$(expr $test_success + $inner_success)\n+\t\ttest_fixed=$(expr $test_fixed + $inner_fixed)\n+\t\ttest_broken=$(expr $test_broken + $inner_broken)\n+\t\ttest_failed=$(expr $test_failed + $inner_failed)\n+\tdone\n+\ttest_done\n+fi\n+\n # Use -P to resolve symlinks in our working directory so that the cwd\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$TRASH_DIRECTORY\" || exit 1\n-- \n1.8.3.rc2.393.g8636c0b\n"},{"id":"217648","messageId":"CA+EOSBm1rBXiXoe8ux3DsN_YxcPDZ1JhUJ6F1wmkVGd-dbocpg@mail.gmail.com","threadId":"33843","inReplyTo":"043f7ee12630ae267bcde88e92a7dfacbf41e730.1368736093.git.trast@inf.ethz.ch","subject":"Re: [PATCH 1/6] test-lib: enable MALLOC_* for the actual tests","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2013-05-16T21:28:30Z","receivedAt":"2013-05-16T21:28:30Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"2013/5/16 Thomas Rast :\n> 1b3185f (MALLOC_CHECK: various clean-ups, 2012-09-14) moved around the\n> MALLOC_CHECK_ and MALLOC_PERTURB_ assignments, intending to limit\n> their effect to only the test runs. However, they were actually\n> enabled only during test cleanup. Call setup/teardown_malloc_check\n> also around the evaluation of the actual test snippet.\n>\n> Signed-off-by: Thomas Rast \n> ---\n> t/test-lib.sh | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index ca6bdef..229f5f7 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -337,8 +337,10 @@ test_eval_ () {\n> test_run_ () {\n> test_cleanup=:\n> expecting_failure=$2\n> + setup_malloc_check\n> test_eval_ \"$1\"\n> eval_ret=$?\n> + teardown_malloc_check\n>\n> if test -z \"$immediate\" || test $eval_ret = 0 || test -n \"$expecting_failure\"\n> then\n> --\n> 1.8.3.rc2.393.g8636c0b\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\nGood.\n"},{"id":"217649","messageId":"7v61yi4kbd.fsf@alter.siamese.dyndns.org","threadId":"33843","inReplyTo":"043f7ee12630ae267bcde88e92a7dfacbf41e730.1368736093.git.trast@inf.ethz.ch","subject":"Re: [PATCH 1/6] test-lib: enable MALLOC_* for the actual tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-16T22:43:50Z","receivedAt":"2013-05-16T22:43:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> 1b3185f (MALLOC_CHECK: various clean-ups, 2012-09-14) moved around the\n> MALLOC_CHECK_ and MALLOC_PERTURB_ assignments, intending to limit\n> their effect to only the test runs. However, they were actually\n> enabled only during test cleanup. Call setup/teardown_malloc_check\n> also around the evaluation of the actual test snippet.\n\nSorry about the breakage.\n\n> Signed-off-by: Thomas Rast \n> ---\n> t/test-lib.sh | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index ca6bdef..229f5f7 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -337,8 +337,10 @@ test_eval_ () {\n> test_run_ () {\n> \ttest_cleanup=:\n> \texpecting_failure=$2\n> +\tsetup_malloc_check\n> \ttest_eval_ \"$1\"\n> \teval_ret=$?\n> +\tteardown_malloc_check\n> \n> \tif test -z \"$immediate\" || test $eval_ret = 0 || test -n \"$expecting_failure\"\n> \tthen\n"},{"id":"217650","messageId":"CABURp0pZQFB37oBDab1h3r8w7nj6jb+HXwPrbn=9pQhfLkTMMQ@mail.gmail.com","threadId":"33843","inReplyTo":"f440021d75345b1242e54f47697c3d2ac9593e99.1368736093.git.trast@inf.ethz.ch","subject":"Re: [PATCH 5/6] test-lib: allow prefixing a custom string before \"ok N\" etc.","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-05-16T22:53:15Z","receivedAt":"2013-05-16T22:53:15Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Thu, May 16, 2013 at 4:50 PM, Thomas Rast wrote:\n> This is not really meant for external use, but allows the next commit\n> to neatly distinguish between sub-tests and the main run.\n\nMaybe we do not care about standards for this library or for your\nuse-case, but placing this prefix before the \"{ok,not ok}\" breaks the\nTAProtocol.\nhttp://podwiki.hexten.net/TAP/TAP.html?page=TAP\n\nMaybe you can put the prefix _after_ the \"{ok, not ok}\" and test number.\n\nPhil\n"},{"id":"217675","messageId":"5195C4B1.3010201@viscovery.net","threadId":"33843","inReplyTo":"9b7e6a882a21b7e1734ec6fd90d3cf0208bb68f1.1368736093.git.trast@inf.ethz.ch","subject":"Re: [PATCH 2/6] test-lib: refactor $GIT_SKIP_TESTS matching","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-05-17T05:48:33Z","receivedAt":"2013-05-17T05:48:33Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 5/16/2013 22:50, schrieb Thomas Rast:\n> +match_pattern_list () {\n> +\targ=\"$1\"\n> +\tshift\n> +\ttest -z \"$*\" && return 1\n> +\tfor pat in $@\n\nYou should have double-quotes around $@ here, but then you can just as\nwell abbreviate to\n\n\tfor pat\n\nand you don't need the 'test -z \"$*' check anymore.\n\n-- Hannes\n"},{"id":"217679","messageId":"87fvxmc9y0.fsf@linux-k42r.v.cablecom.net","threadId":"33843","inReplyTo":"CABURp0pZQFB37oBDab1h3r8w7nj6jb+HXwPrbn=9pQhfLkTMMQ@mail.gmail.com","subject":"Re: [PATCH 5/6] test-lib: allow prefixing a custom string before \"ok N\" etc.","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-17T08:00:39Z","receivedAt":"2013-05-17T08:00:39Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Phil Hord writes:\n\n> On Thu, May 16, 2013 at 4:50 PM, Thomas Rast wrote:\n>> This is not really meant for external use, but allows the next commit\n>> to neatly distinguish between sub-tests and the main run.\n>\n> Maybe we do not care about standards for this library or for your\n> use-case, but placing this prefix before the \"{ok,not ok}\" breaks the\n> TAProtocol.\n> http://podwiki.hexten.net/TAP/TAP.html?page=TAP\n>\n> Maybe you can put the prefix _after_ the \"{ok, not ok}\" and test number.\n\nActually that was half on purpose. You will notice I did not document\nthat option, as it is intended only to be used to distinguish between\nthe parallel runs implemented in [6/6].\n\nThose parallel runs look something like\n\n[4] ok 1 - plain\n[4] ok 2 - plain nested in bare\n[...snip until othes catch up...]\n[4] ok 33 - re-init to update git link\n[4] ok 34 - re-init to move gitdir\n[3] ok 1 - plain\n[2] ok 1 - plain\n[4] ok 35 - re-init to move gitdir symlink\n[4] # still have 2 known breakage(s)\n[4] # passed all remaining 33 test(s)\n[4] 1..35\n[3] ok 2 - plain nested in bare\n\nIt's invalid TAP no matter what: there are N plans and the ok/not ok\nlines from N runs all intermingled. So I'd rather not even pretend that\nit is valid in any way.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"217680","messageId":"87y5beav7d.fsf@linux-k42r.v.cablecom.net","threadId":"33843","inReplyTo":"5195C4B1.3010201@viscovery.net","subject":"Re: [PATCH 2/6] test-lib: refactor $GIT_SKIP_TESTS matching","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-17T08:04:22Z","receivedAt":"2013-05-17T08:04:22Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Johannes Sixt writes:\n\n> Am 5/16/2013 22:50, schrieb Thomas Rast:\n>> +match_pattern_list () {\n>> +\targ=\"$1\"\n>> +\tshift\n>> +\ttest -z \"$*\" && return 1\n>> +\tfor pat in $@\n>\n> You should have double-quotes around $@ here, but then you can just as\n> well abbreviate to\n>\n> \tfor pat\n>\n> and you don't need the 'test -z \"$*' check anymore.\n\nHmm, actually the quotes wouldn't help, because it currently reads\n\n-\tfor skp in $GIT_SKIP_TESTS\n-\tdo\n-\t\tcase $this_test.$test_count in\n-\t\t$skp)\n-\t\t\tto_skip=t\n-\t\t\tbreak\n-\t\tesac\n-\tdone\n\nso the splitting already happens, and in fact needs to, so that one can\npass multiple patterns. Or am I missing something?\n\nBut the 'for pat' with implicit $@ sounds nice regardless, thanks.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"217711","messageId":"CABURp0rjaQq28gAB_WkopE6J+3xDm9GJwZS6zNJ=ia_PFDyjmA@mail.gmail.com","threadId":"33843","inReplyTo":"87fvxmc9y0.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 5/6] test-lib: allow prefixing a custom string before \"ok N\" etc.","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-05-17T13:00:13Z","receivedAt":"2013-05-17T13:00:13Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Fri, May 17, 2013 at 4:00 AM, Thomas Rast wrote:\n> Phil Hord writes:\n>\n>> On Thu, May 16, 2013 at 4:50 PM, Thomas Rast wrote:\n>>> This is not really meant for external use, but allows the next commit\n>>> to neatly distinguish between sub-tests and the main run.\n>>\n>> Maybe we do not care about standards for this library or for your\n>> use-case, but placing this prefix before the \"{ok,not ok}\" breaks the\n>> TAProtocol.\n>> http://podwiki.hexten.net/TAP/TAP.html?page=TAP\n>>\n>> Maybe you can put the prefix _after_ the \"{ok, not ok}\" and test number.\n>\n> Actually that was half on purpose. You will notice I did not document\n> that option, as it is intended only to be used to distinguish between\n> the parallel runs implemented in [6/6].\n>\n> Those parallel runs look something like\n>\n> [4] ok 1 - plain\n> [4] ok 2 - plain nested in bare\n> [...snip until othes catch up...]\n> [4] ok 33 - re-init to update git link\n> [4] ok 34 - re-init to move gitdir\n> [3] ok 1 - plain\n> [2] ok 1 - plain\n> [4] ok 35 - re-init to move gitdir symlink\n> [4] # still have 2 known breakage(s)\n> [4] # passed all remaining 33 test(s)\n> [4] 1..35\n> [3] ok 2 - plain nested in bare\n>\n> It's invalid TAP no matter what: there are N plans and the ok/not ok\n> lines from N runs all intermingled. So I'd rather not even pretend that\n> it is valid in any way.\n\nYes, I guessed this might have been the goal. Maybe you can mention\nit in the commit message.\n\nI hope some future change might even unwind these back into a valid\ncontinuous TAP stream. But at least for now, if someone needs such a\nstream, she can unwind it herself.\n\nPhil\n"},{"id":"217724","messageId":"7vfvxl1rj9.fsf@alter.siamese.dyndns.org","threadId":"33843","inReplyTo":"87y5beav7d.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 2/6] test-lib: refactor $GIT_SKIP_TESTS matching","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-17T16:48:26Z","receivedAt":"2013-05-17T16:48:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Johannes Sixt writes:\n>\n>> Am 5/16/2013 22:50, schrieb Thomas Rast:\n>>> +match_pattern_list () {\n>>> +\targ=\"$1\"\n>>> +\tshift\n>>> +\ttest -z \"$*\" && return 1\n>>> +\tfor pat in $@\n>>\n>> You should have double-quotes around $@ here, but then you can just as\n>> well abbreviate to\n>>\n>> \tfor pat\n>>\n>> and you don't need the 'test -z \"$*' check anymore.\n>\n> Hmm, actually the quotes wouldn't help, because it currently reads\n>\n> -\tfor skp in $GIT_SKIP_TESTS\n> -\tdo\n> -\t\tcase $this_test.$test_count in\n> -\t\t$skp)\n> -\t\t\tto_skip=t\n> -\t\t\tbreak\n> -\t\tesac\n> -\tdone\n>\n> so the splitting already happens, and in fact needs to, so that one can\n> pass multiple patterns. Or am I missing something?\n\nI think you read it right. I do pass multiple test number patterns\nregularly and not quoting there is exactly for support that.\n\n> But the 'for pat' with implicit $@ sounds nice regardless, thanks.\n\n'for pat' is equivalent to 'for pat in \"$@\"', not 'for pat in $@';\nwould it still be useful when you need them split at $IFS?\n"},{"id":"217729","messageId":"87sj1lbkvm.fsf@linux-k42r.v.cablecom.net","threadId":"33843","inReplyTo":"7vfvxl1rj9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/6] test-lib: refactor $GIT_SKIP_TESTS matching","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-17T17:02:05Z","receivedAt":"2013-05-17T17:02:05Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano writes:\n\n> Thomas Rast writes:\n>\n>> Johannes Sixt writes:\n>>\n>>> Am 5/16/2013 22:50, schrieb Thomas Rast:\n>>>> +match_pattern_list () {\n>>>> +\targ=\"$1\"\n>>>> +\tshift\n>>>> +\ttest -z \"$*\" && return 1\n>>>> +\tfor pat in $@\n>>>\n>>> You should have double-quotes around $@ here, but then you can just as\n>>> well abbreviate to\n>>>\n>>> \tfor pat\n>>>\n>>> and you don't need the 'test -z \"$*' check anymore.\n>>\n>> Hmm, actually the quotes wouldn't help, because it currently reads\n>>\n>> -\tfor skp in $GIT_SKIP_TESTS\n>> -\tdo\n>> -\t\tcase $this_test.$test_count in\n>> -\t\t$skp)\n>> -\t\t\tto_skip=t\n>> -\t\t\tbreak\n>> -\t\tesac\n>> -\tdone\n[...]\n>> But the 'for pat' with implicit $@ sounds nice regardless, thanks.\n>\n> 'for pat' is equivalent to 'for pat in \"$@\"', not 'for pat in $@';\n> would it still be useful when you need them split at $IFS?\n\nAt this point the splitting has already happened in the caller when it\ndoes the (refactored)\n\n+\tif match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS\n\nSo $@ and \"$@\" is actually the same thing.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"217738","messageId":"7vobc9zfkx.fsf@alter.siamese.dyndns.org","threadId":"33843","inReplyTo":"87sj1lbkvm.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 2/6] test-lib: refactor $GIT_SKIP_TESTS matching","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-17T17:22:38Z","receivedAt":"2013-05-17T17:22:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> At this point the splitting has already happened in the caller when it\n> does the (refactored)\n>\n> +\tif match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS\n>\n> So $@ and \"$@\" is actually the same thing.\n\nAh, then it is perfect. Thanks.\n"},{"id":"217764","messageId":"5196A128.7070005@kdbg.org","threadId":"33843","inReplyTo":"87sj1lbkvm.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 2/6] test-lib: refactor $GIT_SKIP_TESTS matching","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-05-17T21:29:12Z","receivedAt":"2013-05-17T21:29:12Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 17.05.2013 19:02, schrieb Thomas Rast:\n> At this point the splitting has already happened in the caller when it\n> does the (refactored)\n> \n> +\tif match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS\n> \n> So $@ and \"$@\" is actually the same thing.\n\nNot in general: If you omit the double-quotes, the individual words\nstill undergo path name expansion and the result depends on the files\nthat happen to match patterns given in $GIT_SKIP_TESTS.\n\nThis is not a new problem at the call site of the new function, but we\nshouldn't duplicate the same problem in the implementation of the function.\n\n-- Hannes\n"},{"id":"218745","messageId":"20130529045356.GA31762@sigill.intra.peff.net","threadId":"33843","inReplyTo":"cover.1368736093.git.trast@inf.ethz.ch","subject":"Re: [PATCH 0/6] --valgrind improvements","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-05-29T04:53:56Z","receivedAt":"2013-05-29T04:53:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 16, 2013 at 10:50:11PM +0200, Thomas Rast wrote:\n\n> One open issue with the last patch that currently eludes me: if I\n> combine --valgrind-parallel with any --valgrind=*, there are lots of\n> errors as (apparently) the valgrind wrapper setups race against each\n> other. However, without any --valgrind=* (thus defaulting to\n> 'memcheck') this doesn't happen.\n\nI noticed two racy error messages. If you do:\n\n cd t &&\n make clean &&\n ./some-test --valgrind-parallel=8\n\nyou will get complaints from mkdir about existing directories, as we use\nmkdir as a poor man's O_EXCL to create lockfiles. These error messages\nare harmless (we loop and try again), and we should perhaps just squelch\nthe stderr from mkdir. Although that might make weird situations hard to\ndiagnose, like another error that prevents creating the lockfile, so\nmaybe it is better to just live with the extra output (after the\ndirectory is built once, it does not happen at all).\n\nI also notice:\n\n $ ./t4052-* --valgrind-parallel=8 --valgrind=memcheck\n ...\n ./t4052-stat-output.sh: 572: ./test-lib.sh: cannot open\n /home/peff/compile/git/t/valgrind/bin/git-send-email.perl: No such\n file\n\nLine 572 is checking the \"#!\" line of the _source_ file. So it shouldn't\nbe checking t/valgrind/bin in the first place. It looks like it comes\nfrom this loop:\n\n for path in $PATH\n do\n ls \"$path\"/git-* 2> /dev/null |\n while read file\n do\n make_valgrind_symlink \"$file\"\n done\n done\n\nas t/valgrind/bin seems to be in the $PATH of the parallel\nsub-processes. Hmm. It looks like you set up the valgrind dir in the\nparent test process, and _then_ call the sub-processes in parallel. We\nshouldn't need to do any valgrind setup at all in the subprocesses,\nshould we? They would just be replicating what the parent already did.\n\n-Peff\n"},{"id":"218746","messageId":"20130529050000.GB31762@sigill.intra.peff.net","threadId":"33843","inReplyTo":"10e5eacece7fc9657c312b5c51747d23ae9ef40b.1368736093.git.trast@inf.ethz.ch","subject":"Re: [PATCH 3/6] test-lib: verbose mode for only tests matching a pattern","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-05-29T05:00:00Z","receivedAt":"2013-05-29T05:00:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 16, 2013 at 10:50:14PM +0200, Thomas Rast wrote:\n\n> With the new --verbose-only= option, one can enable --verbose\n> at a per-test granularity. The pattern is matched against the test\n> number, e.g.\n> \n> ./t0000-basic.sh --verbose-only='2[0-2]'\n> \n> to see only the full output of test 20-22, while showing the rest in the\n> one-liner format.\n> \n> This is arguably not *too* useful on its own, but makes the next patch\n> easier to follow.\n\nHmm, I don't think this is quite right. Try:\n\n ./t4052-stat-output.sh --verbose-only=85\n\nThe script and test number aren't important; I just picked these at\nrandom, but they show the issue clearly. The output I get is:\n\n [...]\n ok 83 - log respects prefix greater than COLUMNS (big change)\n ok 84 - log --graph respects prefix greater than COLUMNS (big change)\n Switched to a new branch 'branch'\n ok 85 - merge --stat respects COLUMNS (big change)\n\n expecting success: \n COLUMNS=100 git merge --stat --no-ff master >output &&\n grep \" | \" output >actual\n test_cmp expect actual\n\n ok 86 - merge --stat respects COLUMNS (long filename)\n\nSo we see 83 and 84 non-verbose, which is good. And we see the actual\noutput from 85 (the output from a \"git checkout\"). But we do not see the\n\"expecting success\" for it. We see it for the _next_ test, which we\nshould not see at all. So I think your toggling is happening in the\nwrong spot, but I haven't looked further than that.\n\n-Peff\n"},{"id":"218747","messageId":"20130529050732.GC31762@sigill.intra.peff.net","threadId":"33843","inReplyTo":"20130529050000.GB31762@sigill.intra.peff.net","subject":"Re: [PATCH 3/6] test-lib: verbose mode for only tests matching a pattern","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-05-29T05:07:32Z","receivedAt":"2013-05-29T05:07:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 29, 2013 at 01:00:00AM -0400, Jeff King wrote:\n\n> So we see 83 and 84 non-verbose, which is good. And we see the actual\n> output from 85 (the output from a \"git checkout\"). But we do not see the\n> \"expecting success\" for it. We see it for the _next_ test, which we\n> should not see at all. So I think your toggling is happening in the\n> wrong spot, but I haven't looked further than that.\n\nI think you want something more like:\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 5251009..75351f5 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -349,6 +349,7 @@ test_expect_failure () {\n \texport test_prereq\n \tif ! test_skip \"$@\"\n \tthen\n+\t\ttoggle_verbose\n \t\tsay >&3 \"checking known breakage: $2\"\n \t\tif test_run_ \"$2\" expecting_failure\n \t\tthen\n@@ -367,6 +368,7 @@ test_expect_success () {\n \texport test_prereq\n \tif ! test_skip \"$@\"\n \tthen\n+\t\ttoggle_verbose\n \t\tsay >&3 \"expecting success: $2\"\n \t\tif test_run_ \"$2\"\n \t\tthen\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex b4e81bc..165e84e 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -400,7 +400,6 @@ setup_test_eval () {\n setup_test_eval () {\n \tsetup_malloc_check\n \ttoggle_valgrind\n-\ttoggle_verbose\n }\n teardown_test_eval () {\n \tteardown_malloc_check\n\nHowever, I'm not sure the toggle is the right thing. However, the whole\ntoggle thing seems weird to me, as there is a big \"gap\" between\nfinishing test X and starting test X+1 where we inherit the verbosity\n(and valgrind) settings from X. In general we frown upon doing much at\nall outside of test_expect_*, but I would think that:\n\n test_expect_success 'one' '...'\n git foo\n test_expect_success 'two' '...'\n\nwhen run with \"--valgrind-only=1\" would not run the intermediate \"git\nfoo\" with valgrind. I would have expected the implementation to be more\nlike:\n\n maybe_turn_on_valgrind\n maybe_turn_on_verbose\n run_the_actual_test\n maybe_turn_off_verbose\n maybe_turn_off_valgrind\n\nrather than relying on the next test to return to normal. I doubt that\nit matters too much in practice, though.\n\n-Peff\n"},{"id":"218864","messageId":"7vmwrdu0z5.fsf@alter.siamese.dyndns.org","threadId":"33843","inReplyTo":"20130529050732.GC31762@sigill.intra.peff.net","subject":"Re: [PATCH 3/6] test-lib: verbose mode for only tests matching a pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-29T17:53:18Z","receivedAt":"2013-05-29T17:53:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, May 29, 2013 at 01:00:00AM -0400, Jeff King wrote:\n>\n>> So we see 83 and 84 non-verbose, which is good. And we see the actual\n>> output from 85 (the output from a \"git checkout\"). But we do not see the\n>> \"expecting success\" for it. We see it for the _next_ test, which we\n>> should not see at all. So I think your toggling is happening in the\n>> wrong spot, but I haven't looked further than that.\n>\n> I think you want something more like:\n>\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index 5251009..75351f5 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -349,6 +349,7 @@ test_expect_failure () {\n> \texport test_prereq\n> \tif ! test_skip \"$@\"\n> \tthen\n> +\t\ttoggle_verbose\n> \t\tsay >&3 \"checking known breakage: $2\"\n> \t\tif test_run_ \"$2\" expecting_failure\n> \t\tthen\n> @@ -367,6 +368,7 @@ test_expect_success () {\n> \texport test_prereq\n> \tif ! test_skip \"$@\"\n> \tthen\n> +\t\ttoggle_verbose\n> \t\tsay >&3 \"expecting success: $2\"\n> \t\tif test_run_ \"$2\"\n> \t\tthen\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index b4e81bc..165e84e 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -400,7 +400,6 @@ setup_test_eval () {\n> setup_test_eval () {\n> \tsetup_malloc_check\n> \ttoggle_valgrind\n> -\ttoggle_verbose\n> }\n> teardown_test_eval () {\n> \tteardown_malloc_check\n>\n> However, I'm not sure the toggle is the right thing. However, the whole\n> toggle thing seems weird to me, as there is a big \"gap\" between\n> finishing test X and starting test X+1 where we inherit the verbosity\n> (and valgrind) settings from X. In general we frown upon doing much at\n> all outside of test_expect_*, but I would think that:\n>\n> test_expect_success 'one' '...'\n> git foo\n> test_expect_success 'two' '...'\n>\n> when run with \"--valgrind-only=1\" would not run the intermediate \"git\n> foo\" with valgrind. I would have expected the implementation to be more\n> like:\n>\n> maybe_turn_on_valgrind\n> maybe_turn_on_verbose\n> run_the_actual_test\n> maybe_turn_off_verbose\n> maybe_turn_off_valgrind\n>\n> rather than relying on the next test to return to normal.\n\nThat matches my expectation as well (I had the same thought while\nreading the series).\n\nThanks.\n"},{"id":"221041","messageId":"cover.1371460265.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1368736093.git.trast@inf.ethz.ch","subject":"[PATCH v2 0/6] --valgrind improvements","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-17T09:18:45Z","receivedAt":"2013-06-17T09:18:45Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Here's the promised reroll. It integrates everyone's suggestions (I\nhope I didn't miss any), most notably the two by Peff:\n\n* --valgrind-parallel only does the valgrind wrapper setup in the\n children, avoiding lock contention and confusion because it tries to\n wrap the wrappers.\n\n* more careful --verbose-only toggling (with some extra care about the\n blank lines in between)\n\n\n\nThomas Rast (6):\n test-lib: enable MALLOC_* for the actual tests\n test-lib: refactor $GIT_SKIP_TESTS matching\n test-lib: verbose mode for only tests matching a pattern\n test-lib: valgrind for only tests matching a pattern\n test-lib: allow prefixing a custom string before \"ok N\" etc.\n test-lib: support running tests under valgrind in parallel\n\n t/README | 10 ++\n t/test-lib-functions.sh | 2 +\n t/test-lib.sh | 248 ++++++++++++++++++++++++++++++++++++++----------\n t/valgrind/valgrind.sh | 3 +\n 4 files changed, 213 insertions(+), 50 deletions(-)\n\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221043","messageId":"fc300eeacd6c42dbdc00fe33216189a88d606412.1371460265.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371460265.git.trast@inf.ethz.ch","subject":"[PATCH v2 1/6] test-lib: enable MALLOC_* for the actual tests","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-17T09:18:46Z","receivedAt":"2013-06-17T09:18:46Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"1b3185f (MALLOC_CHECK: various clean-ups, 2012-09-14) moved around the\nMALLOC_CHECK_ and MALLOC_PERTURB_ assignments, intending to limit\ntheir effect to only the test runs. However, they were actually\nenabled only during test cleanup. Call setup/teardown_malloc_check\nalso around the evaluation of the actual test snippet.\n\nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex eff3a65..35da859 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -337,8 +337,10 @@ test_eval_ () {\n test_run_ () {\n \ttest_cleanup=:\n \texpecting_failure=$2\n+\tsetup_malloc_check\n \ttest_eval_ \"$1\"\n \teval_ret=$?\n+\tteardown_malloc_check\n \n \tif test -z \"$immediate\" || test $eval_ret = 0 || test -n \"$expecting_failure\"\n \tthen\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221042","messageId":"4795e6f40e7ce62fc4c414e8cbbd2aa777d3fbb0.1371460265.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371460265.git.trast@inf.ethz.ch","subject":"[PATCH v2 2/6] test-lib: refactor $GIT_SKIP_TESTS matching","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-17T09:18:47Z","receivedAt":"2013-06-17T09:18:47Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"It's already used twice, and we will have more of the same kind of\nmatching in a minute.\n\nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 41 ++++++++++++++++++++++++-----------------\n 1 file changed, 24 insertions(+), 17 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 35da859..d9a74ff 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -328,6 +328,20 @@ test_debug () {\n \ttest \"$debug\" = \"\" || eval \"$1\"\n }\n \n+match_pattern_list () {\n+\targ=\"$1\"\n+\tshift\n+\ttest -z \"$*\" && return 1\n+\tfor pat\n+\tdo\n+\t\tcase \"$arg\" in\n+\t\t$pat)\n+\t\t\treturn 0\n+\t\tesac\n+\tdone\n+\treturn 1\n+}\n+\n test_eval_ () {\n \t# This is a separate function because some tests use\n \t# \"return\" to end a test_expect_success block early.\n@@ -358,14 +372,10 @@ test_run_ () {\n test_skip () {\n \ttest_count=$(($test_count+1))\n \tto_skip=\n-\tfor skp in $GIT_SKIP_TESTS\n-\tdo\n-\t\tcase $this_test.$test_count in\n-\t\t$skp)\n-\t\t\tto_skip=t\n-\t\t\tbreak\n-\t\tesac\n-\tdone\n+\tif match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS\n+\tthen\n+\t\tto_skip=t\n+\tfi\n \tif test -z \"$to_skip\" && test -n \"$test_prereq\" &&\n \t ! test_have_prereq \"$test_prereq\"\n \tthen\n@@ -630,15 +640,12 @@ cd -P \"$TRASH_DIRECTORY\" || exit 1\n \n this_test=${0##*/}\n this_test=${this_test%%-*}\n-for skp in $GIT_SKIP_TESTS\n-do\n-\tcase \"$this_test\" in\n-\t$skp)\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-done\n+if match_pattern_list \"$this_test\" $GIT_SKIP_TESTS\n+then\n+\tsay_color info >&3 \"skipping test $this_test altogether\"\n+\tskip_all=\"skip all tests in $this_test\"\n+\ttest_done\n+fi\n \n # Provide an implementation of the 'yes' utility\n yes () {\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221046","messageId":"8739d3ba5947c60d402030fc35d47115e7a51b0f.1371460265.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371460265.git.trast@inf.ethz.ch","subject":"[PATCH v2 3/6] test-lib: verbose mode for only tests matching a pattern","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-17T09:18:48Z","receivedAt":"2013-06-17T09:18:48Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"With the new --verbose-only= option, one can enable --verbose\nat a per-test granularity. The pattern is matched against the test\nnumber, e.g.\n\n ./t0000-basic.sh --verbose-only='2[0-2]'\n\nto see only the full output of test 20-22, while showing the rest in the\none-liner format.\n\nAs suggested by Jeff King, this takes care to wrap the entire test_expect_*\nblock, but nothing else, in the verbose toggling. To that end we use\na new pair of hook functions. The placement is a bit weird because we\nneed to wait until the beginning of test_skip for $test_count to be\nincremented.\n\nThis is arguably not *too* useful on its own, but makes the next patch\neasier to follow.\n\nHelped-by: Jeff King \nSigned-off-by: Thomas Rast \n---\n t/README | 5 +++++\n t/test-lib-functions.sh | 2 ++\n t/test-lib.sh | 44 ++++++++++++++++++++++++++++++++++++++++++--\n 3 files changed, 49 insertions(+), 2 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 35b3c5c..f4e6299 100644\n--- a/t/README\n+++ b/t/README\n@@ -76,6 +76,11 @@ appropriately before running \"make\".\n \tcommand being run and their output if any are also\n \toutput.\n \n+--verbose-only=::\n+\tLike --verbose, but the effect is limited to tests with\n+\tnumbers matching . The number matched against is\n+\tsimply the running count of the test within the file.\n+\n --debug::\n \tThis may help the person who is developing a new test.\n \tIt causes the command defined with test_debug to run.\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 5251009..0eac1dd 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -358,6 +358,7 @@ test_expect_failure () {\n \t\tfi\n \tfi\n \techo >&3 \"\"\n+\ttest_teardown_hook_\n }\n \n test_expect_success () {\n@@ -376,6 +377,7 @@ test_expect_success () {\n \t\tfi\n \tfi\n \techo >&3 \"\"\n+\ttest_teardown_hook_\n }\n \n # test_external runs external test scripts that provide continuous\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex d9a74ff..84e5f03 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -184,6 +184,9 @@ do\n \t\thelp=t; shift ;;\n \t-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)\n \t\tverbose=t; shift ;;\n+\t--verbose-only=*)\n+\t\tverbose_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t-q|--q|--qu|--qui|--quie|--quiet)\n \t\t# Ignore --quiet under a TAP::Harness. Saying how many tests\n \t\t# passed without the ok/not ok details is always an error.\n@@ -342,6 +345,44 @@ match_pattern_list () {\n \treturn 1\n }\n \n+maybe_teardown_verbose () {\n+\ttest -z \"$verbose_only\" && return\n+\texec 4>/dev/null 3>/dev/null\n+\tverbose=\n+}\n+\n+last_verbose=t\n+maybe_setup_verbose () {\n+\ttest -z \"$verbose_only\" && return\n+\tif match_pattern_list $test_count $verbose_only\n+\tthen\n+\t\texec 4>&2 3>&1\n+\t\t# Emit a delimiting blank line when going from\n+\t\t# non-verbose to verbose. Within verbose mode the\n+\t\t# delimiter is printed by test_expect_*. The choice\n+\t\t# of the initial $last_verbose is such that before\n+\t\t# test 1, we do not print it.\n+\t\ttest -z \"$last_verbose\" && echo >&3 \"\"\n+\t\tverbose=t\n+\telse\n+\t\texec 4>/dev/null 3>/dev/null\n+\t\tverbose=\n+\tfi\n+\tlast_verbose=$verbose\n+}\n+\n+# Called from test_skip after it has incremented $test_count. This\n+# means it runs before any test-specific code and output.\n+test_setup_hook_ () {\n+\tmaybe_setup_verbose\n+}\n+\n+# Called at the end of test_expect_*. This means it runs after all\n+# test-specific code and output.\n+test_teardown_hook_ () {\n+\tmaybe_teardown_verbose\n+}\n+\n test_eval_ () {\n \t# This is a separate function because some tests use\n \t# \"return\" to end a test_expect_success block early.\n@@ -358,9 +399,7 @@ test_run_ () {\n \n \tif test -z \"$immediate\" || test $eval_ret = 0 || test -n \"$expecting_failure\"\n \tthen\n-\t\tsetup_malloc_check\n \t\ttest_eval_ \"$test_cleanup\"\n-\t\tteardown_malloc_check\n \tfi\n \tif test \"$verbose\" = \"t\" && test -n \"$HARNESS_ACTIVE\"\n \tthen\n@@ -372,6 +411,7 @@ test_run_ () {\n test_skip () {\n \ttest_count=$(($test_count+1))\n \tto_skip=\n+\ttest_setup_hook_\n \tif match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS\n \tthen\n \t\tto_skip=t\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221044","messageId":"2b24f531f3bdcb0e4553c5701c91d462d928d13c.1371460265.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371460265.git.trast@inf.ethz.ch","subject":"[PATCH v2 4/6] test-lib: valgrind for only tests matching a pattern","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-17T09:18:49Z","receivedAt":"2013-06-17T09:18:49Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"With the new --valgrind-only= option, one can enable\n--valgrind at a per-test granularity, exactly analogous to\n--verbose-only from the previous commit.\n\nThe options are wired such that --valgrind implies --verbose (as\nbefore), but --valgrind-only= implies\n--verbose-only= unless --verbose is also in effect.\n\nSigned-off-by: Thomas Rast \n---\n t/README | 5 +++++\n t/test-lib.sh | 36 +++++++++++++++++++++++++++++++++++-\n t/valgrind/valgrind.sh | 3 +++\n 3 files changed, 43 insertions(+), 1 deletion(-)\n\ndiff --git a/t/README b/t/README\nindex f4e6299..abe991f 100644\n--- a/t/README\n+++ b/t/README\n@@ -126,6 +126,11 @@ appropriately before running \"make\".\n \tthe 't/valgrind/' directory and use the commands under\n \t't/valgrind/bin/'.\n \n+--valgrind-only=::\n+\tLike --valgrind, but the effect is limited to tests with\n+\tnumbers matching . The number matched against is\n+\tsimply the running count of the test within the file.\n+\n --tee::\n \tIn addition to printing the test output to the terminal,\n \twrite it to files named 't/test-results/$TEST_NAME.out'.\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 84e5f03..40bd7da 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -201,6 +201,9 @@ do\n \t--valgrind=*)\n \t\tvalgrind=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n \t\tshift ;;\n+\t--valgrind-only=*)\n+\t\tvalgrind_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t--tee)\n \t\tshift ;; # was handled already\n \t--root=*)\n@@ -211,7 +214,14 @@ do\n \tesac\n done\n \n-test -n \"$valgrind\" && verbose=t\n+if test -n \"$valgrind_only\"\n+then\n+\ttest -z \"$valgrind\" && valgrind=memcheck\n+\ttest -z \"$verbose\" && verbose_only=\"$valgrind_only\"\n+elif test -n \"$valgrind\"\n+then\n+\tverbose=t\n+fi\n \n if test -n \"$color\"\n then\n@@ -371,15 +381,36 @@ maybe_setup_verbose () {\n \tlast_verbose=$verbose\n }\n \n+maybe_teardown_valgrind () {\n+\ttest -z \"$GIT_VALGRIND\" && return\n+\tGIT_VALGRIND_ENABLED=\n+}\n+\n+maybe_setup_valgrind () {\n+\ttest -z \"$GIT_VALGRIND\" && return\n+\tif test -z \"$valgrind_only\"\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=t\n+\t\treturn\n+\tfi\n+\tGIT_VALGRIND_ENABLED=\n+\tif match_pattern_list $test_count $valgrind_only\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=t\n+\tfi\n+}\n+\n # Called from test_skip after it has incremented $test_count. This\n # means it runs before any test-specific code and output.\n test_setup_hook_ () {\n \tmaybe_setup_verbose\n+\tmaybe_setup_valgrind\n }\n \n # Called at the end of test_expect_*. This means it runs after all\n # test-specific code and output.\n test_teardown_hook_ () {\n+\tmaybe_teardown_valgrind\n \tmaybe_teardown_verbose\n }\n \n@@ -590,6 +621,9 @@ then\n \texport GIT_VALGRIND\n \tGIT_VALGRIND_MODE=\"$valgrind\"\n \texport GIT_VALGRIND_MODE\n+\tGIT_VALGRIND_ENABLED=t\n+\ttest -n \"$valgrind_only\" && GIT_VALGRIND_ENABLED=\n+\texport GIT_VALGRIND_ENABLED\n elif test -n \"$GIT_TEST_INSTALLED\"\n then\n \tGIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path) ||\ndiff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh\nindex 6b87c91..4215303 100755\n--- a/t/valgrind/valgrind.sh\n+++ b/t/valgrind/valgrind.sh\n@@ -4,6 +4,9 @@ base=$(basename \"$0\")\n \n TOOL_OPTIONS='--leak-check=no'\n \n+test -z \"$GIT_VALGRIND_ENABLED\" &&\n+exec \"$GIT_VALGRIND\"/../../\"$base\" \"$@\"\n+\n case \"$GIT_VALGRIND_MODE\" in\n memcheck-fast)\n \t;;\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221045","messageId":"cc07d2c6e7282df1c6a1e1dcac23205b9c4b8667.1371460265.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371460265.git.trast@inf.ethz.ch","subject":"[PATCH v2 5/6] test-lib: allow prefixing a custom string before \"ok N\" etc.","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-17T09:18:50Z","receivedAt":"2013-06-17T09:18:50Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"This is not really meant for external use, and thus not documented. It\nallows the next commit to neatly distinguish between sub-tests and the\nmain run.\n\nThe format is intentionally not valid TAP. The use in the next commit\nwould not result in anything valid either way, and it seems better to\nmake it obvious.\n\nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 27 +++++++++++++++------------\n 1 file changed, 15 insertions(+), 12 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 40bd7da..aaf6084 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -209,6 +209,9 @@ do\n \t--root=*)\n \t\troot=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n \t\tshift ;;\n+\t--statusprefix=*)\n+\t\tstatusprefix=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t*)\n \t\techo \"error: unknown test option '$1'\" >&2; exit 1 ;;\n \tesac\n@@ -316,12 +319,12 @@ trap 'die' EXIT\n \n test_ok_ () {\n \ttest_success=$(($test_success + 1))\n-\tsay_color \"\" \"ok $test_count - $@\"\n+\tsay_color \"\" \"${statusprefix}ok $test_count - $@\"\n }\n \n test_failure_ () {\n \ttest_failure=$(($test_failure + 1))\n-\tsay_color error \"not ok $test_count - $1\"\n+\tsay_color error \"${statusprefix}not ok $test_count - $1\"\n \tshift\n \techo \"$@\" | sed -e 's/^/#\t/'\n \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n@@ -329,12 +332,12 @@ test_failure_ () {\n \n test_known_broken_ok_ () {\n \ttest_fixed=$(($test_fixed+1))\n-\tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n+\tsay_color error \"${statusprefix}ok $test_count - $@ # TODO known breakage vanished\"\n }\n \n test_known_broken_failure_ () {\n \ttest_broken=$(($test_broken+1))\n-\tsay_color warn \"not ok $test_count - $@ # TODO known breakage\"\n+\tsay_color warn \"${statusprefix}not ok $test_count - $@ # TODO known breakage\"\n }\n \n test_debug () {\n@@ -460,8 +463,8 @@ test_skip () {\n \t\t\tof_prereq=\" of $test_prereq\"\n \t\tfi\n \n-\t\tsay_color skip >&3 \"skipping test: $@\"\n-\t\tsay_color skip \"ok $test_count # skip $1 (missing $missing_prereq${of_prereq})\"\n+\t\tsay_color skip >&3 \"${statusprefix}skipping test: $@\"\n+\t\tsay_color skip \"${statusprefix}ok $test_count # skip $1 (missing $missing_prereq${of_prereq})\"\n \t\t: true\n \t\t;;\n \t*)\n@@ -497,11 +500,11 @@ test_done () {\n \n \tif test \"$test_fixed\" != 0\n \tthen\n-\t\tsay_color error \"# $test_fixed known breakage(s) vanished; please update test(s)\"\n+\t\tsay_color error \"${statusprefix}# $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\tsay_color warn \"${statusprefix}# still have $test_broken known breakage(s)\"\n \tfi\n \tif test \"$test_broken\" != 0 || test \"$test_fixed\" != 0\n \tthen\n@@ -524,9 +527,9 @@ test_done () {\n \t\tthen\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\t\tsay_color pass \"${statusprefix}# passed all $msg\"\n \t\t\tfi\n-\t\t\tsay \"1..$test_count$skip_all\"\n+\t\t\tsay \"${statusprefix}1..$test_count$skip_all\"\n \t\tfi\n \n \t\ttest -d \"$remove_trash\" &&\n@@ -540,8 +543,8 @@ test_done () {\n \t*)\n \t\tif test $test_external_has_tap -eq 0\n \t\tthen\n-\t\t\tsay_color error \"# failed $test_failure among $msg\"\n-\t\t\tsay \"1..$test_count\"\n+\t\t\tsay_color error \"${statusprefix}# failed $test_failure among $msg\"\n+\t\t\tsay \"${statusprefix}1..$test_count\"\n \t\tfi\n \n \t\texit 1 ;;\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221047","messageId":"d68924be9b176de44c91d0378810e1120d507fd2.1371460265.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371460265.git.trast@inf.ethz.ch","subject":"[PATCH v2 6/6] test-lib: support running tests under valgrind in parallel","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-17T09:18:51Z","receivedAt":"2013-06-17T09:18:51Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"With the new --valgrind-parallel= option, we support running the\ntests in a single test script under valgrind in parallel using 'n'\nprocesses.\n\nThis really follows the dumbest approach possible, as follows:\n\n* We spawn the test script 'n' times, using a throw-away\n TEST_OUTPUT_DIRECTORY. Each of the instances is given options that\n ensures that it only runs every n-th test under valgrind, but\n together they cover the entire range.\n\n* We add up the numbers from the individual tests, and provide the\n usual output.\n\nThis is really a gross hack at this point, and should be improved. In\nparticular we should keep the actual outputs somewhere more easily\ndiscoverable, and summarize them to the user.\n\nNevertheless, this is already workable and gives a speedup of more\nthan 2 on a dual-core (hyperthreaded) machine, using n=4. This is\nexpected since the overhead of valgrind is so big (on the order of 20x\nunder good conditions, and a large startup overhead at every git\ninvocation) that redundantly running the non-valgrind tests in between\nis not that expensive.\n\nHelped-by: Jeff King \nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 106 ++++++++++++++++++++++++++++++++++++++++++++++------------\n 1 file changed, 84 insertions(+), 22 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex aaf6084..ca293c6 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -204,6 +204,15 @@ do\n \t--valgrind-only=*)\n \t\tvalgrind_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n \t\tshift ;;\n+\t--valgrind-parallel=*)\n+\t\tvalgrind_parallel=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n+\t--valgrind-only-stride=*)\n+\t\tvalgrind_only_stride=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n+\t--valgrind-only-offset=*)\n+\t\tvalgrind_only_offset=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t--tee)\n \t\tshift ;; # was handled already\n \t--root=*)\n@@ -217,7 +226,7 @@ do\n \tesac\n done\n \n-if test -n \"$valgrind_only\"\n+if test -n \"$valgrind_only\" || test -n \"$valgrind_only_stride\"\n then\n \ttest -z \"$valgrind\" && valgrind=memcheck\n \ttest -z \"$verbose\" && verbose_only=\"$valgrind_only\"\n@@ -367,7 +376,9 @@ maybe_teardown_verbose () {\n last_verbose=t\n maybe_setup_verbose () {\n \ttest -z \"$verbose_only\" && return\n-\tif match_pattern_list $test_count $verbose_only\n+\tif match_pattern_list $test_count $verbose_only ||\n+\t\t{ test -n \"$valgrind_only_stride\" &&\n+\t\texpr $test_count \"%\" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null; }\n \tthen\n \t\texec 4>&2 3>&1\n \t\t# Emit a delimiting blank line when going from\n@@ -391,7 +402,7 @@ maybe_teardown_valgrind () {\n \n maybe_setup_valgrind () {\n \ttest -z \"$GIT_VALGRIND\" && return\n-\tif test -z \"$valgrind_only\"\n+\tif test -z \"$valgrind_only\" && test -z \"$valgrind_only_stride\"\n \tthen\n \t\tGIT_VALGRIND_ENABLED=t\n \t\treturn\n@@ -400,6 +411,10 @@ maybe_setup_valgrind () {\n \tif match_pattern_list $test_count $valgrind_only\n \tthen\n \t\tGIT_VALGRIND_ENABLED=t\n+\telif test -n \"$valgrind_only_stride\" &&\n+\t\texpr $test_count \"%\" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=t\n \tfi\n }\n \n@@ -552,6 +567,9 @@ test_done () {\n \tesac\n }\n \n+\n+# Set up a directory that we can put in PATH which redirects all git\n+# calls to 'valgrind git ...'.\n if test -n \"$valgrind\"\n then\n \tmake_symlink () {\n@@ -599,33 +617,42 @@ then\n \t\tmake_symlink \"$symlink_target\" \"$GIT_VALGRIND/bin/$base\" || exit\n \t}\n \n-\t# override all git executables in TEST_DIRECTORY/..\n-\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind\n-\tmkdir -p \"$GIT_VALGRIND\"/bin\n-\tfor file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*\n-\tdo\n-\t\tmake_valgrind_symlink $file\n-\tdone\n-\t# special-case the mergetools loadables\n-\tmake_symlink \"$GIT_BUILD_DIR\"/mergetools \"$GIT_VALGRIND/bin/mergetools\"\n-\tOLDIFS=$IFS\n-\tIFS=:\n-\tfor path in $PATH\n-\tdo\n-\t\tls \"$path\"/git-* 2> /dev/null |\n-\t\twhile read file\n+\t# In the case of --valgrind-parallel, we only need to do the\n+\t# wrapping once, in the main script. The worker children all\n+\t# have $valgrind_only_stride set, so we can skip based on that.\n+\tif test -z \"$valgrind_stride\"\n+\tthen\n+\t\t# override all git executables in TEST_DIRECTORY/..\n+\t\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind\n+\t\tmkdir -p \"$GIT_VALGRIND\"/bin\n+\t\tfor file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*\n \t\tdo\n-\t\t\tmake_valgrind_symlink \"$file\"\n+\t\t\tmake_valgrind_symlink $file\n \t\tdone\n-\tdone\n-\tIFS=$OLDIFS\n+\t\t# special-case the mergetools loadables\n+\t\tmake_symlink \"$GIT_BUILD_DIR\"/mergetools \"$GIT_VALGRIND/bin/mergetools\"\n+\t\tOLDIFS=$IFS\n+\t\tIFS=:\n+\t\tfor path in $PATH\n+\t\tdo\n+\t\t\tls \"$path\"/git-* 2> /dev/null |\n+\t\t\twhile read file\n+\t\t\tdo\n+\t\t\t\tmake_valgrind_symlink \"$file\"\n+\t\t\tdone\n+\t\tdone\n+\t\tIFS=$OLDIFS\n+\tfi\n \tPATH=$GIT_VALGRIND/bin:$PATH\n \tGIT_EXEC_PATH=$GIT_VALGRIND/bin\n \texport GIT_VALGRIND\n \tGIT_VALGRIND_MODE=\"$valgrind\"\n \texport GIT_VALGRIND_MODE\n \tGIT_VALGRIND_ENABLED=t\n-\ttest -n \"$valgrind_only\" && GIT_VALGRIND_ENABLED=\n+\tif test -n \"$valgrind_only\" || test -n \"$valgrind_only_stride\"\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=\n+\tfi\n \texport GIT_VALGRIND_ENABLED\n elif test -n \"$GIT_TEST_INSTALLED\"\n then\n@@ -711,6 +738,41 @@ then\n else\n \tmkdir -p \"$TRASH_DIRECTORY\"\n fi\n+\n+# Gross hack to spawn N sub-instances of the tests in parallel, and\n+# summarize the results. Note that if this is enabled, the script\n+# terminates at the end of this 'if' block.\n+if test -n \"$valgrind_parallel\"\n+then\n+\tfor i in $(test_seq 1 $valgrind_parallel)\n+\tdo\n+\t\troot=\"$TRASH_DIRECTORY/vgparallel-$i\"\n+\t\tmkdir \"$root\"\n+\t\tTEST_OUTPUT_DIRECTORY=\"$root\" \\\n+\t\t\t${SHELL_PATH} \"$0\" \\\n+\t\t\t--root=\"$root\" --statusprefix=\"[$i] \" \\\n+\t\t\t--valgrind=\"$valgrind\" \\\n+\t\t\t--valgrind-only-stride=\"$valgrind_parallel\" \\\n+\t\t\t--valgrind-only-offset=\"$i\" &\n+\t\tpids=\"$pids $!\"\n+\tdone\n+\ttrap \"kill $pids\" INT TERM HUP\n+\twait $pids\n+\ttrap - INT TERM HUP\n+\tfor i in $(test_seq 1 $valgrind_parallel)\n+\tdo\n+\t\troot=\"$TRASH_DIRECTORY/vgparallel-$i\"\n+\t\teval \"$(cat \"$root/test-results/$(basename \"$0\" .sh)\"-*.counts |\n+\t\t\tsed 's/^\\([a-z][a-z]*\\) \\([0-9][0-9]*\\)/inner_\\1=\\2/')\"\n+\t\ttest_count=$(expr $test_count + $inner_total)\n+\t\ttest_success=$(expr $test_success + $inner_success)\n+\t\ttest_fixed=$(expr $test_fixed + $inner_fixed)\n+\t\ttest_broken=$(expr $test_broken + $inner_broken)\n+\t\ttest_failure=$(expr $test_failure + $inner_failed)\n+\tdone\n+\ttest_done\n+fi\n+\n # Use -P to resolve symlinks in our working directory so that the cwd\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$TRASH_DIRECTORY\" || exit 1\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221143","messageId":"20130618053710.GB5916@sigill.intra.peff.net","threadId":"33843","inReplyTo":"8739d3ba5947c60d402030fc35d47115e7a51b0f.1371460265.git.trast@inf.ethz.ch","subject":"Re: [PATCH v2 3/6] test-lib: verbose mode for only tests matching a pattern","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-06-18T05:37:12Z","receivedAt":"2013-06-18T05:37:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 17, 2013 at 11:18:48AM +0200, Thomas Rast wrote:\n\n> As suggested by Jeff King, this takes care to wrap the entire test_expect_*\n> block, but nothing else, in the verbose toggling. To that end we use\n> a new pair of hook functions. The placement is a bit weird because we\n> need to wait until the beginning of test_skip for $test_count to be\n> incremented.\n\nI guess it is not surprising because I suggested it, but I find the new\nsetup/teardown logic easier to follow than the old toggle. The weird\nplacement did throw me for a minute while reading the patch. I wonder if\nit is worth pulling the increment out of test_skip, to have something\nlike this in test_expect_*:\n\n test_start ;# increment number, run setup hooks\n if ! test_skip\n then\n ...\n fi\n test_finish ;# teardown hooks\n\nThen it is a bit easier to see that each start has a finish (whereas in\nthe current version, the setups in test_skip are matched by individual\nteardowns in each caller). I did not look too hard at it, though, so I\nwouldn't be surprised if there is some other hidden order dependency\nthat makes that not work. :)\n\n> +# Called from test_skip after it has incremented $test_count. This\n> +# means it runs before any test-specific code and output.\n> +test_setup_hook_ () {\n> +\tmaybe_setup_verbose\n> +}\n> +\n> +# Called at the end of test_expect_*. This means it runs after all\n> +# test-specific code and output.\n> +test_teardown_hook_ () {\n> +\tmaybe_teardown_verbose\n> +}\n\nSo these do your verbose setup/teardown. Makes sense.\n\nBut then what is this hunk doing:\n\n> test_eval_ () {\n> \t# This is a separate function because some tests use\n> \t# \"return\" to end a test_expect_success block early.\n> @@ -358,9 +399,7 @@ test_run_ () {\n> \n> \tif test -z \"$immediate\" || test $eval_ret = 0 || test -n \"$expecting_failure\"\n> \tthen\n> -\t\tsetup_malloc_check\n> \t\ttest_eval_ \"$test_cleanup\"\n> -\t\tteardown_malloc_check\n> \tfi\n> \tif test \"$verbose\" = \"t\" && test -n \"$HARNESS_ACTIVE\"\n> \tthen\n\nAt first I thought these should go into your setup/teardown hooks, but I\ndon't think so. They are made redundant by patch 1/6, which wraps all of\ntest_eval with malloc setup/teardown, aren't they? Should this hunk be\nin patch 1?\n\n-Peff\n"},{"id":"221144","messageId":"20130618054607.GC5916@sigill.intra.peff.net","threadId":"33843","inReplyTo":"cover.1371460265.git.trast@inf.ethz.ch","subject":"Re: [PATCH v2 0/6] --valgrind improvements","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-06-18T05:46:07Z","receivedAt":"2013-06-18T05:46:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 17, 2013 at 11:18:45AM +0200, Thomas Rast wrote:\n\n> Here's the promised reroll. It integrates everyone's suggestions (I\n> hope I didn't miss any), most notably the two by Peff:\n\nThanks. With the exception of a few comments on patch 3, this looks good\nto me.\n\nI agree with your \"gross hack\" assessment of patch 6/6 (besides the\ngeneral implementation, it would fail pretty badly for any test script\nwhich had global state outside of the filesystem, like the lib-httpd\ntests). But I also think it cannot not cause too much damage, being both\nconfined to the test scripts and only having an effect if the option is\nenabled. So I'd be fine with it going in just to see if people end up\nfinding it useful.\n\n-Peff\n"},{"id":"221150","messageId":"51C0064B.8060408@viscovery.net","threadId":"33843","inReplyTo":"4795e6f40e7ce62fc4c414e8cbbd2aa777d3fbb0.1371460265.git.trast@inf.ethz.ch","subject":"Re: [PATCH v2 2/6] test-lib: refactor $GIT_SKIP_TESTS matching","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-06-18T07:03:39Z","receivedAt":"2013-06-18T07:03:39Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 6/17/2013 11:18, schrieb Thomas Rast:\n> +match_pattern_list () {\n> +\targ=\"$1\"\n> +\tshift\n> +\ttest -z \"$*\" && return 1\n> +\tfor pat\n> +\tdo\n> +\t\tcase \"$arg\" in\n> +\t\t$pat)\n> +\t\t\treturn 0\n> +\t\tesac\n> +\tdone\n> +\treturn 1\n> +}\n\nWatch this failing test case:\n\n GIT_SKIP_TESTS=\"t950[012]\" ./t3006-ls-files-long.sh\n\n'pat' is too short and too sweet to be used as a shell variable name in a\nlibrary function. ;)\n\n-- Hannes\n"},{"id":"221161","messageId":"87fvwfddyn.fsf@linux-k42r.v.cablecom.net","threadId":"33843","inReplyTo":"51C0064B.8060408@viscovery.net","subject":"Re: [PATCH v2 2/6] test-lib: refactor $GIT_SKIP_TESTS matching","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-18T08:23:44Z","receivedAt":"2013-06-18T08:23:44Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Johannes Sixt writes:\n\n> Am 6/17/2013 11:18, schrieb Thomas Rast:\n>> +match_pattern_list () {\n>> +\targ=\"$1\"\n>> +\tshift\n>> +\ttest -z \"$*\" && return 1\n>> +\tfor pat\n>> +\tdo\n>> +\t\tcase \"$arg\" in\n>> +\t\t$pat)\n>> +\t\t\treturn 0\n>> +\t\tesac\n>> +\tdone\n>> +\treturn 1\n>> +}\n>\n> Watch this failing test case:\n>\n> GIT_SKIP_TESTS=\"t950[012]\" ./t3006-ls-files-long.sh\n>\n> 'pat' is too short and too sweet to be used as a shell variable name in a\n> library function. ;)\n\nGah! Serves me right for not testing test-lib. Thanks for noticing.\n\n/me goes stand in the corner for a while\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"221163","messageId":"8761xbdcxk.fsf@linux-k42r.v.cablecom.net","threadId":"33843","inReplyTo":"20130618053710.GB5916@sigill.intra.peff.net","subject":"Re: [PATCH v2 3/6] test-lib: verbose mode for only tests matching a pattern","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-18T08:45:59Z","receivedAt":"2013-06-18T08:45:59Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King writes:\n\n> On Mon, Jun 17, 2013 at 11:18:48AM +0200, Thomas Rast wrote:\n>\n>> As suggested by Jeff King, this takes care to wrap the entire test_expect_*\n>> block, but nothing else, in the verbose toggling. To that end we use\n>> a new pair of hook functions. The placement is a bit weird because we\n>> need to wait until the beginning of test_skip for $test_count to be\n>> incremented.\n[...]\n> test_start ;# increment number, run setup hooks\n> if ! test_skip\n> then\n> ...\n> fi\n> test_finish ;# teardown hooks\n>\n> Then it is a bit easier to see that each start has a finish (whereas in\n> the current version, the setups in test_skip are matched by individual\n> teardowns in each caller). I did not look too hard at it, though, so I\n> wouldn't be surprised if there is some other hidden order dependency\n> that makes that not work. :)\n\nNo, I think that's actually very reasonable. I'll do it that way in v3.\n\n> But then what is this hunk doing:\n>\n>> test_eval_ () {\n>> \t# This is a separate function because some tests use\n>> \t# \"return\" to end a test_expect_success block early.\n>> @@ -358,9 +399,7 @@ test_run_ () {\n>> \n>> \tif test -z \"$immediate\" || test $eval_ret = 0 || test -n \"$expecting_failure\"\n>> \tthen\n>> -\t\tsetup_malloc_check\n>> \t\ttest_eval_ \"$test_cleanup\"\n>> -\t\tteardown_malloc_check\n>> \tfi\n>> \tif test \"$verbose\" = \"t\" && test -n \"$HARNESS_ACTIVE\"\n>> \tthen\n\nThanks for catching this -- it's just a mis-edit that would effectively\nrevert 1/6.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"221200","messageId":"cover.1371557431.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371460265.git.trast@inf.ethz.ch","subject":"[PATCH v3 0/8] --valgrind improvements","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-18T12:25:56Z","receivedAt":"2013-06-18T12:25:56Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Changes from v2:\n\n- $pat renamed to $pattern_ to avoid collisions (thanks j6t)\n\n- New patch 3 that tests --verbose operation, and additions to patch\n (now) 4 that test --verbose-only. (Adding similar tests for\n --valgrind[-only] and associated options would be nice, but is much\n harder because the user may not have valgrind.)\n\n- Rearranged the hooking in (now) 4, 6 and 8 to make it more obvious\n what is going on, as per Peff's comments\n\n- Fixed a misspelled variable that prevented the \"valgrind setup only\n once\" logic from working\n\nThomas Rast (8):\n test-lib: enable MALLOC_* for the actual tests\n test-lib: refactor $GIT_SKIP_TESTS matching\n test-lib: rearrange start/end of test_expect_* and test_skip\n test-lib: self-test that --verbose works\n test-lib: verbose mode for only tests matching a pattern\n test-lib: valgrind for only tests matching a pattern\n test-lib: allow prefixing a custom string before \"ok N\" etc.\n test-lib: support running tests under valgrind in parallel\n\n t/README | 10 ++\n t/t0000-basic.sh | 54 ++++++++++-\n t/test-lib-functions.sh | 6 +-\n t/test-lib.sh | 244 ++++++++++++++++++++++++++++++++++++++----------\n t/valgrind/valgrind.sh | 3 +\n 5 files changed, 265 insertions(+), 52 deletions(-)\n\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221201","messageId":"60de7a05a5ad90c4ade5570e4cd6e83f84a06568.1371557431.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371557431.git.trast@inf.ethz.ch","subject":"[PATCH v3 1/8] test-lib: enable MALLOC_* for the actual tests","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-18T12:25:57Z","receivedAt":"2013-06-18T12:25:57Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"1b3185f (MALLOC_CHECK: various clean-ups, 2012-09-14) moved around the\nMALLOC_CHECK_ and MALLOC_PERTURB_ assignments, intending to limit\ntheir effect to only the test runs. However, they were actually\nenabled only during test cleanup. Call setup/teardown_malloc_check\nalso around the evaluation of the actual test snippet.\n\nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex eff3a65..35da859 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -337,8 +337,10 @@ test_eval_ () {\n test_run_ () {\n \ttest_cleanup=:\n \texpecting_failure=$2\n+\tsetup_malloc_check\n \ttest_eval_ \"$1\"\n \teval_ret=$?\n+\tteardown_malloc_check\n \n \tif test -z \"$immediate\" || test $eval_ret = 0 || test -n \"$expecting_failure\"\n \tthen\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221203","messageId":"ad912196db92e34793348462db2e03333ec9eb21.1371557431.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371557431.git.trast@inf.ethz.ch","subject":"[PATCH v3 2/8] test-lib: refactor $GIT_SKIP_TESTS matching","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-18T12:25:58Z","receivedAt":"2013-06-18T12:25:58Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"It's already used twice, and we will have more of the same kind of\nmatching in a minute.\n\nHelped-by: Johannes Sixt \nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 41 ++++++++++++++++++++++++-----------------\n 1 file changed, 24 insertions(+), 17 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 35da859..4fa141a 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -328,6 +328,20 @@ test_debug () {\n \ttest \"$debug\" = \"\" || eval \"$1\"\n }\n \n+match_pattern_list () {\n+\targ=\"$1\"\n+\tshift\n+\ttest -z \"$*\" && return 1\n+\tfor pattern_\n+\tdo\n+\t\tcase \"$arg\" in\n+\t\t$pattern_)\n+\t\t\treturn 0\n+\t\tesac\n+\tdone\n+\treturn 1\n+}\n+\n test_eval_ () {\n \t# This is a separate function because some tests use\n \t# \"return\" to end a test_expect_success block early.\n@@ -358,14 +372,10 @@ test_run_ () {\n test_skip () {\n \ttest_count=$(($test_count+1))\n \tto_skip=\n-\tfor skp in $GIT_SKIP_TESTS\n-\tdo\n-\t\tcase $this_test.$test_count in\n-\t\t$skp)\n-\t\t\tto_skip=t\n-\t\t\tbreak\n-\t\tesac\n-\tdone\n+\tif match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS\n+\tthen\n+\t\tto_skip=t\n+\tfi\n \tif test -z \"$to_skip\" && test -n \"$test_prereq\" &&\n \t ! test_have_prereq \"$test_prereq\"\n \tthen\n@@ -630,15 +640,12 @@ cd -P \"$TRASH_DIRECTORY\" || exit 1\n \n this_test=${0##*/}\n this_test=${this_test%%-*}\n-for skp in $GIT_SKIP_TESTS\n-do\n-\tcase \"$this_test\" in\n-\t$skp)\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-done\n+if match_pattern_list \"$this_test\" $GIT_SKIP_TESTS\n+then\n+\tsay_color info >&3 \"skipping test $this_test altogether\"\n+\tskip_all=\"skip all tests in $this_test\"\n+\ttest_done\n+fi\n \n # Provide an implementation of the 'yes' utility\n yes () {\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221202","messageId":"0c11f94a00644b0fe8d2fcc3620ece530868f5af.1371557431.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371557431.git.trast@inf.ethz.ch","subject":"[PATCH v3 3/8] test-lib: rearrange start/end of test_expect_* and test_skip","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-18T12:25:59Z","receivedAt":"2013-06-18T12:25:59Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"This moves\n\n* the early setup part from test_skip to a new function test_start_\n\n* the final common parts of test_expect_* to a new function\n test_finish_\n\nto make the next commit more obvious.\n\nSigned-off-by: Thomas Rast \n---\n t/test-lib-functions.sh | 6 ++++--\n t/test-lib.sh | 9 ++++++++-\n 2 files changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 5251009..ad6d60a 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -343,6 +343,7 @@ test_declared_prereq () {\n }\n \n test_expect_failure () {\n+\ttest_start_\n \ttest \"$#\" = 3 && { test_prereq=$1; shift; } || test_prereq=\n \ttest \"$#\" = 2 ||\n \terror \"bug in the test script: not 2 or 3 parameters to test-expect-failure\"\n@@ -357,10 +358,11 @@ test_expect_failure () {\n \t\t\ttest_known_broken_failure_ \"$1\"\n \t\tfi\n \tfi\n-\techo >&3 \"\"\n+\ttest_finish_\n }\n \n test_expect_success () {\n+\ttest_start_\n \ttest \"$#\" = 3 && { test_prereq=$1; shift; } || test_prereq=\n \ttest \"$#\" = 2 ||\n \terror \"bug in the test script: not 2 or 3 parameters to test-expect-success\"\n@@ -375,7 +377,7 @@ test_expect_success () {\n \t\t\ttest_failure_ \"$@\"\n \t\tfi\n \tfi\n-\techo >&3 \"\"\n+\ttest_finish_\n }\n \n # test_external runs external test scripts that provide continuous\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 4fa141a..e99b0ea 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -369,8 +369,15 @@ test_run_ () {\n \treturn \"$eval_ret\"\n }\n \n-test_skip () {\n+test_start_ () {\n \ttest_count=$(($test_count+1))\n+}\n+\n+test_finish_ () {\n+\techo >&3 \"\"\n+}\n+\n+test_skip () {\n \tto_skip=\n \tif match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS\n \tthen\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221207","messageId":"81378a78ec46f876a2b426be99cbb6c3f85498f9.1371557431.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371557431.git.trast@inf.ethz.ch","subject":"[PATCH v3 4/8] test-lib: self-test that --verbose works","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-18T12:26:00Z","receivedAt":"2013-06-18T12:26:00Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"t0000 contains some light self-tests of test-lib.sh, but --verbose was\nnot covered. Add a test.\n\nThe only catch is that the presence of a test harness influences the\noutput (specifically, the presence of some empty lines). The easiest\nfix is to unset TEST_HARNESS for the sub-test scripts. This means\nthat we no longer check whether test-lib.sh works under the harness;\nhowever, since running everything under 'prove' seems far more common\nthan the esoteric --verbose-only feature introduced in the next\ncommit, this seems the smaller risk.\n\nSigned-off-by: Thomas Rast \n---\n t/t0000-basic.sh | 28 +++++++++++++++++++++++++++-\n 1 file changed, 27 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex cefe33d..b568c06 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -47,8 +47,10 @@ test_expect_failure 'pretend we have a known breakage' '\n \n run_sub_test_lib_test () {\n \tname=\"$1\" descr=\"$2\" # stdin is the body of the test code\n+\tshift 2\n \tmkdir \"$name\" &&\n \t(\n+\t\tunset HARNESS_ACTIVE\n \t\tcd \"$name\" &&\n \t\tcat >\"$name.sh\" <<-EOF &&\n \t\t#!$SHELL_PATH\n@@ -65,7 +67,7 @@ run_sub_test_lib_test () {\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\t./\"$name.sh\" \"$@\" >out 2>err\n \t)\n }\n \n@@ -215,6 +217,30 @@ test_expect_success 'pretend we have a mix of all possible results' \"\n \tEOF\n \"\n \n+test_expect_success 'test --verbose' '\n+\ttest_must_fail run_sub_test_lib_test \\\n+\t\ttest-verbose \"test verbose\" --verbose <<-\\EOF &&\n+\ttest_expect_success \"passing test\" true\n+\ttest_expect_success \"test with output\" \"echo foo\"\n+\ttest_expect_success \"failing test\" false\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test test-verbose <<-\\EOF\n+\t> expecting success: true\n+\t> ok 1 - passing test\n+\t>\n+\t> expecting success: echo foo\n+\t> foo\n+\t>\n+\t> ok 2 - test with output\n+\t> expecting success: false\n+\t> not ok 3 - failing test\n+\t> #\tfalse\n+\t> # failed 1 among 3 test(s)\n+\t> 1..3\n+\tEOF\n+'\n+\n test_set_prereq HAVEIT\n haveit=no\n test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221206","messageId":"a48122be29cc9088cc0aa388e9148bc62c1712dd.1371557431.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371557431.git.trast@inf.ethz.ch","subject":"[PATCH v3 5/8] test-lib: verbose mode for only tests matching a pattern","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-18T12:26:01Z","receivedAt":"2013-06-18T12:26:01Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"With the new --verbose-only= option, one can enable --verbose\nat a per-test granularity. The pattern is matched against the test\nnumber, e.g.\n\n ./t0000-basic.sh --verbose-only='2[0-2]'\n\nto see only the full output of test 20-22, while showing the rest in the\none-liner format.\n\nAs suggested by Jeff King, this takes care to wrap the entire\ntest_expect_* block, but nothing else, in the verbose toggling. We\ncan use the test_start/end functions from the previous commit for the\npurpose.\n\nThis is arguably not *too* useful on its own, but makes the next patch\neasier to follow.\n\nHelped-by: Jeff King \nSigned-off-by: Thomas Rast \n---\n t/README | 5 +++++\n t/t0000-basic.sh | 30 ++++++++++++++++++++++++++++--\n t/test-lib.sh | 31 +++++++++++++++++++++++++++++++\n 3 files changed, 64 insertions(+), 2 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 35b3c5c..f4e6299 100644\n--- a/t/README\n+++ b/t/README\n@@ -76,6 +76,11 @@ appropriately before running \"make\".\n \tcommand being run and their output if any are also\n \toutput.\n \n+--verbose-only=::\n+\tLike --verbose, but the effect is limited to tests with\n+\tnumbers matching . The number matched against is\n+\tsimply the running count of the test within the file.\n+\n --debug::\n \tThis may help the person who is developing a new test.\n \tIt causes the command defined with test_debug to run.\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex b568c06..0d86039 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -225,17 +225,43 @@ test_expect_success 'test --verbose' '\n \ttest_expect_success \"failing test\" false\n \ttest_done\n \tEOF\n+\tmv test-verbose/out test-verbose/out+\n+\tgrep -v \"^Initialized empty\" test-verbose/out+ >test-verbose/out &&\n \tcheck_sub_test_lib_test test-verbose <<-\\EOF\n \t> expecting success: true\n \t> ok 1 - passing test\n-\t>\n+\t> Z\n \t> expecting success: echo foo\n \t> foo\n-\t>\n \t> ok 2 - test with output\n+\t> Z\n \t> expecting success: false\n \t> not ok 3 - failing test\n \t> #\tfalse\n+\t> Z\n+\t> # failed 1 among 3 test(s)\n+\t> 1..3\n+\tEOF\n+'\n+\n+test_expect_success 'test --verbose-only' '\n+\ttest_must_fail run_sub_test_lib_test \\\n+\t\ttest-verbose-only-2 \"test verbose-only=2\" \\\n+\t\t--verbose-only=2 <<-\\EOF &&\n+\ttest_expect_success \"passing test\" true\n+\ttest_expect_success \"test with output\" \"echo foo\"\n+\ttest_expect_success \"failing test\" false\n+\ttest_done\n+\tEOF\n+\tcheck_sub_test_lib_test test-verbose-only-2 <<-\\EOF\n+\t> ok 1 - passing test\n+\t> Z\n+\t> expecting success: echo foo\n+\t> foo\n+\t> ok 2 - test with output\n+\t> Z\n+\t> not ok 3 - failing test\n+\t> #\tfalse\n \t> # failed 1 among 3 test(s)\n \t> 1..3\n \tEOF\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex e99b0ea..2bceb92 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -184,6 +184,9 @@ do\n \t\thelp=t; shift ;;\n \t-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)\n \t\tverbose=t; shift ;;\n+\t--verbose-only=*)\n+\t\tverbose_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t-q|--q|--qu|--qui|--quie|--quiet)\n \t\t# Ignore --quiet under a TAP::Harness. Saying how many tests\n \t\t# passed without the ok/not ok details is always an error.\n@@ -342,6 +345,32 @@ match_pattern_list () {\n \treturn 1\n }\n \n+maybe_teardown_verbose () {\n+\ttest -z \"$verbose_only\" && return\n+\texec 4>/dev/null 3>/dev/null\n+\tverbose=\n+}\n+\n+last_verbose=t\n+maybe_setup_verbose () {\n+\ttest -z \"$verbose_only\" && return\n+\tif match_pattern_list $test_count $verbose_only\n+\tthen\n+\t\texec 4>&2 3>&1\n+\t\t# Emit a delimiting blank line when going from\n+\t\t# non-verbose to verbose. Within verbose mode the\n+\t\t# delimiter is printed by test_expect_*. The choice\n+\t\t# of the initial $last_verbose is such that before\n+\t\t# test 1, we do not print it.\n+\t\ttest -z \"$last_verbose\" && echo >&3 \"\"\n+\t\tverbose=t\n+\telse\n+\t\texec 4>/dev/null 3>/dev/null\n+\t\tverbose=\n+\tfi\n+\tlast_verbose=$verbose\n+}\n+\n test_eval_ () {\n \t# This is a separate function because some tests use\n \t# \"return\" to end a test_expect_success block early.\n@@ -371,10 +400,12 @@ test_run_ () {\n \n test_start_ () {\n \ttest_count=$(($test_count+1))\n+\tmaybe_setup_verbose\n }\n \n test_finish_ () {\n \techo >&3 \"\"\n+\tmaybe_teardown_verbose\n }\n \n test_skip () {\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221204","messageId":"b9bc80e3a1e335ff6122dcc42c97247c6aa7ef56.1371557431.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371557431.git.trast@inf.ethz.ch","subject":"[PATCH v3 6/8] test-lib: valgrind for only tests matching a pattern","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-18T12:26:02Z","receivedAt":"2013-06-18T12:26:02Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"With the new --valgrind-only= option, one can enable\n--valgrind at a per-test granularity, exactly analogous to\n--verbose-only from the previous commit.\n\nThe options are wired such that --valgrind implies --verbose (as\nbefore), but --valgrind-only= implies\n--verbose-only= unless --verbose is also in effect.\n\nSigned-off-by: Thomas Rast \n---\n t/README | 5 +++++\n t/test-lib.sh | 36 +++++++++++++++++++++++++++++++++++-\n t/valgrind/valgrind.sh | 3 +++\n 3 files changed, 43 insertions(+), 1 deletion(-)\n\ndiff --git a/t/README b/t/README\nindex f4e6299..abe991f 100644\n--- a/t/README\n+++ b/t/README\n@@ -126,6 +126,11 @@ appropriately before running \"make\".\n \tthe 't/valgrind/' directory and use the commands under\n \t't/valgrind/bin/'.\n \n+--valgrind-only=::\n+\tLike --valgrind, but the effect is limited to tests with\n+\tnumbers matching . The number matched against is\n+\tsimply the running count of the test within the file.\n+\n --tee::\n \tIn addition to printing the test output to the terminal,\n \twrite it to files named 't/test-results/$TEST_NAME.out'.\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 2bceb92..817ab43 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -201,6 +201,9 @@ do\n \t--valgrind=*)\n \t\tvalgrind=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n \t\tshift ;;\n+\t--valgrind-only=*)\n+\t\tvalgrind_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t--tee)\n \t\tshift ;; # was handled already\n \t--root=*)\n@@ -211,7 +214,14 @@ do\n \tesac\n done\n \n-test -n \"$valgrind\" && verbose=t\n+if test -n \"$valgrind_only\"\n+then\n+\ttest -z \"$valgrind\" && valgrind=memcheck\n+\ttest -z \"$verbose\" && verbose_only=\"$valgrind_only\"\n+elif test -n \"$valgrind\"\n+then\n+\tverbose=t\n+fi\n \n if test -n \"$color\"\n then\n@@ -371,6 +381,25 @@ maybe_setup_verbose () {\n \tlast_verbose=$verbose\n }\n \n+maybe_teardown_valgrind () {\n+\ttest -z \"$GIT_VALGRIND\" && return\n+\tGIT_VALGRIND_ENABLED=\n+}\n+\n+maybe_setup_valgrind () {\n+\ttest -z \"$GIT_VALGRIND\" && return\n+\tif test -z \"$valgrind_only\"\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=t\n+\t\treturn\n+\tfi\n+\tGIT_VALGRIND_ENABLED=\n+\tif match_pattern_list $test_count $valgrind_only\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=t\n+\tfi\n+}\n+\n test_eval_ () {\n \t# This is a separate function because some tests use\n \t# \"return\" to end a test_expect_success block early.\n@@ -401,10 +430,12 @@ test_run_ () {\n test_start_ () {\n \ttest_count=$(($test_count+1))\n \tmaybe_setup_verbose\n+\tmaybe_setup_valgrind\n }\n \n test_finish_ () {\n \techo >&3 \"\"\n+\tmaybe_teardown_valgrind\n \tmaybe_teardown_verbose\n }\n \n@@ -588,6 +619,9 @@ then\n \texport GIT_VALGRIND\n \tGIT_VALGRIND_MODE=\"$valgrind\"\n \texport GIT_VALGRIND_MODE\n+\tGIT_VALGRIND_ENABLED=t\n+\ttest -n \"$valgrind_only\" && GIT_VALGRIND_ENABLED=\n+\texport GIT_VALGRIND_ENABLED\n elif test -n \"$GIT_TEST_INSTALLED\"\n then\n \tGIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path) ||\ndiff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh\nindex 6b87c91..4215303 100755\n--- a/t/valgrind/valgrind.sh\n+++ b/t/valgrind/valgrind.sh\n@@ -4,6 +4,9 @@ base=$(basename \"$0\")\n \n TOOL_OPTIONS='--leak-check=no'\n \n+test -z \"$GIT_VALGRIND_ENABLED\" &&\n+exec \"$GIT_VALGRIND\"/../../\"$base\" \"$@\"\n+\n case \"$GIT_VALGRIND_MODE\" in\n memcheck-fast)\n \t;;\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221208","messageId":"7b3790c16289ff9ece9097fbfcea86db21bbeb37.1371557431.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371557431.git.trast@inf.ethz.ch","subject":"[PATCH v3 7/8] test-lib: allow prefixing a custom string before \"ok N\" etc.","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-18T12:26:03Z","receivedAt":"2013-06-18T12:26:03Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"This is not really meant for external use, and thus not documented. It\nallows the next commit to neatly distinguish between sub-tests and the\nmain run.\n\nThe format is intentionally not valid TAP. The use in the next commit\nwould not result in anything valid either way, and it seems better to\nmake it obvious.\n\nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 27 +++++++++++++++------------\n 1 file changed, 15 insertions(+), 12 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 817ab43..f2abff9 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -209,6 +209,9 @@ do\n \t--root=*)\n \t\troot=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n \t\tshift ;;\n+\t--statusprefix=*)\n+\t\tstatusprefix=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t*)\n \t\techo \"error: unknown test option '$1'\" >&2; exit 1 ;;\n \tesac\n@@ -316,12 +319,12 @@ trap 'die' EXIT\n \n test_ok_ () {\n \ttest_success=$(($test_success + 1))\n-\tsay_color \"\" \"ok $test_count - $@\"\n+\tsay_color \"\" \"${statusprefix}ok $test_count - $@\"\n }\n \n test_failure_ () {\n \ttest_failure=$(($test_failure + 1))\n-\tsay_color error \"not ok $test_count - $1\"\n+\tsay_color error \"${statusprefix}not ok $test_count - $1\"\n \tshift\n \techo \"$@\" | sed -e 's/^/#\t/'\n \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n@@ -329,12 +332,12 @@ test_failure_ () {\n \n test_known_broken_ok_ () {\n \ttest_fixed=$(($test_fixed+1))\n-\tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n+\tsay_color error \"${statusprefix}ok $test_count - $@ # TODO known breakage vanished\"\n }\n \n test_known_broken_failure_ () {\n \ttest_broken=$(($test_broken+1))\n-\tsay_color warn \"not ok $test_count - $@ # TODO known breakage\"\n+\tsay_color warn \"${statusprefix}not ok $test_count - $@ # TODO known breakage\"\n }\n \n test_debug () {\n@@ -458,8 +461,8 @@ test_skip () {\n \t\t\tof_prereq=\" of $test_prereq\"\n \t\tfi\n \n-\t\tsay_color skip >&3 \"skipping test: $@\"\n-\t\tsay_color skip \"ok $test_count # skip $1 (missing $missing_prereq${of_prereq})\"\n+\t\tsay_color skip >&3 \"${statusprefix}skipping test: $@\"\n+\t\tsay_color skip \"${statusprefix}ok $test_count # skip $1 (missing $missing_prereq${of_prereq})\"\n \t\t: true\n \t\t;;\n \t*)\n@@ -495,11 +498,11 @@ test_done () {\n \n \tif test \"$test_fixed\" != 0\n \tthen\n-\t\tsay_color error \"# $test_fixed known breakage(s) vanished; please update test(s)\"\n+\t\tsay_color error \"${statusprefix}# $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\tsay_color warn \"${statusprefix}# still have $test_broken known breakage(s)\"\n \tfi\n \tif test \"$test_broken\" != 0 || test \"$test_fixed\" != 0\n \tthen\n@@ -522,9 +525,9 @@ test_done () {\n \t\tthen\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\t\tsay_color pass \"${statusprefix}# passed all $msg\"\n \t\t\tfi\n-\t\t\tsay \"1..$test_count$skip_all\"\n+\t\t\tsay \"${statusprefix}1..$test_count$skip_all\"\n \t\tfi\n \n \t\ttest -d \"$remove_trash\" &&\n@@ -538,8 +541,8 @@ test_done () {\n \t*)\n \t\tif test $test_external_has_tap -eq 0\n \t\tthen\n-\t\t\tsay_color error \"# failed $test_failure among $msg\"\n-\t\t\tsay \"1..$test_count\"\n+\t\t\tsay_color error \"${statusprefix}# failed $test_failure among $msg\"\n+\t\t\tsay \"${statusprefix}1..$test_count\"\n \t\tfi\n \n \t\texit 1 ;;\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221205","messageId":"1f5236603e050962b3792c59170d56a2a10a482f.1371557431.git.trast@inf.ethz.ch","threadId":"33843","inReplyTo":"cover.1371557431.git.trast@inf.ethz.ch","subject":"[PATCH v3 8/8] test-lib: support running tests under valgrind in parallel","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-18T12:26:04Z","receivedAt":"2013-06-18T12:26:04Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"With the new --valgrind-parallel= option, we support running the\ntests in a single test script under valgrind in parallel using 'n'\nprocesses.\n\nThis really follows the dumbest approach possible, as follows:\n\n* We spawn the test script 'n' times, using a throw-away\n TEST_OUTPUT_DIRECTORY. Each of the instances is given options that\n ensures that it only runs every n-th test under valgrind, but\n together they cover the entire range.\n\n* We add up the numbers from the individual tests, and provide the\n usual output.\n\nThis is really a gross hack at this point, and should be improved. In\nparticular we should keep the actual outputs somewhere more easily\ndiscoverable, and summarize them to the user.\n\nNevertheless, this is already workable and gives a speedup of more\nthan 2 on a dual-core (hyperthreaded) machine, using n=4. This is\nexpected since the overhead of valgrind is so big (on the order of 20x\nunder good conditions, and a large startup overhead at every git\ninvocation) that redundantly running the non-valgrind tests in between\nis not that expensive.\n\nHelped-by: Jeff King \nSigned-off-by: Thomas Rast \n---\n t/test-lib.sh | 106 ++++++++++++++++++++++++++++++++++++++++++++++------------\n 1 file changed, 84 insertions(+), 22 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex f2abff9..acc7f2a 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -204,6 +204,15 @@ do\n \t--valgrind-only=*)\n \t\tvalgrind_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n \t\tshift ;;\n+\t--valgrind-parallel=*)\n+\t\tvalgrind_parallel=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n+\t--valgrind-only-stride=*)\n+\t\tvalgrind_only_stride=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n+\t--valgrind-only-offset=*)\n+\t\tvalgrind_only_offset=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tshift ;;\n \t--tee)\n \t\tshift ;; # was handled already\n \t--root=*)\n@@ -217,7 +226,7 @@ do\n \tesac\n done\n \n-if test -n \"$valgrind_only\"\n+if test -n \"$valgrind_only\" || test -n \"$valgrind_only_stride\"\n then\n \ttest -z \"$valgrind\" && valgrind=memcheck\n \ttest -z \"$verbose\" && verbose_only=\"$valgrind_only\"\n@@ -367,7 +376,9 @@ maybe_teardown_verbose () {\n last_verbose=t\n maybe_setup_verbose () {\n \ttest -z \"$verbose_only\" && return\n-\tif match_pattern_list $test_count $verbose_only\n+\tif match_pattern_list $test_count $verbose_only ||\n+\t\t{ test -n \"$valgrind_only_stride\" &&\n+\t\texpr $test_count \"%\" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null; }\n \tthen\n \t\texec 4>&2 3>&1\n \t\t# Emit a delimiting blank line when going from\n@@ -391,7 +402,7 @@ maybe_teardown_valgrind () {\n \n maybe_setup_valgrind () {\n \ttest -z \"$GIT_VALGRIND\" && return\n-\tif test -z \"$valgrind_only\"\n+\tif test -z \"$valgrind_only\" && test -z \"$valgrind_only_stride\"\n \tthen\n \t\tGIT_VALGRIND_ENABLED=t\n \t\treturn\n@@ -400,6 +411,10 @@ maybe_setup_valgrind () {\n \tif match_pattern_list $test_count $valgrind_only\n \tthen\n \t\tGIT_VALGRIND_ENABLED=t\n+\telif test -n \"$valgrind_only_stride\" &&\n+\t\texpr $test_count \"%\" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=t\n \tfi\n }\n \n@@ -550,6 +565,9 @@ test_done () {\n \tesac\n }\n \n+\n+# Set up a directory that we can put in PATH which redirects all git\n+# calls to 'valgrind git ...'.\n if test -n \"$valgrind\"\n then\n \tmake_symlink () {\n@@ -597,33 +615,42 @@ then\n \t\tmake_symlink \"$symlink_target\" \"$GIT_VALGRIND/bin/$base\" || exit\n \t}\n \n-\t# override all git executables in TEST_DIRECTORY/..\n-\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind\n-\tmkdir -p \"$GIT_VALGRIND\"/bin\n-\tfor file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*\n-\tdo\n-\t\tmake_valgrind_symlink $file\n-\tdone\n-\t# special-case the mergetools loadables\n-\tmake_symlink \"$GIT_BUILD_DIR\"/mergetools \"$GIT_VALGRIND/bin/mergetools\"\n-\tOLDIFS=$IFS\n-\tIFS=:\n-\tfor path in $PATH\n-\tdo\n-\t\tls \"$path\"/git-* 2> /dev/null |\n-\t\twhile read file\n+\t# In the case of --valgrind-parallel, we only need to do the\n+\t# wrapping once, in the main script. The worker children all\n+\t# have $valgrind_only_stride set, so we can skip based on that.\n+\tif test -z \"$valgrind_only_stride\"\n+\tthen\n+\t\t# override all git executables in TEST_DIRECTORY/..\n+\t\tGIT_VALGRIND=$TEST_DIRECTORY/valgrind\n+\t\tmkdir -p \"$GIT_VALGRIND\"/bin\n+\t\tfor file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*\n \t\tdo\n-\t\t\tmake_valgrind_symlink \"$file\"\n+\t\t\tmake_valgrind_symlink $file\n \t\tdone\n-\tdone\n-\tIFS=$OLDIFS\n+\t\t# special-case the mergetools loadables\n+\t\tmake_symlink \"$GIT_BUILD_DIR\"/mergetools \"$GIT_VALGRIND/bin/mergetools\"\n+\t\tOLDIFS=$IFS\n+\t\tIFS=:\n+\t\tfor path in $PATH\n+\t\tdo\n+\t\t\tls \"$path\"/git-* 2> /dev/null |\n+\t\t\twhile read file\n+\t\t\tdo\n+\t\t\t\tmake_valgrind_symlink \"$file\"\n+\t\t\tdone\n+\t\tdone\n+\t\tIFS=$OLDIFS\n+\tfi\n \tPATH=$GIT_VALGRIND/bin:$PATH\n \tGIT_EXEC_PATH=$GIT_VALGRIND/bin\n \texport GIT_VALGRIND\n \tGIT_VALGRIND_MODE=\"$valgrind\"\n \texport GIT_VALGRIND_MODE\n \tGIT_VALGRIND_ENABLED=t\n-\ttest -n \"$valgrind_only\" && GIT_VALGRIND_ENABLED=\n+\tif test -n \"$valgrind_only\" || test -n \"$valgrind_only_stride\"\n+\tthen\n+\t\tGIT_VALGRIND_ENABLED=\n+\tfi\n \texport GIT_VALGRIND_ENABLED\n elif test -n \"$GIT_TEST_INSTALLED\"\n then\n@@ -709,6 +736,41 @@ then\n else\n \tmkdir -p \"$TRASH_DIRECTORY\"\n fi\n+\n+# Gross hack to spawn N sub-instances of the tests in parallel, and\n+# summarize the results. Note that if this is enabled, the script\n+# terminates at the end of this 'if' block.\n+if test -n \"$valgrind_parallel\"\n+then\n+\tfor i in $(test_seq 1 $valgrind_parallel)\n+\tdo\n+\t\troot=\"$TRASH_DIRECTORY/vgparallel-$i\"\n+\t\tmkdir \"$root\"\n+\t\tTEST_OUTPUT_DIRECTORY=\"$root\" \\\n+\t\t\t${SHELL_PATH} \"$0\" \\\n+\t\t\t--root=\"$root\" --statusprefix=\"[$i] \" \\\n+\t\t\t--valgrind=\"$valgrind\" \\\n+\t\t\t--valgrind-only-stride=\"$valgrind_parallel\" \\\n+\t\t\t--valgrind-only-offset=\"$i\" &\n+\t\tpids=\"$pids $!\"\n+\tdone\n+\ttrap \"kill $pids\" INT TERM HUP\n+\twait $pids\n+\ttrap - INT TERM HUP\n+\tfor i in $(test_seq 1 $valgrind_parallel)\n+\tdo\n+\t\troot=\"$TRASH_DIRECTORY/vgparallel-$i\"\n+\t\teval \"$(cat \"$root/test-results/$(basename \"$0\" .sh)\"-*.counts |\n+\t\t\tsed 's/^\\([a-z][a-z]*\\) \\([0-9][0-9]*\\)/inner_\\1=\\2/')\"\n+\t\ttest_count=$(expr $test_count + $inner_total)\n+\t\ttest_success=$(expr $test_success + $inner_success)\n+\t\ttest_fixed=$(expr $test_fixed + $inner_fixed)\n+\t\ttest_broken=$(expr $test_broken + $inner_broken)\n+\t\ttest_failure=$(expr $test_failure + $inner_failed)\n+\tdone\n+\ttest_done\n+fi\n+\n # Use -P to resolve symlinks in our working directory so that the cwd\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$TRASH_DIRECTORY\" || exit 1\n-- \n1.8.3.1.530.g6f90e57\n"},{"id":"221279","messageId":"7v4ncv5lff.fsf@alter.siamese.dyndns.org","threadId":"33843","inReplyTo":"0c11f94a00644b0fe8d2fcc3620ece530868f5af.1371557431.git.trast@inf.ethz.ch","subject":"Re: [PATCH v3 3/8] test-lib: rearrange start/end of test_expect_* and test_skip","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-18T18:21:56Z","receivedAt":"2013-06-18T18:21:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 4fa141a..e99b0ea 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -369,8 +369,15 @@ test_run_ () {\n> \treturn \"$eval_ret\"\n> }\n> \n> -test_skip () {\n> +test_start_ () {\n> \ttest_count=$(($test_count+1))\n> +}\n> +\n> +test_finish_ () {\n> +\techo >&3 \"\"\n> +}\n> +\n> +test_skip () {\n> \tto_skip=\n> \tif match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS\n> \tthen\n\nThis puzzled me for a few minutes, until I realized that the code\nbefore this patch was using the call to test_skip, whose primary\npurpose is to answer \"do we want to run this test, or do we want to\nskip it?\", as a way to increment the test_count variable. Arguably\neach test would call test_skip once, so it may not be too bad, but\nit does look like it is depending on a subtle side-effect.\n\nThat increment does logically belong to \"now we are starting a new\ntest\" much better. This change makes perfect sense.\n\nThanks.\n"}]}