[PATCH v2 0/4] remote-helpers: trivial test fixes

[PATCH v2 0/4] remote-helpers: trivial test fixes

[Felipe Contreras]

Hi,

A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the
outsanding shell portability issue. The rest is the same.

Felipe Contreras (4):
  remote-bzr: avoid echo -n
  remote-helpers: fix the run of all tests
  remote-bzr: remove stale check code for tests
  remote-hg: fix hg-git test-case

 contrib/remote-helpers/Makefile          |  1 +
 contrib/remote-helpers/test-bzr.sh       | 16 +---------------
 contrib/remote-helpers/test-hg-hg-git.sh |  1 -
 3 files changed, 2 insertions(+), 16 deletions(-)

-- 
1.8.2

[PATCH v2 1/4] remote-bzr: avoid echo -n

[Felipe Contreras]

It's not portable, as reported by test-lint.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-bzr.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 70aa8a0..b81052b 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -136,7 +136,7 @@ test_expect_success 'special modes' '
   (cd gitrepo &&
   git cat-file -p HEAD:link > ../actual) &&
 
-  echo -n content > expected &&
+  printf content > expected &&
   test_cmp expected actual
 '
 
-- 
1.8.2

[PATCH v2 2/4] remote-helpers: fix the run of all tests

[Felipe Contreras]

We don't need to check for duplicate test numbers, we don't have them,
and either way test-lint-duplicates doesn't work in this situation.

Also, while we are on it, enable test-lint-shell-syntax to check for sh
errors.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile
index 9a76575..239161d 100644
--- a/contrib/remote-helpers/Makefile
+++ b/contrib/remote-helpers/Makefile
@@ -3,6 +3,7 @@ TESTS := $(wildcard test*.sh)
 export T := $(addprefix $(CURDIR)/,$(TESTS))
 export MAKE := $(MAKE) -e
 export PATH := $(CURDIR):$(PATH)
+export TEST_LINT := test-lint-executable test-lint-shell-syntax
 
 test:
 	$(MAKE) -C ../../t $@
-- 
1.8.2

[PATCH v2 3/4] remote-bzr: remove stale check code for tests

[Felipe Contreras]

The fastimport plugin was only required in the early stage of
development.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-bzr.sh | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index b81052b..8450432 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -17,20 +17,6 @@ if ! "$PYTHON_PATH" -c 'import bzrlib'; then
 	test_done
 fi
 
