[PATCH] t7610: cd inside subshell instead of around

[PATCH] t7610: cd inside subshell instead of around

[Brian Gernhardt]

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

This ensures that the test doesn't get caught in the subdirectory if
there is an error in the subshell.

Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---
 t/t7610-mergetool.sh |   49 ++++++++++++++++++++++++++-----------------------
 1 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index f5a7bf4..3c436e1 100755
--- a/t/t7610-mergetool.sh
+++ 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"
+    )
 '
 
 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") &&
-    cd ..
+    (
+        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"
+    )
 '
 
 test_expect_success 'mergetool skips autoresolved' '
@@ -96,16 +98,17 @@ test_expect_success 'mergetool skips autoresolved' '
 '
 
 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") &&
-    cd ..
+    (
+        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"
+    )
 '
 
 test_done
-- 
1.7.2.2.399.g628fd

Re: [PATCH] t7610: cd inside subshell instead of around

[Jonathan Nieder]

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

Re: [PATCH] t7610: cd inside subshell instead of around

[Brian Gernhardt]

On Aug 23, 2010, at 11:05 PM, Jonathan Nieder wrote:

> 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.

Fair enough.  I just took a stab at the bit that irritated me the most.  Thanks for cleaning up the rest.

~~ Brian

Re: [PATCH] t7610: cd inside subshell instead of around

[Jonathan Nieder]

Brian Gernhardt wrote:

> Thanks for cleaning up the rest.

Glad you liked it.  BTW I did not mean to imply your cleanup was
half-hearted or anything.  On the contrary, I was very happy with it.

[PATCH v2] t7610 (mergetool): more nitpicks

[Jonathan Nieder]

 - 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.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jonathan Nieder wrote:

>  test_expect_success 'mergetool crlf' '
>      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
> +	git checkout -b test2 branch1 &&
> -    test_must_fail git merge master >/dev/null 2>&1 &&
> +	test_must_fail git merge master &&
> +	test_when_finished "git reset --hard" &&
> -    ( 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 ) &&
> +	yes "" | git mergetool file1 &&
> +	yes "" | git mergetool file2 &&
> +	yes "" | git mergetool subdir/file3 &&
> -    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")" &&
> +
> +	test_cmp file1.expected file1 &&
> +	test_cmp file2.expected file2 &&
> +	test_cmp sub.expected subdir/file3 &&
> -    git commit -m "branch1 resolved with mergetool - autocrlf" &&
> +
> +	git commit -m "branch1 resolved with mergetool - autocrlf"
> -    git config core.autocrlf false &&
> -    git reset --hard
>  '

As Junio noticed, the net effect is to run "reset --hard" before
the "[core] autocrlf" configuration is removed, which could be
an undesirable behavior change.  So this version runs "reset --hard"
after unsetting autocrlf for good measure.

 t/t7610-mergetool.sh |  187 +++++++++++++++++++++++++++++---------------------
 1 files changed, 108 insertions(+), 79 deletions(-)

diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 3bd7404..a7f05b0 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -3,112 +3,141 @@
 # 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 &&
+		git reset --hard
+	" &&
+	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 &&
+	echo master updated >file1.expected &&
+	echo master new >file2.expected &&
+	echo master new sub >sub.expected &&
+
 	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"
-    )
+	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

Re: [PATCH v2] t7610 (mergetool): more nitpicks

[David Aguilar]

On Tue, Aug 24, 2010 at 07:25:52PM -0500, Jonathan Nieder wrote:
>  - use tabs to indent
> [...]

Cool.  I'd like to do the same to git-mergetool.sh too.
Until now I've stuck to the existing style.
My editor is setup to display tabs visually which makes it easy
for me to emulate existing code but not everyone does that.

Any objections to a patch that replaces the mixed 4-space+tab
indents with pure tabs?

It would make the git-mergetool consistent with
git-mergetool--lib, git-difftool--helper, and their tests.

I don't know if pure-tabs is the preferred style for *.sh
scripts.  It's not explicitly mentioned in CodingGuidelines'
shell script section.  Updating all of the *.sh scripts, though,
seems like code churn so I wouldn't recommend that unless we
were going to be changing the scripts anyways.

-- 

	David

Re: [PATCH v2] t7610 (mergetool): more nitpicks

[Charles Bailey]

On Wed, Aug 25, 2010 at 12:40:38AM -0700, David Aguilar wrote:
> On Tue, Aug 24, 2010 at 07:25:52PM -0500, Jonathan Nieder wrote:
> >  - use tabs to indent
> > [...]
> 
> Cool.  I'd like to do the same to git-mergetool.sh too.
> Until now I've stuck to the existing style.
> My editor is setup to display tabs visually which makes it easy
> for me to emulate existing code but not everyone does that.
> 
> Any objections to a patch that replaces the mixed 4-space+tab
> indents with pure tabs?

Just the same objection as the last time:

http://thread.gmane.org/gmane.comp.version-control.git/115069/focus=115192

We've already tidyied up mergetool to be consistent, I don't see what
touching 90% of the lines achieves other than getting yourself
'blamed' for everything in mergetool.

Charles.

Re: [PATCH v2] t7610 (mergetool): more nitpicks

[David Aguilar]

On Aug 25, 2010, at 12:56 AM, Charles Bailey <charles@hashpling.org>  
wrote:

> On Wed, Aug 25, 2010 at 12:40:38AM -0700, David Aguilar wrote:
>> On Tue, Aug 24, 2010 at 07:25:52PM -0500, Jonathan Nieder wrote:
>>> - use tabs to indent
>>> [...]
>>
>> Cool.  I'd like to do the same to git-mergetool.sh too.
>> Until now I've stuck to the existing style.
>> My editor is setup to display tabs visually which makes it easy
>> for me to emulate existing code but not everyone does that.
>>
>> Any objections to a patch that replaces the mixed 4-space+tab
>> indents with pure tabs?
>
> Just the same objection as the last time:
>
> http://thread.gmane.org/gmane.comp.version-control.git/115069/focus=115192
>
> We've already tidyied up mergetool to be consistent, I don't see what
> touching 90% of the lines achieves other than getting yourself
> 'blamed' for everything in mergetool.
>
> Charles.

Too true.

Instead of that I should probably get around to factoring out the tool- 
specific parts like we discussed in the past.

Thanks Charles,
-- 
         David