All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Brian Gernhardt <brian@gernhardtsoftware.com>
Cc: Git List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Charles Bailey <charles@hashpling.org>
Subject: Re: [PATCH] t7610: cd inside subshell instead of around
Date: Mon, 23 Aug 2010 22:05:24 -0500	[thread overview]
Message-ID: <20100824030524.GF17406@burratino> (raw)
In-Reply-To: <1282617444-641-1-git-send-email-brian@gernhardtsoftware.com>

Brian Gernhardt wrote:

> Instead of using `cd dir && (...) && cd..` use `(cd dir && ...)`

Thank you.

> +++ b/t/t7610-mergetool.sh
> @@ -68,22 +68,24 @@ test_expect_success 'mergetool crlf' '
>  '
>  
>  test_expect_success 'mergetool in subdir' '
> -    git checkout -b test3 branch1
> -    cd subdir && (
> -    test_must_fail git merge master >/dev/null 2>&1 &&
> -    ( yes "" | git mergetool file3 >/dev/null 2>&1 ) &&
> -    test "$(cat file3)" = "master new sub") &&
> -    cd ..
> +    git checkout -b test3 branch1 &&
> +    (
> +        cd subdir &&
> +        test_must_fail git merge master >/dev/null 2>&1 &&
> +        ( yes "" | git mergetool file3 >/dev/null 2>&1 ) &&
> +        test "$(cat file3)" = "master new sub"
> +    )


While we're looking at this test script, some more nitpicks for
squashing in. :)

-- 8< --
Subject: t7610 (mergetool): more nitpicks

 - use tabs to indent
 - do not redirect output away unnecessarily
 - avoid a subshell for 'yes "" | git mergetool file3'
 - use test_tick for reproducible, increasing timestamps
 - use test_cmp instead of 'test $foo = bar'; the former is much
   nicer to debug with --verbose since it produces a diff.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t7610-mergetool.sh |  186 ++++++++++++++++++++++++++++---------------------
 1 files changed, 106 insertions(+), 80 deletions(-)

diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 3c436e1..908f934 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -3,112 +3,138 @@
 # Copyright (c) 2008 Charles Bailey
 #
 
-test_description='git mergetool
+test_description='Testing basic merge tool invocation
 
-Testing basic merge tool invocation'
+All the mergetool tests work by checking out a temporary branch based
+off branch1 and then merging in master and checking the results of
+running mergetool.
+'
 
 . ./test-lib.sh
 
-# All the mergetool test work by checking out a temporary branch based
-# off 'branch1' and then merging in master and checking the results of
-# running mergetool
-
 test_expect_success 'setup' '
-    git config rerere.enabled true &&
-    echo master >file1 &&
-    mkdir subdir &&
-    echo master sub >subdir/file3 &&
-    git add file1 subdir/file3 &&
-    git commit -m "added file1" &&
+	git config rerere.enabled true &&
+	echo master >file1 &&
+	mkdir subdir &&
+	echo master sub >subdir/file3 &&
+	git add file1 subdir/file3 &&
+	test_tick &&
+	git commit -m "added file1" &&
 
-    git checkout -b branch1 master &&
-    echo branch1 change >file1 &&
-    echo branch1 newfile >file2 &&
-    echo branch1 sub >subdir/file3 &&
-    git add file1 file2 subdir/file3 &&
-    git commit -m "branch1 changes" &&
+	git checkout -b branch1 master &&
+	echo branch1 change >file1 &&
+	echo branch1 newfile >file2 &&
+	echo branch1 sub >subdir/file3 &&
+	git add file1 file2 subdir/file3 &&
+	test_tick &&
+	git commit -m "branch1 changes" &&
 
-    git checkout master &&
-    echo master updated >file1 &&
-    echo master new >file2 &&
-    echo master new sub >subdir/file3 &&
-    git add file1 file2 subdir/file3 &&
-    git commit -m "master updates" &&
+	git checkout master &&
+	echo master updated >file1 &&
+	echo master new >file2 &&
+	echo master new sub >subdir/file3 &&
+	git add file1 file2 subdir/file3 &&
+	test_tick &&
+	git commit -m "master updates" &&
 
-    git config merge.tool mytool &&
-    git config mergetool.mytool.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
-    git config mergetool.mytool.trustExitCode true
+	git config merge.tool mytool &&
+	git config mergetool.mytool.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
+	git config mergetool.mytool.trustExitCode true
 '
 
 test_expect_success 'custom mergetool' '