-cmd='
-import bzrlib
-bzrlib.initialize()
-import bzrlib.plugin
-bzrlib.plugin.load_plugins()
-import bzrlib.plugins.fastimport
-'
-
-if ! "$PYTHON_PATH" -c "$cmd"; then
-	echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2
-	skip_all='skipping remote-bzr tests; bzr-fastimport not available'
-	test_done
-fi
-
 check () {
 	(cd $1 &&
 	git log --format='%s' -1 &&
-- 
1.8.2

[PATCH v2 4/4] remote-hg: fix hg-git test-case

[Felipe Contreras]

There was some lingering code that shouldn't have been there in the
first place.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-hg-hg-git.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
index 7e3967f..3f253b7 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -140,7 +140,6 @@ test_expect_success 'executable bit' '
 		git_clone_$x hgrepo-$x gitrepo2-$x &&
 		git_log gitrepo2-$x > log-$x
 	done &&
-	cp -r log-* output-* /tmp/foo/ &&
 
 	test_cmp output-hg output-git &&
 	test_cmp log-hg log-git
-- 
1.8.2

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

[Torsten Bögershausen]

On 04.04.13 17:36, Felipe Contreras wrote:
> Hi,
> 
> A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the
> outsanding shell portability issue. The rest is the same.
> 
> Felipe Contreras (4):
>   remote-bzr: avoid echo -n
>   remote-helpers: fix the run of all tests
>   remote-bzr: remove stale check code for tests
>   remote-hg: fix hg-git test-case
> 
>  contrib/remote-helpers/Makefile          |  1 +
>  contrib/remote-helpers/test-bzr.sh       | 16 +---------------
>  contrib/remote-helpers/test-hg-hg-git.sh |  1 -
>  3 files changed, 2 insertions(+), 16 deletions(-)
> 
Sorry being late, now I installed bzr and hg on one of my machines 

One defect found: "\s" is not portable on all grep versions
A "*" is not a "basic regular expression", so we need to use egrep


diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 5f81dfa..2e80c11 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -115,7 +115,7 @@ test_expect_success 'update bookmark' '
   git push
   ) &&
 
-  hg -R hgrepo bookmarks | grep "devel\s\+3:"
+  hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:"
 '
2 mninor nits:

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 8450432..7970f9e 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -13,7 +13,7 @@ if ! test_have_prereq PYTHON; then
 fi
 
 if ! "$PYTHON_PATH" -c 'import bzrlib'; then
-       skip_all='skipping remote-bzr tests; bzr not available'
+       skip_all='skipping remote-bzr tests; python bzrlib not available'
        test_done
 fi
 
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
index 3f253b7..5d87282 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -21,7 +21,7 @@ if ! "$PYTHON_PATH" -c 'import mercurial'; then
 fi
 
 if ! "$PYTHON_PATH" -c 'import hggit'; then
-       skip_all='skipping remote-hg tests; hg-git not available'
+       skip_all='skipping remote-hg tests; python hggit not available'
        test_done
 fi

(And as a micro-nit: the indenting deserves some better indentation:
TAB could be used for *.sh, and sub-shells could be indentented one TAB:


test_expect_success 'update bookmark' '
	test_when_finished "rm -rf gitrepo*" &&
	(
		cd hgrepo &&
		hg bookmark devel

/Torsten

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

[Torsten Bögershausen]

On 06.04.13 19:03, Torsten Bögershausen wrote:
> On 04.04.13 17:36, Felipe Contreras wrote:
>> Hi,
>>
>> A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the
>> outsanding shell portability issue. The rest is the same.
>>
>> Felipe Contreras (4):
>>   remote-bzr: avoid echo -n
>>   remote-helpers: fix the run of all tests
>>   remote-bzr: remove stale check code for tests
>>   remote-hg: fix hg-git test-case
>>
>>  contrib/remote-helpers/Makefile          |  1 +
>>  contrib/remote-helpers/test-bzr.sh       | 16 +---------------
>>  contrib/remote-helpers/test-hg-hg-git.sh |  1 -
>>  3 files changed, 2 insertions(+), 16 deletions(-)
>>
> Sorry being late, now I installed bzr and hg on one of my machines 
> 
> One defect found: "\s" is not portable on all grep versions
> A "*" is not a "basic regular expression", so we need to use egrep

Sorry for confusion:
A "*" is a basic regular expression, but not the "+"
"+" is an extended regular expression, which is understood
by some  grep versions (gnu ?).

Felipe: Should I send a patch, or wait for a re-roll?

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

[Felipe Contreras]

On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 04.04.13 17:36, Felipe Contreras wrote:
>> Hi,
>>
>> A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the
>> outsanding shell portability issue. The rest is the same.
>>
>> Felipe Contreras (4):
>>   remote-bzr: avoid echo -n
>>   remote-helpers: fix the run of all tests
>>   remote-bzr: remove stale check code for tests
>>   remote-hg: fix hg-git test-case
>>
>>  contrib/remote-helpers/Makefile          |  1 +
>>  contrib/remote-helpers/test-bzr.sh       | 16 +---------------
>>  contrib/remote-helpers/test-hg-hg-git.sh |  1 -
>>  3 files changed, 2 insertions(+), 16 deletions(-)
>>
> Sorry being late, now I installed bzr and hg on one of my machines
>
> One defect found: "\s" is not portable on all grep versions
> A "*" is not a "basic regular expression", so we need to use egrep
>
>
> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
> index 5f81dfa..2e80c11 100755
> --- a/contrib/remote-helpers/test-hg.sh
> +++ b/contrib/remote-helpers/test-hg.sh
> @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' '
>    git push
>    ) &&
>
> -  hg -R hgrepo bookmarks | grep "devel\s\+3:"
> +  hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:"
>  '

I would rather use [ \t] instead.

> 2 mninor nits:
>
> diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
> index 8450432..7970f9e 100755
> --- a/contrib/remote-helpers/test-bzr.sh
> +++ b/contrib/remote-helpers/test-bzr.sh
> @@ -13,7 +13,7 @@ if ! test_have_prereq PYTHON; then
>  fi
>
>  if ! "$PYTHON_PATH" -c 'import bzrlib'; then
> -       skip_all='skipping remote-bzr tests; bzr not available'
> +       skip_all='skipping remote-bzr tests; python bzrlib not available'

"python bzrlib" is meaningless to the user; bazaar provides it;
basically all the code is there.

>         test_done
>  fi
>
> diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
> index 3f253b7..5d87282 100755
> --- a/contrib/remote-helpers/test-hg-hg-git.sh
> +++ b/contrib/remote-helpers/test-hg-hg-git.sh
> @@ -21,7 +21,7 @@ if ! "$PYTHON_PATH" -c 'import mercurial'; then
>  fi
>
>  if ! "$PYTHON_PATH" -c 'import hggit'; then
> -       skip_all='skipping remote-hg tests; hg-git not available'
> +       skip_all='skipping remote-hg tests; python hggit not available'

Same. Google for 'python hggit' and the result is hg-git; and that's
what you would actually tell your package manager to install.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

[Torsten Bögershausen]

On 06.04.13 19:29, Felipe Contreras wrote:
> On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen <tboegi@web.de> wrote:
>> On 04.04.13 17:36, Felipe Contreras wrote:
>>> Hi,
>>>
>>> A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the
>>> outsanding shell portability issue. The rest is the same.
>>>
>>> Felipe Contreras (4):
>>>   remote-bzr: avoid echo -n
>>>   remote-helpers: fix the run of all tests
>>>   remote-bzr: remove stale check code for tests
>>>   remote-hg: fix hg-git test-case
>>>
>>>  contrib/remote-helpers/Makefile          |  1 +
>>>  contrib/remote-helpers/test-bzr.sh       | 16 +---------------
>>>  contrib/remote-helpers/test-hg-hg-git.sh |  1 -
>>>  3 files changed, 2 insertions(+), 16 deletions(-)
>>>
>> Sorry being late, now I installed bzr and hg on one of my machines
>>
>> One defect found: "\s" is not portable on all grep versions
>> A "*" is not a "basic regular expression", so we need to use egrep
>>
>>
>> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
>> index 5f81dfa..2e80c11 100755
>> --- a/contrib/remote-helpers/test-hg.sh
>> +++ b/contrib/remote-helpers/test-hg.sh
>> @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' '
>>    git push
>>    ) &&
>>
>> -  hg -R hgrepo bookmarks | grep "devel\s\+3:"
>> +  hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:"
>>  '
> 
> I would rather use [ \t] instead.
That doesn't work on e.g. Mac OS.
[:space:] is actually portable
/Torsten

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

[Felipe Contreras]

On Sat, Apr 6, 2013 at 11:45 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 06.04.13 19:29, Felipe Contreras wrote:
>> On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen <tboegi@web.de> wrote:

>>> --- a/contrib/remote-helpers/test-hg.sh
>>> +++ b/contrib/remote-helpers/test-hg.sh
>>> @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' '
>>>    git push
>>>    ) &&
>>>
>>> -  hg -R hgrepo bookmarks | grep "devel\s\+3:"
>>> +  hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:"
>>>  '
>>
>> I would rather use [ \t] instead.
> That doesn't work on e.g. Mac OS.
> [:space:] is actually portable

