All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] minor fixes to some svn tests
@ 2016-05-13 19:47 Jeff King
  2016-05-13 19:48 ` [PATCH 1/3] t/lib-git-svn: drop $remote_git_svn and $git_svn_id Jeff King
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Jeff King @ 2016-05-13 19:47 UTC (permalink / raw)
  To: Git List; +Cc: Armin Kunaschik, Eric Wong, Junio C Hamano

On Fri, May 13, 2016 at 02:23:25PM -0400, Jeff King wrote:

> This covers all cases detected by:
> 
>   git grep 'test -z [^"]'
> 
> (though note that has a few false positives for tests which
> need an extra layer of quoting to do '\"').

I looked at those false positives, and they're really gratuitous uses
of double-quotes that cause the extra layer of quoting.

So I set off to fix that, but along the way found an actual bug (in the
final patch here), along with a few minor readability fixups.

  [1/3]: t/lib-git-svn: drop $remote_git_svn and $git_svn_id
  [2/3]: t9100: enclose all test code in single-quotes
  [3/3]: t9107: use "return 1" instead of "exit 1"

-Peff

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] t/lib-git-svn: drop $remote_git_svn and $git_svn_id
  2016-05-13 19:47 [PATCH 0/3] minor fixes to some svn tests Jeff King
@ 2016-05-13 19:48 ` Jeff King
  2016-05-13 19:48 ` [PATCH 2/3] t9100: enclose all test code in single-quotes Jeff King
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Jeff King @ 2016-05-13 19:48 UTC (permalink / raw)
  To: Git List; +Cc: Armin Kunaschik, Eric Wong, Junio C Hamano

These variables were added in 16805d3 (t/t91XX-svn: start
removing use of "git-" from these tests, 2008-09-08) so that
running:

  git grep git-

would return fewer hits. At the time, we were transitioning
away from the use of the "dashed" git-foo form.

That transition has been over for years, and grepping for
"git-" in the test suite yields thousands of hits anyway
(all presumably false positives).

With their original purpose gone, these variables serve only
to obfuscate the tests. Let's get rid of them.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/lib-git-svn.sh                              |  3 ---
 t/t9100-git-svn-basic.sh                      | 38 +++++++++++++--------------
 t/t9101-git-svn-props.sh                      | 12 ++++-----
 t/t9102-git-svn-deep-rmdir.sh                 |  2 +-
 t/t9106-git-svn-commit-diff-clobber.sh        |  6 ++---
 t/t9107-git-svn-migrate.sh                    | 14 +++++-----
 t/t9110-git-svn-use-svm-props.sh              | 18 ++++++-------
 t/t9111-git-svn-use-svnsync-props.sh          | 18 ++++++-------
 t/t9120-git-svn-clone-with-percent-escapes.sh |  6 ++---
 t/t9123-git-svn-rebuild-with-rewriteroot.sh   |  2 +-
 t/t9153-git-svn-rewrite-uuid.sh               |  4 +--
 11 files changed, 60 insertions(+), 63 deletions(-)

diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 6a50b87..fb88232 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -1,8 +1,5 @@
 . ./test-lib.sh
 
-remotes_git_svn=remotes/git""-svn
-git_svn_id=git""-svn-id
-
 if test -n "$NO_SVN_TESTS"
 then
 	skip_all='skipping git svn tests, NO_SVN_TESTS defined'
diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index 22d8367..6ec73ee 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -45,13 +45,13 @@ test_expect_success "checkout from svn" 'svn co "$svnrepo" "$SVN_TREE"'
 
 name='try a deep --rmdir with a commit'
 test_expect_success "$name" '
-	git checkout -f -b mybranch ${remotes_git_svn} &&
+	git checkout -f -b mybranch remotes/git-svn &&
 	mv dir/a/b/c/d/e/file dir/file &&
 	cp dir/file file &&
 	git update-index --add --remove dir/a/b/c/d/e/file dir/file file &&
 	git commit -m "$name" &&
 	git svn set-tree --find-copies-harder --rmdir \
-		${remotes_git_svn}..mybranch &&
+		remotes/git-svn..mybranch &&
 	svn_cmd up "$SVN_TREE" &&
 	test -d "$SVN_TREE"/dir && test ! -d "$SVN_TREE"/dir/a'
 
@@ -65,14 +65,14 @@ test_expect_success "$name" "
 	git update-index --add dir/file/file &&
 	git commit -m '$name' &&
 	test_must_fail git svn set-tree --find-copies-harder --rmdir \
-		${remotes_git_svn}..mybranch
+		remotes/git-svn..mybranch
 "
 
 
 name='detect node change from directory to file #1'
 test_expect_success "$name" '
 	rm -rf dir "$GIT_DIR"/index &&
-	git checkout -f -b mybranch2 ${remotes_git_svn} &&
+	git checkout -f -b mybranch2 remotes/git-svn &&
 	mv bar/zzz zzz &&
 	rm -rf bar &&
 	mv zzz bar &&
@@ -80,14 +80,14 @@ test_expect_success "$name" '
 	git update-index --add -- bar &&
 	git commit -m "$name" &&
 	test_must_fail git svn set-tree --find-copies-harder --rmdir \
-		${remotes_git_svn}..mybranch2
+		remotes/git-svn..mybranch2
 '
 
 
 name='detect node change from file to directory #2'
 test_expect_success "$name" '
 	rm -f "$GIT_DIR"/index &&
-	git checkout -f -b mybranch3 ${remotes_git_svn} &&
+	git checkout -f -b mybranch3 remotes/git-svn &&
 	rm bar/zzz &&
 	git update-index --remove bar/zzz &&
 	mkdir bar/zzz &&
@@ -95,7 +95,7 @@ test_expect_success "$name" '
 	git update-index --add bar/zzz/yyy &&
 	git commit -m "$name" &&
 	git svn set-tree --find-copies-harder --rmdir \
-		${remotes_git_svn}..mybranch3 &&
+		remotes/git-svn..mybranch3 &&
 	svn_cmd up "$SVN_TREE" &&
 	test -d "$SVN_TREE"/bar/zzz &&
 	test -e "$SVN_TREE"/bar/zzz/yyy
@@ -104,7 +104,7 @@ test_expect_success "$name" '
 name='detect node change from directory to file #2'
 test_expect_success "$name" '
 	rm -f "$GIT_DIR"/index &&
-	git checkout -f -b mybranch4 ${remotes_git_svn} &&
+	git checkout -f -b mybranch4 remotes/git-svn &&
 	rm -rf dir &&
 	git update-index --remove -- dir/file &&
 	touch dir &&
@@ -112,19 +112,19 @@ test_expect_success "$name" '
 	git update-index --add -- dir &&
 	git commit -m "$name" &&
 	test_must_fail git svn set-tree --find-copies-harder --rmdir \
-		${remotes_git_svn}..mybranch4
+		remotes/git-svn..mybranch4
 '
 
 
 name='remove executable bit from a file'
 test_expect_success POSIXPERM "$name" '
 	rm -f "$GIT_DIR"/index &&
-	git checkout -f -b mybranch5 ${remotes_git_svn} &&
+	git checkout -f -b mybranch5 remotes/git-svn &&
 	chmod -x exec.sh &&
 	git update-index exec.sh &&
 	git commit -m "$name" &&
 	git svn set-tree --find-copies-harder --rmdir \
-		${remotes_git_svn}..mybranch5 &&
+		remotes/git-svn..mybranch5 &&
 	svn_cmd up "$SVN_TREE" &&
 	test ! -x "$SVN_TREE"/exec.sh'
 
@@ -135,7 +135,7 @@ test_expect_success POSIXPERM "$name" '
 	git update-index exec.sh &&
 	git commit -m "$name" &&
 	git svn set-tree --find-copies-harder --rmdir \
-		${remotes_git_svn}..mybranch5 &&
+		remotes/git-svn..mybranch5 &&
 	svn_cmd up "$SVN_TREE" &&
 	test -x "$SVN_TREE"/exec.sh'
 
@@ -147,7 +147,7 @@ test_expect_success SYMLINKS "$name" '
 	git update-index exec.sh &&
 	git commit -m "$name" &&
 	git svn set-tree --find-copies-harder --rmdir \
-		${remotes_git_svn}..mybranch5 &&
+		remotes/git-svn..mybranch5 &&
 	svn_cmd up "$SVN_TREE" &&
 	test -h "$SVN_TREE"/exec.sh'
 
@@ -159,7 +159,7 @@ test_expect_success POSIXPERM,SYMLINKS "$name" '
 	git update-index --add file exec-2.sh &&
 	git commit -m "$name" &&
 	git svn set-tree --find-copies-harder --rmdir \
-		${remotes_git_svn}..mybranch5 &&
+		remotes/git-svn..mybranch5 &&
 	svn_cmd up "$SVN_TREE" &&
 	test -x "$SVN_TREE"/file &&
 	test -h "$SVN_TREE"/exec-2.sh'
@@ -172,7 +172,7 @@ test_expect_success POSIXPERM,SYMLINKS "$name" '
 	git update-index exec-2.sh &&
 	git commit -m "$name" &&
 	git svn set-tree --find-copies-harder --rmdir \
-		${remotes_git_svn}..mybranch5 &&
+		remotes/git-svn..mybranch5 &&
 	svn_cmd up "$SVN_TREE" &&
 	test -f "$SVN_TREE"/exec-2.sh &&
 	test ! -h "$SVN_TREE"/exec-2.sh &&
@@ -194,7 +194,7 @@ GIT_SVN_ID=alt
 export GIT_SVN_ID
 test_expect_success "$name" \
     'git svn init "$svnrepo" && git svn fetch &&
-     git rev-list --pretty=raw ${remotes_git_svn} | grep ^tree | uniq > a &&
+     git rev-list --pretty=raw remotes/git-svn | grep ^tree | uniq > a &&
      git rev-list --pretty=raw remotes/alt | grep ^tree | uniq > b &&
      test_cmp a b'
 
@@ -219,7 +219,7 @@ test_expect_success POSIXPERM,SYMLINKS "$name" "test_cmp a expected"
 
 test_expect_success 'exit if remote refs are ambigious' "
         git config --add svn-remote.svn.fetch \
-                              bar:refs/${remotes_git_svn} &&
+		bar:refs/remotes/git-svn &&
 	test_must_fail git svn migrate
 "
 
@@ -227,7 +227,7 @@ test_expect_success 'exit if init-ing a would clobber a URL' '
         svnadmin create "${PWD}/svnrepo2" &&
         svn mkdir -m "mkdir bar" "${svnrepo}2/bar" &&
         git config --unset svn-remote.svn.fetch \
-                                "^bar:refs/${remotes_git_svn}$" &&
+		"^bar:refs/remotes/git-svn$" &&
 	test_must_fail git svn init "${svnrepo}2/bar"
         '
 
@@ -237,7 +237,7 @@ test_expect_success \
         git config --get svn-remote.svn.fetch \
                               "^bar:refs/remotes/bar$" &&
         git config --get svn-remote.svn.fetch \
-                              "^:refs/${remotes_git_svn}$"
+			      "^:refs/remotes/git-svn$"
         '
 
 test_expect_success 'dcommit $rev does not clobber current branch' '
diff --git a/t/t9101-git-svn-props.sh b/t/t9101-git-svn-props.sh
index e8173d5..07bfb63 100755
--- a/t/t9101-git-svn-props.sh
+++ b/t/t9101-git-svn-props.sh
@@ -73,11 +73,11 @@ test_expect_success 'fetch revisions from svn' 'git svn fetch'
 
 name='test svn:keywords ignoring'
 test_expect_success "$name" \
-	'git checkout -b mybranch ${remotes_git_svn} &&
+	'git checkout -b mybranch remotes/git-svn &&
 	echo Hi again >> kw.c &&
 	git commit -a -m "test keywords ignoring" &&
-	git svn set-tree ${remotes_git_svn}..mybranch &&
-	git pull . ${remotes_git_svn}'
+	git svn set-tree remotes/git-svn..mybranch &&
+	git pull . remotes/git-svn'
 
 expect='/* $Id$ */'
 got="$(sed -ne 2p kw.c)"
@@ -95,7 +95,7 @@ test_expect_success "propset CR on crlf files" '
 
 test_expect_success 'fetch and pull latest from svn and checkout a new wc' \
 	'git svn fetch &&
-	 git pull . ${remotes_git_svn} &&
+	 git pull . remotes/git-svn &&
 	 svn_cmd co "$svnrepo" new_wc'
 
 for i in crlf ne_crlf lf ne_lf cr ne_cr empty_cr empty_lf empty empty_crlf
@@ -117,7 +117,7 @@ cd test_wc
 	 svn_cmd commit -m "propset CRLF on cr files"'
 cd ..
 test_expect_success 'fetch and pull latest from svn' \
-	'git svn fetch && git pull . ${remotes_git_svn}'
+	'git svn fetch && git pull . remotes/git-svn'
 
 b_cr="$(git hash-object cr)"
 b_ne_cr="$(git hash-object ne_cr)"
@@ -168,7 +168,7 @@ cat >create-ignore-index.expect <<\EOF
 EOF
 
 test_expect_success 'test create-ignore' "
-	git svn fetch && git pull . ${remotes_git_svn} &&
+	git svn fetch && git pull . remotes/git-svn &&
 	git svn create-ignore &&
 	cmp ./.gitignore create-ignore.expect &&
 	cmp ./deeply/.gitignore create-ignore.expect &&
diff --git a/t/t9102-git-svn-deep-rmdir.sh b/t/t9102-git-svn-deep-rmdir.sh
index eb70f48..66cd511 100755
--- a/t/t9102-git-svn-deep-rmdir.sh
+++ b/t/t9102-git-svn-deep-rmdir.sh
@@ -17,7 +17,7 @@ test_expect_success 'initialize repo' '
 test_expect_success 'mirror via git svn' '
 	git svn init "$svnrepo" &&
 	git svn fetch &&
-	git checkout -f -b test-rmdir ${remotes_git_svn}
+	git checkout -f -b test-rmdir remotes/git-svn
 	'
 
 test_expect_success 'Try a commit on rmdir' '
diff --git a/t/t9106-git-svn-commit-diff-clobber.sh b/t/t9106-git-svn-commit-diff-clobber.sh
index f6d7ac7..dbe8dea 100755
--- a/t/t9106-git-svn-commit-diff-clobber.sh
+++ b/t/t9106-git-svn-commit-diff-clobber.sh
@@ -44,7 +44,7 @@ test_expect_success 'commit complementing change from git' '
 test_expect_success 'dcommit fails to commit because of conflict' '
 	git svn init "$svnrepo" &&
 	git svn fetch &&
-	git reset --hard refs/${remotes_git_svn} &&
+	git reset --hard refs/remotes/git-svn &&
 	svn_cmd co "$svnrepo" t.svn &&
 	(
 		cd t.svn &&
@@ -59,7 +59,7 @@ test_expect_success 'dcommit fails to commit because of conflict' '
 	'
 
 test_expect_success 'dcommit does the svn equivalent of an index merge' "
-	git reset --hard refs/${remotes_git_svn} &&
+	git reset --hard refs/remotes/git-svn &&
 	echo 'index merge' > file2 &&
 	git update-index --add file2 &&
 	git commit -a -m 'index merge' &&
@@ -81,7 +81,7 @@ test_expect_success 'commit another change from svn side' '
 	'
 
 test_expect_success 'multiple dcommit from git svn will not clobber svn' "
-	git reset --hard refs/${remotes_git_svn} &&
+	git reset --hard refs/remotes/git-svn &&
 	echo new file >> new-file &&
 	git update-index --add new-file &&
 	git commit -a -m 'new file' &&
diff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh
index 9060198..6efc2ab 100755
--- a/t/t9107-git-svn-migrate.sh
+++ b/t/t9107-git-svn-migrate.sh
@@ -19,9 +19,9 @@ test_expect_success 'setup old-looking metadata' '
 	git svn init "$svnrepo" &&
 	git svn fetch &&
 	rm -rf "$GIT_DIR"/svn &&
-	git update-ref refs/heads/git-svn-HEAD refs/${remotes_git_svn} &&
-	git update-ref refs/heads/svn-HEAD refs/${remotes_git_svn} &&
-	git update-ref -d refs/${remotes_git_svn} refs/${remotes_git_svn}
+	git update-ref refs/heads/git-svn-HEAD refs/remotes/git-svn &&
+	git update-ref refs/heads/svn-HEAD refs/remotes/git-svn &&
+	git update-ref -d refs/remotes/git-svn refs/remotes/git-svn
 	'
 
 head=$(git rev-parse --verify refs/heads/git-svn-HEAD^0)
@@ -35,11 +35,11 @@ test_expect_success 'initialize old-style (v0) git svn layout' '
 	echo "$svnrepo" > "$GIT_DIR"/svn/info/url &&
 	git svn migrate &&
 	! test -d "$GIT_DIR"/git-svn &&
-	git rev-parse --verify refs/${remotes_git_svn}^0 &&
+	git rev-parse --verify refs/remotes/git-svn^0 &&
 	git rev-parse --verify refs/remotes/svn^0 &&
 	test "$(git config --get svn-remote.svn.url)" = "$svnrepo_escaped" &&
 	test $(git config --get svn-remote.svn.fetch) = \
-             ":refs/${remotes_git_svn}"
+		":refs/remotes/git-svn"
 	'
 
 test_expect_success 'initialize a multi-repository repo' '
@@ -66,7 +66,7 @@ test_expect_success 'initialize a multi-repository repo' '
 	grep "^tags/0\.1:refs/remotes/origin/tags/0\.1$" fetch.out &&
 	grep "^tags/0\.2:refs/remotes/origin/tags/0\.2$" fetch.out &&
 	grep "^tags/0\.3:refs/remotes/origin/tags/0\.3$" fetch.out &&
-	grep "^:refs/${remotes_git_svn}" fetch.out
+	grep "^:refs/remotes/git-svn" fetch.out
 	'
 
 # refs should all be different, but the trees should all be the same:
@@ -104,7 +104,7 @@ test_expect_success 'migrate --minimize on old inited layout' '
 	grep "^tags/0\.1:refs/remotes/origin/tags/0\.1$" fetch.out &&
 	grep "^tags/0\.2:refs/remotes/origin/tags/0\.2$" fetch.out &&
 	grep "^tags/0\.3:refs/remotes/origin/tags/0\.3$" fetch.out &&
-	grep "^:refs/${remotes_git_svn}" fetch.out
+	grep "^:refs/remotes/git-svn" fetch.out
 	'
 
 test_expect_success  ".rev_db auto-converted to .rev_map.UUID" '
diff --git a/t/t9110-git-svn-use-svm-props.sh b/t/t9110-git-svn-use-svm-props.sh
index 29fbdfd..dde0a3c 100755
--- a/t/t9110-git-svn-use-svm-props.sh
+++ b/t/t9110-git-svn-use-svm-props.sh
@@ -22,31 +22,31 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
 bar_url=http://mayonaise/svnrepo/bar
 test_expect_success 'verify metadata for /bar' "
 	git cat-file commit refs/remotes/bar | \
-	   grep '^${git_svn_id}: $bar_url@12 $uuid$' &&
+	   grep '^git-svn-id: $bar_url@12 $uuid$' &&
 	git cat-file commit refs/remotes/bar~1 | \
-	   grep '^${git_svn_id}: $bar_url@11 $uuid$' &&
+	   grep '^git-svn-id: $bar_url@11 $uuid$' &&
 	git cat-file commit refs/remotes/bar~2 | \
-	   grep '^${git_svn_id}: $bar_url@10 $uuid$' &&
+	   grep '^git-svn-id: $bar_url@10 $uuid$' &&
 	git cat-file commit refs/remotes/bar~3 | \
-	   grep '^${git_svn_id}: $bar_url@9 $uuid$' &&
+	   grep '^git-svn-id: $bar_url@9 $uuid$' &&
 	git cat-file commit refs/remotes/bar~4 | \
-	   grep '^${git_svn_id}: $bar_url@6 $uuid$' &&
+	   grep '^git-svn-id: $bar_url@6 $uuid$' &&
 	git cat-file commit refs/remotes/bar~5 | \
-	   grep '^${git_svn_id}: $bar_url@1 $uuid$'
+	   grep '^git-svn-id: $bar_url@1 $uuid$'
 	"
 
 e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
 test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
 	git cat-file commit refs/remotes/e | \
-	   grep '^${git_svn_id}: $e_url@1 $uuid$'
+	   grep '^git-svn-id: $e_url@1 $uuid$'
 	"
 
 dir_url=http://mayonaise/svnrepo/dir
 test_expect_success 'verify metadata for /dir' "
 	git cat-file commit refs/remotes/dir | \
-	   grep '^${git_svn_id}: $dir_url@2 $uuid$' &&
+	   grep '^git-svn-id: $dir_url@2 $uuid$' &&
 	git cat-file commit refs/remotes/dir~1 | \
-	   grep '^${git_svn_id}: $dir_url@1 $uuid$'
+	   grep '^git-svn-id: $dir_url@1 $uuid$'
 	"
 
 test_expect_success 'find commit based on SVN revision number' "
diff --git a/t/t9111-git-svn-use-svnsync-props.sh b/t/t9111-git-svn-use-svnsync-props.sh
index bd081c2..22b6e5e 100755
--- a/t/t9111-git-svn-use-svnsync-props.sh
+++ b/t/t9111-git-svn-use-svnsync-props.sh
@@ -21,31 +21,31 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
 bar_url=http://mayonaise/svnrepo/bar
 test_expect_success 'verify metadata for /bar' "
 	git cat-file commit refs/remotes/bar | \
-	   grep '^${git_svn_id}: $bar_url@12 $uuid$' &&
+	   grep '^git-svn-id: $bar_url@12 $uuid$' &&
 	git cat-file commit refs/remotes/bar~1 | \
-	   grep '^${git_svn_id}: $bar_url@11 $uuid$' &&
+	   grep '^git-svn-id: $bar_url@11 $uuid$' &&
 	git cat-file commit refs/remotes/bar~2 | \
-	   grep '^${git_svn_id}: $bar_url@10 $uuid$' &&
+	   grep '^git-svn-id: $bar_url@10 $uuid$' &&
 	git cat-file commit refs/remotes/bar~3 | \
-	   grep '^${git_svn_id}: $bar_url@9 $uuid$' &&
+	   grep '^git-svn-id: $bar_url@9 $uuid$' &&
 	git cat-file commit refs/remotes/bar~4 | \
-	   grep '^${git_svn_id}: $bar_url@6 $uuid$' &&
+	   grep '^git-svn-id: $bar_url@6 $uuid$' &&
 	git cat-file commit refs/remotes/bar~5 | \
-	   grep '^${git_svn_id}: $bar_url@1 $uuid$'
+	   grep '^git-svn-id: $bar_url@1 $uuid$'
 	"
 
 e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
 test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
 	git cat-file commit refs/remotes/e | \
-	   grep '^${git_svn_id}: $e_url@1 $uuid$'
+	   grep '^git-svn-id: $e_url@1 $uuid$'
 	"
 
 dir_url=http://mayonaise/svnrepo/dir
 test_expect_success 'verify metadata for /dir' "
 	git cat-file commit refs/remotes/dir | \
-	   grep '^${git_svn_id}: $dir_url@2 $uuid$' &&
+	   grep '^git-svn-id: $dir_url@2 $uuid$' &&
 	git cat-file commit refs/remotes/dir~1 | \
-	   grep '^${git_svn_id}: $dir_url@1 $uuid$'
+	   grep '^git-svn-id: $dir_url@1 $uuid$'
 	"
 
 test_done
diff --git a/t/t9120-git-svn-clone-with-percent-escapes.sh b/t/t9120-git-svn-clone-with-percent-escapes.sh
index 1c84ce1..59465b1 100755
--- a/t/t9120-git-svn-clone-with-percent-escapes.sh
+++ b/t/t9120-git-svn-clone-with-percent-escapes.sh
@@ -22,7 +22,7 @@ test_expect_success 'test clone with percent escapes' '
 	git svn clone "$svnrepo/pr%20ject" clone &&
 	(
 		cd clone &&
-		git rev-parse refs/${remotes_git_svn}
+		git rev-parse refs/remotes/git-svn
 	)
 '
 
@@ -42,7 +42,7 @@ test_expect_success 'test clone trunk with percent escapes and minimize-url' '
 	git svn clone --minimize-url "$svnrepo/pr%20ject/trunk" minimize &&
 	(
 		cd minimize &&
-		git rev-parse refs/${remotes_git_svn}
+		git rev-parse refs/remotes/git-svn
 	)
 '
 
@@ -50,7 +50,7 @@ test_expect_success 'test clone trunk with percent escapes' '
 	git svn clone "$svnrepo/pr%20ject/trunk" trunk &&
 	(
 		cd trunk &&
-		git rev-parse refs/${remotes_git_svn}
+		git rev-parse refs/remotes/git-svn
 	)
 '
 
diff --git a/t/t9123-git-svn-rebuild-with-rewriteroot.sh b/t/t9123-git-svn-rebuild-with-rewriteroot.sh
index fd81847..ead4045 100755
--- a/t/t9123-git-svn-rebuild-with-rewriteroot.sh
+++ b/t/t9123-git-svn-rebuild-with-rewriteroot.sh
@@ -17,7 +17,7 @@ rm -rf import
 test_expect_success 'init, fetch and checkout repository' '
 	git svn init --rewrite-root=http://invalid.invalid/ "$svnrepo" &&
 	git svn fetch &&
-	git checkout -b mybranch ${remotes_git_svn}
+	git checkout -b mybranch remotes/git-svn
 	'
 
 test_expect_success 'remove rev_map' '
diff --git a/t/t9153-git-svn-rewrite-uuid.sh b/t/t9153-git-svn-rewrite-uuid.sh
index 88a2cfa..372ef15 100755
--- a/t/t9153-git-svn-rewrite-uuid.sh
+++ b/t/t9153-git-svn-rewrite-uuid.sh
@@ -17,9 +17,9 @@ test_expect_success 'load svn repo' "
 
 test_expect_success 'verify uuid' "
 	git cat-file commit refs/remotes/git-svn~0 | \
-	   grep '^${git_svn_id}: .*@2 $uuid$' &&
+	   grep '^git-svn-id: .*@2 $uuid$' &&
 	git cat-file commit refs/remotes/git-svn~1 | \
-	   grep '^${git_svn_id}: .*@1 $uuid$'
+	   grep '^git-svn-id: .*@1 $uuid$'
 	"
 
 test_done
-- 
2.8.2.825.gea31738

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] t9100: enclose all test code in single-quotes
  2016-05-13 19:47 [PATCH 0/3] minor fixes to some svn tests Jeff King
  2016-05-13 19:48 ` [PATCH 1/3] t/lib-git-svn: drop $remote_git_svn and $git_svn_id Jeff King
