git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Emeric Fermas <emeric.fermas@gmail.com>
Subject: [PATCH 1/3] t5701: modernize style
Date: Fri, 25 May 2012 23:42:53 -0400	[thread overview]
Message-ID: <20120526034253.GA14332@sigill.intra.peff.net> (raw)
In-Reply-To: <20120526034226.GA14287@sigill.intra.peff.net>

This test is pretty old and did not follow some of our more
modern best practices. In particular:

  1. It chdir'd all over the place, leaving later tests to
     deal with the fallout. Do our chdirs in subshells
     instead.

  2. It did not use test_must_fail.

  3. It did not use test_line_count.

  4. It checked for the non-existence of a ref by looking in the
     .git/refs directory (since we pack refs during clone
     these days, this will always be succeed, making the
     test useless).

     Note that one call to "-e .git/refs/..." remains,
     because it is checking for the existence of a symbolic
     ref, not a ref itself.

Signed-off-by: Jeff King <peff@peff.net>
---
I mostly wanted to pull out the repo_is_hardlinked logic for tests I was
about to add, but once I got cleaning, I couldn't stop. And who can
argue with that diffstat?

 t/t5701-clone-local.sh | 73 ++++++++++++++------------------------------------
 1 file changed, 20 insertions(+), 53 deletions(-)

diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
index 6972258..c6feca4 100755
--- a/t/t5701-clone-local.sh
+++ b/t/t5701-clone-local.sh
@@ -3,7 +3,10 @@
 test_description='test local clone'
 . ./test-lib.sh
 
-D=`pwd`
+repo_is_hardlinked() {
+	find "$1/objects" -type f -links 1 >output &&
+	test_line_count = 0 output
+}
 
 test_expect_success 'preparing origin repository' '
 	: >file && git add . && git commit -m1 &&
@@ -19,105 +22,72 @@ test_expect_success 'preparing origin repository' '
 '
 
 test_expect_success 'local clone without .git suffix' '
-	cd "$D" &&
 	git clone -l -s a b &&
-	cd b &&
+	(cd b &&
 	test "$(GIT_CONFIG=.git/config git config --bool core.bare)" = false &&
-	git fetch
+	git fetch)
 '
 
 test_expect_success 'local clone with .git suffix' '
-	cd "$D" &&
 	git clone -l -s a.git c &&
-	cd c &&
-	git fetch
+	(cd c && git fetch)
 '
 
 test_expect_success 'local clone from x' '
-	cd "$D" &&
 	git clone -l -s x y &&
-	cd y &&
-	git fetch
+	(cd y && git fetch)
 '
 
 test_expect_success 'local clone from x.git that does not exist' '
-	cd "$D" &&
-	if git clone -l -s x.git z
-	then
-		echo "Oops, should have failed"
-		false
-	else
-		echo happy
-	fi
+	test_must_fail git clone -l -s x.git z
 '
 
 test_expect_success 'With -no-hardlinks, local will make a copy' '
-	cd "$D" &&
 	git clone --bare --no-hardlinks x w &&
-	cd w &&
-	linked=$(find objects -type f ! -links 1 | wc -l) &&
-	test 0 = $linked
+	! repo_is_hardlinked w
 '
 
 test_expect_success 'Even without -l, local will make a hardlink' '
-	cd "$D" &&
 	rm -fr w &&
 	git clone -l --bare x w &&
-	cd w &&
-	copied=$(find objects -type f -links 1 | wc -l) &&
-	test 0 = $copied
+	repo_is_hardlinked w
 '
 
 test_expect_success 'local clone of repo with nonexistent ref in HEAD' '
-	cd "$D" &&
 	echo "ref: refs/heads/nonexistent" > a.git/HEAD &&
 	git clone a d &&
-	cd d &&
+	(cd d &&
 	git fetch &&
-	test ! -e .git/refs/remotes/origin/HEAD'
+	test ! -e .git/refs/remotes/origin/HEAD)
+'
 
 test_expect_success 'bundle clone without .bundle suffix' '
