[PATCH] Skip t3403 selftests if stdin is not a terminal

[PATCH] Skip t3403 selftests if stdin is not a terminal

[Gerrit Pape]

sh t3403-rebase-skip.sh </dev/null fails because stdin is not connected
to a terminal, as in the Debian autobuild environment.  This disbales
the test 3 and 7 in this case.

Signed-off-by: Gerrit Pape <pape@smarden.org>

---
 t/t3403-rebase-skip.sh |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh
index 8ab63c5..983d152 100755
--- a/t/t3403-rebase-skip.sh
+++ b/t/t3403-rebase-skip.sh
@@ -39,10 +39,16 @@ test_expect_success setup '
 
 test_expect_failure 'rebase with git am -3 (default)' 'git rebase master'
 
-test_expect_success 'rebase --skip with am -3' '
-	git reset --hard HEAD &&
-	git rebase --skip
-	'
+if tty -s
+then
+	test_expect_success 'rebase --skip with am -3' '
+		git reset --hard HEAD &&
+		git rebase --skip
+		'
+else
+	echo 'stdin is not a terminal, skip test.'
+fi
+
 test_expect_success 'checkout skip-merge' 'git checkout -f skip-merge'
 
 test_expect_failure 'rebase with --merge' 'git rebase --merge master'
@@ -52,8 +58,13 @@ test_expect_success 'rebase --skip with 
 	git rebase --skip
 	'
 
-test_expect_success 'merge and reference trees equal' \
-	'test -z "`git-diff-tree skip-merge skip-reference`"'
+if tty -s
+then
+	test_expect_success 'merge and reference trees equal' \
+		'test -z "`git-diff-tree skip-merge skip-reference`"'
+else
+	echo 'stdin is not a terminal, skip test.'
+fi
 
 test_debug 'gitk --all & sleep 1'
 
-- 
1.4.2.1

Re: [PATCH] Skip t3403 selftests if stdin is not a terminal

[Junio C Hamano]

Gerrit Pape <pape@smarden.org> writes:

> sh t3403-rebase-skip.sh </dev/null fails because stdin is not connected
> to a terminal, as in the Debian autobuild environment.  This disbales
> the test 3 and 7 in this case.

Disabling these tests somehow feels as if you are shooting the
messenger who reports breakage of the commands they try to test.

Is it expected that the git Porcelainish commands involved in
these particular tests not to work without terminal?  If not
maybe we should fix them, not the test.

Re: [PATCH] Skip t3403 selftests if stdin is not a terminal

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Gerrit Pape <pape@smarden.org> writes:
>
>> sh t3403-rebase-skip.sh </dev/null fails because stdin is not connected
>> to a terminal, as in the Debian autobuild environment.  This disbales
>> the test 3 and 7 in this case.
>
> Disabling these tests somehow feels as if you are shooting the
> messenger who reports breakage of the commands they try to test.
>
> Is it expected that the git Porcelainish commands involved in
> these particular tests not to work without terminal?  If not
> maybe we should fix them, not the test.

How about this instead?

-- >8 --
Fix git-am safety checks

An earlier commit cbd64af added a check that prevents "git-am"
to run without its standard input connected to a terminal while
resuming operation.  This was to catch a user error to try
feeding a new patch from its standard input while recovery.

The assumption of the check was that it is an indication that a
new patch is being fed if the standard input is not connected to
a terminal.  It is however not quite correct (the standard input
can be /dev/null if the user knows the operation does not need
any input, for example).  This broke t3403 when the test was run
with its standard input connected to /dev/null.

When git-am is given an explicit command such as --skip, there
is no reason to insist that the standard input is a terminal; we
are not going to read a new patch anyway.

Credit goes to Gerrit Pape for noticing and reporting the
problem with t3403-rebase-skip test.

---
diff --git a/git-am.sh b/git-am.sh
index d0af786..66a8e3d 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -166,10 +166,25 @@ fi
 
 if test -d "$dotest"
 then
-	if test ",$#," != ",0," || ! tty -s
-	then
-		die "previous dotest directory $dotest still exists but mbox given."
-	fi
+	case "$#,$skip$resolved" in    
+	0,*t*)
+		# Explicit resume command and we do not have file, so
+		# we are happy.
+		: ;;
+	0,)
+		# No file input but without resume parameters; catch
+		# user error to feed us a patch from standard input
+		# when there is already .dotest.  This is somewhat
+		# unreliable -- stdin could be /dev/null for example
+		# and the caller did not intend to feed us a patch but
+		# wanted to continue unattended.
+		tty -s
+		;;
+	*)
+		false
+		;;
+	esac ||
+	die "previous dotest directory $dotest still exists but mbox given."
 	resume=yes
 else
 	# Make sure we are not given --skip nor --resolved

Re: [PATCH] Skip t3403 selftests if stdin is not a terminal

[Gerrit Pape]

On Fri, Sep 15, 2006 at 11:19:02PM -0700, Junio C Hamano wrote:
> Junio C Hamano <junkio@cox.net> writes:
> > Gerrit Pape <pape@smarden.org> writes:
> >> sh t3403-rebase-skip.sh </dev/null fails because stdin is not connected
> >> to a terminal, as in the Debian autobuild environment.  This disbales
> >> the test 3 and 7 in this case.
> >
> > Disabling these tests somehow feels as if you are shooting the
> > messenger who reports breakage of the commands they try to test.
> >
> > Is it expected that the git Porcelainish commands involved in
> > these particular tests not to work without terminal?  If not
> > maybe we should fix them, not the test.
> 
> How about this instead?

Yes, this is the better fix, I agree.  The selftests work fine for me
with this patch applied.

Thanks, Gerrit.