@ 2016-05-13 19:48 ` Jeff King
  2016-05-13 19:50 ` [PATCH 3/3] t9107: use "return 1" instead of "exit 1" Jeff King
  2016-05-13 20:49 ` [PATCH 0/3] minor fixes to some svn tests Jeff King
  3 siblings, 0 replies; 5+ messages in thread
From: Jeff King @ 2016-05-13 19:48 UTC (permalink / raw)
  To: Git List; +Cc: Armin Kunaschik, Eric Wong, Junio C Hamano

A few tests here use double-quotes around the snippets of
shell code to run the tests. None of these tests wants to do
any interpolation at all, and it just leads to an extra
layer of quoting around all double-quotes and dollar signs
inside the snippet.  Let's switch to single quotes, like
most other test scripts.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t9100-git-svn-basic.sh | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index 6ec73ee..28082b1 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -217,11 +217,11 @@ EOF
 
 test_expect_success POSIXPERM,SYMLINKS "$name" "test_cmp a expected"
 
-test_expect_success 'exit if remote refs are ambigious' "
+test_expect_success 'exit if remote refs are ambigious' '
         git config --add svn-remote.svn.fetch \
 		bar:refs/remotes/git-svn &&
 	test_must_fail git svn migrate
-"
+'
 
 test_expect_success 'exit if init-ing a would clobber a URL' '
         svnadmin create "${PWD}/svnrepo2" &&
@@ -259,26 +259,26 @@ test_expect_success 'dcommit $rev does not clobber current branch' '
 	git branch -D my-bar
 	'
 
-test_expect_success 'able to dcommit to a subdirectory' "
+test_expect_success 'able to dcommit to a subdirectory' '
 	git svn fetch -i bar &&
 	git checkout -b my-bar refs/remotes/bar &&
 	echo abc > d &&
 	git update-index --add d &&
-	git commit -m '/bar/d should be in the log' &&
+	git commit -m "/bar/d should be in the log" &&
 	git svn dcommit -i bar &&
-	test -z \"\$(git diff refs/heads/my-bar refs/remotes/bar)\" &&
+	test -z "$(git diff refs/heads/my-bar refs/remotes/bar)" &&
 	mkdir newdir &&
 	echo new > newdir/dir &&
 	git update-index --add newdir/dir &&
-	git commit -m 'add a new directory' &&
+	git commit -m "add a new directory" &&
 	git svn dcommit -i bar &&
-	test -z \"\$(git diff refs/heads/my-bar refs/remotes/bar)\" &&
+	test -z "$(git diff refs/heads/my-bar refs/remotes/bar)" &&
 	echo foo >> newdir/dir &&
 	git update-index newdir/dir &&
-	git commit -m 'modify a file in new directory' &&
+	git commit -m "modify a file in new directory" &&
 	git svn dcommit -i bar &&
-	test -z \"\$(git diff refs/heads/my-bar refs/remotes/bar)\"
-	"
+	test -z "$(git diff refs/heads/my-bar refs/remotes/bar)"
+'
 
 test_expect_success 'dcommit should not fail with a touched file' '
 	test_commit "commit-new-file-foo2" foo2 &&
@@ -291,13 +291,13 @@ test_expect_success 'rebase should not fail with a touched file' '
 	git svn rebase
 '
 
-test_expect_success 'able to set-tree to a subdirectory' "
+test_expect_success 'able to set-tree to a subdirectory' '
 	echo cba > d &&
 	git update-index d &&
-	git commit -m 'update /bar/d' &&
+	git commit -m "update /bar/d" &&
 	git svn set-tree -i bar HEAD &&
-	test -z \"\$(git diff refs/heads/my-bar refs/remotes/bar)\"
-	"
+	test -z "$(git diff refs/heads/my-bar refs/remotes/bar)"
+'
 
 test_expect_success 'git-svn works in a bare repository' '
 	mkdir bare-repo &&
-- 
2.8.2.825.gea31738

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] t9107: use "return 1" instead of "exit 1"
  2016-05-13 19:47 [PATCH 0/3] minor fixes to some svn tests Jeff King
  2016-05-13 19:48 ` [PATCH 1/3] t/lib-git-svn: drop $remote_git_svn and $git_svn_id Jeff King
  2016-05-13 19:48 ` [PATCH 2/3] t9100: enclose all test code in single-quotes Jeff King
@ 2016-05-13 19:50 ` Jeff King
  2016-05-13 20:49 ` [PATCH 0/3] minor fixes to some svn tests Jeff King
  3 siblings, 0 replies; 5+ messages in thread
From: Jeff King @ 2016-05-13 19:50 UTC (permalink / raw)
  To: Git List; +Cc: Armin Kunaschik, Eric Wong, Junio C Hamano

When a test runs a loop, it cannot rely on the usual
&&-chaining to propagate a failure inside the loop; it needs
to break out with a failure signal. However, unless you are
in a subshell, doing so with "exit 1" will exit the entire
test script, not just the test snippet we are in (and cause
the harness to complain that test_done was never reached).

So the fundamental point of this patch is s/exit/return/.
But while we're there, let's fix a number of style and
readability issues:

  - snippets in double-quotes need an extra layer of quoting
    for their meta-characters; let's avoid that by using
    single quotes

  - accumulating loop output by appending to a file in each
    iteration is brittle, as it can be affected by content
    left in the file by earlier tests. Instead, it's better
    to redirect stdout for the whole loop, so we know the
    output only comes from that loop.

  - using "test -z" to check that diff output is empty is
    overly verbose; we can just ask diff to use --exit-code.

  - we can factor out long lists of refs to make it more
    obvious we're using the same ones in each loop

  - subshells are unnecessary when ending an &&-chain with
    "|| return 1"

  - minor style fixups like space-after-redirection, and
    "do" and "done" on their own lines

Signed-off-by: Jeff King <peff@peff.net>
---
This covers all of the "|| exit 1" cases I could find. If you grep,
there are a number of them, but they are all inside subshells (where
it's the correct thing to use).

This is a bugfix, but it's not _that_ big a deal, in the sense that
using "exit 1" would definitely signal to the caller that the test
failed. :) But it disrupts things like the TAP output.

 t/t9107-git-svn-migrate.sh | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh
index 6efc2ab..2908aef 100755
--- a/t/t9107-git-svn-migrate.sh
+++ b/t/t9107-git-svn-migrate.sh
@@ -56,9 +56,11 @@ test_expect_success 'initialize a multi-repository repo' '
 	                        "^tags/\*:refs/remotes/origin/tags/\*$" &&
 	git config --add svn-remote.svn.fetch "branches/a:refs/remotes/origin/a" &&
 	git config --add svn-remote.svn.fetch "branches/b:refs/remotes/origin/b" &&
-	for i in tags/0.1 tags/0.2 tags/0.3; do
+	for i in tags/0.1 tags/0.2 tags/0.3
+	do
 		git config --add svn-remote.svn.fetch \
-		                 $i:refs/remotes/origin/$i || exit 1; done &&
+			$i:refs/remotes/origin/$i || return 1
+	done &&
 	git config --get-all svn-remote.svn.fetch > fetch.out &&
 	grep "^trunk:refs/remotes/origin/trunk$" fetch.out &&
 	grep "^branches/a:refs/remotes/origin/a$" fetch.out &&
@@ -70,30 +72,38 @@ test_expect_success 'initialize a multi-repository repo' '
 	'
 
 # refs should all be different, but the trees should all be the same:
-test_expect_success 'multi-fetch works on partial urls + paths' "
+test_expect_success 'multi-fetch works on partial urls + paths' '
+	refs="trunk a b tags/0.1 tags/0.2 tags/0.3" &&
 	git svn multi-fetch &&
-	for i in trunk a b tags/0.1 tags/0.2 tags/0.3; do
-		git rev-parse --verify refs/remotes/origin/\$i^0 >> refs.out || exit 1;
-	    done &&
-	test -z \"\$(sort < refs.out | uniq -d)\" &&
-	for i in trunk a b tags/0.1 tags/0.2 tags/0.3; do
-	  for j in trunk a b tags/0.1 tags/0.2 tags/0.3; do
-		if test \$j != \$i; then continue; fi
-	    test -z \"\$(git diff refs/remotes/origin/\$i \
-				 refs/remotes/origin/\$j)\" ||exit 1; done; done
-	"
+	for i in $refs
+	do
+		git rev-parse --verify refs/remotes/origin/$i^0 || return 1;
+	done >refs.out &&
+	test -z "$(sort <refs.out | uniq -d)" &&
+	>expect &&
+	for i in $refs
+	do
+		for j in $refs
+		do
+			git diff --exit-code refs/remotes/origin/$i refs/remotes/origin/$j ||
+				return 1
+		done
+	done
+'
 
 test_expect_success 'migrate --minimize on old inited layout' '
 	git config --unset-all svn-remote.svn.fetch &&
 	git config --unset-all svn-remote.svn.url &&
 	rm -rf "$GIT_DIR"/svn &&
-	for i in $(cat fetch.out); do
+	for i in $(cat fetch.out)
+	do
 		path=$(expr $i : "\([^:]*\):.*$")
 		ref=$(expr $i : "[^:]*:\(refs/remotes/.*\)$")
 		if test -z "$ref"; then continue; fi
 		if test -n "$path"; then path="/$path"; fi
-		( mkdir -p "$GIT_DIR"/svn/$ref/info/ &&
-		echo "$svnrepo"$path > "$GIT_DIR"/svn/$ref/info/url ) || exit 1;
+		mkdir -p "$GIT_DIR"/svn/$ref/info/ &&
+		echo "$svnrepo"$path >"$GIT_DIR"/svn/$ref/info/url ||
+		return 1
 	done &&
 	git svn migrate --minimize &&
 	test -z "$(git config -l | grep "^svn-remote\.git-svn\.")" &&
-- 
2.8.2.825.gea31738

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] minor fixes to some svn tests
  2016-05-13 19:47 [PATCH 0/3] minor fixes to some svn tests Jeff King
                   ` (2 preceding siblings ...)
  2016-05-13 19:50 ` [PATCH 3/3] t9107: use "return 1" instead of "exit 1" Jeff King
@ 2016-05-13 20:49 ` Jeff King
  3 siblings, 0 replies; 5+ messages in thread
