* [PATCH 1/2] perf-lib: split starting the test from the execution
@ 2013-09-17 12:10 Thomas Gummerer
2013-09-17 12:10 ` [PATCH 2/2] perf-lib: add test_perf_cleanup target Thomas Gummerer
0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gummerer @ 2013-09-17 12:10 UTC (permalink / raw)
To: git; +Cc: trast, gitster, t.gummerer
Separate the execution part to make future changes to the tests simpler.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
t/perf/perf-lib.sh | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index c61d535..95e483c 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -148,13 +148,8 @@ exit $ret' >&3 2>&4
return "$eval_ret"
}
-
-test_perf () {
+perf_test_ () {
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
if ! test_skip "$@"
then
base=$(basename "$0" .sh)
@@ -191,6 +186,14 @@ test_perf () {
test_finish_
}
+test_perf () {
+ 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
+ perf_test_ "$1" "$2"
+}
+
# We extend test_done to print timings at the end (./run disables this
# and does it after running everything)
test_at_end_hook_ () {
--
1.8.3.4.1238.ga800761
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] perf-lib: add test_perf_cleanup target
2013-09-17 12:10 [PATCH 1/2] perf-lib: split starting the test from the execution Thomas Gummerer
@ 2013-09-17 12:10 ` Thomas Gummerer
2013-09-17 17:43 ` Junio C Hamano
0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gummerer @ 2013-09-17 12:10 UTC (permalink / raw)
To: git; +Cc: trast, gitster, t.gummerer
Currently there is no way to clean up the changes that have been made
with test_perf for the next run. Add a way to reset the repository to
the state before the test for testing commands that modify the git
repository, e.g. for perf testing git add.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
This enables me to do something like (hint, hint, hint ;-)):
Test HEAD~1 HEAD
------------------------------------------------------------------------------------
....
0003.16: v5 update-index file 0.19(0.12+0.06) 0.08(0.06+0.01) -57.9%
There are no performance tests currently using this, but since I have
it anyway for a POC of partial writing of index-v5 (which is ugly and
will have to wait a bit until I'm ready to send it to the list) I think
this may be a worthwhile addition others can use in the meantime.
t/perf/README | 11 +++++++++++
t/perf/perf-lib.sh | 25 +++++++++++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/t/perf/README b/t/perf/README
index 8848c14..72f8a7b 100644
--- a/t/perf/README
+++ b/t/perf/README
@@ -123,6 +123,17 @@ tests, use
command2
'
+For performance tests that need cleaning up after them that should not
+be timed, use
+
+ test_perf_cleanup 'descriptive string' '
+ command1 &&
+ command2
+ ' '
+ cleanupcommand1 &&
+ cleanupcommand2
+ '
+
test_perf spawns a subshell, for lack of better options. This means
that
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 95e483c..11a93f1 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -174,6 +174,22 @@ perf_test_ () {
test_failure_ "$@"
break
fi
+ say >&3 "cleaning up: $3"
+ if test "$#" = 3
+ then
+ if test_run_ "$3"
+ then
+ if test -z "$verbose"; then
+ echo -n " c$i"
+ else
+ echo "* cleaning up run $i/$GIT_PERF_REPEAT_COUNT:"
+ fi
+ else
+ test -z "$verbose" && echo
+ test_failure_ "$@"
+ break
+ fi
+ fi
done
if test -z "$verbose"; then
echo " ok"
@@ -194,6 +210,15 @@ test_perf () {
perf_test_ "$1" "$2"
}
+test_perf_cleanup () {
+ test_start_
+ test "$#" = 4 && { test_prereq=$1; shift; } || test_prereq=
+ test "$#" = 3 ||
+ error "bug in the test script: not 3 or 4 parameters to test-expect-success"
+ export test_prereq
+ perf_test_ "$1" "$2" "$3"
+}
+
# We extend test_done to print timings at the end (./run disables this
# and does it after running everything)
test_at_end_hook_ () {
--
1.8.3.4.1238.ga800761
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] perf-lib: add test_perf_cleanup target
2013-09-17 12:10 ` [PATCH 2/2] perf-lib: add test_perf_cleanup target Thomas Gummerer
@ 2013-09-17 17:43 ` Junio C Hamano
2013-09-19 11:42 ` Thomas Gummerer
0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2013-09-17 17:43 UTC (permalink / raw)
To: Thomas Gummerer; +Cc: git, trast
Thomas Gummerer <t.gummerer@gmail.com> writes:
> +For performance tests that need cleaning up after them that should not
> +be timed, use
> +
> + test_perf_cleanup 'descriptive string' '
> + command1 &&
> + command2
> + ' '
> + cleanupcommand1 &&
> + cleanupcommand2
> + '
> +
Hmm, if "not timing the clean-up actions" is the primary goal, is it
an option to reuse test_when_finished for this (you may have to make
sure that the commands run inside it are not timed; I didn't check).
One thing I felt uneasy about the above construct is that it makes
cleanupcommand2 responsible for handling cases where not just
command2 but also command1 might have failed.
Compared to that, test-when-finished allows you to control what
clean-up to do depending on what have already been done, i.e.
test_when_finished 'undo what command1 would have done' &&
command1 &&
test_when_finished 'undo what command2 would have done' &&
command2 &&
...
The second "undo command2" must be prepared for the case where
command2 may have failed in the middle, but it can at least rely on
command1 having succeeded when it is run.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] perf-lib: add test_perf_cleanup target
2013-09-17 17:43 ` Junio C Hamano
@ 2013-09-19 11:42 ` Thomas Gummerer
2013-09-19 17:19 ` Junio C Hamano
0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gummerer @ 2013-09-19 11:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, trast
Junio C Hamano <gitster@pobox.com> writes:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
>> +For performance tests that need cleaning up after them that should not
>> +be timed, use
>> +
>> + test_perf_cleanup 'descriptive string' '
>> + command1 &&
>> + command2
>> + ' '
>> + cleanupcommand1 &&
>> + cleanupcommand2
>> + '
>> +
>
> Hmm, if "not timing the clean-up actions" is the primary goal, is it
> an option to reuse test_when_finished for this (you may have to make
> sure that the commands run inside it are not timed; I didn't check).
Maybe I wasn't clear enough. The goal is to allow tests to have
everything cleaned up after every run. This can then be used for
commands that change the index (or other things) and get back to the
original state after that. For example something like this:
file=$(git ls-files | tail -n 30 | head -1)
test_expect_success "change a file" "
echo 'something' >>$file
"
test_perf_cleanup "v5 update-index file" "
git update-index $file
" "
git reset
"
test_when_finished on the other hand only cleans up when the whole test
is finished.
> One thing I felt uneasy about the above construct is that it makes
> cleanupcommand2 responsible for handling cases where not just
> command2 but also command1 might have failed.
>
> Compared to that, test-when-finished allows you to control what
> clean-up to do depending on what have already been done, i.e.
>
> test_when_finished 'undo what command1 would have done' &&
> command1 &&
> test_when_finished 'undo what command2 would have done' &&
> command2 &&
> ...
>
> The second "undo command2" must be prepared for the case where
> command2 may have failed in the middle, but it can at least rely on
> command1 having succeeded when it is run.
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. This consistent with the normal tests with the immediate
flag passed to them. (All performance tests have the --immediate flag
on implicitly).
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] perf-lib: add test_perf_cleanup target
2013-09-19 11:42 ` Thomas Gummerer
@ 2013-09-19 17:19 ` Junio C Hamano
2013-09-19 19:52 ` Junio C Hamano
0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2013-09-19 17:19 UTC (permalink / raw)
To: Thomas Gummerer; +Cc: git, trast
Thomas Gummerer <t.gummerer@gmail.com> 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.
Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] perf-lib: add test_perf_cleanup target
2013-09-19 17:19 ` Junio C Hamano
@ 2013-09-19 19:52 ` Junio C Hamano
2013-09-19 20:11 ` Junio C Hamano
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Junio C Hamano @ 2013-09-19 19:52 UTC (permalink / raw)
To: Thomas Gummerer; +Cc: git, trast
Junio C Hamano <gitster@pobox.com> writes:
> Thomas Gummerer <t.gummerer@gmail.com> 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 &&
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] perf-lib: add test_perf_cleanup target
2013-09-19 19:52 ` Junio C Hamano
@ 2013-09-19 20:11 ` Junio C Hamano
2013-09-20 22:14 ` Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 0/3] Add cleanup action to perf-lib Thomas Gummerer
2 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2013-09-19 20:11 UTC (permalink / raw)
To: Thomas Gummerer; +Cc: git, trast
Junio C Hamano <gitster@pobox.com> writes:
> 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 ...
In any case, will queue your version as-is, at least for now.
Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] perf-lib: add test_perf_cleanup target
2013-09-19 19:52 ` Junio C Hamano
2013-09-19 20:11 ` Junio C Hamano
@ 2013-09-20 22:14 ` Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 0/3] Add cleanup action to perf-lib Thomas Gummerer
2 siblings, 0 replies; 12+ messages in thread
From: Thomas Gummerer @ 2013-09-20 22:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, trast
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Thomas Gummerer <t.gummerer@gmail.com> 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.
It defines any actions you want to happen after each round of tests that
should not be timed. Usually that would mean any action that resets a
modified repository to its original state. I'm not really sure about
the name, but test_perf_with_cleanup seems good to me.
> 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.
Yeah, that was my first thought too, but as you explained that's not
possible. Just thinking out loud, we could drop the prerequisite check
for performance tests, as no performance tests currently use it. Future
performance tests may use it though and it would be inconsistent with
the rest of the test-suite.
> 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).
I think this makes most sense. The cleanup part would only be needed
for the perf tests, but the flexibility of this wouldn't hurt. The
cleanup would then look something like this:
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index b2a277b..4dfdd28 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -176,6 +176,21 @@ test_perf () {
test_failure_ "$@"
break
fi
+ if ! test -z "$cleanup_action"; then
+ say >&3 "cleaning up: $cleanup_action"
+ if test_run_ "$cleanup_action"
+ then
+ if test -z "$verbose"; then
+ echo -n " c$i"
+ else
+ echo "* cleaning up run $i/$GIT_PERF_REPEAT_COUNT:"
+ fi
+ else
+ test -z $verbose && echo
+ test_failure_ "$@"
+ break
+ fi
+ fi
done
if test -z "$verbose"; then
echo " ok"
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 473b21d..4bad14f 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -360,6 +360,12 @@ test_expect_parse () {
test_prereq=$2
shift
;;
+ --cleanup)
+ test $# -gt 1 ||
+ error "bug in the test script: --cleanup needs a parameter"
+ cleanup_action=$2
+ shift
+ ;;
--)
shift
break
I'll try to send an updated patch series, probably over the weekend with
this suggestions included (It's time to go to bed now).
> 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 &&
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 0/3] Add cleanup action to perf-lib
2013-09-19 19:52 ` Junio C Hamano
2013-09-19 20:11 ` Junio C Hamano
2013-09-20 22:14 ` Thomas Gummerer
@ 2013-09-23 21:08 ` Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 1/3] test-lib: introduce "modern" style tests Thomas Gummerer
` (2 more replies)
2 siblings, 3 replies; 12+ messages in thread
From: Thomas Gummerer @ 2013-09-23 21:08 UTC (permalink / raw)
To: gitster; +Cc: trast, git, t.gummerer
This patch series comes out of the discussion at $gmane/234874, adding
a new (modern) form of writing tests. This form allows easier
extensibility of test cases. In the next patch a --cleanup option is
added for performance tests. The option does nothing for normal
tests, as test_when_finished is the better option for them.
The last patch adds a few simple tests to show the capabilities of the
new --cleanup option.
Junio C Hamano (1):
test-lib: introduce "modern" style tests
Thomas Gummerer (1):
perf-lib: add cleanup option
Thomas Rast (1):
p0003-index.sh: add perf test for the index formats
t/README | 24 +++++++++++++--
t/perf/README | 21 ++++++++++++-
t/perf/p0003-index.sh | 56 +++++++++++++++++++++++++++++++++++
t/perf/perf-lib.sh | 32 +++++++++++++-------
t/t0008-ignores.sh | 34 ++++++++++-----------
t/test-lib-functions.sh | 78 +++++++++++++++++++++++++++++++++++++++----------
6 files changed, 200 insertions(+), 45 deletions(-)
create mode 100755 t/perf/p0003-index.sh
--
1.8.3.4.1241.g1ce9896
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/3] test-lib: introduce "modern" style tests
2013-09-23 21:08 ` [PATCH v2 0/3] Add cleanup action to perf-lib Thomas Gummerer
@ 2013-09-23 21:08 ` Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 2/3] perf-lib: add cleanup option Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 3/3] p0003-index.sh: add perf test for the index formats Thomas Gummerer
2 siblings, 0 replies; 12+ messages in thread
From: Thomas Gummerer @ 2013-09-23 21:08 UTC (permalink / raw)
To: gitster; +Cc: trast, git, t.gummerer
From: Junio C Hamano <gitster@pobox.com>
Add a new, extensible style for tests that allows the addition of new
parameters other than the prerequitites
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
t/README | 24 +++++++++++++++--
t/perf/README | 12 ++++++++-
t/perf/perf-lib.sh | 17 +++++-------
t/t0008-ignores.sh | 34 +++++++++++------------
t/test-lib-functions.sh | 72 ++++++++++++++++++++++++++++++++++++++-----------
5 files changed, 114 insertions(+), 45 deletions(-)
diff --git a/t/README b/t/README
index 2167125..bf41c29 100644
--- a/t/README
+++ b/t/README
@@ -390,6 +390,12 @@ below), e.g.:
"$PERL_PATH" -e "hlagh() if unf_unf()"
'
+or in the "modern" form, specifying the prerequisite as a parameter, e.g.:
+
+ test_expect_success --prereq PERL 'I need Perl' '
+ "$PERL_PATH" -e "hlagh() if unf_unf()"
+ '
+
The advantage of skipping tests like this is that platforms that don't
have the PERL and other optional dependencies get an indication of how
many tests they're missing.
@@ -422,6 +428,7 @@ There are a handful helper functions defined in the test harness
library for your script to use.
- test_expect_success [<prereq>] <message> <script>
+ - test_expect_success [--prereq <prereq>] [--] <message> <script>
Usually takes two strings as parameters, and evaluates the
<script>. If it yields success, test is considered
@@ -434,19 +441,31 @@ library for your script to use.
'tree=$(git-write-tree)'
If you supply three parameters the first will be taken to be a
- prerequisite; see the test_set_prereq and test_have_prereq
+ prerequisite; if you ues the "modern" test style, you can specify
+ the prerequisites with --prereq; see the test_set_prereq and test_have_prereq
documentation below:
test_expect_success TTY 'git --paginate rev-list uses a pager' \
' ... '
+ test_expect_success --prereq TTY 'git --paginate rev-list uses a pager' \
+ ' ... '
+
You can also supply a comma-separated list of prerequisites, in the
rare case where your test depends on more than one:
test_expect_success PERL,PYTHON 'yo dawg' \
' test $(perl -E 'print eval "1 +" . qx[python -c "print 2"]') == "4" '
+ test_expect_success --prereq PERL,PYTHON 'yo dawg' \
+ ' test $(perl -E 'print eval "1 +" . qx[python -c "print 2"]') == "4" '
+
+ The modern version also allows to distinguish the message from the
+ description and test script with --, in case the message starts
+ with --.
+
- test_expect_failure [<prereq>] <message> <script>
+ - test_expect_failure [--prereq <prereq>] [--] <message> <script>
This is NOT the opposite of test_expect_success, but is used
to mark a test that demonstrates a known breakage. Unlike
@@ -456,7 +475,8 @@ library for your script to use.
tests won't cause -i (immediate) to stop.
Like test_expect_success this function can optionally use a three
- argument invocation with a prerequisite as the first argument.
+ argument invocation with a prerequisite as the first argument, or
+ the modern invocation with the prerequisite as an extra parameter.
- test_debug <script>
diff --git a/t/perf/README b/t/perf/README
index 8848c14..21abbaf 100644
--- a/t/perf/README
+++ b/t/perf/README
@@ -118,11 +118,21 @@ At least one of the first two is required!
You can use test_expect_success as usual. For actual performance
tests, use
- test_perf 'descriptive string' '
+ test_perf [<prereq>] 'descriptive string' '
+ command1 &&
+ command2
+ '
+
+ test_perf [--prereq <prereq>] [--] 'descriptive string' '
command1 &&
command2
'
+prereq is an optional parameter to test_perf, and the performance
+tests are only executed if the prerequisite is fulfilled. The modern
+version also allows to distinguish the message from the description
+and test script with --, in case the message starts with --.
+
test_perf spawns a subshell, for lack of better options. This means
that
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 181513a..efe8dfd 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -234,7 +234,7 @@ test_expect_success_multi 'empty command line' '' '
stderr_contains "fatal: no path specified"
'
-test_expect_success_multi '--stdin with empty STDIN' '' '
+test_expect_success_multi -- '--stdin with empty STDIN' '' '
test_check_ignore "--stdin" 1 </dev/null &&
test_stderr ""
'
@@ -245,7 +245,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"
@@ -255,7 +255,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'
@@ -263,7 +263,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"
@@ -274,12 +274,12 @@ test_expect_success_multi 'erroneous use of --' '' '
stderr_contains "fatal: no path specified"
'
-test_expect_success_multi '--stdin with superfluous arg' '' '
+test_expect_success_multi -- '--stdin with superfluous arg' '' '
test_check_ignore "--stdin foo" 128 &&
stderr_contains "fatal: cannot specify pathnames with --stdin"
'
-test_expect_success_multi '--stdin -z with superfluous arg' '' '
+test_expect_success_multi -- '--stdin -z with superfluous arg' '' '
test_check_ignore "--stdin -z foo" 128 &&
stderr_contains "fatal: cannot specify pathnames with --stdin"
'
@@ -613,34 +613,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
"
@@ -699,7 +699,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 &&
@@ -707,7 +707,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 &&
@@ -715,7 +715,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 &&
@@ -725,7 +725,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 &&
@@ -733,7 +733,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 &&
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 19cdf0b..473b21d 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -347,20 +347,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_
@@ -368,16 +413,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
--
1.8.3.4.1241.g1ce9896
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/3] perf-lib: add cleanup option
2013-09-23 21:08 ` [PATCH v2 0/3] Add cleanup action to perf-lib Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 1/3] test-lib: introduce "modern" style tests Thomas Gummerer
@ 2013-09-23 21:08 ` Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 3/3] p0003-index.sh: add perf test for the index formats Thomas Gummerer
2 siblings, 0 replies; 12+ messages in thread
From: Thomas Gummerer @ 2013-09-23 21:08 UTC (permalink / raw)
To: gitster; +Cc: trast, git, t.gummerer
Add a --cleanup for the performance tests. This option can be used to
clean up the tested repository after each time the performance tests are
run. The option can be specified for normal tests too, although it will
not do anything for them. Use test_when_finished for those tests.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
t/perf/README | 11 ++++++++++-
t/perf/perf-lib.sh | 15 +++++++++++++++
t/test-lib-functions.sh | 6 ++++++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/t/perf/README b/t/perf/README
index 21abbaf..73a1d1c 100644
--- a/t/perf/README
+++ b/t/perf/README
@@ -123,7 +123,7 @@ tests, use
command2
'
- test_perf [--prereq <prereq>] [--] 'descriptive string' '
+ test_perf [--prereq <prereq>] [--cleanup <cleanup>] [--] 'descriptive string' '
command1 &&
command2
'
@@ -133,6 +133,15 @@ tests are only executed if the prerequisite is fulfilled. The modern
version also allows to distinguish the message from the description
and test script with --, in case the message starts with --.
+cleanup is another optional parameter to test_perf, which is executed
+after every run of the performance test. It can specify actions to
+bring the repository to the original state, in order to be able to
+execute the exact same test multiple times, e.g:
+
+ test_perf --cleanup 'git reset' 'test performance of git add' '
+ git add $somefile
+ '
+
test_perf spawns a subshell, for lack of better options. This means
that
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 6477d38..8ace4a3 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -176,6 +176,21 @@ test_perf () {
test_failure_ "$@"
break
fi
+ if ! test -z "$cleanup_action"; then
+ say >&3 "cleaning up: $cleanup_action"
+ if test_run_ "$cleanup_action"
+ then
+ if test -z "$verbose"; then
+ printf " c%s" "$i"
+ else
+ echo "* cleaning up run $i/$GIT_PERF_REPEAT_COUNT:"
+ fi
+ else
+ test -z $verbose && echo
+ test_failure_ "$@"
+ break
+ fi
+ fi
done
if test -z "$verbose"; then
echo " ok"
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 473b21d..4bad14f 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -360,6 +360,12 @@ test_expect_parse () {
test_prereq=$2
shift
;;
+ --cleanup)
+ test $# -gt 1 ||
+ error "bug in the test script: --cleanup needs a parameter"
+ cleanup_action=$2
+ shift
+ ;;
--)
shift
break
--
1.8.3.4.1241.g1ce9896
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/3] p0003-index.sh: add perf test for the index formats
2013-09-23 21:08 ` [PATCH v2 0/3] Add cleanup action to perf-lib Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 1/3] test-lib: introduce "modern" style tests Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 2/3] perf-lib: add cleanup option Thomas Gummerer
@ 2013-09-23 21:08 ` Thomas Gummerer
2 siblings, 0 replies; 12+ messages in thread
From: Thomas Gummerer @ 2013-09-23 21:08 UTC (permalink / raw)
To: gitster; +Cc: trast, git, t.gummerer
From: Thomas Rast <trast@inf.ethz.ch>
Add a performance test for index version [23]/4 by using
git update-index --index-version=x, thus testing both the reader
and the writer speed of all index formats.
Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
t/perf/p0003-index.sh | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
create mode 100755 t/perf/p0003-index.sh
diff --git a/t/perf/p0003-index.sh b/t/perf/p0003-index.sh
new file mode 100755
index 0000000..f2308c0
--- /dev/null
+++ b/t/perf/p0003-index.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+
+test_description="Tests index versions [23]/4/5"
+
+. ./perf-lib.sh
+
+test_perf_large_repo
+test_checkout_worktree
+
+test_expect_success "convert to v3" "
+ git update-index --index-version=2
+"
+subdir=$(git ls-files | sed 's#/[^/]*$##' | grep -v '^$' | uniq | tail -n 30 | head -1)
+file=$(git ls-files | tail -n 100 | head -1)
+
+test_expect_success "modify a file" "
+ echo 'foo bar' >>$file
+"
+
+test_perf "v[23]: update-index" "
+ git update-index --index-version=2 >/dev/null
+"
+
+test_perf "v[23]: grep nonexistent -- subdir" "
+ test_must_fail git grep nonexistent -- $subdir >/dev/null
+"
+
+test_perf "v[23]: ls-files -- subdir" "
+ git ls-files $subdir >/dev/null
+"
+
+test_perf --cleanup "git reset" "v[23]: update-index -- file" "
+ git update-index $file
+"
+
+test_expect_success "convert to v4" "
+ git update-index --index-version=4
+"
+
+test_perf "v4: update-index" "
+ git update-index --index-version=4 >/dev/null
+"
+
+test_perf "v4: grep nonexistent -- subdir" "
+ test_must_fail git grep nonexistent -- $subdir >/dev/null
+"
+
+test_perf "v4: ls-files -- subdir" "
+ git ls-files $subdir >/dev/null
+"
+
+test_perf --cleanup "git reset" "v4: update-index -- file" "
+ git update-index $file
+"
+
+test_done
--
1.8.3.4.1241.g1ce9896
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-09-23 21:10 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-17 12:10 [PATCH 1/2] perf-lib: split starting the test from the execution Thomas Gummerer
2013-09-17 12:10 ` [PATCH 2/2] perf-lib: add test_perf_cleanup target Thomas Gummerer
2013-09-17 17:43 ` Junio C Hamano
2013-09-19 11:42 ` Thomas Gummerer
2013-09-19 17:19 ` Junio C Hamano
2013-09-19 19:52 ` Junio C Hamano
2013-09-19 20:11 ` Junio C Hamano
2013-09-20 22:14 ` Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 0/3] Add cleanup action to perf-lib Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 1/3] test-lib: introduce "modern" style tests Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 2/3] perf-lib: add cleanup option Thomas Gummerer
2013-09-23 21:08 ` [PATCH v2 3/3] p0003-index.sh: add perf test for the index formats Thomas Gummerer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).