From e1c0c158b13bf78486d6cc2766d6cb69782c5173 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 13 May 2016 16:47:14 -0400 Subject: [PATCH 1/6] t/lib-git-svn: drop $remote_git_svn and $git_svn_id 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 Signed-off-by: Junio C Hamano --- 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 6a50b8793e..fb8823224e 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 22d8367ff3..6ec73ee5d7 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 e8173d5fef..07bfb63777 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 eb70f4839c..66cd51102c 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 f6d7ac7c5f..dbe8deac0d 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 9060198037..6efc2ab4d1 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 29fbdfdd3f..dde0a3c222 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 bd081c2ec3..22b6e5ee7d 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 1c84ce1023..59465b147e 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 fd8184787f..ead404589e 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 88a2cfa233..372ef15685 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 From 577dfd03024f1913c6eba63b43a56d254e694347 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 13 May 2016 16:47:18 -0400 Subject: [PATCH 2/6] t9100,t3419: enclose all test code in single-quotes 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 Signed-off-by: Junio C Hamano --- t/t3419-rebase-patch-id.sh | 12 ++++++------ t/t9100-git-svn-basic.sh | 28 ++++++++++++++-------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh index 217dd79b2e..49f548cdb9 100755 --- a/t/t3419-rebase-patch-id.sh +++ b/t/t3419-rebase-patch-id.sh @@ -73,17 +73,17 @@ do_tests () { run git format-patch --stdout --ignore-if-in-upstream master " - test_expect_success $pr 'detect upstream patch' " + test_expect_success $pr 'detect upstream patch' ' git checkout -q master && scramble file && git add file && - git commit -q -m 'change big file again' && + git commit -q -m "change big file again" && git checkout -q other^{} && git rebase master && - test_must_fail test -n \"\$(git rev-list master...HEAD~)\" - " + test_must_fail test -n "$(git rev-list master...HEAD~)" + ' - test_expect_success $pr 'do not drop patch' " + test_expect_success $pr 'do not drop patch' ' git branch -f squashed master && git checkout -q -f squashed && git reset -q --soft HEAD~2 && @@ -91,7 +91,7 @@ do_tests () { git checkout -q other^{} && test_must_fail git rebase squashed && rm -rf .git/rebase-apply - " + ' } do_tests 500 diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh index 6ec73ee5d7..28082b134f 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 && From f831acc6c6839426e6ccb42d7fd1931863bb07ac Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 13 May 2016 16:47:21 -0400 Subject: [PATCH 3/6] t9107: use "return 1" instead of "exit 1" 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 Signed-off-by: Junio C Hamano --- 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 6efc2ab4d1..b6a99b807f 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 "$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\.")" && From 9874576995c8ecf800e48dead0e34c36defa91b4 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 13 May 2016 16:47:24 -0400 Subject: [PATCH 4/6] t9107: switch inverted single/double quotes in test One of the test snippets in t9107 is enclosed in double quotes, but then uses single quotes to surround an interpolated variable inside the snippet, like: test_expect_success '...' " test -n '$head' " This happens to work because the variable is interpolated _before_ the snippet is run, and the result is eval'd. So as long as the variable does not contain any single quotes, the two are equivalent. And it doesn't, as we know it is a sha1 from rev-parse above. But this construct is unnecessarily confusing. But we can go a step further in cleaning up. The test is really checking that a particular ref has a value. Rather than checking if rev-parse produced output, we can just move rev-parse into the test itself, and rely on the exit code from --verify. Nobody else cares about the $head variable at all. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t9107-git-svn-migrate.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh index b6a99b807f..9f3ef8f2ef 100755 --- a/t/t9107-git-svn-migrate.sh +++ b/t/t9107-git-svn-migrate.sh @@ -24,8 +24,9 @@ test_expect_success 'setup old-looking metadata' ' git update-ref -d refs/remotes/git-svn refs/remotes/git-svn ' -head=$(git rev-parse --verify refs/heads/git-svn-HEAD^0) -test_expect_success 'git-svn-HEAD is a real HEAD' "test -n '$head'" +test_expect_success 'git-svn-HEAD is a real HEAD' ' + git rev-parse --verify refs/heads/git-svn-HEAD^0 +' svnrepo_escaped=$(echo $svnrepo | sed 's/ /%20/') From 2a86cb6dc4fbc894df1e858d94cdd45c26ccf9d0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 13 May 2016 16:47:28 -0400 Subject: [PATCH 5/6] t9103: modernize test style The main goal here was to avoid double-quotes for surrounding the test snippet, since it makes the code hard to read (and to grep for common problems). But while we're here, we can fix a few other things: - use test_path_* helpers, which are more robust and give better error messages - only "cd" inside a subshell, which leaves the environment pristine if further tests are added - consistently quote shell arguments. These aren't wrong if we assume find-rev output doesn't have any whitespace, but it doesn't hurt to be careful. - replace the old-style 'test x$foo = x' with 'test -z "$foo"'. Besides the quoting fix, this is the form we generally use in our test suite. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t9103-git-svn-tracked-directory-removed.sh | 22 +++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/t/t9103-git-svn-tracked-directory-removed.sh b/t/t9103-git-svn-tracked-directory-removed.sh index 3413164cb1..b28271345c 100755 --- a/t/t9103-git-svn-tracked-directory-removed.sh +++ b/t/t9103-git-svn-tracked-directory-removed.sh @@ -23,17 +23,19 @@ test_expect_success 'make history for tracking' ' test_expect_success 'clone repo with git' ' git svn clone -s "$svnrepo" x && - test -f x/FOLLOWME && - test ! -f x/README + test_path_is_file x/FOLLOWME && + test_path_is_missing x/README ' -test_expect_success 'make sure r2 still has old file' " - cd x && - test -n \"\$(git svn find-rev r1)\" && - git reset --hard \$(git svn find-rev r1) && - test -f README && - test ! -f FOLLOWME && - test x\$(git svn find-rev r2) = x -" +test_expect_success 'make sure r2 still has old file' ' + ( + cd x && + test -n "$(git svn find-rev r1)" && + git reset --hard "$(git svn find-rev r1)" && + test_path_is_file README && + test_path_is_missing FOLLOWME && + test -z "$(git svn find-rev r2)" + ) +' test_done From 268ef4d3d07e4cf9909ac060b63b195509391190 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 13 May 2016 16:47:33 -0400 Subject: [PATCH 6/6] always quote shell arguments to test -z/-n In shell code like: test -z $foo test -n $foo that does not quote its arguments, it's easy to think that it is actually looking at the contents of $foo in each case. But if $foo is empty, then "test" does not see any argument at all! The results are quite subtle. POSIX specifies that test's behavior depends on the number of arguments it sees, and if $foo is empty, it sees only one. The behavior in this case is: 1 argument: Exit true (0) if $1 is not null; otherwise, exit false. So in the "-z $foo" case, if $foo is empty, then we check that "-z" is non-null, and it returns success. Which happens to match what we expected. But for "-n $foo", if $foo is empty, we'll see that "-n" is non-null and still return success. That's the opposite of what we intended! Furthermore, if $foo contains whitespace, we'll end up with more than 2 arguments. The results in this case are generally unspecified (unless the first part of $foo happens to be a valid binary operator, in which case the results are specified but certainly not what we intended). And on top of this, even though "test -z $foo" _should_ work for the empty case, some older shells (reportedly ksh88) complain about the missing argument. So let's make sure we consistently quote our variable arguments to "test". After this patch, the results of: git grep 'test -[zn] [^"]' are empty. Reported-by: Armin Kunaschik Helped-by: Junio C Hamano Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-rebase--interactive.sh | 4 ++-- git-stash.sh | 4 ++-- t/t4151-am-abort.sh | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 9ea30756f1..470413b9f9 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -866,12 +866,12 @@ add_exec_commands () { # $3: the input filename check_commit_sha () { badsha=0 - if test -z $1 + if test -z "$1" then badsha=1 else sha1_verif="$(git rev-parse --verify --quiet $1^{commit})" - if test -z $sha1_verif + if test -z "$sha1_verif" then badsha=1 fi diff --git a/git-stash.sh b/git-stash.sh index c7c65e25f5..c7509e8da4 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -185,7 +185,7 @@ store_stash () { git update-ref --create-reflog -m "$stash_msg" $ref_stash $w_commit ret=$? - test $ret != 0 && test -z $quiet && + test $ret != 0 && test -z "$quiet" && die "$(eval_gettext "Cannot update \$ref_stash with \$w_commit")" return $ret } @@ -277,7 +277,7 @@ save_stash () { git clean --force --quiet -d $CLEAN_X_OPTION fi - if test "$keep_index" = "t" && test -n $i_tree + if test "$keep_index" = "t" && test -n "$i_tree" then git read-tree --reset -u $i_tree fi diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index ea5ace99a1..9473c2779e 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -82,7 +82,7 @@ test_expect_success 'am -3 --abort removes otherfile-4' ' test 4 = "$(cat otherfile-4)" && git am --abort && test_cmp_rev initial HEAD && - test -z $(git ls-files -u) && + test -z "$(git ls-files -u)" && test_path_is_missing otherfile-4 '