[PATCH] t/lib-terminal: make TTY a lazy prerequisite
On Fri, Mar 14, 2014 at 02:47:14PM -0700, Junio C Hamano wrote:
Something like the patch below (looks like we should be using $PERL_PATH
instead of perl, too).
Actually, we don't need to do this, as of 94221d2 (t: use perl instead
of $PERL_PATH where applicable, 2013-10-28). If only the author of
that commit were here to correct me...
;-) Also a SP between test_terminal and (), perhaps.
Fixed below. Here it is with a commit message.
-- 8 --
Subject: t/lib-terminal: make TTY a lazy prerequisite
When lib-terminal.sh is sourced by a test script, we
immediately set up the TTY prerequisite. We do so inside a
test_expect_success, because that nicely isolates any
generated output.
However, this early test can interfere with a script that
later wants to skip all tests (e.g., t5541 then goes on to
set up the httpd server, and wants to skip_all if that
fails). TAP output doesn't let us skip everything after we
have already run at least one test.
We could fix this by reordering the inclusion of
lib-terminal.sh in t5541 to go after the httpd setup. That
solves this case, but we might eventually hit a case with
circular dependencies, where either lib-*.sh include might
want to skip_all after the other has run a test. So
instead, let's just remove the ordering constraint entirely
by doing the setup inside a test_lazy_prereq construct,
rather than in a regular test. We never cared about the
test outcome anyway (it was written to always succeed).
Note that in addition to setting up the prerequisite, the
current test also defines test_terminal. Since we can't
affect the environment from a lazy_prereq, we have to hoist
that out. We previously depended on it _not_ being defined
when the TTY prereq isn't set as a way to ensure that tests
properly declare their dependency on TTY. However, we still
cover the case (see the in-code comment for details).
Reported-by: Jens Lehmann jens.lehm...@web.de
Signed-off-by: Jeff King p...@peff.net
---
t/lib-terminal.sh | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 9a2dca5..5184549 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -1,6 +1,20 @@
# Helpers for terminal output tests.
-test_expect_success PERL 'set up terminal for tests' '
+# Catch tests which should depend on TTY but forgot to. There's no need
+# to aditionally check that the TTY prereq is set here. If the test declared
+# it and we are running the test, then it must have been set.
+test_terminal () {
+ if ! test_declared_prereq TTY
+ then
+ echo 4 test_terminal: need to declare TTY prerequisite
+ return 127
+ fi
+ perl $TEST_DIRECTORY/test-terminal.perl $@
+}
+
+test_lazy_prereq TTY '
+ test_have_prereq PERL
+
# Reading from the pty master seems to get stuck _sometimes_
# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.
#
@@ -15,21 +29,8 @@ test_expect_success PERL 'set up terminal for tests' '
# After 2000 iterations or so it hangs.
# https://rt.cpan.org/Ticket/Display.html?id=65692
#
- if test $(uname -s) = Darwin
- then
- :
- elif
- perl $TEST_DIRECTORY/test-terminal.perl \
- sh -c test -t 1 test -t 2
- then
- 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 $TEST_DIRECTORY/test-terminal.perl $@
- }
- fi
+ test $(uname -s) != Darwin
+
+ perl $TEST_DIRECTORY/test-terminal.perl \
+ sh -c test -t 1 test -t 2
'
--
1.9.0.417.gc6bea4f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-terminal: make TTY a lazy prerequisite
Jeff King p...@peff.net writes:
On Fri, Mar 14, 2014 at 02:47:14PM -0700, Junio C Hamano wrote:
Something like the patch below (looks like we should be using $PERL_PATH
instead of perl, too).
Actually, we don't need to do this, as of 94221d2 (t: use perl instead
of $PERL_PATH where applicable, 2013-10-28). If only the author of
that commit were here to correct me...
Yuck. I forgot all about that, too.
I wonder if that commit (actually the one before it) invites subtle
bugs by tempting us to say
sane_unset VAR
VAR=VAL perl -e 0
test ${VAR+isset} != isset
-- 8 --
Subject: t/lib-terminal: make TTY a lazy prerequisite
When lib-terminal.sh is sourced by a test script, we
immediately set up the TTY prerequisite. We do so inside a
test_expect_success, because that nicely isolates any
generated output.
However, this early test can interfere with a script that
later wants to skip all tests (e.g., t5541 then goes on to
set up the httpd server, and wants to skip_all if that
fails). TAP output doesn't let us skip everything after we
have already run at least one test.
We could fix this by reordering the inclusion of
lib-terminal.sh in t5541 to go after the httpd setup. That
solves this case, but we might eventually hit a case with
circular dependencies, where either lib-*.sh include might
want to skip_all after the other has run a test. So
instead, let's just remove the ordering constraint entirely
by doing the setup inside a test_lazy_prereq construct,
rather than in a regular test. We never cared about the
test outcome anyway (it was written to always succeed).
Note that in addition to setting up the prerequisite, the
current test also defines test_terminal. Since we can't
affect the environment from a lazy_prereq, we have to hoist
that out. We previously depended on it _not_ being defined
when the TTY prereq isn't set as a way to ensure that tests
properly declare their dependency on TTY. However, we still
cover the case (see the in-code comment for details).
Reported-by: Jens Lehmann jens.lehm...@web.de
Signed-off-by: Jeff King p...@peff.net
---
Thanks.
t/lib-terminal.sh | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 9a2dca5..5184549 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -1,6 +1,20 @@
# Helpers for terminal output tests.
-test_expect_success PERL 'set up terminal for tests' '
+# Catch tests which should depend on TTY but forgot to. There's no need
+# to aditionally check that the TTY prereq is set here. If the test declared
+# it and we are running the test, then it must have been set.
+test_terminal () {
+ if ! test_declared_prereq TTY
+ then
+ echo 4 test_terminal: need to declare TTY prerequisite
+ return 127
+ fi
+ perl $TEST_DIRECTORY/test-terminal.perl $@
+}
+
+test_lazy_prereq TTY '
+ test_have_prereq PERL
+
# Reading from the pty master seems to get stuck _sometimes_
# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.
#
@@ -15,21 +29,8 @@ test_expect_success PERL 'set up terminal for tests' '
# After 2000 iterations or so it hangs.
# https://rt.cpan.org/Ticket/Display.html?id=65692
#
- if test $(uname -s) = Darwin
- then
- :
- elif
- perl $TEST_DIRECTORY/test-terminal.perl \
- sh -c test -t 1 test -t 2
- then
- 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 $TEST_DIRECTORY/test-terminal.perl $@
- }
- fi
+ test $(uname -s) != Darwin
+
+ perl $TEST_DIRECTORY/test-terminal.perl \
+ sh -c test -t 1 test -t 2
'
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-terminal: make TTY a lazy prerequisite
Am 14.03.2014 22:57, schrieb Jeff King:
On Fri, Mar 14, 2014 at 02:47:14PM -0700, Junio C Hamano wrote:
Something like the patch below (looks like we should be using $PERL_PATH
instead of perl, too).
Actually, we don't need to do this, as of 94221d2 (t: use perl instead
of $PERL_PATH where applicable, 2013-10-28). If only the author of
that commit were here to correct me...
;-) Also a SP between test_terminal and (), perhaps.
Fixed below. Here it is with a commit message.
Thanks, this fixes the problem for me :-)
-- 8 --
Subject: t/lib-terminal: make TTY a lazy prerequisite
When lib-terminal.sh is sourced by a test script, we
immediately set up the TTY prerequisite. We do so inside a
test_expect_success, because that nicely isolates any
generated output.
However, this early test can interfere with a script that
later wants to skip all tests (e.g., t5541 then goes on to
set up the httpd server, and wants to skip_all if that
fails). TAP output doesn't let us skip everything after we
have already run at least one test.
We could fix this by reordering the inclusion of
lib-terminal.sh in t5541 to go after the httpd setup. That
solves this case, but we might eventually hit a case with
circular dependencies, where either lib-*.sh include might
want to skip_all after the other has run a test. So
instead, let's just remove the ordering constraint entirely
by doing the setup inside a test_lazy_prereq construct,
rather than in a regular test. We never cared about the
test outcome anyway (it was written to always succeed).
Note that in addition to setting up the prerequisite, the
current test also defines test_terminal. Since we can't
affect the environment from a lazy_prereq, we have to hoist
that out. We previously depended on it _not_ being defined
when the TTY prereq isn't set as a way to ensure that tests
properly declare their dependency on TTY. However, we still
cover the case (see the in-code comment for details).
Reported-by: Jens Lehmann jens.lehm...@web.de
Signed-off-by: Jeff King p...@peff.net
---
t/lib-terminal.sh | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 9a2dca5..5184549 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -1,6 +1,20 @@
# Helpers for terminal output tests.
-test_expect_success PERL 'set up terminal for tests' '
+# Catch tests which should depend on TTY but forgot to. There's no need
+# to aditionally check that the TTY prereq is set here. If the test declared
+# it and we are running the test, then it must have been set.
+test_terminal () {
+ if ! test_declared_prereq TTY
+ then
+ echo 4 test_terminal: need to declare TTY prerequisite
+ return 127
+ fi
+ perl $TEST_DIRECTORY/test-terminal.perl $@
+}
+
+test_lazy_prereq TTY '
+ test_have_prereq PERL
+
# Reading from the pty master seems to get stuck _sometimes_
# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.
#
@@ -15,21 +29,8 @@ test_expect_success PERL 'set up terminal for tests' '
# After 2000 iterations or so it hangs.
# https://rt.cpan.org/Ticket/Display.html?id=65692
#
- if test $(uname -s) = Darwin
- then
- :
- elif
- perl $TEST_DIRECTORY/test-terminal.perl \
- sh -c test -t 1 test -t 2
- then
- 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 $TEST_DIRECTORY/test-terminal.perl $@
- }
- fi
+ test $(uname -s) != Darwin
+
+ perl $TEST_DIRECTORY/test-terminal.perl \
+ sh -c test -t 1 test -t 2
'
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/lib-terminal: make TTY a lazy prerequisite
On Fri, Mar 14, 2014 at 03:05:45PM -0700, Junio C Hamano wrote:
Actually, we don't need to do this, as of 94221d2 (t: use perl instead
of $PERL_PATH where applicable, 2013-10-28). If only the author of
that commit were here to correct me...
Yuck. I forgot all about that, too.
I wonder if that commit (actually the one before it) invites subtle
bugs by tempting us to say
sane_unset VAR
VAR=VAL perl -e 0
test ${VAR+isset} != isset
I dunno. A more subtle case is:
write_script foo -\EOF
perl ...
EOF
which uses the real perl and not the function. So it's not as airtight
as I would like, but I think it may be a net win, as the common case can
just use perl.
Hmph. It seems like I raised both of those concerns initially:
http://article.gmane.org/gmane.comp.version-control.git/236879
We can revisit it if you want. I think the only options besides leaving
it or reverting it would be to put perl into bin-wrappers as a wrapper
script. That's fine for the tests, but I suspect it might annoy people
who use bin-wrappers to run git straight out of the build directory
without installing.
-- 8 --
Subject: t/lib-terminal: make TTY a lazy prerequisite
[...]
Thanks.
By the way, I checked for other cases that could use the same treatment
by grepping for test_expect_* in t/lib-*.sh. Most of them are inside
functions, so presumably the scripts call them at the appropriate time.
The exceptions are:
1. lib-read-tree-m-3way.sh; this one has a whole battery of tests
that sourced into t1000 and t4002. It could be split into functions
and modernized, but it's probably not worth the effort. It's not
causing ordering problems, and it's not likely to get used
elsewhere.
2. lib-pager.sh; this one is weird, as it is really about setting the
$less variable to git's default pager. And then the prereq is
really just checking that said pager is syntactically simple, I
think, so we can override it by writing to a file with the same
name. At least that's my impression; frankly I found it a bit
confusing to read.
Converting it to a lazy prereq wouldn't work because we care about
its side effect of setting the less variable. There are no
ordering issues with it currently, so I'm inclined to leave it.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
