test-lib: add the test_bash convenience function
PATCH 1/1 test-lib: add the test_bash convenience function
Since 781f76b15 (test-lib: redirect stdin of tests) you can't simply put a "bash &&" into a test for debugging purposes anymore. Instead you'll have to use "bash <&6 >&3 2>&4".
As that invocation is not that easy to remember add the test_bash convenience function. This function also checks if the -v flag is given and will complain if that is not the case instead of letting the test hang until ^D is pressed.
---
I was tempted to call that method "run_bash" but after looking around in test-lib.sh "test_bash" seemed like a better name.
t/test-lib.sh | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-)
t/test-lib.sh +11 -0
@@ -329,6 +329,17 @@ test_tick () { export GIT_COMMITTER_DATE GIT_AUTHOR_DATE } +# Stop execution and start a bash shell. This is useful for debugging tests +# and only makes sense together with "-v". + +test_bash () { + if test "$verbose" = t; then + bash <&6 >&3 2>&4 + else + say >&5 "skipping test_bash as it makes no sense without -v" + fi +} + # Call test_commit with the arguments "<message> [<file> [<contents>]]" # # This will commit a file with the given contents and the given commit
Signature · 1 line
1.7.9.rc1.1.g46aa0.dirty
Quoted 3 lines · Jens Lehmann
Since
781f76b15(test-lib: redirect stdin of tests) you can't simply put a "bash &&" into a test for debugging purposes anymore. Instead you'll have to use "bash <&6 >&3 2>&4".
Yeah, an unfortunate side effect.
If you're not relying on particular state in the middle of a chain of commands, you can just put the "bash" outside of the test_expect_*. But sometimes you do care about having it in the middle.
Quoted 4 lines
As that invocation is not that easy to remember add the
test_bashconvenience function. This function also checks if the -v flag is given and will complain if that is not the case instead of letting the test hang until ^D is pressed.
Nice. Many times I have added such a "bash" or "gdb" invocation then forgotten "-v", only to scratch my head at why the test seemed to be hanging.
Two minor nits on the patch itself:
t/test-lib.sh test_tick () { · line 341
Quoted 10 lines
t/test-lib.sh +10
-0+# Stop execution and start a bash shell. This is useful for debugging tests +# and only makes sense together with "-v". + +test_bash () { + if test "$verbose" = t; then + bash <&6 >&3 2>&4 + else + say >&5 "skipping test_bash as it makes no sense without -v" + fi +}
1. It may be worth putting a warning in the comment that this is never to be used in a real test, but only temporarily inserted.
2. I do this not just with bash, but with "gdb". I wonder if it is worth making this "test_foo bash", for some value of "foo" (the ones that occur to me are "debug" and "run", but of course they are taken).
Actually, I wonder if the existing test_debug could handle this already (though you do have to remember to add "--debug" to your command line, then).
-Peff
PATCH 1/1 test_interactive: interactive debugging in test scripts
For test debugging, pause test execution and spawn a shell to allow examination of internal test state. The shell has access to all exported variables from the test. Exit the shell to continue the test. Calls to this function should never be included in committed code. The "--verbose" option is required.
---
[email protected] wrote on Sun, 15 Jan 2012 18:24 -0500:
Quoted 9 lines · Jens Lehmann
Quoted 4 lines
As that invocation is not that easy to remember add the
test_bashconvenience function. This function also checks if the -v flag is given and will complain if that is not the case instead of letting the test hang until ^D is pressed.Nice. Many times I have added such a "bash" or "gdb" invocation then forgotten "-v", only to scratch my head at why the test seemed to be hanging.
Yes, good catch noticing the need for "-v".
Here's something similar that I've been playing around with locally. I export HOME and TERM in the debug shell to make sure all the features around dotfiles and color/editing work nicely. Also just use SHELL directly, not SHELL_PATH or bash, to cater to other shell users.
And it is necessary to export any test variables you want to use in the debug shell. I often cut-n-paste lines containing TEST_DIRECTORY and TRASH_DIRECTORY; there could be others, in test scripts and helper libraries too.
Quoted 7 lines
2. I do this not just with bash, but with "gdb". I wonder if it is worth making this "
test_foobash", for some value of "foo" (the ones that occur to me are "debug" and "run", but of course they are taken).Actually, I wonder if the existing
test_debugcould handle this already (though you do have to remember to add "--debug" to your command line, then).
While it would be nice to use:
test_interactive gdb --args git ...
the path is setup to invoke the script in bin-wrappers/git, requiring either --with-dashes or something like
test_interactive gdb --args "$GIT_EXEC_PATH"/git ...
both of which I'm sure to forget. I have a "test_gdb_git" that can be used in place of "git" in a script for exactly this purpose, but feel it's not general enough to warrant inclusion. It's easy to start a shell then invoke gdb by hand.
Between mine and Jens' there is hopefully something widely useful here.
-- Pete
t/README | 9 +++++++++ t/test-lib.sh | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 0 deletions(-)
t/README +9 -0
@@ -548,6 +548,15 @@ library for your script to use. ... ' + - test_interactive + + For test debugging, pause test execution and spawn a shell + to allow examination of internal test state. The shell has + access to all exported variables from the test. Exit the shell + to continue the test. Calls to this function should never be + included in committed code. This function requires the "-v" + ("--verbose") option. + Prerequisites -------------
t/test-lib.sh +34 -0
@@ -32,7 +32,9 @@ done,*) esac # Keep the original TERM for say_color +# and the original HOME for interactive debugging ORIGINAL_TERM=$TERM +ORIGINAL_HOME="$HOME" # For repeatability, reset the environment to known value. LANG=C
@@ -473,6 +475,36 @@ test_debug () { test "$debug" = "" || eval "$1" } +# +# Add to a test script to spawn a shell during execution. This +# spawns a shell allowing inspection of internal test state. Exit +# the shell to continue with the test. Example: +# +# test_expect_success 'test' ' +# git do-something && +# test_interactive && +# test_cmp expected current +# ' +# +# Be sure to remove the debug lines before submitting: it only +# works with "-v". +# +test_interactive () { + if test -z "$verbose" ; then + error >&5 "test_interactive requires --verbose" + fi + say >&3 "Interactive debugging" + say >&3 "Exit shell to continue test" + ( + # restore important original environment variables + exec 0<&6 3>&5 2>&4 + export HOME="$ORIGINAL_HOME" + export TERM="$ORIGINAL_TERM" + exec $SHELL + ) + return 0 +} + test_eval_ () { # This is a separate function because some tests use # "return" to end a test_expect_success block early.
@@ -901,6 +933,7 @@ then # itself. TEST_DIRECTORY=$(pwd) fi +export TEST_DIRECTORY GIT_BUILD_DIR="$TEST_DIRECTORY"/.. if test -n "$valgrind"
@@ -1038,6 +1071,7 @@ case "$test" in /*) TRASH_DIRECTORY="$test" ;; *) TRASH_DIRECTORY="$TEST_DIRECTORY/$test" ;; esac +export TRASH_DIRECTORY test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY rm -fr "$test" || { GIT_EXIT_OK=t
Signature · 1 line
1.7.9.rc0.47.gc9457
Re: [PATCH] test_interactive: interactive debugging in test scripts
Quoted 4 lines · Pete Wyckoff
And it is necessary to export any test variables you want to use in the debug shell. I often cut-n-paste lines containing
TEST_DIRECTORYandTRASH_DIRECTORY; there could be others, in test scripts and helper libraries too.
Yeah, exporting a few common ones would be helpful. I really wish there was a way to ask a shell to stop running the script and start doing interactive things in the current shell context, but no such thing exists AFAIK (you can hack it with a read-eval loop, but you are missing many of the useful input-handling bits like tab completion. Hmm, I wonder if bash's "read -e" would be enough, though).
I kind of wonder if that is over-engineering, though. I'd like a perfect debugging environment, but the fact of the matter is that writing, maintaining, and making it work in every case that comes up is probably a lot more work than just fiddling with the scripts now and then. This code isn't even meant to be run except in such fiddling circumstances.
Quoted 8 lines
While it would be nice to use:
test_interactivegdb --args git ...the path is setup to invoke the script in bin-wrappers/git, requiring either --with-dashes or something like
test_interactivegdb --args "$GIT_EXEC_PATH"/git ...
Yeah. I have before patched the bin-wrappers script to accept a GIT_WRAPPER_PREFIX variable, so you can just set that and have it run gdb on your invocation. But even that's not enough for externals. I've been tempted to actually carry around a run-time option to exec externals via gdb, but I didn't want to pollute the regular code base (and you can usually get by with just running "gdb git-foo" directly).
-Peff
Re: [PATCH] test_interactive: interactive debugging in test scripts
In reply to Pete WyckoffAm 16.01.2012 16:49, schrieb Pete Wyckoff:
Quoted 2 lines
Between mine and Jens' there is hopefully something widely useful here.
Just while I was planning a v2 and thought about renaming "test_bash" to "test_interactive", let it take a command to run as optional argument and document it better you came up with this :-)
So I vote for your patch as it takes my initial idea even further. I really like that HOME, TERM and SHELL are honored in your version leaving the user with a fully functional shell of his choice.
Re: [PATCH] test_interactive: interactive debugging in test scripts
Quoted 3 lines · Jens Lehmann
So I vote for your patch as it takes my initial idea even further. I really like that HOME, TERM and SHELL are honored in your version leaving the user with a fully functional shell of his choice.
I'm actually mildly negative on this feature, as it interferes with the tests themselves. Probably TERM and SHELL don't matter. But $HOME means git will read your personal .gitconfig, not any config (or lack thereof) in the trash directory.
I've personally been left head-scratching in the past by:
$ ./tXXXX-foo -v -i ... expecting success: git foo not ok - 1 git foo
$ cd trash\ directory.tXXXX-foo $ git foo ;# and it works!
where the culprit was a non-clean environment caused by my personal git config (or very occasionally, something else in $HOME).
-Peff
Re: [PATCH] test_interactive: interactive debugging in test scripts
Am 16.01.2012 21:11, schrieb Jeff King:
Quoted 10 lines · Jens Lehmann
Quoted 4 lines
So I vote for your patch as it takes my initial idea even further. I really like that HOME, TERM and SHELL are honored in your version leaving the user with a fully functional shell of his choice.
I'm actually mildly negative on this feature, as it interferes with the tests themselves. Probably TERM and SHELL don't matter. But $HOME means git will read your personal .gitconfig, not any config (or lack thereof) in the trash directory.
Good point, I haven't thought of that ... so yes, at least $HOME should go.
Re: [PATCH] test_interactive: interactive debugging in test scripts
[email protected] wrote on Mon, 16 Jan 2012 21:48 +0100:
Quoted 7 lines
Am 16.01.2012 21:11, schrieb Jeff King:
Quoted 6 lines · Jens Lehmann
Quoted 4 lines
So I vote for your patch as it takes my initial idea even further. I really like that HOME, TERM and SHELL are honored in your version leaving the user with a fully functional shell of his choice.
Quoted 7 lines
Quoted 4 lines
I'm actually mildly negative on this feature, as it interferes with the tests themselves. Probably TERM and SHELL don't matter. But $HOME means git will read your personal .gitconfig, not any config (or lack thereof) in the trash directory.
Good point, I haven't thought of that ... so yes, at least $HOME should go.
I, too, have stumbled over differences that are due to picking up something in $HOME. It takes a minute to realize what's going on.
TERM can interfere with at least one test: c2116a1 (test-lib: fix TERM to dumb for test repeatability, 2008-03-06). SHELL can cause issues when it is more feature-ful than SHELL_PATH.
On the other hand, it's frustrating to work in an environment without my shell aliases, git aliases, readline customizations, personal path, and assorted environment variables.
I'm comfortable with the (rare?) possibility of confusion. But perhaps it is unwise to support a feature with so many caveats, even if it is only for debugging.
-- Pete
Quoted 9 lines · Jeff King
Nice. Many times I have added such a "bash" or "gdb" invocation then forgotten "-v", only to scratch my head at why the test seemed to be hanging.
Two minor nits on the patch itself:
...
1. It may be worth putting a warning in the comment that this is never to be used in a real test, but only temporarily inserted.
Quoted 7 lines
2. I do this not just with bash, but with "gdb". I wonder if it is worth making this "
test_foobash", for some value of "foo" (the ones that occur to me are "debug" and "run", but of course they are taken).Actually, I wonder if the existing
test_debugcould handle this already (though you do have to remember to add "--debug" to your command line, then).
I wondered the same thing from a different angle. My first reaction was "Why is this called 'bash' not 'sh'?" which naturally led to the same direction as yours "why not an arbitrary command 'test_debug xxx'?"
test_pause perhaps?
PATCH 1/1 test-lib: add the test_pause convenience function
Since 781f76b15 (test-lib: redirect stdin of tests) you can't simply put a "bash &&" into a test for debugging purposes anymore. Instead you'll have to use "bash <&6 >&3 2>&4".
As that invocation is not that easy to remember add the test_pause convenience function. This function also checks if the -v flag is given and will error out if that is not the case instead of letting the test hang until ^D is pressed.
---
Am 16.01.2012 23:51, schrieb Junio C Hamano:
Quoted 19 lines
Quoted 9 lines · Jeff King
Nice. Many times I have added such a "bash" or "gdb" invocation then forgotten "-v", only to scratch my head at why the test seemed to be hanging.
Two minor nits on the patch itself:
...
1. It may be worth putting a warning in the comment that this is never to be used in a real test, but only temporarily inserted.Quoted 7 lines
2. I do this not just with bash, but with "gdb". I wonder if it is worth making this "
test_foobash", for some value of "foo" (the ones that occur to me are "debug" and "run", but of course they are taken).Actually, I wonder if the existing
test_debugcould handle this already (though you do have to remember to add "--debug" to your command line, then).
Quoted 5 lines
I wondered the same thing from a different angle. My first reaction was "Why is this called 'bash' not 'sh'?" which naturally led to the same direction as yours "why not an arbitrary command '
test_debugxxx'?"
test_pauseperhaps?
I really don't care deeply about the name, so test_pause is absolutely ok for me. I added some documentation in t/README too and made it an error when --verbose is not used.
Is it ok to invoke bash here or should sh be used?
t/README | 13 +++++++++++++ t/test-lib.sh | 13 +++++++++++++ 2 files changed, 26 insertions(+), 0 deletions(-)
t/README +13 -0
@@ -548,6 +548,19 @@ library for your script to use. ... ' + - test_pause + + This command is useful for writing and debugging tests and must be + removed before submitting. It halts the execution of the test and + spawns a shell in the trash directory. Exit the shell to continue + the test. Example: + + test_expect_success 'test' ' + git do-something >actual && + test_pause && + test_cmp expected actual + ' + Prerequisites -------------
t/test-lib.sh +13 -0
@@ -329,6 +329,19 @@ test_tick () { export GIT_COMMITTER_DATE GIT_AUTHOR_DATE } +# Stop execution and start a shell. This is useful for debugging tests and +# only makes sense together with "-v". +# +# Be sure to remove all invocations of this command before submitting. + +test_pause () { + if test "$verbose" = t; then + bash <&6 >&3 2>&4 + else + error >&5 "test_pause requires --verbose" + fi +} + # Call test_commit with the arguments "<message> [<file> [<contents>]]" # # This will commit a file with the given contents and the given commit
Signature · 1 line
1.7.9.rc1.2.g0b847.dirty
Re: [PATCH] test-lib: add the test_pause convenience function
Quoted 3 lines · Jens Lehmann
I really don't care deeply about the name, so
test_pauseis absolutely ok for me. I added some documentation in t/README too and made it an error when --verbose is not used.
I don't care about the name at all, either.
What I cared was more about the hardcoded "bash". Believe it or not, there are boxes that lack it, and there are people who prefer other shells for their interactive work. At the very least, invoke "$SHELL_PATH" instead of bash there, perhaps?
If we wanted to allow an ad-hoc debugging of test scripts to sprinkle "test_pause $cmd", we might need to do something like:
t/test-lib.sh test_tick () { · line 339
Quoted 3 lines
t/test-lib.sh +3
-0+test_pause () { + if test "$verbose" = t; then + bash <&6 >&3 2>&4
${1-"$SHELL_PATH"} <&6 >&3 2>&4t/test-lib.sh test_tick () { · line 343
Quoted 4 lines
t/test-lib.sh +4
-0+ else + error >&5 "test_pause requires --verbose" + fi +}
but I do not think that is worth it. The debugging developer should easily be able to run gdb or whatever from the interactive shell you are giving here.
PATCH v2 1/1 test-lib: add the test_pause convenience function
Since 781f76b15 (test-lib: redirect stdin of tests) you can't simply put a "bash &&" into a test for debugging purposes anymore. Instead you'll have to use "bash <&6 >&3 2>&4".
As that invocation is not that easy to remember add the test_pause convenience function. It invokes "$SHELL_PATH" to provide a sane shell for the user.
This function also checks if the -v flag is given and will error out if that is not the case instead of letting the test hang until ^D is pressed.
---
Am 17.01.2012 20:15, schrieb Junio C Hamano:
Quoted 5 lines
Jens Lehmann <[email protected]> writes:
What I cared was more about the hardcoded "bash". Believe it or not, there are boxes that lack it, and there are people who prefer other shells for their interactive work. At the very least, invoke "$SHELL_PATH" instead of bash there, perhaps?
Sure, I changed that in this version and explained it in the commit message.
Quoted 7 lines
If we wanted to allow an ad-hoc debugging of test scripts to sprinkle "
test_pause$cmd", we might need to do something like:Review
Quoted 3 lines
(quoted diff) +3
-0+test_pause () { + if test "$verbose" = t; then + bash <&6 >&3 2>&4${1-"$SHELL_PATH"} <&6 >&3 2>&4
Quoted 8 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ else + error >&5 "test_pause requires --verbose" + fi +}but I do not think that is worth it. The debugging developer should easily be able to run gdb or whatever from the interactive shell you are giving here.
That's what I always do, so I'm fine with what this patch provides. And now the fact that you can temporarily pause a test and explore the trash directory is documented too ;-)
t/README | 13 +++++++++++++ t/test-lib.sh | 13 +++++++++++++ 2 files changed, 26 insertions(+), 0 deletions(-)
t/README +13 -0
@@ -548,6 +548,19 @@ library for your script to use. ... ' + - test_pause + + This command is useful for writing and debugging tests and must be + removed before submitting. It halts the execution of the test and + spawns a shell in the trash directory. Exit the shell to continue + the test. Example: + + test_expect_success 'test' ' + git do-something >actual && + test_pause && + test_cmp expected actual + ' + Prerequisites -------------
t/test-lib.sh +13 -0
@@ -329,6 +329,19 @@ test_tick () { export GIT_COMMITTER_DATE GIT_AUTHOR_DATE } +# Stop execution and start a shell. This is useful for debugging tests and +# only makes sense together with "-v". +# +# Be sure to remove all invocations of this command before submitting. + +test_pause () { + if test "$verbose" = t; then + "$SHELL_PATH" <&6 >&3 2>&4 + else + error >&5 "test_pause requires --verbose" + fi +} + # Call test_commit with the arguments "<message> [<file> [<contents>]]" # # This will commit a file with the given contents and the given commit
Signature · 1 line
1.7.9.rc1.1.g8dae2