# [PATCH] Skip t3403 selftests if stdin is not a terminal 4 messages from 2006-09-15 to 2006-09-19. Participants: Gerrit Pape, Junio C Hamano. Thread: https://gitlist.dev/t/5571 ## Gerrit Pape, 2006-09-15 12:59 Subject: [PATCH] Skip t3403 selftests if stdin is not a terminal Message-ID: <20060915125910.10514.qmail@26499ea44f2ee3.315fe32.mid.smarden.org> URL: https://gitlist.dev/e/20060915125910.10514.qmail%4026499ea44f2ee3.315fe32.mid.smarden.org ``` sh t3403-rebase-skip.sh --- 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 ``` ## Junio C Hamano, 2006-09-16 05:39 Subject: Re: [PATCH] Skip t3403 selftests if stdin is not a terminal Message-ID: <7vk644e5f5.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vk644e5f5.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20060915125910.10514.qmail@26499ea44f2ee3.315fe32.mid.smarden.org> ``` Gerrit Pape writes: > sh t3403-rebase-skip.sh 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, 2006-09-16 06:19 Subject: Re: [PATCH] Skip t3403 selftests if stdin is not a terminal Message-ID: <7v3base3l5.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v3base3l5.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <7vk644e5f5.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano writes: > Gerrit Pape writes: > >> sh t3403-rebase-skip.sh > 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 ``` ## Gerrit Pape, 2006-09-19 17:04 Subject: Re: [PATCH] Skip t3403 selftests if stdin is not a terminal Message-ID: <20060919170442.28169.qmail@e5754f1244ec85.315fe32.mid.smarden.org> URL: https://gitlist.dev/e/20060919170442.28169.qmail%40e5754f1244ec85.315fe32.mid.smarden.org In-Reply-To: <7v3base3l5.fsf@assigned-by-dhcp.cox.net> ``` On Fri, Sep 15, 2006 at 11:19:02PM -0700, Junio C Hamano wrote: > Junio C Hamano writes: > > Gerrit Pape writes: > >> sh t3403-rebase-skip.sh >> 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. ```