[PATCHv1] git-p4: test harness directory handling tidyup

[PATCHv1] git-p4: test harness directory handling tidyup

[Luke Diamand]

Junio pointed out that the git-p4 test harness (t9800-git-p4.sh) has
tests that 'cd' into a directory, do stuff, and then cd back out. If
the tests fail partway through then the process is left in the wrong
directory for the next test.

This patch changes it to use the "(cd dir && do_test)" pattern instead.

Luke Diamand (1):
  git-p4: test harness directory changing tidyup

 t/t9800-git-p4.sh |   37 ++++++++++++++++++++++---------------
 1 files changed, 22 insertions(+), 15 deletions(-)

[PATCHv1] git-p4: test harness directory handling tidyup

[Luke Diamand]

The git-p4 test harness relied a lot on cd'ing to the target directory
and then cd'ing back explicitly. That caused problems if the test failed
partway through. i.e.
  cd $git && stuff && cd "$TRASH_DIRECTORY"

Instead, use:
  (cd $git && stuff)

Signed-off-by: Luke Diamand <luke@diamand.org>
Suggested-by: Junio C Hamano <gitster@pobox.com>
---
 t/t9800-git-p4.sh |   37 ++++++++++++++++++++++---------------
 1 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 888ad54..122e91d 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -28,6 +28,8 @@ test_expect_success setup '
 '
 
 test_expect_success 'add p4 files' '
+	export P4CLIENT=client &&
+	(
 	cd "$cli" &&
 	p4 client -i <<-EOF &&
 	Client: client
@@ -35,14 +37,13 @@ test_expect_success 'add p4 files' '
 	Root: $cli
 	View: //depot/... //client/...
 	EOF
-	export P4CLIENT=client &&
 	echo file1 >file1 &&
 	p4 add file1 &&
 	p4 submit -d "file1" &&
 	echo file2 >file2 &&
 	p4 add file2 &&
-	p4 submit -d "file2" &&
-	cd "$TRASH_DIRECTORY"
+	p4 submit -d "file2"
+	)
 '
 
 test_expect_success 'basic git-p4 clone' '
@@ -100,34 +101,37 @@ test_expect_success 'exit when p4 fails to produce marshaled output' '
 '
 
 test_expect_success 'add p4 files with wildcards in the names' '
+	(
 	cd "$cli" &&
 	echo file-wild-hash >file-wild#hash &&
 	echo file-wild-star >file-wild\*star &&
 	echo file-wild-at >file-wild@at &&
 	echo file-wild-percent >file-wild%percent &&
 	p4 add -f file-wild* &&
-	p4 submit -d "file wildcards" &&
-	cd "$TRASH_DIRECTORY"
+	p4 submit -d "file wildcards"
+	)
 '
 
 test_expect_success 'wildcard files git-p4 clone' '
 	"$GITP4" clone --dest="$git" //depot &&
+	(
 	cd "$git" &&
 	test -f file-wild#hash &&
 	test -f file-wild\*star &&
 	test -f file-wild@at &&
-	test -f file-wild%percent &&
-	cd "$TRASH_DIRECTORY" &&
+	test -f file-wild%percent
+	)
 	rm -rf "$git" && mkdir "$git"
 '
 
 test_expect_success 'clone bare' '
 	"$GITP4" clone --dest="$git" --bare //depot &&
+	(
 	cd "$git" &&
 	test ! -d .git &&
 	bare=`git config --get core.bare` &&
-	test "$bare" = true &&
-	cd "$TRASH_DIRECTORY" &&
+	test "$bare" = true
+	)
 	rm -rf "$git" && mkdir "$git"
 '
 
@@ -175,6 +179,7 @@ test_expect_success 'preserve users' '
 	p4_add_user bob Bob &&
 	p4_grant_admin alice &&
 	"$GITP4" clone --dest="$git" //depot &&
+	(
 	cd "$git" &&
 	echo "username: a change by alice" >> file1 &&
 	echo "username: a change by bob" >> file2 &&
@@ -183,8 +188,8 @@ test_expect_success 'preserve users' '
 	git config git-p4.skipSubmitEditCheck true &&
 	P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
 	p4_check_commit_author file1 alice &&
-	p4_check_commit_author file2 bob &&
-	cd "$TRASH_DIRECTORY" &&
+	p4_check_commit_author file2 bob
+	)
 	rm -rf "$git" && mkdir "$git"
 '
 
@@ -192,18 +197,20 @@ test_expect_success 'preserve users' '
 # not submit change to p4 (git diff should show deltas).
 test_expect_success 'refuse to preserve users without perms' '
 	"$GITP4" clone --dest="$git" //depot &&
+	(
 	cd "$git" &&
 	echo "username-noperms: a change by alice" >> file1 &&
 	git commit --author "Alice <alice@localhost>" -m "perms: a change by alice" file1 &&
 	! P4EDITOR=touch P4USER=bob P4PASSWD=secret "$GITP4" commit --preserve-user &&
-	! git diff --exit-code HEAD..p4/master > /dev/null &&
-	cd "$TRASH_DIRECTORY" &&
+	! git diff --exit-code HEAD..p4/master > /dev/null
+	)
 	rm -rf "$git" && mkdir "$git"
 '
 
 # What happens with unknown author? Without allowMissingP4Users it should fail.
 test_expect_success 'preserve user where author is unknown to p4' '
 	"$GITP4" clone --dest="$git" //depot &&
+	(
 	cd "$git" &&
 	git config git-p4.skipSubmitEditCheck true
 	echo "username-bob: a change by bob" >> file1 &&
@@ -217,8 +224,8 @@ test_expect_success 'preserve user where author is unknown to p4' '
 	git config git-p4.preserveUser true &&
 	P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit &&
 	git diff --exit-code HEAD..p4/master > /dev/null &&
-	p4_check_commit_author file1 alice &&
-	cd "$TRASH_DIRECTORY" &&
+	p4_check_commit_author file1 alice
+	)
 	rm -rf "$git" && mkdir "$git"
 '
 
-- 
1.7.1

Re: [PATCHv1] git-p4: test harness directory handling tidyup

[Pete Wyckoff]

luke@diamand.org wrote on Thu, 12 May 2011 06:14 +0100:
> The git-p4 test harness relied a lot on cd'ing to the target directory
> and then cd'ing back explicitly. That caused problems if the test failed
> partway through. i.e.
>   cd $git && stuff && cd "$TRASH_DIRECTORY"
> 
> Instead, use:
>   (cd $git && stuff)
> 
> Signed-off-by: Luke Diamand <luke@diamand.org>
> Suggested-by: Junio C Hamano <gitster@pobox.com>

I'm glad that Junio noticed and that you took the time to fix
this.  It was indeed a problem.

Acked-by: Pete Wyckoff <pw@padd.com>