* [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).