Re: [PATCH v2 2/5] Use -y where possible in test t7610-mergetool

[Denton Liu <liu.denton@gmail.com>]

On Fri, Mar 03, 2017 at 11:39:30AM -0800, Junio C Hamano wrote:
> Denton Liu <liu.denton@gmail.com> writes:
> 
> > In these tests, there are many situations where
> > 'echo "" | git mergetool' is used. This replaces all of those
> > occurrences with 'git mergetool -y' for simplicity and readability.
> 
> "-y where _possible_" is not necessarily a good thing to do in
> tests.  We do want to make sure that both "yes" from the input and
> "-y" from the command line work.  Changes to "-y" done only for the
> tests that are not about accepting the interactive input from the
> end-user is a very good idea, but "replaces all of those" makes me
> worried.
The 'custom mergetool' test case seems like a good place to introduce an
interactive test. Would the following patch to my patch work?

diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index b6a419258..71624583c 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -135,8 +135,8 @@ test_expect_success 'custom mergetool' '
 	test_expect_code 1 git merge master >/dev/null 2>&1 &&
 	git mergetool -y both >/dev/null 2>&1 &&
 	git mergetool -y file1 file1 &&
-	git mergetool -y file2 "spaced name" >/dev/null 2>&1 &&
-	git mergetool -y subdir/file3 >/dev/null 2>&1 &&
+	( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) &&
+	( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
 	( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
 	( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
 	( yes "l" | git mergetool submod >/dev/null 2>&1 ) &&

> > -	( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
> > -	( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
> > -	( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) &&
> > -	( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> > -	( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
> > +	git mergetool file1 >/dev/null 2>&1 &&
> > +	git mergetool file2 >/dev/null 2>&1 &&
> > +	git mergetool "spaced name" >/dev/null 2>&1 &&
> > +	git mergetool both >/dev/null 2>&1 &&
> > +	git mergetool subdir/file3 >/dev/null 2>&1 &&
> 
> The reason for the lack of "-y" in the rewrite, in contrast to what
> was done in the previous hunk we saw, is not quite obvious.
> 
Sorry, it was my mistake. I had forgotten to replace the '-y'. I have
corrected this for a future revision.