From: Jeff King @ 2016-05-13 20:49 UTC (permalink / raw)
  To: Git List; +Cc: Armin Kunaschik, Eric Wong, Junio C Hamano

On Fri, May 13, 2016 at 03:47:16PM -0400, Jeff King wrote:

> On Fri, May 13, 2016 at 02:23:25PM -0400, Jeff King wrote:
> 
> > This covers all cases detected by:
> > 
> >   git grep 'test -z [^"]'
> > 
> > (though note that has a few false positives for tests which
> > need an extra layer of quoting to do '\"').
> 
> I looked at those false positives, and they're really gratuitous uses
> of double-quotes that cause the extra layer of quoting.
> 
> So I set off to fix that, but along the way found an actual bug (in the
> final patch here), along with a few minor readability fixups.
> 
>   [1/3]: t/lib-git-svn: drop $remote_git_svn and $git_svn_id
>   [2/3]: t9100: enclose all test code in single-quotes
>   [3/3]: t9107: use "return 1" instead of "exit 1"

Note that I ended up rolling these into the series posted at:

  http://thread.gmane.org/gmane.comp.version-control.git/294539/focus=294577

Or <20160513204654.GA10684@sigill.intra.peff.net>, since I know Eric has
his own list archive. :)

-Peff

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-05-13 20:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-13 19:47 [PATCH 0/3] minor fixes to some svn tests Jeff King
2016-05-13 19:48 ` [PATCH 1/3] t/lib-git-svn: drop $remote_git_svn and $git_svn_id Jeff King
2016-05-13 19:48 ` [PATCH 2/3] t9100: enclose all test code in single-quotes Jeff King
2016-05-13 19:50 ` [PATCH 3/3] t9107: use "return 1" instead of "exit 1" Jeff King
2016-05-13 20:49 ` [PATCH 0/3] minor fixes to some svn tests Jeff King

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.