Why wouldn't it work? This is from their manpage:

A bracket expression is a list of characters enclosed by [ and ].  It
matches any single character in
that list; if the first character of the list is the caret ^ then it
matches any character not in the
list.  For example, the regular expression [0123456789] matches any
single digit.

-- 
Felipe Contreras

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

[Torsten Bögershausen]

On 06.04.13 19:58, Felipe Contreras wrote:
> On Sat, Apr 6, 2013 at 11:45 AM, Torsten Bögershausen <tboegi@web.de> wrote:
>> On 06.04.13 19:29, Felipe Contreras wrote:
>>> On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> 
>>>> --- a/contrib/remote-helpers/test-hg.sh
>>>> +++ b/contrib/remote-helpers/test-hg.sh
>>>> @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' '
>>>>    git push
>>>>    ) &&
>>>>
>>>> -  hg -R hgrepo bookmarks | grep "devel\s\+3:"
>>>> +  hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:"
>>>>  '
>>>
>>> I would rather use [ \t] instead.
>> That doesn't work on e.g. Mac OS.
>> [:space:] is actually portable
> 
> Why wouldn't it work? This is from their manpage:
> 
> A bracket expression is a list of characters enclosed by [ and ].  It
> matches any single character in
> that list; if the first character of the list is the caret ^ then it
> matches any character not in the
> list.  For example, the regular expression [0123456789] matches any
> single digit.
> 

It's not about the "bracket list".
It's about using \t as an abreviation for TAB.
The "backslash n" as an replacement for TAB is quite often understood by many programs.
It is not demanded to be understood by all grep implementations, please see below.
Instead of using \t you can use a literal TAB.
Contact your local editor how to put that inte source code ;-)

 
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap09.html#tag_09_03

9.3.2 BRE Ordinary Characters

An ordinary character is a BRE that matches itself: any character in the supported character set, except for the BRE special characters listed in BRE Special Characters.

The interpretation of an ordinary character preceded by a backslash ( '\' ) is undefined, except for:

    The characters ')', '(', '{', and '}'

    The digits 1 to 9 inclusive (see BREs Matching Multiple Characters)

    A character inside a bracket expression