[PATCH] valgrind: support test helpers

[PATCH] valgrind: support test helpers

[René Scharfe]

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 <l.s.r@web.de>
---
 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" "$@"
-- 
2.10.1

Re: [PATCH] valgrind: support test helpers

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 764 bytes --]

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 Scharfe <l.s.r@web.de>

Apart from the missing accent ("é") in your SOB: ACK.

Ciao,
Dscho

Re: [PATCH] valgrind: support test helpers

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> 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 <l.s.r@web.de>
> ---

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

[René Scharfe]

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é Scharfe]

Am 28.10.2016 um 10:51 schrieb Johannes Schindelin:
> On Fri, 28 Oct 2016, René Scharfe wrote:
>> Signed-off-by: Rene Scharfe <l.s.r@web.de>
>
> Apart 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 "<C3><A9>" 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é