Tay Ray Chuan wrote:
> - use the test_terminal script even when running with "-v"
> if IO::Pty is available, to allow commands like
>
> test_terminal foo >out 2>err
>
> - add a separate TTYREDIR prerequisite which is only set
> when the test_terminal script is usable
>
> - write the "need to declare TTY prerequisite" message to fd 4,
> where it will be printed when running tests with -v, rather
> than being swallowed up by an unrelated redireciton.
The patches up to this one look good to me. This one behaves
as advertised, but I think the API is lousy --- it is just
begging people to use the TTY prereq where TTYREDIR is needed.
Better to change TTY to mean TTYREDIR and drop support for
test_terminal on systems without IO::Pty:
-- 8< --
Subject: test_terminal: ensure redirections work reliably
For terminal tests that capture output/stderr, the TTY prerequisite
warning does not quite work for commands like
test_terminal foo >out 2>err
because the warning gets "swallowed" up by the redirection that's
supposed only to be done by the subcommand.
Even worse, the outcome depends on whether stdout was already a
terminal (in which case test_terminal is a noop) or not (in which case
test_terminal introduces a pseudo-tty in the middle of the pipeline).
$ test_terminal.perl sh -c 'test -t 1 && echo >&2 YES' >out
YES
$ sh -c 'test -t 1 && echo >&2 YES' >out
$
So:
- use the test_terminal script even when running with "-v".
- skip tests that require a terminal when the test_terminal
script is unusable because IO::Pty is not installed.
- write the "need to declare TTY prerequisite" message to fd 4,
where it will be printed when running tests with -v, rather
than being swallowed up by an unrelated redireciton.
Noticed-by: Tay Ray Chuan <rctay89@xxxxxxxxx>
Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
---
The only other sane alternative I can think of is to introduce
TTYNOREDIR, since at least people wouldn't be tempted to use
that. Distinguishing between
test_expect_success 'foo' '
test_terminal bar >out 2>err
'
and
test_expect_success 'foo' '
test_terminal bar
'
from a script run as
sh t1234-some-script.sh >log 2>err.log
does not seem to be easy without OS-specific hacks like
"readlink /dev/fd/1".
t/lib-terminal.sh | 38 ++++++++++----------------------------
1 files changed, 10 insertions(+), 28 deletions(-)
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 5e7ee9a..c383b57 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -1,37 +1,19 @@
#!/bin/sh
test_expect_success 'set up terminal for tests' '
- if test -t 1 && test -t 2
- then
- >have_tty
- elif
+ if
test_have_prereq PERL &&
"$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl \
sh -c "test -t 1 && test -t 2"
then
- >test_terminal_works
+ test_set_prereq TTY &&
+ test_terminal () {
+ if ! test_declared_prereq TTY
+ then
+ echo >&4 "test_terminal: need to declare TTY prerequisite"
+ return 127
+ fi
+ "$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl "$@"
+ }
fi
'
-
-if test -e have_tty
-then
- test_terminal_() { "$@"; }
- test_set_prereq TTY
-elif test -e test_terminal_works
-then
- test_terminal_() {
- "$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl "$@"
- }
- test_set_prereq TTY
-else
- say "# no usable terminal, so skipping some tests"
-fi
-
-test_terminal () {
- if ! test_declared_prereq TTY
- then
- echo >&2 'test_terminal: need to declare TTY prerequisite'
- return 127
- fi
- test_terminal_ "$@"
-}
--
1.7.2.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html