Skip to content

Commit 26a0730

Browse files
trastgitster
authored andcommitted
Revert "test-lib: support running tests under valgrind in parallel"
This reverts commit ad0e623. --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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 5f737ac commit 26a0730

File tree

1 file changed

+22
-84
lines changed

1 file changed

+22
-84
lines changed

t/test-lib.sh

Lines changed: 22 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -205,15 +205,6 @@ do
205205
--valgrind-only=*)
206206
valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
207207
shift ;;
208-
--valgrind-parallel=*)
209-
valgrind_parallel=$(expr "z$1" : 'z[^=]*=\(.*\)')
210-
shift ;;
211-
--valgrind-only-stride=*)
212-
valgrind_only_stride=$(expr "z$1" : 'z[^=]*=\(.*\)')
213-
shift ;;
214-
--valgrind-only-offset=*)
215-
valgrind_only_offset=$(expr "z$1" : 'z[^=]*=\(.*\)')
216-
shift ;;
217208
--tee)
218209
shift ;; # was handled already
219210
--root=*)
@@ -227,7 +218,7 @@ do
227218
esac
228219
done
229220

230-
if test -n "$valgrind_only" || test -n "$valgrind_only_stride"
221+
if test -n "$valgrind_only"
231222
then
232223
test -z "$valgrind" && valgrind=memcheck
233224
test -z "$verbose" && verbose_only="$valgrind_only"
@@ -377,9 +368,7 @@ maybe_teardown_verbose () {
377368
last_verbose=t
378369
maybe_setup_verbose () {
379370
test -z "$verbose_only" && return
380-
if match_pattern_list $test_count $verbose_only ||
381-
{ test -n "$valgrind_only_stride" &&
382-
expr $test_count "%" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null; }
371+
if match_pattern_list $test_count $verbose_only
383372
then
384373
exec 4>&2 3>&1
385374
# Emit a delimiting blank line when going from
@@ -403,17 +392,13 @@ maybe_teardown_valgrind () {
403392

404393
maybe_setup_valgrind () {
405394
test -z "$GIT_VALGRIND" && return
406-
if test -z "$valgrind_only" && test -z "$valgrind_only_stride"
395+
if test -z "$valgrind_only"
407396
then
408397
GIT_VALGRIND_ENABLED=t
409398
return
410399
fi
411400
GIT_VALGRIND_ENABLED=
412401
if match_pattern_list $test_count $valgrind_only
413-
then
414-
GIT_VALGRIND_ENABLED=t
415-
elif test -n "$valgrind_only_stride" &&
416-
expr $test_count "%" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null
417402
then
418403
GIT_VALGRIND_ENABLED=t
419404
fi
@@ -568,9 +553,6 @@ test_done () {
568553
esac
569554
}
570555

571-
572-
# Set up a directory that we can put in PATH which redirects all git
573-
# calls to 'valgrind git ...'.
574556
if test -n "$valgrind"
575557
then
576558
make_symlink () {
@@ -618,42 +600,33 @@ then
618600
make_symlink "$symlink_target" "$GIT_VALGRIND/bin/$base" || exit
619601
}
620602

621-
# In the case of --valgrind-parallel, we only need to do the
622-
# wrapping once, in the main script. The worker children all
623-
# have $valgrind_only_stride set, so we can skip based on that.
624-
if test -z "$valgrind_only_stride"
625-
then
626-
# override all git executables in TEST_DIRECTORY/..
627-
GIT_VALGRIND=$TEST_DIRECTORY/valgrind
628-
mkdir -p "$GIT_VALGRIND"/bin
629-
for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
630-
do
631-
make_valgrind_symlink $file
632-
done
633-
# special-case the mergetools loadables
634-
make_symlink "$GIT_BUILD_DIR"/mergetools "$GIT_VALGRIND/bin/mergetools"
635-
OLDIFS=$IFS
636-
IFS=:
637-
for path in $PATH
603+
# override all git executables in TEST_DIRECTORY/..
604+
GIT_VALGRIND=$TEST_DIRECTORY/valgrind
605+
mkdir -p "$GIT_VALGRIND"/bin
606+
for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
607+
do
608+
make_valgrind_symlink $file
609+
done
610+
# special-case the mergetools loadables
611+
make_symlink "$GIT_BUILD_DIR"/mergetools "$GIT_VALGRIND/bin/mergetools"
612+
OLDIFS=$IFS
613+
IFS=:
614+
for path in $PATH
615+
do
616+
ls "$path"/git-* 2> /dev/null |
617+
while read file
638618
do
639-
ls "$path"/git-* 2> /dev/null |
640-
while read file
641-
do
642-
make_valgrind_symlink "$file"
643-
done
619+
make_valgrind_symlink "$file"
644620
done
645-
IFS=$OLDIFS
646-
fi
621+
done
622+
IFS=$OLDIFS
647623
PATH=$GIT_VALGRIND/bin:$PATH
648624
GIT_EXEC_PATH=$GIT_VALGRIND/bin
649625
export GIT_VALGRIND
650626
GIT_VALGRIND_MODE="$valgrind"
651627
export GIT_VALGRIND_MODE
652628
GIT_VALGRIND_ENABLED=t
653-
if test -n "$valgrind_only" || test -n "$valgrind_only_stride"
654-
then
655-
GIT_VALGRIND_ENABLED=
656-
fi
629+
test -n "$valgrind_only" && GIT_VALGRIND_ENABLED=
657630
export GIT_VALGRIND_ENABLED
658631
elif test -n "$GIT_TEST_INSTALLED"
659632
then
@@ -739,41 +712,6 @@ then
739712
else
740713
mkdir -p "$TRASH_DIRECTORY"
741714
fi
742-
743-
# Gross hack to spawn N sub-instances of the tests in parallel, and
744-
# summarize the results. Note that if this is enabled, the script
745-
# terminates at the end of this 'if' block.
746-
if test -n "$valgrind_parallel"
747-
then
748-
for i in $(test_seq 1 $valgrind_parallel)
749-
do
750-
root="$TRASH_DIRECTORY/vgparallel-$i"
751-
mkdir "$root"
752-
TEST_OUTPUT_DIRECTORY="$root" \
753-
${SHELL_PATH} "$0" \
754-
--root="$root" --statusprefix="[$i] " \
755-
--valgrind="$valgrind" \
756-
--valgrind-only-stride="$valgrind_parallel" \
757-
--valgrind-only-offset="$i" &
758-
pids="$pids $!"
759-
done
760-
trap "kill $pids" INT TERM HUP
761-
wait $pids
762-
trap - INT TERM HUP
763-
for i in $(test_seq 1 $valgrind_parallel)
764-
do
765-
root="$TRASH_DIRECTORY/vgparallel-$i"
766-
eval "$(cat "$root/test-results/$(basename "$0" .sh)"-*.counts |
767-
sed 's/^\([a-z][a-z]*\) \([0-9][0-9]*\)/inner_\1=\2/')"
768-
test_count=$(expr $test_count + $inner_total)
769-
test_success=$(expr $test_success + $inner_success)
770-
test_fixed=$(expr $test_fixed + $inner_fixed)
771-
test_broken=$(expr $test_broken + $inner_broken)
772-
test_failure=$(expr $test_failure + $inner_failed)
773-
done
774-
test_done
775-
fi
776-
777715
# Use -P to resolve symlinks in our working directory so that the cwd
778716
# in subprocesses like git equals our $PWD (for pathname comparisons).
779717
cd -P "$TRASH_DIRECTORY" || exit 1

0 commit comments

Comments
 (0)