-    git checkout -b test1 branch1 &&
-    test_must_fail git merge master >/dev/null 2>&1 &&
-    ( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
-    ( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
-    ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
-    test "$(cat file1)" = "master updated" &&
-    test "$(cat file2)" = "master new" &&
-    test "$(cat subdir/file3)" = "master new sub" &&
-    git commit -m "branch1 resolved with mergetool"
+	echo master updated >file1.expected &&
+	echo master new >file2.expected &&
+	echo master new sub >sub.expected &&
+
+	git checkout -b test1 branch1 &&
+	test_must_fail git merge master &&
+	yes "" | git mergetool file1 &&
+	yes "" | git mergetool file2 &&
+	yes "" | git mergetool subdir/file3 &&
+
+	test_cmp file1.expected file1 &&
+	test_cmp file2.expected file2 &&
+	test_cmp sub.expected subdir/file3 &&
+
+	git commit -m "branch1 resolved with mergetool"
 '
 
 test_expect_success 'mergetool crlf' '
-    git config core.autocrlf true &&
-    git checkout -b test2 branch1
-    test_must_fail git merge master >/dev/null 2>&1 &&
-    ( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
-    ( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
-    ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
-    test "$(printf x | cat file1 -)" = "$(printf "master updated\r\nx")" &&
-    test "$(printf x | cat file2 -)" = "$(printf "master new\r\nx")" &&
-    test "$(printf x | cat subdir/file3 -)" = "$(printf "master new sub\r\nx")" &&
-    git commit -m "branch1 resolved with mergetool - autocrlf" &&
-    git config core.autocrlf false &&
-    git reset --hard
+	git config core.autocrlf true &&
+	test_when_finished "git config --unset core.autocrlf" &&
+	echo master updated | append_cr >file1.expected &&
+	echo master new | append_cr >file2.expected &&
+	echo master new sub | append_cr >sub.expected &&
+
+	git checkout -b test2 branch1 &&
+	test_must_fail git merge master &&
+	test_when_finished "git reset --hard" &&
+	yes "" | git mergetool file1 &&
+	yes "" | git mergetool file2 &&
+	yes "" | git mergetool subdir/file3 &&
+
+	test_cmp file1.expected file1 &&
+	test_cmp file2.expected file2 &&
+	test_cmp sub.expected subdir/file3 &&
+
+	git commit -m "branch1 resolved with mergetool - autocrlf"
 '
 
 test_expect_success 'mergetool in subdir' '
-    git checkout -b test3 branch1 &&
-    (
-        cd subdir &&
-        test_must_fail git merge master >/dev/null 2>&1 &&
-        ( yes "" | git mergetool file3 >/dev/null 2>&1 ) &&
-        test "$(cat file3)" = "master new sub"
-    )
+	echo master new sub >sub.expected &&
+	git checkout -b test3 branch1 &&
+	(
+		cd subdir &&
+		test_must_fail git merge master &&
+		yes "" | git mergetool file3 &&
+		test_cmp ../sub.expected file3
+	)
 '
 
 test_expect_success 'mergetool on file in parent dir' '
-    (
-        cd subdir && 
-        ( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) &&
-        ( yes "" | git mergetool ../file2 >/dev/null 2>&1 ) &&
-        test "$(cat ../file1)" = "master updated" &&
-        test "$(cat ../file2)" = "master new" &&
-        git commit -m "branch1 resolved with mergetool - subdir"
-    )
+	echo master updated >file1.expected &&
+	echo master new >file2.expected &&
+	(
+		cd subdir &&
+		yes "" | git mergetool ../file1 &&
+		yes "" | git mergetool ../file2 &&
+		test_cmp ../file1.expected ../file1 &&
+		test_cmp ../file2.expected ../file2 &&
+		git commit -m "branch1 resolved with mergetool - subdir"
+	)
 '
 
 test_expect_success 'mergetool skips autoresolved' '
-    git checkout -b test4 branch1 &&
-    test_must_fail git merge master &&
-    test -n "$(git ls-files -u)" &&
-    output="$(git mergetool --no-prompt)" &&
-    test "$output" = "No files need merging" &&
-    git reset --hard
+	echo "No files need merging" >expected &&
+	git checkout -b test4 branch1 &&
+	test_must_fail git merge master &&
+	test_when_finished "git reset --hard" &&
+	test_must_fail git update-index --refresh &&
+	git mergetool --no-prompt >actual &&
+	test_cmp expected actual
 '
 
 test_expect_success 'mergetool merges all from subdir' '
-    (
-        cd subdir &&
-        git config rerere.enabled false &&
-        test_must_fail git merge master &&
-        git mergetool --no-prompt &&
-        test "$(cat ../file1)" = "master updated" &&
-        test "$(cat ../file2)" = "master new" &&
-        test "$(cat file3)" = "master new sub" &&
-        git add ../file1 ../file2 file3 &&
-        git commit -m "branch2 resolved by mergetool from subdir"
-    )
+	echo master updated >file1.expected &&
+	echo master new >file2.expected &&
+	echo master new sub >sub.expected &&
+
+	git config rerere.enabled false &&
+	test_when_finished "git config rerere.enabled true" &&
+	mv .git/rr-cache .git/rr-cache-moved &&
+	test_when_finished "mv .git/rr-cache-moved .git/rr-cache" &&
+	(
+		cd subdir &&
+		test_must_fail git merge master &&
+		test_when_finished "git reset --hard" &&
+		git mergetool --no-prompt &&
+		test_cmp ../file1.expected ../file1 &&
+		test_cmp ../file2.expected ../file2 &&
+		test_cmp ../sub.expected file3 &&
+		git add ../file1 ../file2 file3 &&
+		git commit -m "branch2 resolved by mergetool from subdir"
+	)
 '
 
 test_done
-- 
1.7.2.2

  reply	other threads:[~2010-08-24  3:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-24  2:37 [PATCH] t7610: cd inside subshell instead of around Brian Gernhardt
2010-08-24  3:05 ` Jonathan Nieder [this message]
2010-08-24  3:51   ` Brian Gernhardt
2010-08-24  5:03     ` Jonathan Nieder
2010-08-25  0:25   ` [PATCH v2] t7610 (mergetool): more nitpicks Jonathan Nieder
2010-08-25  7:40     ` David Aguilar
2010-08-25  7:56       ` Charles Bailey
2010-08-26  8:38         ` David Aguilar

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=20100824030524.GF17406@burratino \
    --to=jrnieder@gmail.com \
    --cc=brian@gernhardtsoftware.com \
    --cc=charles@hashpling.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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 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.