Re: [PATCH] t9902: protect test from stray build artifacts
Jeff King p...@peff.net writes: On Thu, Jan 24, 2013 at 08:19:30PM -0800, Junio C Hamano wrote: Ahh, ok, we show one element per line and just make sure bundle is there, and we do not care what other buns appear in the output. Not so quick, though. The lower level read from help -a is only run once and its output kept in a two-level cache hierarchy; we need to reset both. Ugh, I didn't even think about that. I wonder if it would be simpler if the completion tests actually ran a new bash for each test. That would be slower, but it somehow seems cleaner. I agree 100% with that. Let's leave this fix as-is, at least as a tentative fix while git check-ignore graduates into the upcoming release, and let somebody who is interested work on an update to this test script to do so as an independent topic. It starts to look a bit too intimately tied to the implementation of what is being tested for my taste, though. [...] +test_expect_success 'help -a read correctly by command list generator' ' +__git_all_commands= +__git_porcelain_commands= +GIT_TESTING_COMMAND_COMPLETION= +run_completion git bun +grep ^bundle $ out +' Agreed. I could take or leave it at this point. It's nice to check that changes to help -a will not break it, but ultimately it feels a bit too contrived to catch anything useful. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t9902: protect test from stray build artifacts
On Thu, Jan 24, 2013 at 03:07:42PM -0800, Junio C Hamano wrote: When you have random build artifacts in your build directory, left behind by running make while on another branch, the git help -a command run by __git_list_all_commands in the completion script that is being tested does not have a way to know that they are not part of the subcommands this build will ship. Such extra subcommands may come from the user's $PATH. They will interfere with the tests that expect a certain prefix to uniquely expand to a known completion. Instrument the completion script and give it a way for us to tell what (subset of) subcommands we are going to ship. Also add a test to git --help prefixTAB expansion. It needs to show not just commands but some selected documentation pages. Based on an idea by Jeff King. Signed-off-by: Junio C Hamano gits...@pobox.com --- contrib/completion/git-completion.bash | 11 ++- t/t9902-completion.sh | 25 - 2 files changed, 34 insertions(+), 2 deletions(-) This looks good to me. The only thing I might add is a test just to double-check that git help -a is parsed correctly. Like: test_expect_success 'command completion works without test harness' ' GIT_TESTING_COMMAND_COMPLETION= run_completion git bun grep ^bundle\$ out ' (we know we are running bash here, so the one-shot variable is OK to be used with a function). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t9902: protect test from stray build artifacts
Jeff King wrote: On Thu, Jan 24, 2013 at 03:07:42PM -0800, Junio C Hamano wrote[1]: Instrument the completion script and give it a way for us to tell what (subset of) subcommands we are going to ship. [...] The only thing I might add is a test just to double-check that git help -a is parsed correctly. Like: test_expect_success 'command completion works without test harness' ' GIT_TESTING_COMMAND_COMPLETION= run_completion git bun grep ^bundle\$ out ' Yes. Since there are no other 'git help -a' tests, I think we need this. Aside from that, the fix looks good to me. Jonathan [1] http://thread.gmane.org/gmane.comp.version-control.git/214167/focus=214469 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t9902: protect test from stray build artifacts
Jeff King p...@peff.net writes: This looks good to me. The only thing I might add is a test just to double-check that git help -a is parsed correctly. Like: test_expect_success 'command completion works without test harness' ' GIT_TESTING_COMMAND_COMPLETION= run_completion git bun grep ^bundle\$ out ' (we know we are running bash here, so the one-shot variable is OK to be used with a function). I think you meant ^bundle $ there, but don't we have the same problem when there is an end-user subcommand git bunny? Ahh, ok, we show one element per line and just make sure bundle is there, and we do not care what other buns appear in the output. Will squash in, then. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t9902: protect test from stray build artifacts
On Thu, Jan 24, 2013 at 08:11:07PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: This looks good to me. The only thing I might add is a test just to double-check that git help -a is parsed correctly. Like: test_expect_success 'command completion works without test harness' ' GIT_TESTING_COMMAND_COMPLETION= run_completion git bun grep ^bundle\$ out ' (we know we are running bash here, so the one-shot variable is OK to be used with a function). I think you meant ^bundle $ there, but don't we have the same problem when there is an end-user subcommand git bunny? Ahh, ok, we show one element per line and just make sure bundle is there, and we do not care what other buns appear in the output. Exactly. At least that was the intent; I typed it straight into my MUA. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t9902: protect test from stray build artifacts
Junio C Hamano gits...@pobox.com writes: Jeff King p...@peff.net writes: This looks good to me. The only thing I might add is a test just to double-check that git help -a is parsed correctly. Like: test_expect_success 'command completion works without test harness' ' GIT_TESTING_COMMAND_COMPLETION= run_completion git bun grep ^bundle\$ out ' (we know we are running bash here, so the one-shot variable is OK to be used with a function). I think you meant ^bundle $ there, but don't we have the same problem when there is an end-user subcommand git bunny? Ahh, ok, we show one element per line and just make sure bundle is there, and we do not care what other buns appear in the output. Will squash in, then. Not so quick, though. The lower level read from help -a is only run once and its output kept in a two-level cache hierarchy; we need to reset both. It starts to look a bit too intimately tied to the implementation of what is being tested for my taste, though. t/t9902-completion.sh | 10 ++ 1 file changed, 10 insertions(+) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index adc1372..bb6ee1a 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -286,4 +286,14 @@ test_expect_success 'send-email' ' test_completion git send-email ma master ' +# This is better to be at the end, as it resets the state by tweaking +# the internals. +test_expect_success 'help -a read correctly by command list generator' ' + __git_all_commands= + __git_porcelain_commands= + GIT_TESTING_COMMAND_COMPLETION= + run_completion git bun + grep ^bundle $ out +' + test_done -- 1.8.1.1.525.gdace530 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t9902: protect test from stray build artifacts
On Thu, Jan 24, 2013 at 08:19:30PM -0800, Junio C Hamano wrote: Ahh, ok, we show one element per line and just make sure bundle is there, and we do not care what other buns appear in the output. Not so quick, though. The lower level read from help -a is only run once and its output kept in a two-level cache hierarchy; we need to reset both. Ugh, I didn't even think about that. I wonder if it would be simpler if the completion tests actually ran a new bash for each test. That would be slower, but it somehow seems cleaner. It starts to look a bit too intimately tied to the implementation of what is being tested for my taste, though. [...] +test_expect_success 'help -a read correctly by command list generator' ' + __git_all_commands= + __git_porcelain_commands= + GIT_TESTING_COMMAND_COMPLETION= + run_completion git bun + grep ^bundle $ out +' Agreed. I could take or leave it at this point. It's nice to check that changes to help -a will not break it, but ultimately it feels a bit too contrived to catch anything useful. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
