On Tue, Nov 12, 2019 at 01:24:38PM +0100, SZEDER Gábor wrote:
> With './t1234-foo.sh -r 5,6' we can run only specific test cases in a
> test script, but our test framwork still evaluates all lazy prereqs
> that the excluded test cases might depend on. This is unnecessary and
> produces verbose and trace output that can be distracting. This has
> been an issue ever since the '-r|--run=' options were introduced in
> 0445e6f0a1 (test-lib: '--run' to run only specific tests, 2014-04-30),
> because that commit added the check of the list of test cases
> specified with '-r' after evaluating the prereqs.
>
> Avoid this unnecessary prereq evaluation by checking the list of test
> cases specified with '-r' before looking at the prereqs.
>
> Note that GIT_SKIP_TESTS has always been checked before the prereqs,
> so prereqs necessary for tests skipped that way were not evaluated.
Thanks. This definitely seems like an improvement. I verified that it
works in practice, and the patch itself looks obviously correct.
One of the things I had to double check is how to_skip is handled, since
we check it in the moved block. It's fine, but I wondered if the whole
thing would be easier as an if/else cascade with an early return. Like
the patch below (on top of your patch; try "diff -w" to quiet the
indentation noise).
But maybe it's not worth the churn (and if we pursue it at all, it
should not hold up your patch).
-Peff
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 24b541f494..a1784114cb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -993,49 +993,38 @@ test_finish_ () {
}
test_skip () {
- to_skip=
skipped_reason=
if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
then
- to_skip=t
skipped_reason="GIT_SKIP_TESTS"
- fi
- if test -z "$to_skip" && test -n "$run_list" &&
+ elif test -n "$run_list" &&
! match_test_selector_list '--run' $test_count "$run_list"
then
- to_skip=t
skipped_reason="--run"
- fi
- if test -z "$to_skip" && test -n "$test_prereq" &&
+ elif
+ if test -n "$test_prereq" &&
! test_have_prereq "$test_prereq"
then
- to_skip=t
-
of_prereq=
if test "$missing_prereq" != "$test_prereq"
then
of_prereq=" of $test_prereq"
fi
skipped_reason="missing $missing_prereq${of_prereq}"
+ else
+ return 1
fi
- case "$to_skip" in
- t)
- if test -n "$write_junit_xml"
- then
- message="$(xml_attr_encode "$skipped_reason")"
- write_junit_xml_testcase "$1" \
- " <skipped message=\"$message\" />"
- fi
+ if test -n "$write_junit_xml"
+ then
+ message="$(xml_attr_encode "$skipped_reason")"
+ write_junit_xml_testcase "$1" \
+ " <skipped message=\"$message\" />"
+ fi
- say_color skip >&3 "skipping test: $@"
- say_color skip "ok $test_count # skip $1 ($skipped_reason)"
- : true
- ;;
- *)
- false
- ;;
- esac
+ say_color skip >&3 "skipping test: $@"
+ say_color skip "ok $test_count # skip $1 ($skipped_reason)"
+ return 0
}
# stub; perf-lib overrides it
