Re: [BUG] range expressions in GIT_SKIP_TESTS are broken in master (was [BUG] question mark in GIT_SKIP_TESTS is broken in master)
From: Jeff King <hidden>
Date: 2021-06-16 08:29:18
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2021-06-16 · Re: [BUG] range expressions in GIT_SKIP_TESTS are broken in master (was [BUG] question mark in GIT_SKIP_TESTS is broken in master) · Junio C Hamano <hidden>
- 2021-06-16 · Re: [BUG] range expressions in GIT_SKIP_TESTS are broken in master (was [BUG] question mark in GIT_SKIP_TESTS is broken in master) · Junio C Hamano <hidden>
- 2021-06-15 · [BUG] range expressions in GIT_SKIP_TESTS are broken in master (was [BUG] question mark in GIT_SKIP_TESTS is broken in master) · Andrei Rybak <hidden>
On Wed, Jun 16, 2021 at 01:12:09PM +0900, Junio C Hamano wrote:
The variable $GIT_SKIP_TESTS on this line:
if match_pattern_list "$this_test" $GIT_SKIP_TESTS
globs to t5000. We don't quote the variable because we want them
separated at $IFS boundaries, but we didn't want the glob specials
in its value to take any effect. Sigh.Good find. It's surprisingly hard to do field-splitting without pathname globbing in pure shell. I couldn't find a way without using "set -f". That's in POSIX, but it feels funny tweaking a global that can effect how other code runs. We can at least constraint it to a subshell close to the point of use:
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 54938c6427..093104d04f 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh@@ -732,14 +732,15 @@ match_pattern_list () { arg="$1" shift test -z "$*" && return 1 - for pattern_ + (set -f + for pattern_ in $* do case "$arg" in $pattern_) - return 0 + exit 0 esac done - return 1 + exit 1) } match_test_selector_list () {
@@ -848,7 +849,7 @@ maybe_teardown_verbose () { last_verbose=t maybe_setup_verbose () { test -z "$verbose_only" && return - if match_pattern_list $test_count $verbose_only + if match_pattern_list $test_count "$verbose_only" then exec 4>&2 3>&1 # Emit a delimiting blank line when going from
@@ -878,7 +879,7 @@ maybe_setup_valgrind () { return fi GIT_VALGRIND_ENABLED= - if match_pattern_list $test_count $valgrind_only + if match_pattern_list $test_count "$valgrind_only" then GIT_VALGRIND_ENABLED=t fi
@@ -1006,7 +1007,7 @@ test_finish_ () { test_skip () { to_skip= skipped_reason= - if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS + if match_pattern_list $this_test.$test_count "$GIT_SKIP_TESTS" then to_skip=t skipped_reason="GIT_SKIP_TESTS"
@@ -1346,7 +1347,7 @@ fi remove_trash= this_test=${0##*/} this_test=${this_test%%-*} -if match_pattern_list "$this_test" $GIT_SKIP_TESTS +if match_pattern_list "$this_test" "$GIT_SKIP_TESTS" then say_color info >&3 "skipping test $this_test altogether" skip_all="skip all tests in $this_test"
If that isn't portable for some reason, I think we could fall back on
splitting with an external tool. You can't feed the result as a function
argument (you run into the same problem!) but you can use "read" to
split on newlines, like:
echo "$GIT_SKIP_TESTS" |
tr ' ' '\n' |
while read pattern
do
echo "got $pattern"
done
That does put the shell code on the right-hand side of a pipe, which
means it's constrained in terms of setting variables, etc. But that
would be acceptable for our use here.
I dunno. Maybe somebody else can come up with something more clever (or
maybe I am just missing something obvious).
-Peff