{"thread":{"id":"31893","subject":"[PATCH 0/2] completion: test simplifications","startedAt":"2012-10-22T01:38:59Z","lastAt":"2012-10-30T23:02:01Z","messageCount":7,"participants":["Felipe Contreras","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"201673","messageId":"1350869941-22485-1-git-send-email-felipe.contreras@gmail.com","threadId":"31893","inReplyTo":null,"subject":"[PATCH 0/2] completion: test simplifications","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-22T01:38:59Z","receivedAt":"2012-10-22T01:38:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"As the subject says. These came up in a discussion with SZEDER.\n\nFelipe Contreras (2):\n  completion: refactor __gitcomp related tests\n  completion: simplify __gitcomp test helper\n\n t/t9902-completion.sh | 71 +++++++++++++++------------------------------------\n 1 file changed, 21 insertions(+), 50 deletions(-)\n\n-- \n1.8.0\n"},{"id":"201675","messageId":"1350869941-22485-2-git-send-email-felipe.contreras@gmail.com","threadId":"31893","inReplyTo":"1350869941-22485-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/2] completion: refactor __gitcomp related tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-22T01:39:00Z","receivedAt":"2012-10-22T01:39:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Lots of duplicated code!\n\nNo functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 72 ++++++++++++++++-----------------------------------\n 1 file changed, 23 insertions(+), 49 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex cbd0fb6..1c6952a 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -72,87 +72,61 @@ test_completion_long ()\n \n newline=$'\\n'\n \n-test_expect_success '__gitcomp - trailing space - options' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n-\t--reuse-message=Z\n-\t--reedit-message=Z\n-\t--reset-author Z\n-\tEOF\n+test_gitcomp ()\n+{\n+\tsed -e 's/Z$//' > expected &&\n \t(\n \t\tlocal -a COMPREPLY &&\n-\t\tcur=\"--re\" &&\n-\t\t__gitcomp \"--dry-run --reuse-message= --reedit-message=\n-\t\t\t\t--reset-author\" &&\n+\t\tcur=\"$1\" &&\n+\t\tshift &&\n+\t\t__gitcomp \"$@\" &&\n \t\tIFS=\"$newline\" &&\n \t\techo \"${COMPREPLY[*]}\" > out\n \t) &&\n \ttest_cmp expected out\n+}\n+\n+test_expect_success '__gitcomp - trailing space - options' '\n+\ttest_gitcomp \"--re\" \"--dry-run --reuse-message= --reedit-message=\n+\t\t--reset-author\" <<-EOF\n+\t--reuse-message=Z\n+\t--reedit-message=Z\n+\t--reset-author Z\n+\tEOF\n '\n \n test_expect_success '__gitcomp - trailing space - config keys' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n+\ttest_gitcomp \"br\" \"branch. branch.autosetupmerge\n+\t\tbranch.autosetuprebase browser.\" <<-\\EOF\n \tbranch.Z\n \tbranch.autosetupmerge Z\n \tbranch.autosetuprebase Z\n \tbrowser.Z\n \tEOF\n-\t(\n-\t\tlocal -a COMPREPLY &&\n-\t\tcur=\"br\" &&\n-\t\t__gitcomp \"branch. branch.autosetupmerge\n-\t\t\t\tbranch.autosetuprebase browser.\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n-\t) &&\n-\ttest_cmp expected out\n '\n \n test_expect_success '__gitcomp - option parameter' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n+\ttest_gitcomp \"--strategy=re\" \"octopus ours recursive resolve subtree\" \\\n+\t\t\"\" \"re\" <<-\\EOF\n \trecursive Z\n \tresolve Z\n \tEOF\n-\t(\n-\t\tlocal -a COMPREPLY &&\n-\t\tcur=\"--strategy=re\" &&\n-\t\t__gitcomp \"octopus ours recursive resolve subtree\n-\t\t\t\" \"\" \"re\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n-\t) &&\n-\ttest_cmp expected out\n '\n \n test_expect_success '__gitcomp - prefix' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n+\ttest_gitcomp \"branch.me\" \"remote merge mergeoptions rebase\" \\\n+\t\t\"branch.maint.\" \"me\" <<-\\EOF\n \tbranch.maint.merge Z\n \tbranch.maint.mergeoptions Z\n \tEOF\n-\t(\n-\t\tlocal -a COMPREPLY &&\n-\t\tcur=\"branch.me\" &&\n-\t\t__gitcomp \"remote merge mergeoptions rebase\n-\t\t\t\" \"branch.maint.\" \"me\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n-\t) &&\n-\ttest_cmp expected out\n '\n \n test_expect_success '__gitcomp - suffix' '\n-\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n+\ttest_gitcomp \"branch.me\" \"master maint next pu\" \"branch.\" \\\n+\t\t\"ma\" \".\" <<-\\EOF\n \tbranch.master.Z\n \tbranch.maint.Z\n \tEOF\n-\t(\n-\t\tlocal -a COMPREPLY &&\n-\t\tcur=\"branch.me\" &&\n-\t\t__gitcomp \"master maint next pu\n-\t\t\t\" \"branch.\" \"ma\" \".\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n-\t) &&\n-\ttest_cmp expected out\n '\n \n test_expect_success 'basic' '\n-- \n1.8.0\n"},{"id":"201674","messageId":"1350869941-22485-3-git-send-email-felipe.contreras@gmail.com","threadId":"31893","inReplyTo":"1350869941-22485-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/2] completion: simplify __gitcomp test helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-22T01:39:01Z","receivedAt":"2012-10-22T01:39:01Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"By using print_comp as suggested by SZEDER Gábor.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 13 +++++--------\n 1 file changed, 5 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 1c6952a..2e7fc06 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -74,15 +74,12 @@ newline=$'\\n'\n \n test_gitcomp ()\n {\n+\tlocal -a COMPREPLY &&\n \tsed -e 's/Z$//' > expected &&\n-\t(\n-\t\tlocal -a COMPREPLY &&\n-\t\tcur=\"$1\" &&\n-\t\tshift &&\n-\t\t__gitcomp \"$@\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n-\t) &&\n+\tcur=\"$1\" &&\n+\tshift &&\n+\t__gitcomp \"$@\" &&\n+\tprint_comp &&\n \ttest_cmp expected out\n }\n \n-- \n1.8.0\n"},{"id":"202241","messageId":"20121030212725.GA15709@goldbirke","threadId":"31893","inReplyTo":"1350869941-22485-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 2/2] completion: simplify __gitcomp test helper","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-10-30T21:27:25Z","receivedAt":"2012-10-30T21:27:25Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Oct 22, 2012 at 03:39:01AM +0200, Felipe Contreras wrote:\n> By using print_comp as suggested by SZEDER Gábor.\n> \n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  t/t9902-completion.sh | 13 +++++--------\n>  1 file changed, 5 insertions(+), 8 deletions(-)\n> \n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index 1c6952a..2e7fc06 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -74,15 +74,12 @@ newline=$'\\n'\n\nThis $newline variable was only used to set IFS to a newline inside SQ\nblocks.  AFAICS after this change there are no such places left,\nbecause print_comp() takes care of IFS, so $newline is not necessary\nanymore.\n\n>  test_gitcomp ()\n>  {\n> +\tlocal -a COMPREPLY &&\n>  \tsed -e 's/Z$//' > expected &&\n> -\t(\n> -\t\tlocal -a COMPREPLY &&\n> -\t\tcur=\"$1\" &&\n> -\t\tshift &&\n> -\t\t__gitcomp \"$@\" &&\n> -\t\tIFS=\"$newline\" &&\n> -\t\techo \"${COMPREPLY[*]}\" > out\n> -\t) &&\n> +\tcur=\"$1\" &&\n> +\tshift &&\n> +\t__gitcomp \"$@\" &&\n> +\tprint_comp &&\n>  \ttest_cmp expected out\n>  }\n>  \n> -- \n> 1.8.0\n> \n"},{"id":"202248","messageId":"CAMP44s34yBSurkiCb7bvJFt82_F=hdkFLDyd0o=9Yrsn-TRWqg@mail.gmail.com","threadId":"31893","inReplyTo":"20121030212725.GA15709@goldbirke","subject":"Re: [PATCH 2/2] completion: simplify __gitcomp test helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-30T21:43:10Z","receivedAt":"2012-10-30T21:43:10Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Oct 30, 2012 at 10:27 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> On Mon, Oct 22, 2012 at 03:39:01AM +0200, Felipe Contreras wrote:\n>> By using print_comp as suggested by SZEDER Gábor.\n>>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  t/t9902-completion.sh | 13 +++++--------\n>>  1 file changed, 5 insertions(+), 8 deletions(-)\n>>\n>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n>> index 1c6952a..2e7fc06 100755\n>> --- a/t/t9902-completion.sh\n>> +++ b/t/t9902-completion.sh\n>> @@ -74,15 +74,12 @@ newline=$'\\n'\n>\n> This $newline variable was only used to set IFS to a newline inside SQ\n> blocks.  AFAICS after this change there are no such places left,\n> because print_comp() takes care of IFS, so $newline is not necessary\n> anymore.\n\nRight, I thought I did that =/\n\n-- \nFelipe Contreras\n"},{"id":"202262","messageId":"20121030224558.GN12052@goldbirke","threadId":"31893","inReplyTo":"1350869941-22485-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 1/2] completion: refactor __gitcomp related tests","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-10-30T22:45:58Z","receivedAt":"2012-10-30T22:45:58Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Oct 22, 2012 at 03:39:00AM +0200, Felipe Contreras wrote:\n> Lots of duplicated code!\n> \n> No functional changes.\n\nI'm not sure.\nI'm all for removing duplicated application code, but I'm usually more\nconservative when it comes to test code.  The more logic, the more\npossibility for bugs in tests.  So tests should be dead simple, even\nif that means some duplicated test code or the lack of convenience\nfunctions.\nWhile this might be considered just a matter of personal preference, ...\n\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  t/t9902-completion.sh | 72 ++++++++++++++++-----------------------------------\n>  1 file changed, 23 insertions(+), 49 deletions(-)\n> \n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index cbd0fb6..1c6952a 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -72,87 +72,61 @@ test_completion_long ()\n>  \n>  newline=$'\\n'\n>  \n> -test_expect_success '__gitcomp - trailing space - options' '\n> -\tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n> -\t--reuse-message=Z\n> -\t--reedit-message=Z\n> -\t--reset-author Z\n> -\tEOF\n> +test_gitcomp ()\n> +{\n> +\tsed -e 's/Z$//' > expected &&\n>  \t(\n>  \t\tlocal -a COMPREPLY &&\n> -\t\tcur=\"--re\" &&\n> -\t\t__gitcomp \"--dry-run --reuse-message= --reedit-message=\n> -\t\t\t\t--reset-author\" &&\n> +\t\tcur=\"$1\" &&\n> +\t\tshift &&\n> +\t\t__gitcomp \"$@\" &&\n\n... I was really puzzled by how __gitcomp() gets its arguments here,\nand had to think for a while to figure out why it's not broken.\n"},{"id":"202266","messageId":"CAMP44s0aN_FU+51huzdtEKOMaw2yhVy4d=jV6YHMeoCKDeqU8Q@mail.gmail.com","threadId":"31893","inReplyTo":"20121030224558.GN12052@goldbirke","subject":"Re: [PATCH 1/2] completion: refactor __gitcomp related tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-30T23:02:01Z","receivedAt":"2012-10-30T23:02:01Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Oct 30, 2012 at 11:45 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> On Mon, Oct 22, 2012 at 03:39:00AM +0200, Felipe Contreras wrote:\n>> Lots of duplicated code!\n>>\n>> No functional changes.\n>\n> I'm not sure.\n> I'm all for removing duplicated application code, but I'm usually more\n> conservative when it comes to test code.  The more logic, the more\n> possibility for bugs in tests.  So tests should be dead simple, even\n> if that means some duplicated test code or the lack of convenience\n> functions.\n> While this might be considered just a matter of personal preference, ...\n\nFair enough, but my preference is different: test code should be the\nsame as normal code; easy to maintain.\n\n>> +test_gitcomp ()\n>> +{\n>> +     sed -e 's/Z$//' > expected &&\n>>       (\n>>               local -a COMPREPLY &&\n>> -             cur=\"--re\" &&\n>> -             __gitcomp \"--dry-run --reuse-message= --reedit-message=\n>> -                             --reset-author\" &&\n>> +             cur=\"$1\" &&\n>> +             shift &&\n>> +             __gitcomp \"$@\" &&\n>\n> ... I was really puzzled by how __gitcomp() gets its arguments here,\n> and had to think for a while to figure out why it's not broken.\n\nYou mean because \"$@\" is treated in a special way? Without that\nbehavior I'm not sure many things would be possible :)\n\nAnyway, I don't think we should make our life more difficult while\nmaintaining test code... that's my opinion.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}