{"thread":{"id":"32689","subject":"[RFC] Instruct git-completion.bash that we are in test mode","startedAt":"2013-01-21T22:30:10Z","lastAt":"2013-01-25T22:06:41Z","messageCount":15,"participants":["Jean-Noël AVILA","Junio C Hamano","Jeff King","Jean-Noël Avila","Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"207446","messageId":"201301212330.10824.jn.avila@free.fr","threadId":"32689","inReplyTo":null,"subject":"[RFC] Instruct git-completion.bash that we are in test mode","fromName":"Jean-Noël AVILA","fromEmail":"jn.avila@free.fr","sentAt":"2013-01-21T22:30:10Z","receivedAt":"2013-01-21T22:30:10Z","isPatch":false,"sender":{"key":"jn.avila@free.fr","avatar":"https://avatars.githubusercontent.com/u/156172?v=4"},"body":"In test mode, git completion should only propose core commands.\n\nSigned-off-by: Jean-Noel Avila <jn.avila@free.fr>\n---\n\nI reworked the patch so that the test argument is only evaluated\nwhen sourcing the file and there is no environment clutter.\n\nAt least, \"it works for me\".\n\n contrib/completion/git-completion.bash | 8 +++++++-\n t/t9902-completion.sh                  | 2 +-\n 2 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 14dd5e7..ac9fa65 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -531,10 +531,16 @@ __git_complete_strategy ()\n \treturn 1\n }\n \n+if [ \"x$1\" != \"xTEST\" ]; then\n+\t__git_cmdlist () { git help -a|egrep '^  [a-zA-Z0-9]'; }\n+else\n+\t__git_cmdlist () { git help -a| egrep -m 1 -B1000 PATH | egrep '^  [a-zA-Z0-9]'; }\n+fi\n+\n __git_list_all_commands ()\n {\n \tlocal i IFS=\" \"$'\\n'\n-\tfor i in $(git help -a|egrep '^  [a-zA-Z0-9]')\n+\tfor i in $(__git_cmdlist)\n \tdo\n \t\tcase $i in\n \t\t*--*)             : helper pattern;;\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 3cd53f8..51463b2 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -13,7 +13,7 @@ complete ()\n \treturn 0\n }\n \n-. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\"\n+. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\" TEST\n \n # We don't need this function to actually join words or do anything special.\n # Also, it's cleaner to avoid touching bash's internal completion variables.\n-- \n1.8.1.1.271.g02f55e6\n"},{"id":"207453","messageId":"7v7gn6f6ya.fsf@alter.siamese.dyndns.org","threadId":"32689","inReplyTo":"201301212330.10824.jn.avila@free.fr","subject":"Re: [RFC] Instruct git-completion.bash that we are in test mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-21T23:32:29Z","receivedAt":"2013-01-21T23:32:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jean-Noël AVILA\" <jn.avila@free.fr> writes:\n\n> At least, \"it works for me\".\n\nI suspect that your approach will still not fix the case in which\nyou build a branch with a new command git-check-ignore, and then\ncheck out another branch that does not yet have that command without\nfirst running \"make clean\".\n\nDoes the following really pass with your patch?\n\n\tgit checkout origin/next\n        make\n        git checkout origin/maint\n\tgit apply your_patch.mbox\n        make\n        cd t && sh ./t9902-completion.sh\n\n> +\t__git_cmdlist () { git help -a| egrep -m 1 -B1000 PATH | egrep '^  [a-zA-Z0-9]'; }\n\n'egrep' is not even in POSIX in the first place but grep -E ought to\nbe a replacement for it, so I'll let it pass, but \"-m1 -B1000\"?\nPlease stay within portable options.\n\n    git help -a |\n    sed -n -e '/^  [a-z]/p' -e '/^git commands available from elsewhere/q'/'\n\nmight be a good enough substitute, I think, if we were to take your\napproach, but I suspect it needs a lot more to limit the output in\nthe test mode.\n"},{"id":"207459","messageId":"20130122003954.GA23297@sigill.intra.peff.net","threadId":"32689","inReplyTo":"7v7gn6f6ya.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC] Instruct git-completion.bash that we are in test mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-22T00:39:55Z","receivedAt":"2013-01-22T00:39:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 21, 2013 at 03:32:29PM -0800, Junio C Hamano wrote:\n\n> \"Jean-Noël AVILA\" <jn.avila@free.fr> writes:\n> \n> > At least, \"it works for me\".\n> \n> I suspect that your approach will still not fix the case in which\n> you build a branch with a new command git-check-ignore, and then\n> check out another branch that does not yet have that command without\n> first running \"make clean\".\n> \n> Does the following really pass with your patch?\n> \n> \tgit checkout origin/next\n>         make\n>         git checkout origin/maint\n> \tgit apply your_patch.mbox\n>         make\n>         cd t && sh ./t9902-completion.sh\n\nI really hate to suggest this, but should it be more like:\n\n  if test -z \"$FAKE_COMMAND_LIST\"; then\n          __git_cmdlist() {\n                  git help -a | egrep '^  [a-zA-Z0-9]'\n          }\n  else\n          __git_cmdlist() {\n                  printf '%s' \"$FAKE_COMMAND_LIST\"\n          }\n  fi\n\nThat gives us a nice predictable starting point for actually testing the\ncompletion code. The downside is that it  doesn't let us test that we\nremain compatible with the output of \"help -a\". But we could potentially\nadd a single, more liberal test (without $FAKE_COMMAND_LIST, but ready\nto expect extra output) that checks that.\n\n> > +\t__git_cmdlist () { git help -a| egrep -m 1 -B1000 PATH | egrep '^  [a-zA-Z0-9]'; }\n> \n> 'egrep' is not even in POSIX in the first place but grep -E ought to\n> be a replacement for it, so I'll let it pass, but \"-m1 -B1000\"?\n> Please stay within portable options.\n\nIf I recall correctly, egrep is actually more portable than \"grep -E\"\n(and it is already in use, so I think we are OK). I agree on the rest,\nthough. :)\n\n-Peff\n"},{"id":"207470","messageId":"7vham9dej2.fsf@alter.siamese.dyndns.org","threadId":"32689","inReplyTo":"20130122003954.GA23297@sigill.intra.peff.net","subject":"Re: [RFC] Instruct git-completion.bash that we are in test mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-22T04:31:45Z","receivedAt":"2013-01-22T04:31:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I really hate to suggest this, but should it be more like:\n>\n>   if test -z \"$FAKE_COMMAND_LIST\"; then\n>           __git_cmdlist() {\n>                   git help -a | egrep '^  [a-zA-Z0-9]'\n>           }\n>   else\n>           __git_cmdlist() {\n>                   printf '%s' \"$FAKE_COMMAND_LIST\"\n>           }\n>   fi\n>\n> That gives us a nice predictable starting point for actually testing the\n> completion code. The downside is that it  doesn't let us test that we\n> remain compatible with the output of \"help -a\".\n\nYeah, I think this is simpler and more to the point for the test in\nt9902.  If we really want to test something that is the same as, or\nat least any closer than this approach (or my \"help --standard\"), to\nwhat the real users use, the test has to become inherently flaky, so\nI think we should go for the simplicity of this patch shows.\n"},{"id":"207489","messageId":"50FE47F4.20104@gmail.com","threadId":"32689","inReplyTo":"7vham9dej2.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC] Instruct git-completion.bash that we are in test mode","fromName":"Jean-Noël Avila","fromEmail":"avila.jn@gmail.com","sentAt":"2013-01-22T08:04:04Z","receivedAt":"2013-01-22T08:04:04Z","isPatch":false,"sender":{"key":"jn.avila@free.fr","avatar":"https://avatars.githubusercontent.com/u/156172?v=4"},"body":"Le 22/01/2013 05:31, Junio C Hamano a écrit :\n> Jeff King <peff@peff.net>  writes:\n >\n >> I really hate to suggest this, but should it be more like:\n >>\n >> if test -z \"$FAKE_COMMAND_LIST\"; then __git_cmdlist() { git help -a\n >> | egrep '^ [a-zA-Z0-9]' } else __git_cmdlist() { printf '%s'\n >> \"$FAKE_COMMAND_LIST\" } fi\n >>\n >> That gives us a nice predictable starting point for actually\n >> testing the completion code. The downside is that it doesn't let\n >> us test that we remain compatible with the output of \"help -a\".\n >\n > Yeah, I think this is simpler and more to the point for the test in\n > t9902. If we really want to test something that is the same as, or\n > at least any closer than this approach (or my \"help --standard\"), to\n > what the real users use, the test has to become inherently flaky, so\n > I think we should go for the simplicity of this patch shows.\n\nInstead of imposing the list of command, we could use the command\nlist argument to filter the ouput of git help -a. This would ensure that the\ncompletions we want to test are still present in the installation while\nstill restricting them to the test case.\n"},{"id":"207511","messageId":"7vham9b2n3.fsf@alter.siamese.dyndns.org","threadId":"32689","inReplyTo":"50FE47F4.20104@gmail.com","subject":"Re: [RFC] Instruct git-completion.bash that we are in test mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-22T16:31:28Z","receivedAt":"2013-01-22T16:31:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jean-Noël Avila <avila.jn@gmail.com> writes:\n\n> Le 22/01/2013 05:31, Junio C Hamano a écrit :\n>> Jeff King <peff@peff.net>  writes:\n>>\n>>> I really hate to suggest this, but should it be more like:\n>>>\n>>> if test -z \"$FAKE_COMMAND_LIST\"; then __git_cmdlist() { git help -a\n>>> | egrep '^ [a-zA-Z0-9]' } else __git_cmdlist() { printf '%s'\n>>> \"$FAKE_COMMAND_LIST\" } fi\n>>>\n>>> That gives us a nice predictable starting point for actually\n>>> testing the completion code. The downside is that it doesn't let\n>>> us test that we remain compatible with the output of \"help -a\".\n> ...\n> Instead of imposing the list of command, we could use the command\n> list argument to filter the ouput of git help -a. This would ensure that the\n> completions we want to test are still present in the installation while\n> still restricting them to the test case.\n\nIn order to \"filter the output\", you still need to know how output\nfrom \"git help -a\" looks like, and adjust the code to filter when\nthe shape of the output changes.  The effort to do so is pretty\nsimilar to the amount of effort needed to maintain FAKE_COMMAND_LIST\nto look like the output from \"git help -a\".  It is of dubious value\ncompared to the simplicity of \"printf\" FAKE_COMMAND_LIST approach, I\nthink.\n"},{"id":"207731","messageId":"7vehha89j5.fsf_-_@alter.siamese.dyndns.org","threadId":"32689","inReplyTo":"20130122003954.GA23297@sigill.intra.peff.net","subject":"[PATCH] t9902: protect test from stray build artifacts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-24T23:07:42Z","receivedAt":"2013-01-24T23:07:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When you have random build artifacts in your build directory, left\nbehind by running \"make\" while on another branch, the \"git help -a\"\ncommand run by __git_list_all_commands in the completion script that\nis being tested does not have a way to know that they are not part\nof the subcommands this build will ship.  Such extra subcommands may\ncome from the user's $PATH.  They will interfere with the tests that\nexpect a certain prefix to uniquely expand to a known completion.\n\nInstrument the completion script and give it a way for us to tell\nwhat (subset of) subcommands we are going to ship.\n\nAlso add a test to \"git --help <prefix><TAB>\" expansion.  It needs\nto show not just commands but some selected documentation pages.\n\nBased on an idea by Jeff King.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n contrib/completion/git-completion.bash | 11 ++++++++++-\n t/t9902-completion.sh                  | 25 ++++++++++++++++++++++++-\n 2 files changed, 34 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex a4c48e1..6139b50 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -531,10 +531,19 @@ __git_complete_strategy ()\n \treturn 1\n }\n \n+__git_commands () {\n+\tif test -n \"${GIT_TESTING_COMMAND_COMPLETION:-}\"\n+\tthen\n+\t\tprintf \"%s\" \"${GIT_TESTING_COMMAND_COMPLETION}\"\n+\telse\n+\t\tgit help -a|egrep '^  [a-zA-Z0-9]'\n+\tfi\n+}\n+\n __git_list_all_commands ()\n {\n \tlocal i IFS=\" \"$'\\n'\n-\tfor i in $(git help -a|egrep '^  [a-zA-Z0-9]')\n+\tfor i in $(__git_commands)\n \tdo\n \t\tcase $i in\n \t\t*--*)             : helper pattern;;\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 3cd53f8..adc1372 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -13,6 +13,25 @@ complete ()\n \treturn 0\n }\n \n+# Be careful when updating this list:\n+#\n+# (1) The build tree may have build artifact from different branch, or\n+#     the user's $PATH may have a random executable that may begin\n+#     with \"git-check\" that are not part of the subcommands this build\n+#     will ship, e.g.  \"check-ignore\".  The tests for completion for\n+#     subcommand names tests how \"check\" is expanded; we limit the\n+#     possible candidates to \"checkout\" and \"check-attr\" to make sure\n+#     \"check-attr\", which is known by the filter function as a\n+#     subcommand to be thrown out, while excluding other random files\n+#     that happen to begin with \"check\" to avoid letting them get in\n+#     the way.\n+#\n+# (2) A test makes sure that common subcommands are included in the\n+#     completion for \"git <TAB>\", and a plumbing is excluded.  \"add\",\n+#     \"filter-branch\" and \"ls-files\" are listed for this.\n+\n+GIT_TESTING_COMMAND_COMPLETION='add checkout check-attr filter-branch ls-files'\n+\n . \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\"\n \n # We don't need this function to actually join words or do anything special.\n@@ -196,7 +215,6 @@ test_expect_success 'general options plus command' '\n \ttest_completion \"git --paginate check\" \"checkout \" &&\n \ttest_completion \"git --git-dir=foo check\" \"checkout \" &&\n \ttest_completion \"git --bare check\" \"checkout \" &&\n-\ttest_completion \"git --help des\" \"describe \" &&\n \ttest_completion \"git --exec-path=foo check\" \"checkout \" &&\n \ttest_completion \"git --html-path check\" \"checkout \" &&\n \ttest_completion \"git --no-pager check\" \"checkout \" &&\n@@ -207,6 +225,11 @@ test_expect_success 'general options plus command' '\n \ttest_completion \"git --no-replace-objects check\" \"checkout \"\n '\n \n+test_expect_success 'git --help completion' '\n+\ttest_completion \"git --help ad\" \"add \" &&\n+\ttest_completion \"git --help core\" \"core-tutorial \"\n+'\n+\n test_expect_success 'setup for ref completion' '\n \techo content >file1 &&\n \techo more >file2 &&\n"},{"id":"207738","messageId":"20130125011349.GB27657@sigill.intra.peff.net","threadId":"32689","inReplyTo":"7vehha89j5.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH] t9902: protect test from stray build artifacts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-25T01:13:49Z","receivedAt":"2013-01-25T01:13:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 24, 2013 at 03:07:42PM -0800, Junio C Hamano wrote:\n\n> When you have random build artifacts in your build directory, left\n> behind by running \"make\" while on another branch, the \"git help -a\"\n> command run by __git_list_all_commands in the completion script that\n> is being tested does not have a way to know that they are not part\n> of the subcommands this build will ship.  Such extra subcommands may\n> come from the user's $PATH.  They will interfere with the tests that\n> expect a certain prefix to uniquely expand to a known completion.\n> \n> Instrument the completion script and give it a way for us to tell\n> what (subset of) subcommands we are going to ship.\n> \n> Also add a test to \"git --help <prefix><TAB>\" expansion.  It needs\n> to show not just commands but some selected documentation pages.\n> \n> Based on an idea by Jeff King.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  contrib/completion/git-completion.bash | 11 ++++++++++-\n>  t/t9902-completion.sh                  | 25 ++++++++++++++++++++++++-\n>  2 files changed, 34 insertions(+), 2 deletions(-)\n\nThis looks good to me.\n\nThe only thing I might add is a test just to double-check that \"git help\n-a\" is parsed correctly. Like:\n\n  test_expect_success 'command completion works without test harness' '\n           GIT_TESTING_COMMAND_COMPLETION= run_completion \"git bun\" &&\n           grep \"^bundle\\$\" out\n  '\n\n(we know we are running bash here, so the one-shot variable is OK to be\nused with a function).\n\n-Peff\n"},{"id":"207743","messageId":"20130125025910.GB26524@elie.Belkin","threadId":"32689","inReplyTo":"20130125011349.GB27657@sigill.intra.peff.net","subject":"Re: [PATCH] t9902: protect test from stray build artifacts","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-01-25T02:59:10Z","receivedAt":"2013-01-25T02:59:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n> On Thu, Jan 24, 2013 at 03:07:42PM -0800, Junio C Hamano wrote[1]:\n\n>> Instrument the completion script and give it a way for us to tell\n>> what (subset of) subcommands we are going to ship.\n[...]\n> The only thing I might add is a test just to double-check that \"git help\n> -a\" is parsed correctly. Like:\n>\n>   test_expect_success 'command completion works without test harness' '\n>            GIT_TESTING_COMMAND_COMPLETION= run_completion \"git bun\" &&\n>            grep \"^bundle\\$\" out\n>   '\n\nYes.  Since there are no other 'git help -a' tests, I think we need\nthis.\n\nAside from that, the fix looks good to me.\n\nJonathan\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/214167/focus=214469\n"},{"id":"207749","messageId":"7vvcal7vhg.fsf@alter.siamese.dyndns.org","threadId":"32689","inReplyTo":"20130125011349.GB27657@sigill.intra.peff.net","subject":"Re: [PATCH] t9902: protect test from stray build artifacts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-25T04:11:07Z","receivedAt":"2013-01-25T04:11:07Z","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> This looks good to me.\n>\n> The only thing I might add is a test just to double-check that \"git help\n> -a\" is parsed correctly. Like:\n>\n>   test_expect_success 'command completion works without test harness' '\n>            GIT_TESTING_COMMAND_COMPLETION= run_completion \"git bun\" &&\n>            grep \"^bundle\\$\" out\n>   '\n>\n> (we know we are running bash here, so the one-shot variable is OK to be\n> used with a function).\n\nI think you meant \"^bundle $\" there, but don't we have the same\nproblem when there is an end-user subcommand \"git bunny\"?\n\nAhh, ok, we show one element per line and just make sure \"bundle\"\nis there, and we do not care what other buns appear in the output.\n\nWill squash in, then.\n"},{"id":"207750","messageId":"20130125041326.GA30928@sigill.intra.peff.net","threadId":"32689","inReplyTo":"7vvcal7vhg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t9902: protect test from stray build artifacts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-25T04:13:26Z","receivedAt":"2013-01-25T04:13:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 24, 2013 at 08:11:07PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > This looks good to me.\n> >\n> > The only thing I might add is a test just to double-check that \"git help\n> > -a\" is parsed correctly. Like:\n> >\n> >   test_expect_success 'command completion works without test harness' '\n> >            GIT_TESTING_COMMAND_COMPLETION= run_completion \"git bun\" &&\n> >            grep \"^bundle\\$\" out\n> >   '\n> >\n> > (we know we are running bash here, so the one-shot variable is OK to be\n> > used with a function).\n> \n> I think you meant \"^bundle $\" there, but don't we have the same\n> problem when there is an end-user subcommand \"git bunny\"?\n> \n> Ahh, ok, we show one element per line and just make sure \"bundle\"\n> is there, and we do not care what other buns appear in the output.\n\nExactly. At least that was the intent; I typed it straight into my MUA. :)\n\n-Peff\n"},{"id":"207751","messageId":"7vr4l97v3h.fsf@alter.siamese.dyndns.org","threadId":"32689","inReplyTo":"7vvcal7vhg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t9902: protect test from stray build artifacts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-25T04:19:30Z","receivedAt":"2013-01-25T04:19:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> This looks good to me.\n>>\n>> The only thing I might add is a test just to double-check that \"git help\n>> -a\" is parsed correctly. Like:\n>>\n>>   test_expect_success 'command completion works without test harness' '\n>>            GIT_TESTING_COMMAND_COMPLETION= run_completion \"git bun\" &&\n>>            grep \"^bundle\\$\" out\n>>   '\n>>\n>> (we know we are running bash here, so the one-shot variable is OK to be\n>> used with a function).\n>\n> I think you meant \"^bundle $\" there, but don't we have the same\n> problem when there is an end-user subcommand \"git bunny\"?\n>\n> Ahh, ok, we show one element per line and just make sure \"bundle\"\n> is there, and we do not care what other buns appear in the output.\n>\n> Will squash in, then.\n\nNot so quick, though.  The lower level \"read from help -a\" is only\nrun once and its output kept in a two-level cache hierarchy; we need\nto reset both.\n\nIt starts to look a bit too intimately tied to the implementation of\nwhat is being tested for my taste, though.\n\n t/t9902-completion.sh | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex adc1372..bb6ee1a 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -286,4 +286,14 @@ test_expect_success 'send-email' '\n \ttest_completion \"git send-email ma\" \"master \"\n '\n \n+# This is better to be at the end, as it resets the state by tweaking\n+# the internals.\n+test_expect_success 'help -a read correctly by command list generator' '\n+\t__git_all_commands= &&\n+\t__git_porcelain_commands= &&\n+\tGIT_TESTING_COMMAND_COMPLETION= &&\n+\trun_completion \"git bun\" &&\n+\tgrep \"^bundle $\" out\n+'\n+\n test_done\n-- \n1.8.1.1.525.gdace530\n"},{"id":"207753","messageId":"20130125042326.GA31281@sigill.intra.peff.net","threadId":"32689","inReplyTo":"7vr4l97v3h.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t9902: protect test from stray build artifacts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-25T04:23:26Z","receivedAt":"2013-01-25T04:23:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 24, 2013 at 08:19:30PM -0800, Junio C Hamano wrote:\n\n> > Ahh, ok, we show one element per line and just make sure \"bundle\"\n> > is there, and we do not care what other buns appear in the output.\n> >\n> Not so quick, though.  The lower level \"read from help -a\" is only\n> run once and its output kept in a two-level cache hierarchy; we need\n> to reset both.\n\nUgh, I didn't even think about that.\n\nI wonder if it would be simpler if the completion tests actually ran a\nnew bash for each test. That would be slower, but it somehow seems\ncleaner.\n\n> It starts to look a bit too intimately tied to the implementation of\n> what is being tested for my taste, though.\n> [...]\n> +test_expect_success 'help -a read correctly by command list generator' '\n> +\t__git_all_commands= &&\n> +\t__git_porcelain_commands= &&\n> +\tGIT_TESTING_COMMAND_COMPLETION= &&\n> +\trun_completion \"git bun\" &&\n> +\tgrep \"^bundle $\" out\n> +'\n\nAgreed. I could take or leave it at this point. It's nice to check that\nchanges to \"help -a\" will not break it, but ultimately it feels a bit\ntoo contrived to catch anything useful.\n\n-Peff\n"},{"id":"207822","messageId":"7v1ud95cxb.fsf@alter.siamese.dyndns.org","threadId":"32689","inReplyTo":"20130125042326.GA31281@sigill.intra.peff.net","subject":"Re: [PATCH] t9902: protect test from stray build artifacts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-25T18:34:56Z","receivedAt":"2013-01-25T18:34:56Z","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> On Thu, Jan 24, 2013 at 08:19:30PM -0800, Junio C Hamano wrote:\n>\n>> > Ahh, ok, we show one element per line and just make sure \"bundle\"\n>> > is there, and we do not care what other buns appear in the output.\n>> >\n>> Not so quick, though.  The lower level \"read from help -a\" is only\n>> run once and its output kept in a two-level cache hierarchy; we need\n>> to reset both.\n>\n> Ugh, I didn't even think about that.\n>\n> I wonder if it would be simpler if the completion tests actually ran a\n> new bash for each test. That would be slower, but it somehow seems\n> cleaner.\n\nI agree 100% with that.  Let's leave this fix as-is, at least as a\ntentative fix while \"git check-ignore\" graduates into the upcoming\nrelease, and let somebody who is interested work on an update to\nthis test script to do so as an independent topic.\n\n\n>\n>> It starts to look a bit too intimately tied to the implementation of\n>> what is being tested for my taste, though.\n>> [...]\n>> +test_expect_success 'help -a read correctly by command list generator' '\n>> +\t__git_all_commands= &&\n>> +\t__git_porcelain_commands= &&\n>> +\tGIT_TESTING_COMMAND_COMPLETION= &&\n>> +\trun_completion \"git bun\" &&\n>> +\tgrep \"^bundle $\" out\n>> +'\n>\n> Agreed. I could take or leave it at this point. It's nice to check that\n> changes to \"help -a\" will not break it, but ultimately it feels a bit\n> too contrived to catch anything useful.\n>\n> -Peff\n"},{"id":"207846","messageId":"20130125220641.GB23626@sigill.intra.peff.net","threadId":"32689","inReplyTo":"7v1ud95cxb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t9902: protect test from stray build artifacts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-25T22:06:41Z","receivedAt":"2013-01-25T22:06:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 25, 2013 at 10:34:56AM -0800, Junio C Hamano wrote:\n\n> >> Not so quick, though.  The lower level \"read from help -a\" is only\n> >> run once and its output kept in a two-level cache hierarchy; we need\n> >> to reset both.\n> >\n> > Ugh, I didn't even think about that.\n> >\n> > I wonder if it would be simpler if the completion tests actually ran a\n> > new bash for each test. That would be slower, but it somehow seems\n> > cleaner.\n> \n> I agree 100% with that.  Let's leave this fix as-is, at least as a\n> tentative fix while \"git check-ignore\" graduates into the upcoming\n> release, and let somebody who is interested work on an update to\n> this test script to do so as an independent topic.\n\nSounds good to me. Thanks.\n\n-Peff\n"}]}