Re: [PATCH] valgrind: support test helpers
Am 28.10.2016 um 10:51 schrieb Johannes Schindelin: On Fri, 28 Oct 2016, René Scharfe wrote: Signed-off-by: Rene ScharfeApart from the missing accent ("é") in your SOB: ACK. I sign off without accent out of habit, to avoid display problems -- better have a plain "e" than something like "" or worse. But only now I realized that I can fix at least my end by using an UTF-8 locale (e.g. LANG=C.UTF-8 instead of LANG=C) -- together with PuTTY and its settings Window, Translation, Remote character set: UTF-8 and Connection, Data, Terminal-type-string: linux, which I already had. My terminal just got boosted into the 21st century, woohoo! ;) René
Re: [PATCH] valgrind: support test helpers
Am 28.10.2016 um 14:50 schrieb Junio C Hamano: > Hmph. I somehow thought this was supposed to have been fixed by > 503e224180 ("t/test-lib.sh: fix running tests with --valgrind", > 2016-07-11) already. Its title seems to indicate that intention. Probably the quickest test script that calls a helper is t0009-prio-queue.sh, and without my patch it reports something like this, unfortunately: expecting success: test-prio-queue 2 6 3 10 9 5 7 4 5 8 1 dump >actual && test_cmp expect actual ./t0009-prio-queue.sh: 4: eval: test-prio-queue: not found not ok 1 - basic ordering # # test-prio-queue 2 6 3 10 9 5 7 4 5 8 1 dump >actual && # test_cmp expect actual # René
Re: [PATCH] valgrind: support test helpers
René Scharfewrites: > Tests run with --valgrind call git commands through a wrapper script > that invokes valgrind on them. This script (valgrind.sh) is in turn > invoked through symlinks created for each command in t/valgrind/bin/. > > Since e6e7530d (test helpers: move test-* to t/helper/ subdirectory) > these symlinks have been broken for test helpers -- they point to the > old locations in the root of the build directory. Fix that by teaching > the code for creating the links about the new location of the binaries, > and do the same in the wrapper script to allow it to find its payload. > > Signed-off-by: Rene Scharfe > --- Hmph. I somehow thought this was supposed to have been fixed by 503e224180 ("t/test-lib.sh: fix running tests with --valgrind", 2016-07-11) already. > t/test-lib.sh | 9 - > t/valgrind/valgrind.sh | 12 ++-- > 2 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index b859db6..a724181 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -809,7 +809,14 @@ then > return; > > base=$(basename "$1") > - symlink_target=$GIT_BUILD_DIR/$base > + case "$base" in > + test-*) > + symlink_target="$GIT_BUILD_DIR/t/helper/$base" > + ;; > + *) > + symlink_target="$GIT_BUILD_DIR/$base" > + ;; > + esac > # do not override scripts > if test -x "$symlink_target" && > test ! -d "$symlink_target" && > diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh > index 4215303..669ebaf 100755 > --- a/t/valgrind/valgrind.sh > +++ b/t/valgrind/valgrind.sh > @@ -1,11 +1,19 @@ > #!/bin/sh > > base=$(basename "$0") > +case "$base" in > +test-*) > + program="$GIT_VALGRIND/../../t/helper/$base" > + ;; > +*) > + program="$GIT_VALGRIND/../../$base" > + ;; > +esac > > TOOL_OPTIONS='--leak-check=no' > > test -z "$GIT_VALGRIND_ENABLED" && > -exec "$GIT_VALGRIND"/../../"$base" "$@" > +exec "$program" "$@" > > case "$GIT_VALGRIND_MODE" in > memcheck-fast) > @@ -29,4 +37,4 @@ exec valgrind -q --error-exitcode=126 \ > --log-fd=4 \ > --input-fd=4 \ > $GIT_VALGRIND_OPTIONS \ > - "$GIT_VALGRIND"/../../"$base" "$@" > + "$program" "$@"
Re: [PATCH] valgrind: support test helpers
Hi, On Fri, 28 Oct 2016, René Scharfe wrote: > Tests run with --valgrind call git commands through a wrapper script > that invokes valgrind on them. This script (valgrind.sh) is in turn > invoked through symlinks created for each command in t/valgrind/bin/. > > Since e6e7530d (test helpers: move test-* to t/helper/ subdirectory) > these symlinks have been broken for test helpers -- they point to the > old locations in the root of the build directory. Fix that by teaching > the code for creating the links about the new location of the binaries, > and do the same in the wrapper script to allow it to find its payload. > > Signed-off-by: Rene ScharfeApart from the missing accent ("é") in your SOB: ACK. Ciao, Dscho