From mboxrd@z Thu Jan 1 00:00:00 1970
From: Thomas Rast
Subject: [PATCH 1/2] Revert "test-lib: support running tests under valgrind in parallel"
Date: Sat, 19 Oct 2013 23:06:07 +0200
Message-ID:
References:
Cc: Junio C Hamano , Jeff King
To: git@vger.kernel.org
X-From: git-owner@vger.kernel.org Sat Oct 19 23:06:37 2013
Return-path:
Envelope-to: gcvg-git-2@plane.gmane.org
Received: from vger.kernel.org ([209.132.180.67])
by plane.gmane.org with esmtp (Exim 4.69)
(envelope-from )
id 1VXdjF-0005BM-Rz
for gcvg-git-2@plane.gmane.org; Sat, 19 Oct 2013 23:06:34 +0200
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S1752660Ab3JSVGZ (ORCPT );
Sat, 19 Oct 2013 17:06:25 -0400
Received: from psi.thgersdorf.net ([176.9.98.78]:36706 "EHLO mail.psioc.net"
rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
id S1752211Ab3JSVGY (ORCPT );
Sat, 19 Oct 2013 17:06:24 -0400
Received: from localhost (localhost [127.0.0.1])
by localhost.psioc.net (Postfix) with ESMTP id 206AD4D658A;
Sat, 19 Oct 2013 23:06:23 +0200 (CEST)
X-Virus-Scanned: amavisd-new at psioc.net
Received: from mail.psioc.net ([127.0.0.1])
by localhost (mail.psioc.net [127.0.0.1]) (amavisd-new, port 10024)
with LMTP id 5hsacqN9kqVx; Sat, 19 Oct 2013 23:06:09 +0200 (CEST)
Received: from linux-k42r.v.cablecom.net (46-126-8-85.dynamic.hispeed.ch [46.126.8.85])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client did not present a certificate)
by mail.psioc.net (Postfix) with ESMTPSA id 9A8D04D650A;
Sat, 19 Oct 2013 23:06:09 +0200 (CEST)
X-Mailer: git-send-email 1.8.4.1.810.g312044e
In-Reply-To:
Sender: git-owner@vger.kernel.org
Precedence: bulk
List-ID:
X-Mailing-List: git@vger.kernel.org
Archived-At:
This reverts commit ad0e6233320b004f0d686f6887c803e508607bd2.
--valgrind-parallel was broken from the start: during review I made
the whole valgrind setup code conditional on not being a
--valgrind-parallel worker child. But even the children crucially
need $GIT_VALGRIND to be set; it should therefore have been set
outside the conditional.
The fix would be a two-liner, but since the introduction of the
feature, almost four months have passed without anyone noticing that
it is broken. So this feature is not worth the about hundred lines of
test-lib.sh complexity. Revert it.
Signed-off-by: Thomas Rast
---
t/test-lib.sh | 106 ++++++++++++----------------------------------------------
1 file changed, 22 insertions(+), 84 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0fa7dfd..eaf6759 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -205,15 +205,6 @@ do
--valgrind-only=*)
valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
shift ;;
- --valgrind-parallel=*)
- valgrind_parallel=$(expr "z$1" : 'z[^=]*=\(.*\)')
- shift ;;
- --valgrind-only-stride=*)
- valgrind_only_stride=$(expr "z$1" : 'z[^=]*=\(.*\)')
- shift ;;
- --valgrind-only-offset=*)
- valgrind_only_offset=$(expr "z$1" : 'z[^=]*=\(.*\)')
- shift ;;
--tee)
shift ;; # was handled already
--root=*)
@@ -227,7 +218,7 @@ do
esac
done
-if test -n "$valgrind_only" || test -n "$valgrind_only_stride"
+if test -n "$valgrind_only"
then
test -z "$valgrind" && valgrind=memcheck
test -z "$verbose" && verbose_only="$valgrind_only"
@@ -377,9 +368,7 @@ maybe_teardown_verbose () {
last_verbose=t
maybe_setup_verbose () {
test -z "$verbose_only" && return
- if match_pattern_list $test_count $verbose_only ||
- { test -n "$valgrind_only_stride" &&
- expr $test_count "%" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null; }
+ if match_pattern_list $test_count $verbose_only
then
exec 4>&2 3>&1
# Emit a delimiting blank line when going from
@@ -403,7 +392,7 @@ maybe_teardown_valgrind () {
maybe_setup_valgrind () {
test -z "$GIT_VALGRIND" && return
- if test -z "$valgrind_only" && test -z "$valgrind_only_stride"
+ if test -z "$valgrind_only"
then
GIT_VALGRIND_ENABLED=t
return
@@ -412,10 +401,6 @@ maybe_setup_valgrind () {
if match_pattern_list $test_count $valgrind_only
then
GIT_VALGRIND_ENABLED=t
- elif test -n "$valgrind_only_stride" &&
- expr $test_count "%" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null
- then
- GIT_VALGRIND_ENABLED=t
fi
}
@@ -568,9 +553,6 @@ test_done () {
esac
}
-
-# Set up a directory that we can put in PATH which redirects all git
-# calls to 'valgrind git ...'.
if test -n "$valgrind"
then
make_symlink () {
@@ -618,42 +600,33 @@ then
make_symlink "$symlink_target" "$GIT_VALGRIND/bin/$base" || exit
}
- # In the case of --valgrind-parallel, we only need to do the
- # wrapping once, in the main script. The worker children all
- # have $valgrind_only_stride set, so we can skip based on that.
- if test -z "$valgrind_only_stride"
- then
- # override all git executables in TEST_DIRECTORY/..
- GIT_VALGRIND=$TEST_DIRECTORY/valgrind
- mkdir -p "$GIT_VALGRIND"/bin
- for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
- do
- make_valgrind_symlink $file
- done
- # special-case the mergetools loadables
- make_symlink "$GIT_BUILD_DIR"/mergetools "$GIT_VALGRIND/bin/mergetools"
- OLDIFS=$IFS
- IFS=:
- for path in $PATH
+ # override all git executables in TEST_DIRECTORY/..
+ GIT_VALGRIND=$TEST_DIRECTORY/valgrind
+ mkdir -p "$GIT_VALGRIND"/bin
+ for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
+ do
+ make_valgrind_symlink $file
+ done
+ # special-case the mergetools loadables
+ make_symlink "$GIT_BUILD_DIR"/mergetools "$GIT_VALGRIND/bin/mergetools"
+ OLDIFS=$IFS
+ IFS=:
+ for path in $PATH
+ do
+ ls "$path"/git-* 2> /dev/null |
+ while read file
do
- ls "$path"/git-* 2> /dev/null |
- while read file
- do
- make_valgrind_symlink "$file"
- done
+ make_valgrind_symlink "$file"
done
- IFS=$OLDIFS
- fi
+ done
+ IFS=$OLDIFS
PATH=$GIT_VALGRIND/bin:$PATH
GIT_EXEC_PATH=$GIT_VALGRIND/bin
export GIT_VALGRIND
GIT_VALGRIND_MODE="$valgrind"
export GIT_VALGRIND_MODE
GIT_VALGRIND_ENABLED=t
- if test -n "$valgrind_only" || test -n "$valgrind_only_stride"
- then
- GIT_VALGRIND_ENABLED=
- fi
+ test -n "$valgrind_only" && GIT_VALGRIND_ENABLED=
export GIT_VALGRIND_ENABLED
elif test -n "$GIT_TEST_INSTALLED"
then
@@ -730,41 +703,6 @@ then
else
mkdir -p "$TRASH_DIRECTORY"
fi
-
-# Gross hack to spawn N sub-instances of the tests in parallel, and
-# summarize the results. Note that if this is enabled, the script
-# terminates at the end of this 'if' block.
-if test -n "$valgrind_parallel"
-then
- for i in $(test_seq 1 $valgrind_parallel)
- do
- root="$TRASH_DIRECTORY/vgparallel-$i"
- mkdir "$root"
- TEST_OUTPUT_DIRECTORY="$root" \
- ${SHELL_PATH} "$0" \
- --root="$root" --statusprefix="[$i] " \
- --valgrind="$valgrind" \
- --valgrind-only-stride="$valgrind_parallel" \
- --valgrind-only-offset="$i" &
- pids="$pids $!"
- done
- trap "kill $pids" INT TERM HUP
- wait $pids
- trap - INT TERM HUP
- for i in $(test_seq 1 $valgrind_parallel)
- do
- root="$TRASH_DIRECTORY/vgparallel-$i"
- eval "$(cat "$root/test-results/$(basename "$0" .sh)"-*.counts |
- sed 's/^\([a-z][a-z]*\) \([0-9][0-9]*\)/inner_\1=\2/')"
- test_count=$(expr $test_count + $inner_total)
- test_success=$(expr $test_success + $inner_success)
- test_fixed=$(expr $test_fixed + $inner_fixed)
- test_broken=$(expr $test_broken + $inner_broken)
- test_failure=$(expr $test_failure + $inner_failed)
- done
- test_done
-fi
-
# Use -P to resolve symlinks in our working directory so that the cwd
# in subprocesses like git equals our $PWD (for pathname comparisons).
cd -P "$TRASH_DIRECTORY" || exit 1
--
1.8.4.1.810.g312044e