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

STALE3737d

Revision v2 of 2 in this series.

11 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:56:40

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

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:56:40

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

Signed-off-by: Felipe Contreras <redacted>
---
 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

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:56:40

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 <redacted>
---
 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

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:56:40

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

Signed-off-by: Felipe Contreras <redacted>
---
 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

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:56:40

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

Signed-off-by: Felipe Contreras <redacted>
---
 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

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 22:56:42

On 06.04.13 19:03, Torsten Bögershausen wrote:
On 04.04.13 17:36, Felipe Contreras wrote:
quoted
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

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 22:56:42

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

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 22:56:42

On 06.04.13 19:29, Felipe Contreras wrote:
On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen [off-list ref] wrote:
quoted
On 04.04.13 17:36, Felipe Contreras wrote:
quoted
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

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:56:42

On Sat, Apr 6, 2013 at 11:45 AM, Torsten Bögershausen [off-list ref] wrote:
On 06.04.13 19:29, Felipe Contreras wrote:
quoted
On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen [off-list ref] wrote:
quoted
quoted
--- 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

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:56:42

On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen [off-list ref] wrote:
quoted hunk
On 04.04.13 17:36, Felipe Contreras wrote:
quoted
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.
quoted hunk
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.
quoted hunk
        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

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 22:56:42

On 06.04.13 19:58, Felipe Contreras wrote:
On Sat, Apr 6, 2013 at 11:45 AM, Torsten Bögershausen [off-list ref] wrote:
quoted
On 06.04.13 19:29, Felipe Contreras wrote:
quoted
On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen [off-list ref] wrote:
quoted
quoted
quoted
--- 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help