-	cd "$D" &&
 	git clone dir/b3 &&
-	cd b3 &&
-	git fetch
+	(cd b3 && git fetch)
 '
 
 test_expect_success 'bundle clone with .bundle suffix' '
-	cd "$D" &&
 	git clone b1.bundle &&
-	cd b1 &&
-	git fetch
+	(cd b1 && git fetch)
 '
 
 test_expect_success 'bundle clone from b4' '
-	cd "$D" &&
 	git clone b4 bdl &&
-	cd bdl &&
-	git fetch
+	(cd bdl && git fetch)
 '
 
 test_expect_success 'bundle clone from b4.bundle that does not exist' '
-	cd "$D" &&
-	if git clone b4.bundle bb
-	then
-		echo "Oops, should have failed"
-		false
-	else
-		echo happy
-	fi
+	test_must_fail git clone b4.bundle bb
 '
 
 test_expect_success 'bundle clone with nonexistent HEAD' '
-	cd "$D" &&
 	git clone b2.bundle b2 &&
-	cd b2 &&
+	(cd b2 &&
 	git fetch &&
-	test ! -e .git/refs/heads/master
+	test_must_fail git rev-parse --verify refs/heads/master)
 '
 
 test_expect_success 'clone empty repository' '
-	cd "$D" &&
 	mkdir empty &&
 	(cd empty &&
 	 git init &&
@@ -135,7 +105,6 @@ test_expect_success 'clone empty repository' '
 '
 
 test_expect_success 'clone empty repository, and then push should not segfault.' '
-	cd "$D" &&
 	rm -fr empty/ empty-clone/ &&
 	mkdir empty &&
 	(cd empty && git init) &&
@@ -145,13 +114,11 @@ test_expect_success 'clone empty repository, and then push should not segfault.'
 '
 
 test_expect_success 'cloning non-existent directory fails' '
-	cd "$D" &&
 	rm -rf does-not-exist &&
 	test_must_fail git clone does-not-exist
 '
 
 test_expect_success 'cloning non-git directory fails' '
-	cd "$D" &&
 	rm -rf not-a-git-repo not-a-git-repo-clone &&
 	mkdir not-a-git-repo &&
 	test_must_fail git clone not-a-git-repo not-a-git-repo-clone
-- 
1.7.10.1.21.g62fda49.dirty

  reply	other threads:[~2012-05-26  3:43 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-26  3:42 [PATCH 0/3] clone --local fixes Jeff King
2012-05-26  3:42 ` Jeff King [this message]
2012-05-26  3:45 ` [PATCH 2/3] clone: make --local handle URLs Jeff King
2012-05-28 18:31   ` Johannes Sixt
2012-05-28 19:10     ` Jeff King
2012-05-26  3:45 ` [PATCH 3/3] clone: allow --no-local to turn off local optimizations Jeff King
2012-05-26  4:11 ` [PATCH 4/3] clone: send diagnostic messages to stderr Jeff King
2012-05-27  6:32 ` [PATCH 0/3] clone --local fixes Junio C Hamano
2012-05-28  5:36   ` Jeff King
2012-05-29 17:43     ` Junio C Hamano
2012-05-30 11:03       ` Jeff King
2012-05-30 11:08         ` Jeff King
2012-05-30 11:09         ` [PATCH 1/2] docs/clone: mention that --local may be ignored Jeff King
2012-05-30 11:10         ` [PATCH 2/2] clone: allow --no-local to turn off local optimizations Jeff King
2012-05-30 17:20           ` Junio C Hamano
2012-05-30 21:59             ` Jeff King
2012-05-30 22:10               ` Junio C Hamano
2012-05-30 23:21                 ` Jeff King
2012-05-30 23:33                   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120526034253.GA14332@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=emeric.fermas@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).