From: Jonathan Nieder <jrnieder@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Brandon Casey <drafnel@gmail.com>, Jeff King <peff@peff.net>,
git@vger.kernel.org,
Sebastian Celis <sebastian@sebastiancelis.com>,
Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH v3 7/7] t7006-pager: if stdout is not a terminal, make a new one
Date: Sat, 20 Feb 2010 02:50:25 -0600 [thread overview]
Message-ID: <20100220085025.GA26788@progeny.tock> (raw)
In-Reply-To: <7vaav42ya1.fsf@alter.siamese.dyndns.org>
Testing pagination requires (fake or real) access to a terminal so we
can see whether the pagination automatically kicks in, which makes it
hard to get good coverage when running tests without --verbose. There
are a number of ways to work around that:
- Replace all isatty calls with calls to a custom xisatty wrapper
that usually checks for a terminal but can be overridden for tests.
This would be workable, but it would require implementing xisatty
separately in three languages (C, shell, and perl) and making sure
that any code that is to be tested always uses the wrapper.
- Redirect stdout to /dev/tty. This would be problematic because
there might be no terminal available, and even if a terminal is
available, it might not be appropriate to spew output to it.
- Create a new pseudo-terminal on the fly and capture its output.
This patch implements the third approach.
The new test-terminal.perl helper uses IO::Pty from Expect.pm to create
a terminal and executes the program specified by its arguments with
that terminal as stdout. If the IO::Pty module is missing or not
working on a system, the test script will maintain its old behavior
(skipping most of its tests unless GIT_TEST_OPTS includes --verbose).
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
> > On non-Unixlike and pre-SVR4 systems, this functionality should be
> > disabled by passing NO_GRANTPT=YesPlease to the makefile. I do not
> > know whether Cygwin or HP-UX implements the required functions
> > appropriately, so to be safe this patch includes that option for them.
> >
> > On platforms where test-terminal does not work, the test script will
> > maintain its old behavior (skipping most of its tests unless
> > GIT_TEST_OPTS includes --verbose).
>
> If we are truly serious about doing this, it might be worthwhile to check
> how expect (http://expect.nist.gov) does this portably.
I was not interested in making this work perfectly on ancient systems,
because that would involve providing variants for old-style BSD ptys,
STREAMS-based ptys, and the modern simpler ptys. Looking at the expect
source, it seems there are even more variants to worry about than I had
thought.
On the other hand, I do think it would be better to make this work (or
not work) without makefile changes on all platforms. So how about this?
It lets the IO::Pty library used to implement Expect.pm do the dirty
work for us.
The main downsides are that this means less coverage (not everyone has
IO::Pty installed) and it is much slower than the C version.
.gitignore | 1 +
| 33 ++++++++++++++------
t/t7006/test-terminal.perl | 70 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 94 insertions(+), 10 deletions(-)
create mode 100755 t/t7006/test-terminal.perl
diff --git a/.gitignore b/.gitignore
index 8df8f88..6b405ba 100644
--- a/.gitignore
+++ b/.gitignore
@@ -169,6 +169,7 @@
/test-run-command
/test-sha1
/test-sigchain
+/test-terminal
/common-cmds.h
*.tar.gz
*.dsc
--git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 2e9cb9d..da0f962 100644
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -5,18 +5,31 @@ test_description='Test automatic use of a pager.'
. ./test-lib.sh
rm -f stdout_is_tty
-test_expect_success 'is stdout a terminal?' '
+test_expect_success 'set up terminal for tests' '
if test -t 1
then
: > stdout_is_tty
+ elif
+ test_have_prereq PERL &&
+ "$PERL_PATH" "$TEST_DIRECTORY"/t7006/test-terminal.perl \
+ sh -c "test -t 1"
+ then
+ : > test_terminal_works
fi
'
if test -e stdout_is_tty
then
+ test_terminal() { "$@"; }
+ test_set_prereq TTY
+elif test -e test_terminal_works
+then
+ test_terminal() {
+ "$PERL_PATH" "$TEST_DIRECTORY"/t7006/test-terminal.perl "$@"
+ }
test_set_prereq TTY
else
- say stdout is not a terminal, so skipping some tests.
+ say no usable terminal, so skipping some tests
fi
unset GIT_PAGER GIT_PAGER_IN_USE
@@ -30,13 +43,13 @@ test_expect_success 'setup' '
rm -f paginated.out
test_expect_success TTY 'some commands use a pager' '
- git log &&
+ test_terminal git log &&
test -e paginated.out
'
rm -f paginated.out
test_expect_success TTY 'some commands do not use a pager' '
- git rev-list HEAD &&
+ test_terminal git rev-list HEAD &&
! test -e paginated.out
'
@@ -54,7 +67,7 @@ test_expect_success 'no pager when stdout is a regular file' '
rm -f paginated.out
test_expect_success TTY 'git --paginate rev-list uses a pager' '
- git --paginate rev-list HEAD &&
+ test_terminal git --paginate rev-list HEAD &&
test -e paginated.out
'
@@ -66,7 +79,7 @@ test_expect_success 'no pager even with --paginate when stdout is a pipe' '
rm -f paginated.out
test_expect_success TTY 'no pager with --no-pager' '
- git --no-pager log &&
+ test_terminal git --no-pager log &&
! test -e paginated.out
'
@@ -127,7 +140,7 @@ test_expect_success SIMPLEPAGER 'default pager is used by default' '
: > default_pager_used
EOF
chmod +x $less &&
- PATH=.:$PATH git log &&
+ PATH=.:$PATH test_terminal git log &&
test -e default_pager_used
'
@@ -137,7 +150,7 @@ rm -f PAGER_used
test_expect_success TTY 'PAGER overrides default pager' '
PAGER=": > PAGER_used" &&
export PAGER &&
- git log &&
+ test_terminal git log &&
test -e PAGER_used
'
@@ -147,7 +160,7 @@ test_expect_success TTY 'core.pager overrides PAGER' '
PAGER=: &&
export PAGER &&
git config core.pager ": > core.pager_used" &&
- git log &&
+ test_terminal git log &&
test -e core.pager_used
'
@@ -156,7 +169,7 @@ test_expect_success TTY 'GIT_PAGER overrides core.pager' '
git config core.pager : &&
GIT_PAGER=": > GIT_PAGER_used" &&
export GIT_PAGER &&
- git log &&
+ test_terminal git log &&
test -e GIT_PAGER_used
'
diff --git a/t/t7006/test-terminal.perl b/t/t7006/test-terminal.perl
new file mode 100755
index 0000000..b51cfc6
--- /dev/null
+++ b/t/t7006/test-terminal.perl
@@ -0,0 +1,70 @@
+#!/usr/bin/perl
+use strict;
+use warnings;
+use IO::Pty;
+
+# Fork and execute @$argv with stdout redirected to $out.
+sub start_child {
+ my ($argv, $out) = @_;
+ my $pid = fork;
+ if (not defined $pid) {
+ die "fork failed: $!"
+ } elsif ($pid == 0) {
+ open STDOUT, ">&", $out;
+ close $out;
+ exec(@$argv) or die "cannot exec '$argv->[0]': $!"
+ }
+ return $pid;
+}
+
+# Wait for $pid to finish.
+sub finish_child {
+ # Simplified from wait_or_whine() in run-command.c.
+ my ($pid) = @_;
+
+ my $waiting = waitpid($pid, 0);
+ if ($waiting < 0) {
+ die "waitpid failed: $!";
+ } elsif ($? & 127) {
+ my $code = $? & 127;
+ warn "died of signal $code";
+ return $code - 128;
+ } else {
+ return $? >> 8;
+ }
+}
+
+sub xsendfile {
+ my ($out, $in) = @_;
+
+ my $buf;
+
+ # Note: the real sendfile() cannot read from a terminal.
+ for (;;) {
+ my $n = sysread $in, $buf, 4096;
+
+ # It is unspecified by POSIX whether reads
+ # from a disconnected terminal will return
+ # EIO (as in AIX 4.x, IRIX, and Linux) or
+ # end-of-file. Either is fine.
+ if (!defined($n) && $!{EIO}) {
+ return;
+ } elsif (!defined($n)) {
+ die "cannot read from child: $!";
+ } elsif ($n == 0) {
+ return;
+ } else {
+ print $out $buf or die "write error: $!";
+ }
+ }
+}
+
+if ($#ARGV < 1) {
+ die "usage: test-terminal program args";
+}
+my $master = new IO::Pty;
+my $slave = $master->slave;
+my $pid = start_child(\@ARGV, $slave);
+close $slave;
+xsendfile(\*STDOUT, $master);
+exit(finish_child($pid));
--
1.7.0
next prev parent reply other threads:[~2010-02-20 8:50 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-19 6:50 [PATCH v2 0/7] Re: 'git svn log' no longer uses the pager Jonathan Nieder
2010-02-19 6:51 ` [PATCH 1/7] Fix 'git var' usage synopsis Jonathan Nieder
2010-02-19 7:00 ` [PATCH 2/7] Make 'git var GIT_PAGER' always print the configured pager Jonathan Nieder
2010-02-19 7:06 ` [PATCH 3/7] git.1: Clarify the behavior of the --paginate option Jonathan Nieder
2010-02-19 7:09 ` [PATCH 4/7] git svn: Fix launching of pager Jonathan Nieder
2010-02-19 7:12 ` [PATCH 5/7] am: " Jonathan Nieder
2010-02-19 7:18 ` [PATCH 6/7] tests: Add tests for automatic use " Jonathan Nieder
2010-02-20 17:33 ` Junio C Hamano
2010-02-21 2:03 ` [PATCH v2 " Jonathan Nieder
2010-02-21 2:09 ` [PATCH v4 7/7] t7006-pager: if stdout is not a terminal, make a new one Jonathan Nieder
2010-02-21 7:30 ` Jeff King
2010-02-22 8:19 ` [PATCH v2 6/7] tests: Add tests for automatic use of pager Johannes Sixt
2010-02-22 8:46 ` [PATCH 8/7] tests: Fix race condition in t7006-pager Jonathan Nieder
2010-02-22 9:12 ` Jonathan Nieder
2010-02-19 7:23 ` [PATCH/RFC 7/7] t7006-pager: if stdout is not a terminal, make a new one Jonathan Nieder
2010-02-19 8:08 ` Jeff King
2010-02-19 8:19 ` Jonathan Nieder
2010-02-19 8:34 ` Jeff King
2010-02-19 16:25 ` Brandon Casey
2010-02-20 0:29 ` Brandon Casey
2010-02-20 0:39 ` Jonathan Nieder
2010-02-20 3:42 ` Brandon Casey
2010-02-20 5:25 ` [PATCH v2 " Jonathan Nieder
2010-02-20 6:53 ` Junio C Hamano
2010-02-20 8:50 ` Jonathan Nieder [this message]
2010-02-20 9:48 ` [PATCH squash] Simplify test-terminal.perl Jonathan Nieder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100220085025.GA26788@progeny.tock \
--to=jrnieder@gmail.com \
--cc=drafnel@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=peff@peff.net \
--cc=sebastian@sebastiancelis.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).