threads / patch / 5571

patchSkip t3403 selftests if stdin is not a terminal

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

## tl;dr

4 messages between Sep 15, 2006 and Sep 19, 2006. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Gerrit Pape· Sep 15, 2006, 12:59 UTC · lore

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(-)

Show changes to t/t3403-rebase-skip.sh +17 −6

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

Junio C Hamano· Sep 16, 2006, 05:39 UTC · re: Gerrit Pape · lore

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

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.

Junio C Hamano· Sep 16, 2006, 06:19 UTC · re: Junio C Hamano · lore

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

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

Show 12 quoted lines

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

---

Show changes to git-am.sh +19 −4

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

Gerrit Pape· Sep 19, 2006, 17:04 UTC · re: Junio C Hamano · lore

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

On Fri, Sep 15, 2006 at 11:19:02PM -0700, Junio C Hamano wrote:

Show 14 quoted lines

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