Junio C Hamano <gitster@xxxxxxxxx> writes:
> Thomas Gummerer <t.gummerer@xxxxxxxxx> writes:
>
>> When one performance test fails, the testing is aborted and the cleanup
>> commands are not executed anymore, leaving the trash directory in the
>> failed state.
>
> Ah, that I overlooked. In that case, the comments in my previous
> message do not apply.
Having said that, it was unclear to me why we would want a new
test_perf_cleanup added.
The name is misleading (does it define only clean-up actions?) to
say the least, and one way of fixing it would be to call it
test_perf_with_cleanup.
I wondered why this clean-up section cannot be an optional parameter
to test_perf, but that would not fly well because we won't know if
3-arg form is with one prerequisite and no clean-up, or no prereq
with a clean-up, so perhaps adding a new function may be the best
you could do.
But in the longer term, I think we would be better off to allow two
styles (one traditional, another modern) and add new features only
to the "modern" (aka "more easily extensible") form:
test_perf [PREREQ] BODY
test_perf [--prereq PREREQ] [--cleanup CLEANUP] ... BODY
perhaps like this (this is without --cleanup but it should be
obvious how to add a support for it to the command line parser).
The patch to t0008 is primarily to adjust for test labels that begin
with double-dashes that will be mistaken as a new-style option, but
it is unnecessarily big because it uses random custom shell functions
to hide the real calls to underlying test_expect_success X-<.
t/test-lib-functions.sh | 72 ++++++++++++++++++++++++++++++++++++++-----------
t/perf/perf-lib.sh | 17 +++++-------
t/t0008-ignores.sh | 43 +++++++++++++++--------------
3 files changed, 87 insertions(+), 45 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index a7e9aac..10202dc 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -342,20 +342,65 @@ test_declared_prereq () {
return 1
}
+test_expect_parse () {
+ whoami=$1
+ shift
+ test_expect_new_style=
+ while case $# in 0) false ;; esac
+ do
+ case "$1" in
+ --prereq)
+ test $# -gt 1 ||
+ error "bug in the test script: --prereq needs a parameter"
+ test_prereq=$2
+ shift
+ ;;
+ --)
+ shift
+ break
+ ;;
+ --*)
+ error "bug in the test script: unknown option '$1'"
+ ;;
+ *)
+ break
+ ;;
+ esac
+ test_expect_new_style=yes
+ shift
+ done
+
+ # Traditional "test_expect_what [PREREQ] BODY"
+ if test -z "$test_expect_new_style"
+ then
+ test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
+ fi
+
+ if test $# != 2
+ then
+ if test -z "$test_expect_new_style"
+ then
+ error "bug in the test script: not 2 or 3 parameters to $whoami"
+ else
+ error "bug in the test script: not 2 parameters to $whoami"
+ fi
+ fi
+ test_label=$1 test_body=$2
+
+ export test_prereq
+}
+
test_expect_failure () {
test_start_
- test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
- test "$#" = 2 ||
- error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
- export test_prereq
+ test_expect_parse test_expect_failure "$@"
if ! test_skip "$@"
then
- say >&3 "checking known breakage: $2"
- if test_run_ "$2" expecting_failure
+ say >&3 "checking known breakage: $test_body"
+ if test_run_ "$test_body" expecting_failure
then
- test_known_broken_ok_ "$1"
+ test_known_broken_ok_ "$test_label"
else
- test_known_broken_failure_ "$1"
+ test_known_broken_failure_ "$test_label"
fi
fi
test_finish_
@@ -363,16 +408,13 @@ test_expect_failure () {
test_expect_success () {
test_start_
- test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
- test "$#" = 2 ||
- error "bug in the test script: not 2 or 3 parameters to test-expect-success"
- export test_prereq
+ test_expect_parse test_expect_success "$@"
if ! test_skip "$@"
then
- say >&3 "expecting success: $2"
- if test_run_ "$2"
+ say >&3 "expecting success: $test_body"
+ if test_run_ "$test_body"
then
- test_ok_ "$1"
+ test_ok_ "$test_label"
else
test_failure_ "$@"
fi
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index f4eecaa..6477d38 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -151,23 +151,20 @@ exit $ret' >&3 2>&4
test_perf () {
test_start_
- test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
- test "$#" = 2 ||
- error "bug in the test script: not 2 or 3 parameters to test-expect-success"
- export test_prereq
+ test_expect_parse test_perf "$@"
if ! test_skip "$@"
then
base=$(basename "$0" .sh)
echo "$test_count" >>"$perf_results_dir"/$base.subtests
- echo "$1" >"$perf_results_dir"/$base.$test_count.descr
+ echo "$test_label" >"$perf_results_dir"/$base.$test_count.descr
if test -z "$verbose"; then
- printf "%s" "perf $test_count - $1:"
+ printf "%s" "perf $test_count - $test_label:"
else
- echo "perf $test_count - $1:"
+ echo "perf $test_count - $test_label:"
fi
for i in $(test_seq 1 $GIT_PERF_REPEAT_COUNT); do
- say >&3 "running: $2"
- if test_run_perf_ "$2"
+ say >&3 "running: $test_body"
+ if test_run_perf_ "$test_body"
then
if test -z "$verbose"; then
printf " %s" "$i"
@@ -183,7 +180,7 @@ test_perf () {
if test -z "$verbose"; then
echo " ok"
else
- test_ok_ "$1"
+ test_ok_ "$test_label"
fi
base="$perf_results_dir"/"$perf_results_prefix$(basename "$0" .sh)"."$test_count"
"$TEST_DIRECTORY"/perf/min_time.perl test_time.* >"$base".times
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 96f40fe..795de61 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -106,7 +106,7 @@ test_expect_success_multi () {
expect_verbose=$( echo "$expect_all" | grep -v '^:: ' )
expect=$( echo "$expect_verbose" | sed -e 's/.* //' )
- test_expect_success $prereq "$testname" '
+ test_expect_success ${prereq:+--prereq $prereq} -- "$testname" '
expect "$expect" &&
eval "$code"
'
@@ -116,10 +116,11 @@ test_expect_success_multi () {
then
for quiet_opt in '-q' '--quiet'
do
- test_expect_success $prereq "$testname${quiet_opt:+ with $quiet_opt}" "
- expect '' &&
- $code
- "
+ test_expect_success ${prereq:+--prereq $prereq} -- \
+ "$testname${quiet_opt:+ with $quiet_opt}" '
+ expect "" &&
+ eval "$code"
+ '
done
quiet_opt=
fi
@@ -140,7 +141,9 @@ test_expect_success_multi () {
$code
"
opts="$verbose_opt$non_matching_opt"
- test_expect_success $prereq "$testname${opts:+ with $opts}" "$test_code"
+
+ test_expect_success ${prereq:+--prereq $prereq} -- \
+ "$testname${opts:+ with $opts}" "$test_code"
done
done
verbose_opt=
@@ -225,7 +228,7 @@ test_expect_success '-q with multiple args' '
stderr_contains "fatal: --quiet is only valid with a single pathname"
'
-test_expect_success '--quiet with multiple args' '
+test_expect_success -- '--quiet with multiple args' '
expect "" &&
test_check_ignore "--quiet one two" 128 &&
stderr_contains "fatal: --quiet is only valid with a single pathname"
@@ -235,7 +238,7 @@ for verbose_opt in '-v' '--verbose'
do
for quiet_opt in '-q' '--quiet'
do
- test_expect_success "$quiet_opt $verbose_opt" "
+ test_expect_success -- "$quiet_opt $verbose_opt" "
expect '' &&
test_check_ignore '$quiet_opt $verbose_opt foo' 128 &&
stderr_contains 'fatal: cannot have both --quiet and --verbose'
@@ -243,7 +246,7 @@ do
done
done
-test_expect_success '--quiet with multiple args' '
+test_expect_success -- '--quiet with multiple args' '
expect "" &&
test_check_ignore "--quiet one two" 128 &&
stderr_contains "fatal: --quiet is only valid with a single pathname"
@@ -559,34 +562,34 @@ sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
tr ":\t\n" "\0" >expected-verbose0
-test_expect_success '--stdin' '
+test_expect_success -- '--stdin' '
expect_from_stdin <expected-default &&
test_check_ignore "--stdin" <stdin
'
-test_expect_success '--stdin -q' '
+test_expect_success -- '--stdin -q' '
expect "" &&
test_check_ignore "-q --stdin" <stdin
'
-test_expect_success '--stdin -v' '
+test_expect_success -- '--stdin -v' '
expect_from_stdin <expected-verbose &&
test_check_ignore "-v --stdin" <stdin
'
for opts in '--stdin -z' '-z --stdin'
do
- test_expect_success "$opts" "
+ test_expect_success -- "$opts" "
expect_from_stdin <expected-default0 &&
test_check_ignore '$opts' <stdin0
"
- test_expect_success "$opts -q" "
+ test_expect_success -- "$opts -q" "
expect "" &&
test_check_ignore '-q $opts' <stdin0
"
- test_expect_success "$opts -v" "
+ test_expect_success -- "$opts -v" "
expect_from_stdin <expected-verbose0 &&
test_check_ignore '-v $opts' <stdin0
"
@@ -645,7 +648,7 @@ sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
tr ":\t\n" "\0" >expected-verbose0
-test_expect_success '--stdin from subdirectory' '
+test_expect_success -- '--stdin from subdirectory' '
expect_from_stdin <expected-default &&
(
cd a &&
@@ -653,7 +656,7 @@ test_expect_success '--stdin from subdirectory' '
)
'
-test_expect_success '--stdin from subdirectory with -v' '
+test_expect_success -- '--stdin from subdirectory with -v' '
expect_from_stdin <expected-verbose &&
(
cd a &&
@@ -661,7 +664,7 @@ test_expect_success '--stdin from subdirectory with -v' '
)
'
-test_expect_success '--stdin from subdirectory with -v -n' '
+test_expect_success -- '--stdin from subdirectory with -v -n' '
expect_from_stdin <expected-all &&
(
cd a &&
@@ -671,7 +674,7 @@ test_expect_success '--stdin from subdirectory with -v -n' '
for opts in '--stdin -z' '-z --stdin'
do
- test_expect_success "$opts from subdirectory" '
+ test_expect_success -- "$opts from subdirectory" '
expect_from_stdin <expected-default0 &&
(
cd a &&
@@ -679,7 +682,7 @@ do
)
'
- test_expect_success "$opts from subdirectory with -v" '
+ test_expect_success -- "$opts from subdirectory with -v" '
expect_from_stdin <expected-verbose0 &&
(
cd a &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html