Re: [PATCH v2 5/6] completion: refactor __gitcomp related tests
Not asking for a re-roll but am asking for clarification so that I
can locally update before queuing.
Felipe Contreras felipe.contre...@gmail.com writes:
Lots of duplicated code!
... removed, you mean?
No functional changes.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t9902-completion.sh | 76
++-
1 file changed, 27 insertions(+), 49 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 59cdbfd..66c7af6 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -71,87 +71,65 @@ test_completion ()
newline=$'\n'
-test_expect_success '__gitcomp - trailing space - options' '
- sed -e s/Z$// expected -\EOF
- --reuse-message=Z
- --reedit-message=Z
- --reset-author Z
- EOF
+# Test __gitcomp.
+# Arguments are:
+# 1: typed text so far (cur)
+# *: arguments to pass to __gitcomp
s/\*/remainder/, perhaps? I think you shift $1 out and do not pass
it to __gitcomp.
And expected output is from the standard input just like
test_completion?
+test_gitcomp ()
+{
+ sed -e 's/Z$//' expected
(
local -a COMPREPLY
- cur=--re
- __gitcomp --dry-run --reuse-message= --reedit-message=
- --reset-author
+ cur=$1
+ shift
+ __gitcomp $@
IFS=$newline
echo ${COMPREPLY[*]} out
)
test_cmp expected out
+}
+
+test_expect_success '__gitcomp - trailing space - options' '
+ test_gitcomp --re --dry-run --reuse-message= --reedit-message=
+ --reset-author -EOF
+ --reuse-message=Z
+ --reedit-message=Z
+ --reset-author Z
+ EOF
'
Nice shrinkage.
--
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 v2 5/6] completion: refactor __gitcomp related tests
On Fri, Nov 16, 2012 at 8:13 PM, Junio C Hamano gits...@pobox.com wrote:
Not asking for a re-roll but am asking for clarification so that I
can locally update before queuing.
Felipe Contreras felipe.contre...@gmail.com writes:
Lots of duplicated code!
... removed, you mean?
Yes.
No functional changes.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t9902-completion.sh | 76
++-
1 file changed, 27 insertions(+), 49 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 59cdbfd..66c7af6 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -71,87 +71,65 @@ test_completion ()
newline=$'\n'
-test_expect_success '__gitcomp - trailing space - options' '
- sed -e s/Z$// expected -\EOF
- --reuse-message=Z
- --reedit-message=Z
- --reset-author Z
- EOF
+# Test __gitcomp.
+# Arguments are:
+# 1: typed text so far (cur)
+# *: arguments to pass to __gitcomp
s/\*/remainder/, perhaps? I think you shift $1 out and do not pass
it to __gitcomp.
Right, by * I meant the rest.
And expected output is from the standard input just like
test_completion?
Correct.
+test_gitcomp ()
+{
+ sed -e 's/Z$//' expected
(
local -a COMPREPLY
- cur=--re
- __gitcomp --dry-run --reuse-message= --reedit-message=
- --reset-author
+ cur=$1
+ shift
+ __gitcomp $@
IFS=$newline
echo ${COMPREPLY[*]} out
)
test_cmp expected out
+}
+
+test_expect_success '__gitcomp - trailing space - options' '
+ test_gitcomp --re --dry-run --reuse-message= --reedit-message=
+ --reset-author -EOF
+ --reuse-message=Z
+ --reedit-message=Z
+ --reset-author Z
+ EOF
'
Nice shrinkage.
That's a comment about the whole patch series I hope :)
Cheers.
--
Felipe Contreras
--
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 v2 5/6] completion: refactor __gitcomp related tests
On Sun, Nov 11, 2012 at 03:35:57PM +0100, Felipe Contreras wrote:
Lots of duplicated code!
No functional changes.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t9902-completion.sh | 76
++-
1 file changed, 27 insertions(+), 49 deletions(-)
Despite the impressive numbers, these tests are more useful without
this cleanup.
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 59cdbfd..66c7af6 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -71,87 +71,65 @@ test_completion ()
newline=$'\n'
-test_expect_success '__gitcomp - trailing space - options' '
- sed -e s/Z$// expected -\EOF
- --reuse-message=Z
- --reedit-message=Z
- --reset-author Z
- EOF
+# Test __gitcomp.
+# Arguments are:
+# 1: typed text so far (cur)
The first argument is not the typed text so far, but the word
currently containing the cursor position.
+# *: arguments to pass to __gitcomp
+test_gitcomp ()
+{
+ sed -e 's/Z$//' expected
(
local -a COMPREPLY
- cur=--re
- __gitcomp --dry-run --reuse-message= --reedit-message=
- --reset-author
+ cur=$1
+ shift
+ __gitcomp $@
IFS=$newline
echo ${COMPREPLY[*]} out
)
test_cmp expected out
+}
+
+test_expect_success '__gitcomp - trailing space - options' '
+ test_gitcomp --re --dry-run --reuse-message= --reedit-message=
+ --reset-author -EOF
+ --reuse-message=Z
+ --reedit-message=Z
+ --reset-author Z
+ EOF
'
test_expect_success '__gitcomp - trailing space - config keys' '
- sed -e s/Z$// expected -\EOF
+ test_gitcomp br branch. branch.autosetupmerge
+ branch.autosetuprebase browser. -\EOF
branch.Z
branch.autosetupmerge Z
branch.autosetuprebase Z
browser.Z
EOF
- (
- local -a COMPREPLY
- cur=br
- __gitcomp branch. branch.autosetupmerge
- branch.autosetuprebase browser.
- IFS=$newline
- echo ${COMPREPLY[*]} out
- )
- test_cmp expected out
'
test_expect_success '__gitcomp - option parameter' '
- sed -e s/Z$// expected -\EOF
+ test_gitcomp --strategy=re octopus ours recursive resolve subtree \
+ re -\EOF
recursive Z
resolve Z
EOF
- (
- local -a COMPREPLY
- cur=--strategy=re
- __gitcomp octopus ours recursive resolve subtree
- re
- IFS=$newline
- echo ${COMPREPLY[*]} out
- )
- test_cmp expected out
'
test_expect_success '__gitcomp - prefix' '
- sed -e s/Z$// expected -\EOF
+ test_gitcomp branch.me remote merge mergeoptions rebase \
+ branch.maint. me -\EOF
branch.maint.merge Z
branch.maint.mergeoptions Z
EOF
- (
- local -a COMPREPLY
- cur=branch.me
- __gitcomp remote merge mergeoptions rebase
- branch.maint. me
- IFS=$newline
- echo ${COMPREPLY[*]} out
- )
- test_cmp expected out
'
test_expect_success '__gitcomp - suffix' '
- sed -e s/Z$// expected -\EOF
+ test_gitcomp branch.me master maint next pu branch. \
+ ma . -\EOF
branch.master.Z
branch.maint.Z
EOF
- (
- local -a COMPREPLY
- cur=branch.me
- __gitcomp master maint next pu
- branch. ma .
- IFS=$newline
- echo ${COMPREPLY[*]} out
- )
- test_cmp expected out
'
test_expect_success 'basic' '
--
1.8.0
--
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 v2 5/6] completion: refactor __gitcomp related tests
SZEDER Gábor sze...@ira.uka.de writes: On Sun, Nov 11, 2012 at 03:35:57PM +0100, Felipe Contreras wrote: Lots of duplicated code! No functional changes. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- t/t9902-completion.sh | 76 ++- 1 file changed, 27 insertions(+), 49 deletions(-) Despite the impressive numbers, these tests are more useful without this cleanup. Is this because consolidation of the duplicated part of the tests into a single helper makes it harder to instrument one test you are interested in (or developing) for debugging? It indeed is a problem, and cutting and pasting the same code to multiple tests is one way to solve the problem (you can easily instrument the copy in the test you are interested in while leaving others intact), but I do not think that is a good solution. A bugfix or enhancement to the shared (or duplicated) part can be done by touching one place only after this change, while with the current code you have to repeat the same fix to all places, no? +# Test __gitcomp. +# Arguments are: +# 1: typed text so far (cur) The first argument is not the typed text so far, but the word currently containing the cursor position. Care to update this with a follow-up patch, so that I do not have to keep track of minute details ;-)? -- 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
[PATCH v2 5/6] completion: refactor __gitcomp related tests
Lots of duplicated code!
No functional changes.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t9902-completion.sh | 76 ++-
1 file changed, 27 insertions(+), 49 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 59cdbfd..66c7af6 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -71,87 +71,65 @@ test_completion ()
newline=$'\n'
-test_expect_success '__gitcomp - trailing space - options' '
- sed -e s/Z$// expected -\EOF
- --reuse-message=Z
- --reedit-message=Z
- --reset-author Z
- EOF
+# Test __gitcomp.
+# Arguments are:
+# 1: typed text so far (cur)
+# *: arguments to pass to __gitcomp
+test_gitcomp ()
+{
+ sed -e 's/Z$//' expected
(
local -a COMPREPLY
- cur=--re
- __gitcomp --dry-run --reuse-message= --reedit-message=
- --reset-author
+ cur=$1
+ shift
+ __gitcomp $@
IFS=$newline
echo ${COMPREPLY[*]} out
)
test_cmp expected out
+}
+
+test_expect_success '__gitcomp - trailing space - options' '
+ test_gitcomp --re --dry-run --reuse-message= --reedit-message=
+ --reset-author -EOF
+ --reuse-message=Z
+ --reedit-message=Z
+ --reset-author Z
+ EOF
'
test_expect_success '__gitcomp - trailing space - config keys' '
- sed -e s/Z$// expected -\EOF
+ test_gitcomp br branch. branch.autosetupmerge
+ branch.autosetuprebase browser. -\EOF
branch.Z
branch.autosetupmerge Z
branch.autosetuprebase Z
browser.Z
EOF
- (
- local -a COMPREPLY
- cur=br
- __gitcomp branch. branch.autosetupmerge
- branch.autosetuprebase browser.
- IFS=$newline
- echo ${COMPREPLY[*]} out
- )
- test_cmp expected out
'
test_expect_success '__gitcomp - option parameter' '
- sed -e s/Z$// expected -\EOF
+ test_gitcomp --strategy=re octopus ours recursive resolve subtree \
+re -\EOF
recursive Z
resolve Z
EOF
- (
- local -a COMPREPLY
- cur=--strategy=re
- __gitcomp octopus ours recursive resolve subtree
- re
- IFS=$newline
- echo ${COMPREPLY[*]} out
- )
- test_cmp expected out
'
test_expect_success '__gitcomp - prefix' '
- sed -e s/Z$// expected -\EOF
+ test_gitcomp branch.me remote merge mergeoptions rebase \
+ branch.maint. me -\EOF
branch.maint.merge Z
branch.maint.mergeoptions Z
EOF
- (
- local -a COMPREPLY
- cur=branch.me
- __gitcomp remote merge mergeoptions rebase
-branch.maint. me
- IFS=$newline
- echo ${COMPREPLY[*]} out
- )
- test_cmp expected out
'
test_expect_success '__gitcomp - suffix' '
- sed -e s/Z$// expected -\EOF
+ test_gitcomp branch.me master maint next pu branch. \
+ ma . -\EOF
branch.master.Z
branch.maint.Z
EOF
- (
- local -a COMPREPLY
- cur=branch.me
- __gitcomp master maint next pu
-branch. ma .
- IFS=$newline
- echo ${COMPREPLY[*]} out
- )
- test_cmp expected out
'
test_expect_success 'basic' '
--
1.8.0
--
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
