{"thread":{"id":"29371","subject":"[PATCH] test-lib: add the test_bash convenience function","startedAt":"2012-01-15T20:00:41Z","lastAt":"2012-01-17T21:04:31Z","messageCount":12,"participants":["Jens Lehmann","Jeff King","Pete Wyckoff","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"182582","messageId":"4F133069.10308@web.de","threadId":"29371","inReplyTo":null,"subject":"[PATCH] test-lib: add the test_bash convenience function","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-01-15T20:00:41Z","receivedAt":"2012-01-15T20:00:41Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Since 781f76b15 (test-lib: redirect stdin of tests) you can't simply put a\n\"bash &&\" into a test for debugging purposes anymore. Instead you'll have\nto use \"bash <&6 >&3 2>&4\".\n\nAs that invocation is not that easy to remember add the test_bash\nconvenience function. This function also checks if the -v flag is given\nand will complain if that is not the case instead of letting the test\nhang until ^D is pressed.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nI was tempted to call that method \"run_bash\" but after looking around\nin test-lib.sh \"test_bash\" seemed like a better name.\n\n t/test-lib.sh |   11 +++++++++++\n 1 files changed, 11 insertions(+), 0 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex a65dfc7..f9061e0 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -329,6 +329,17 @@ test_tick () {\n \texport GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n }\n\n+# Stop execution and start a bash shell. This is useful for debugging tests\n+# and only makes sense together with \"-v\".\n+\n+test_bash () {\n+\tif test \"$verbose\" = t; then\n+\t\tbash <&6 >&3 2>&4\n+\telse\n+\t\tsay >&5 \"skipping test_bash as it makes no sense without -v\"\n+\tfi\n+}\n+\n # Call test_commit with the arguments \"<message> [<file> [<contents>]]\"\n #\n # This will commit a file with the given contents and the given commit\n-- \n1.7.9.rc1.1.g46aa0.dirty\n"},{"id":"182583","messageId":"20120115232413.GA14724@sigill.intra.peff.net","threadId":"29371","inReplyTo":"4F133069.10308@web.de","subject":"Re: [PATCH] test-lib: add the test_bash convenience function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-15T23:24:13Z","receivedAt":"2012-01-15T23:24:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jan 15, 2012 at 09:00:41PM +0100, Jens Lehmann wrote:\n\n> Since 781f76b15 (test-lib: redirect stdin of tests) you can't simply put a\n> \"bash &&\" into a test for debugging purposes anymore. Instead you'll have\n> to use \"bash <&6 >&3 2>&4\".\n\nYeah, an unfortunate side effect.\n\nIf you're not relying on particular state in the middle of a chain of\ncommands, you can just put the \"bash\" outside of the test_expect_*. But\nsometimes you do care about having it in the middle.\n\n> As that invocation is not that easy to remember add the test_bash\n> convenience function. This function also checks if the -v flag is given\n> and will complain if that is not the case instead of letting the test\n> hang until ^D is pressed.\n\nNice. Many times I have added such a \"bash\" or \"gdb\" invocation then\nforgotten \"-v\", only to scratch my head at why the test seemed to be\nhanging.\n\nTwo minor nits on the patch itself:\n\n> +# Stop execution and start a bash shell. This is useful for debugging tests\n> +# and only makes sense together with \"-v\".\n> +\n> +test_bash () {\n> +\tif test \"$verbose\" = t; then\n> +\t\tbash <&6 >&3 2>&4\n> +\telse\n> +\t\tsay >&5 \"skipping test_bash as it makes no sense without -v\"\n> +\tfi\n> +}\n\n1. It may be worth putting a warning in the comment that this is never\n   to be used in a real test, but only temporarily inserted.\n\n2. I do this not just with bash, but with \"gdb\". I wonder if it is worth\n   making this \"test_foo bash\", for some value of \"foo\" (the ones that\n   occur to me are \"debug\" and \"run\", but of course they are taken).\n\n   Actually, I wonder if the existing test_debug could handle this\n   already (though you do have to remember to add \"--debug\" to your\n   command line, then).\n\n-Peff\n"},{"id":"182614","messageId":"20120116154953.GA21238@padd.com","threadId":"29371","inReplyTo":"20120115232413.GA14724@sigill.intra.peff.net","subject":"[PATCH] test_interactive: interactive debugging in test scripts","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-01-16T15:49:53Z","receivedAt":"2012-01-16T15:49:53Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"For test debugging, pause test execution and spawn a shell to\nallow examination of internal test state.  The shell has access\nto all exported variables from the test.  Exit the shell to\ncontinue the test.  Calls to this function should never be\nincluded in committed code.  The \"--verbose\" option is required.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\npeff@peff.net wrote on Sun, 15 Jan 2012 18:24 -0500:\n> On Sun, Jan 15, 2012 at 09:00:41PM +0100, Jens Lehmann wrote:\n> > As that invocation is not that easy to remember add the test_bash\n> > convenience function. This function also checks if the -v flag is given\n> > and will complain if that is not the case instead of letting the test\n> > hang until ^D is pressed.\n> \n> Nice. Many times I have added such a \"bash\" or \"gdb\" invocation then\n> forgotten \"-v\", only to scratch my head at why the test seemed to be\n> hanging.\n\nYes, good catch noticing the need for \"-v\".\n\nHere's something similar that I've been playing around with\nlocally.  I export HOME and TERM in the debug shell to make\nsure all the features around dotfiles and color/editing work\nnicely.  Also just use SHELL directly, not SHELL_PATH or bash,\nto cater to other shell users.\n\nAnd it is necessary to export any test variables you want to use\nin the debug shell.  I often cut-n-paste lines containing\nTEST_DIRECTORY and TRASH_DIRECTORY; there could be others,\nin test scripts and helper libraries too.\n\n> 2. I do this not just with bash, but with \"gdb\". I wonder if it is worth\n>    making this \"test_foo bash\", for some value of \"foo\" (the ones that\n>    occur to me are \"debug\" and \"run\", but of course they are taken).\n> \n>    Actually, I wonder if the existing test_debug could handle this\n>    already (though you do have to remember to add \"--debug\" to your\n>    command line, then).\n\nWhile it would be nice to use:\n\n    test_interactive gdb --args git ...\n\nthe path is setup to invoke the script in bin-wrappers/git,\nrequiring either --with-dashes or something like\n\n    test_interactive gdb --args \"$GIT_EXEC_PATH\"/git ...\n\nboth of which I'm sure to forget.  I have a \"test_gdb_git\" that\ncan be used in place of \"git\" in a script for exactly this\npurpose, but feel it's not general enough to warrant inclusion.\nIt's easy to start a shell then invoke gdb by hand.\n\nBetween mine and Jens' there is hopefully something widely useful\nhere.\n\n\t\t-- Pete\n\n t/README      |    9 +++++++++\n t/test-lib.sh |   34 ++++++++++++++++++++++++++++++++++\n 2 files changed, 43 insertions(+), 0 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex c85abaf..da4521f 100644\n--- a/t/README\n+++ b/t/README\n@@ -548,6 +548,15 @@ library for your script to use.\n \t\t...\n \t'\n \n+ - test_interactive\n+\n+   For test debugging, pause test execution and spawn a shell\n+   to allow examination of internal test state.  The shell has\n+   access to all exported variables from the test.  Exit the shell\n+   to continue the test.  Calls to this function should never be\n+   included in committed code.  This function requires the \"-v\"\n+   (\"--verbose\") option.\n+\n Prerequisites\n -------------\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex a65dfc7..a834602 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -32,7 +32,9 @@ done,*)\n esac\n \n # Keep the original TERM for say_color\n+# and the original HOME for interactive debugging\n ORIGINAL_TERM=$TERM\n+ORIGINAL_HOME=\"$HOME\"\n \n # For repeatability, reset the environment to known value.\n LANG=C\n@@ -473,6 +475,36 @@ test_debug () {\n \ttest \"$debug\" = \"\" || eval \"$1\"\n }\n \n+#\n+# Add to a test script to spawn a shell during execution.  This\n+# spawns a shell allowing inspection of internal test state.  Exit\n+# the shell to continue with the test.  Example:\n+#\n+# \ttest_expect_success 'test' '\n+# \t\tgit do-something &&\n+# \t\ttest_interactive &&\n+# \t\ttest_cmp expected current\n+#\t'\n+#\n+# Be sure to remove the debug lines before submitting:  it only\n+# works with \"-v\".\n+#\n+test_interactive () {\n+\tif test -z \"$verbose\" ; then\n+\t\terror >&5 \"test_interactive requires --verbose\"\n+\tfi\n+\tsay >&3 \"Interactive debugging\"\n+\tsay >&3 \"Exit shell to continue test\"\n+\t(\n+\t\t# restore important original environment variables\n+\t\texec 0<&6 3>&5 2>&4\n+\t\texport HOME=\"$ORIGINAL_HOME\"\n+\t\texport TERM=\"$ORIGINAL_TERM\"\n+\t\texec $SHELL\n+\t)\n+\treturn 0\n+}\n+\n test_eval_ () {\n \t# This is a separate function because some tests use\n \t# \"return\" to end a test_expect_success block early.\n@@ -901,6 +933,7 @@ then\n \t# itself.\n \tTEST_DIRECTORY=$(pwd)\n fi\n+export TEST_DIRECTORY\n GIT_BUILD_DIR=\"$TEST_DIRECTORY\"/..\n \n if test -n \"$valgrind\"\n@@ -1038,6 +1071,7 @@ case \"$test\" in\n /*) TRASH_DIRECTORY=\"$test\" ;;\n  *) TRASH_DIRECTORY=\"$TEST_DIRECTORY/$test\" ;;\n esac\n+export TRASH_DIRECTORY\n test ! -z \"$debug\" || remove_trash=$TRASH_DIRECTORY\n rm -fr \"$test\" || {\n \tGIT_EXIT_OK=t\n-- \n1.7.9.rc0.47.gc9457\n"},{"id":"182627","messageId":"4F148211.70106@web.de","threadId":"29371","inReplyTo":"20120116154953.GA21238@padd.com","subject":"Re: [PATCH] test_interactive: interactive debugging in test scripts","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-01-16T20:01:21Z","receivedAt":"2012-01-16T20:01:21Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 16.01.2012 16:49, schrieb Pete Wyckoff:\n> Between mine and Jens' there is hopefully something widely useful\n> here.\n\nJust while I was planning a v2 and thought about renaming \"test_bash\"\nto \"test_interactive\", let it take a command to run as optional\nargument and document it better you came up with this :-)\n\nSo I vote for your patch as it takes my initial idea even further. I\nreally like that HOME, TERM and SHELL are honored in your version\nleaving the user with a fully functional shell of his choice.\n"},{"id":"182628","messageId":"20120116201123.GA18699@sigill.intra.peff.net","threadId":"29371","inReplyTo":"4F148211.70106@web.de","subject":"Re: [PATCH] test_interactive: interactive debugging in test scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-16T20:11:23Z","receivedAt":"2012-01-16T20:11:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 16, 2012 at 09:01:21PM +0100, Jens Lehmann wrote:\n\n> So I vote for your patch as it takes my initial idea even further. I\n> really like that HOME, TERM and SHELL are honored in your version\n> leaving the user with a fully functional shell of his choice.\n\nI'm actually mildly negative on this feature, as it interferes with the\ntests themselves. Probably TERM and SHELL don't matter. But $HOME means\ngit will read your personal .gitconfig, not any config (or lack thereof)\nin the trash directory.\n\nI've personally been left head-scratching in the past by:\n\n  $ ./tXXXX-foo -v -i\n  ...\n  expecting success: git foo\n  not ok - 1 git foo\n\n  $ cd trash\\ directory.tXXXX-foo\n  $ git foo ;# and it works!\n\nwhere the culprit was a non-clean environment caused by my personal git\nconfig (or very occasionally, something else in $HOME).\n\n-Peff\n"},{"id":"182629","messageId":"20120116201922.GB18699@sigill.intra.peff.net","threadId":"29371","inReplyTo":"20120116154953.GA21238@padd.com","subject":"Re: [PATCH] test_interactive: interactive debugging in test scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-16T20:19:22Z","receivedAt":"2012-01-16T20:19:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 16, 2012 at 10:49:53AM -0500, Pete Wyckoff wrote:\n\n> And it is necessary to export any test variables you want to use\n> in the debug shell.  I often cut-n-paste lines containing\n> TEST_DIRECTORY and TRASH_DIRECTORY; there could be others,\n> in test scripts and helper libraries too.\n\nYeah, exporting a few common ones would be helpful. I really wish there\nwas a way to ask a shell to stop running the script and start doing\ninteractive things in the current shell context, but no such thing\nexists AFAIK (you can hack it with a read-eval loop, but you are missing\nmany of the useful input-handling bits like tab completion. Hmm, I\nwonder if bash's \"read -e\" would be enough, though).\n\nI kind of wonder if that is over-engineering, though. I'd like a perfect\ndebugging environment, but the fact of the matter is that writing,\nmaintaining, and making it work in every case that comes up is probably\na lot more work than just fiddling with the scripts now and then. This\ncode isn't even meant to be run except in such fiddling circumstances.\n\n> While it would be nice to use:\n> \n>     test_interactive gdb --args git ...\n> \n> the path is setup to invoke the script in bin-wrappers/git,\n> requiring either --with-dashes or something like\n> \n>     test_interactive gdb --args \"$GIT_EXEC_PATH\"/git ...\n\nYeah. I have before patched the bin-wrappers script to accept a\nGIT_WRAPPER_PREFIX variable, so you can just set that and have it run\ngdb on your invocation. But even that's not enough for externals. I've\nbeen tempted to actually carry around a run-time option to exec\nexternals via gdb, but I didn't want to pollute the regular code base\n(and you can usually get by with just running \"gdb git-foo\" directly).\n\n-Peff\n"},{"id":"182631","messageId":"4F148D3B.4070206@web.de","threadId":"29371","inReplyTo":"20120116201123.GA18699@sigill.intra.peff.net","subject":"Re: [PATCH] test_interactive: interactive debugging in test scripts","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-01-16T20:48:59Z","receivedAt":"2012-01-16T20:48:59Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 16.01.2012 21:11, schrieb Jeff King:\n> On Mon, Jan 16, 2012 at 09:01:21PM +0100, Jens Lehmann wrote:\n> \n>> So I vote for your patch as it takes my initial idea even further. I\n>> really like that HOME, TERM and SHELL are honored in your version\n>> leaving the user with a fully functional shell of his choice.\n> \n> I'm actually mildly negative on this feature, as it interferes with the\n> tests themselves. Probably TERM and SHELL don't matter. But $HOME means\n> git will read your personal .gitconfig, not any config (or lack thereof)\n> in the trash directory.\n\nGood point, I haven't thought of that ... so yes, at least $HOME should\ngo.\n"},{"id":"182638","messageId":"20120116220712.GA30785@padd.com","threadId":"29371","inReplyTo":"4F148D3B.4070206@web.de","subject":"Re: [PATCH] test_interactive: interactive debugging in test scripts","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-01-16T22:07:12Z","receivedAt":"2012-01-16T22:07:12Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Jens.Lehmann@web.de wrote on Mon, 16 Jan 2012 21:48 +0100:\n> Am 16.01.2012 21:11, schrieb Jeff King:\n> > On Mon, Jan 16, 2012 at 09:01:21PM +0100, Jens Lehmann wrote:\n> > \n> >> So I vote for your patch as it takes my initial idea even further. I\n> >> really like that HOME, TERM and SHELL are honored in your version\n> >> leaving the user with a fully functional shell of his choice.\n> > \n> > I'm actually mildly negative on this feature, as it interferes with the\n> > tests themselves. Probably TERM and SHELL don't matter. But $HOME means\n> > git will read your personal .gitconfig, not any config (or lack thereof)\n> > in the trash directory.\n> \n> Good point, I haven't thought of that ... so yes, at least $HOME should\n> go.\n\nI, too, have stumbled over differences that are due to picking up\nsomething in $HOME.  It takes a minute to realize what's going\non.\n\nTERM can interfere with at least one test: c2116a1 (test-lib: fix TERM\nto dumb for test repeatability, 2008-03-06).  SHELL can cause\nissues when it is more feature-ful than SHELL_PATH.\n\nOn the other hand, it's frustrating to work in an environment\nwithout my shell aliases, git aliases, readline customizations,\npersonal path, and assorted environment variables.\n\nI'm comfortable with the (rare?) possibility of confusion.  But\nperhaps it is unwise to support a feature with so many caveats,\neven if it is only for debugging.\n\n\t\t-- Pete\n"},{"id":"182643","messageId":"7vk44ruupe.fsf@alter.siamese.dyndns.org","threadId":"29371","inReplyTo":"20120115232413.GA14724@sigill.intra.peff.net","subject":"Re: [PATCH] test-lib: add the test_bash convenience function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-16T22:51:57Z","receivedAt":"2012-01-16T22:51:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Nice. Many times I have added such a \"bash\" or \"gdb\" invocation then\n> forgotten \"-v\", only to scratch my head at why the test seemed to be\n> hanging.\n>\n> Two minor nits on the patch itself:\n> ...\n> 1. It may be worth putting a warning in the comment that this is never\n>    to be used in a real test, but only temporarily inserted.\n>\n> 2. I do this not just with bash, but with \"gdb\". I wonder if it is worth\n>    making this \"test_foo bash\", for some value of \"foo\" (the ones that\n>    occur to me are \"debug\" and \"run\", but of course they are taken).\n>\n>    Actually, I wonder if the existing test_debug could handle this\n>    already (though you do have to remember to add \"--debug\" to your\n>    command line, then).\n\nI wondered the same thing from a different angle. My first reaction was\n\"Why is this called 'bash' not 'sh'?\" which naturally led to the same\ndirection as yours \"why not an arbitrary command 'test_debug xxx'?\"\n\ntest_pause perhaps?\n"},{"id":"182678","messageId":"4F152F7C.9020902@web.de","threadId":"29371","inReplyTo":"7vk44ruupe.fsf@alter.siamese.dyndns.org","subject":"[PATCH] test-lib: add the test_pause convenience function","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-01-17T08:21:16Z","receivedAt":"2012-01-17T08:21:16Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Since 781f76b15 (test-lib: redirect stdin of tests) you can't simply put a\n\"bash &&\" into a test for debugging purposes anymore. Instead you'll have\nto use \"bash <&6 >&3 2>&4\".\n\nAs that invocation is not that easy to remember add the test_pause\nconvenience function. This function also checks if the -v flag is given\nand will error out if that is not the case instead of letting the test\nhang until ^D is pressed.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nAm 16.01.2012 23:51, schrieb Junio C Hamano:\n> Jeff King <peff@peff.net> writes:\n> \n>> Nice. Many times I have added such a \"bash\" or \"gdb\" invocation then\n>> forgotten \"-v\", only to scratch my head at why the test seemed to be\n>> hanging.\n>>\n>> Two minor nits on the patch itself:\n>> ...\n>> 1. It may be worth putting a warning in the comment that this is never\n>>    to be used in a real test, but only temporarily inserted.\n>>\n>> 2. I do this not just with bash, but with \"gdb\". I wonder if it is worth\n>>    making this \"test_foo bash\", for some value of \"foo\" (the ones that\n>>    occur to me are \"debug\" and \"run\", but of course they are taken).\n>>\n>>    Actually, I wonder if the existing test_debug could handle this\n>>    already (though you do have to remember to add \"--debug\" to your\n>>    command line, then).\n> \n> I wondered the same thing from a different angle. My first reaction was\n> \"Why is this called 'bash' not 'sh'?\" which naturally led to the same\n> direction as yours \"why not an arbitrary command 'test_debug xxx'?\"\n> \n> test_pause perhaps?\n\nI really don't care deeply about the name, so test_pause is absolutely\nok for me. I added some documentation in t/README too and made it an\nerror when --verbose is not used.\n\nIs it ok to invoke bash here or should sh be used?\n\n\n t/README      |   13 +++++++++++++\n t/test-lib.sh |   13 +++++++++++++\n 2 files changed, 26 insertions(+), 0 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex c85abaf..c09c582 100644\n--- a/t/README\n+++ b/t/README\n@@ -548,6 +548,19 @@ library for your script to use.\n \t\t...\n \t'\n\n+ - test_pause\n+\n+\tThis command is useful for writing and debugging tests and must be\n+\tremoved before submitting. It halts the execution of the test and\n+\tspawns a shell in the trash directory. Exit the shell to continue\n+\tthe test. Example:\n+\n+\ttest_expect_success 'test' '\n+\t\tgit do-something >actual &&\n+\t\ttest_pause &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n Prerequisites\n -------------\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex a65dfc7..85084c4 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -329,6 +329,19 @@ test_tick () {\n \texport GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n }\n\n+# Stop execution and start a shell. This is useful for debugging tests and\n+# only makes sense together with \"-v\".\n+#\n+# Be sure to remove all invocations of this command before submitting.\n+\n+test_pause () {\n+\tif test \"$verbose\" = t; then\n+\t\tbash <&6 >&3 2>&4\n+\telse\n+\t\terror >&5 \"test_pause requires --verbose\"\n+\tfi\n+}\n+\n # Call test_commit with the arguments \"<message> [<file> [<contents>]]\"\n #\n # This will commit a file with the given contents and the given commit\n-- \n1.7.9.rc1.2.g0b847.dirty\n"},{"id":"182690","messageId":"7vboq2rvhl.fsf@alter.siamese.dyndns.org","threadId":"29371","inReplyTo":"4F152F7C.9020902@web.de","subject":"Re: [PATCH] test-lib: add the test_pause convenience function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-17T19:15:34Z","receivedAt":"2012-01-17T19:15:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> I really don't care deeply about the name, so test_pause is absolutely\n> ok for me. I added some documentation in t/README too and made it an\n> error when --verbose is not used.\n\nI don't care about the name at all, either.\n\nWhat I cared was more about the hardcoded \"bash\". Believe it or not, there\nare boxes that lack it, and there are people who prefer other shells for\ntheir interactive work. At the very least, invoke \"$SHELL_PATH\" instead of\nbash there, perhaps?\n\nIf we wanted to allow an ad-hoc debugging of test scripts to sprinkle\n\"test_pause $cmd\", we might need to do something like:\n\n> +test_pause () {\n> +\tif test \"$verbose\" = t; then\n> +\t\tbash <&6 >&3 2>&4\n\t\t${1-\"$SHELL_PATH\"} <&6 >&3 2>&4\n> +\telse\n> +\t\terror >&5 \"test_pause requires --verbose\"\n> +\tfi\n> +}\n\nbut I do not think that is worth it. The debugging developer should easily\nbe able to run gdb or whatever from the interactive shell you are giving\nhere.\n"},{"id":"182699","messageId":"4F15E25F.2000002@web.de","threadId":"29371","inReplyTo":"7vboq2rvhl.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] test-lib: add the test_pause convenience function","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-01-17T21:04:31Z","receivedAt":"2012-01-17T21:04:31Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Since 781f76b15 (test-lib: redirect stdin of tests) you can't simply put a\n\"bash &&\" into a test for debugging purposes anymore. Instead you'll have\nto use \"bash <&6 >&3 2>&4\".\n\nAs that invocation is not that easy to remember add the test_pause\nconvenience function. It invokes \"$SHELL_PATH\" to provide a sane shell\nfor the user.\n\nThis function also checks if the -v flag is given and will error out if\nthat is not the case instead of letting the test hang until ^D is pressed.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nAm 17.01.2012 20:15, schrieb Junio C Hamano:\n> Jens Lehmann <Jens.Lehmann@web.de> writes:\n> What I cared was more about the hardcoded \"bash\". Believe it or not, there\n> are boxes that lack it, and there are people who prefer other shells for\n> their interactive work. At the very least, invoke \"$SHELL_PATH\" instead of\n> bash there, perhaps?\n\nSure, I changed that in this version and explained it in the commit\nmessage.\n\n> If we wanted to allow an ad-hoc debugging of test scripts to sprinkle\n> \"test_pause $cmd\", we might need to do something like:\n> \n>> +test_pause () {\n>> +\tif test \"$verbose\" = t; then\n>> +\t\tbash <&6 >&3 2>&4\n> \t\t${1-\"$SHELL_PATH\"} <&6 >&3 2>&4\n>> +\telse\n>> +\t\terror >&5 \"test_pause requires --verbose\"\n>> +\tfi\n>> +}\n> \n> but I do not think that is worth it. The debugging developer should easily\n> be able to run gdb or whatever from the interactive shell you are giving\n> here.\n\nThat's what I always do, so I'm fine with what this patch provides. And\nnow the fact that you can temporarily pause a test and explore the trash\ndirectory is documented too ;-)\n\n\n t/README      |   13 +++++++++++++\n t/test-lib.sh |   13 +++++++++++++\n 2 files changed, 26 insertions(+), 0 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex c85abaf..c09c582 100644\n--- a/t/README\n+++ b/t/README\n@@ -548,6 +548,19 @@ library for your script to use.\n \t\t...\n \t'\n\n+ - test_pause\n+\n+\tThis command is useful for writing and debugging tests and must be\n+\tremoved before submitting. It halts the execution of the test and\n+\tspawns a shell in the trash directory. Exit the shell to continue\n+\tthe test. Example:\n+\n+\ttest_expect_success 'test' '\n+\t\tgit do-something >actual &&\n+\t\ttest_pause &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n Prerequisites\n -------------\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex a65dfc7..709a300 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -329,6 +329,19 @@ test_tick () {\n \texport GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n }\n\n+# Stop execution and start a shell. This is useful for debugging tests and\n+# only makes sense together with \"-v\".\n+#\n+# Be sure to remove all invocations of this command before submitting.\n+\n+test_pause () {\n+\tif test \"$verbose\" = t; then\n+\t\t\"$SHELL_PATH\" <&6 >&3 2>&4\n+\telse\n+\t\terror >&5 \"test_pause requires --verbose\"\n+\tfi\n+}\n+\n # Call test_commit with the arguments \"<message> [<file> [<contents>]]\"\n #\n # This will commit a file with the given contents and the given commit\n-- \n1.7.9.rc1.1.g8dae2\n"}]}