{"thread":{"id":"36160","subject":"[PATCH] t5541: don't call start_httpd after sourcing lib-terminal.sh","startedAt":"2014-03-14T21:18:32Z","lastAt":"2014-03-15T01:55:29Z","messageCount":7,"participants":["Jens Lehmann","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"236749","messageId":"53237228.10809@web.de","threadId":"36160","inReplyTo":null,"subject":"[PATCH] t5541: don't call start_httpd after sourcing lib-terminal.sh","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2014-03-14T21:18:32Z","receivedAt":"2014-03-14T21:18:32Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Since 83d842dc8 \"make test\" using prove fails for some setups in t5541\nwith:\n\n   \"Parse errors: No plan found in TAP output\"\n\nRunning t5541 on its own fails with:\n\n   \"error: Can't use skip_all after running some tests\"\n\nThis happens because \"start_httpd\" (which determines if the test is to\nbe skipped) is called after sourcing lib-terminal.sh (which sets up the\nterminal using test_expect_success).\n\nFix that by calling \"start_httpd\" before sourcing lib-terminal.sh.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nSince recently t5541 fails for me on master and pu. I'm not sure what\ndetail in my setup causes this breakage (I have httpd installed and it\nis running), but this patch fixes it for me.\n\n\n t/t5541-http-push-smart.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh\nindex 73af16f..597fb96 100755\n--- a/t/t5541-http-push-smart.sh\n+++ b/t/t5541-http-push-smart.sh\n@@ -13,8 +13,8 @@ fi\n\n ROOT_PATH=\"$PWD\"\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n-. \"$TEST_DIRECTORY\"/lib-terminal.sh\n start_httpd\n+. \"$TEST_DIRECTORY\"/lib-terminal.sh\n\n test_expect_success 'setup remote repository' '\n \tcd \"$ROOT_PATH\" &&\n-- \n1.9.0.168.g1119394\n"},{"id":"236752","messageId":"20140314213715.GA10299@sigill.intra.peff.net","threadId":"36160","inReplyTo":"53237228.10809@web.de","subject":"Re: [PATCH] t5541: don't call start_httpd after sourcing lib-terminal.sh","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-14T21:37:15Z","receivedAt":"2014-03-14T21:37:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 14, 2014 at 10:18:32PM +0100, Jens Lehmann wrote:\n\n> Since 83d842dc8 \"make test\" using prove fails for some setups in t5541\n> with:\n> \n>    \"Parse errors: No plan found in TAP output\"\n> \n> Running t5541 on its own fails with:\n> \n>    \"error: Can't use skip_all after running some tests\"\n> \n> This happens because \"start_httpd\" (which determines if the test is to\n> be skipped) is called after sourcing lib-terminal.sh (which sets up the\n> terminal using test_expect_success).\n> \n> Fix that by calling \"start_httpd\" before sourcing lib-terminal.sh.\n\nThanks, your solution seems reasonable. lib-terminal runs a test behind\nour back when we source it, which is a little funny.\n\nPotentially we could turn its test into a lazy prereq, but I think that\ndoes not quite work. In addition to setting the TTY prereq, it defines\nthe test_terminal function, and lazy prereqs happen in a subshell, IIRC.\n\nOne option would be to _always_ define test_terminal. Right now we rely\non it failing to exist to catch tests which should fail to correctly\ndepend on the TTY prerequisite. But we could just as easily have it\nreport failure in such a case.\n\nSomething like the patch below (looks like we should be using $PERL_PATH\ninstead of \"perl\", too).\n\n> Since recently t5541 fails for me on master and pu. I'm not sure what\n> detail in my setup causes this breakage (I have httpd installed and it\n> is running), but this patch fixes it for me.\n\nYeah, this is because we now try to run the tests by default, and skip\nthem if webserver setup fails. If you want to know why it's failing on\nyour machine, try running with \"-v -i\" to see output, and/or looking in\nhttpd/error.log in the trash directory.\n\n---\ndiff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\nindex 9a2dca5..55b708f 100644\n--- a/t/lib-terminal.sh\n+++ b/t/lib-terminal.sh\n@@ -1,35 +1,36 @@\n # Helpers for terminal output tests.\n \n-test_expect_success PERL 'set up terminal for tests' '\n+# Catch tests which should depend on TTY but forgot to. There's no need\n+# to check that TTY is set here. If the test declared it and we are running\n+# it, then it is set.\n+test_terminal() {\n+\tif ! test_declared_prereq TTY\n+\tthen\n+\t\techo >&4 \"test_terminal: need to declare TTY prerequisite\"\n+\t\treturn 127\n+\tfi\n+\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n+}\n+\n+test_lazy_prereq TTY '\n+\ttest_have_prereq PERL &&\n+\n \t# Reading from the pty master seems to get stuck _sometimes_\n \t# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.\n \t#\n \t# Reproduction recipe: run\n \t#\n \t#\ti=0\n \t#\twhile ./test-terminal.perl echo hi $i\n \t#\tdo\n \t#\t\t: $((i = $i + 1))\n \t#\tdone\n \t#\n \t# After 2000 iterations or so it hangs.\n \t# https://rt.cpan.org/Ticket/Display.html?id=65692\n \t#\n-\tif test \"$(uname -s)\" = Darwin\n-\tthen\n-\t\t:\n-\telif\n-\t\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \\\n-\t\t\tsh -c \"test -t 1 && test -t 2\"\n-\tthen\n-\t\ttest_set_prereq TTY &&\n-\t\ttest_terminal () {\n-\t\t\tif ! test_declared_prereq TTY\n-\t\t\tthen\n-\t\t\t\techo >&4 \"test_terminal: need to declare TTY prerequisite\"\n-\t\t\t\treturn 127\n-\t\t\tfi\n-\t\t\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n-\t\t}\n-\tfi\n+\ttest \"$(uname -s)\" != Darwin &&\n+\n+\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \\\n+\t\tsh -c \"test -t 1 && test -t 2\"\n '\n"},{"id":"236753","messageId":"xmqqtxb0fo65.fsf@gitster.dls.corp.google.com","threadId":"36160","inReplyTo":"20140314213715.GA10299@sigill.intra.peff.net","subject":"Re: [PATCH] t5541: don't call start_httpd after sourcing lib-terminal.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-14T21:47:14Z","receivedAt":"2014-03-14T21:47:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> One option would be to _always_ define test_terminal....\n\nThat looks like the right direction to go.\n\n> Something like the patch below (looks like we should be using $PERL_PATH\n> instead of \"perl\", too).\n\n;-)  Also a SP between test_terminal and (), perhaps.\n\n> diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\n> index 9a2dca5..55b708f 100644\n> --- a/t/lib-terminal.sh\n> +++ b/t/lib-terminal.sh\n> @@ -1,35 +1,36 @@\n>  # Helpers for terminal output tests.\n>  \n> -test_expect_success PERL 'set up terminal for tests' '\n> +# Catch tests which should depend on TTY but forgot to. There's no need\n> +# to check that TTY is set here. If the test declared it and we are running\n> +# it, then it is set.\n> +test_terminal() {\n> +\tif ! test_declared_prereq TTY\n> +\tthen\n> +\t\techo >&4 \"test_terminal: need to declare TTY prerequisite\"\n> +\t\treturn 127\n> +\tfi\n> +\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n> +}\n> +\n> +test_lazy_prereq TTY '\n> +\ttest_have_prereq PERL &&\n> +\n>  \t# Reading from the pty master seems to get stuck _sometimes_\n>  \t# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.\n>  \t#\n>  \t# Reproduction recipe: run\n>  \t#\n>  \t#\ti=0\n>  \t#\twhile ./test-terminal.perl echo hi $i\n>  \t#\tdo\n>  \t#\t\t: $((i = $i + 1))\n>  \t#\tdone\n>  \t#\n>  \t# After 2000 iterations or so it hangs.\n>  \t# https://rt.cpan.org/Ticket/Display.html?id=65692\n>  \t#\n> -\tif test \"$(uname -s)\" = Darwin\n> -\tthen\n> -\t\t:\n> -\telif\n> -\t\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \\\n> -\t\t\tsh -c \"test -t 1 && test -t 2\"\n> -\tthen\n> -\t\ttest_set_prereq TTY &&\n> -\t\ttest_terminal () {\n> -\t\t\tif ! test_declared_prereq TTY\n> -\t\t\tthen\n> -\t\t\t\techo >&4 \"test_terminal: need to declare TTY prerequisite\"\n> -\t\t\t\treturn 127\n> -\t\t\tfi\n> -\t\t\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n> -\t\t}\n> -\tfi\n> +\ttest \"$(uname -s)\" != Darwin &&\n> +\n> +\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \\\n> +\t\tsh -c \"test -t 1 && test -t 2\"\n>  '\n"},{"id":"236755","messageId":"20140314215723.GB10299@sigill.intra.peff.net","threadId":"36160","inReplyTo":"xmqqtxb0fo65.fsf@gitster.dls.corp.google.com","subject":"[PATCH] t/lib-terminal: make TTY a lazy prerequisite","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-14T21:57:23Z","receivedAt":"2014-03-14T21:57:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 14, 2014 at 02:47:14PM -0700, Junio C Hamano wrote:\n\n> > Something like the patch below (looks like we should be using $PERL_PATH\n> > instead of \"perl\", too).\n\nActually, we don't need to do this, as of 94221d2 (t: use perl instead\nof \"$PERL_PATH\" where applicable, 2013-10-28). If only the author of\nthat commit were here to correct me...\n\n> ;-)  Also a SP between test_terminal and (), perhaps.\n\nFixed below. Here it is with a commit message.\n\n-- >8 --\nSubject: t/lib-terminal: make TTY a lazy prerequisite\n\nWhen lib-terminal.sh is sourced by a test script, we\nimmediately set up the TTY prerequisite. We do so inside a\ntest_expect_success, because that nicely isolates any\ngenerated output.\n\nHowever, this early test can interfere with a script that\nlater wants to skip all tests (e.g., t5541 then goes on to\nset up the httpd server, and wants to skip_all if that\nfails). TAP output doesn't let us skip everything after we\nhave already run at least one test.\n\nWe could fix this by reordering the inclusion of\nlib-terminal.sh in t5541 to go after the httpd setup.  That\nsolves this case, but we might eventually hit a case with\ncircular dependencies, where either lib-*.sh include might\nwant to skip_all after the other has run a test.  So\ninstead, let's just remove the ordering constraint entirely\nby doing the setup inside a test_lazy_prereq construct,\nrather than in a regular test.  We never cared about the\ntest outcome anyway (it was written to always succeed).\n\nNote that in addition to setting up the prerequisite, the\ncurrent test also defines test_terminal. Since we can't\naffect the environment from a lazy_prereq, we have to hoist\nthat out. We previously depended on it _not_ being defined\nwhen the TTY prereq isn't set as a way to ensure that tests\nproperly declare their dependency on TTY. However, we still\ncover the case (see the in-code comment for details).\n\nReported-by: Jens Lehmann <Jens.Lehmann@web.de>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/lib-terminal.sh | 37 +++++++++++++++++++------------------\n 1 file changed, 19 insertions(+), 18 deletions(-)\n\ndiff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\nindex 9a2dca5..5184549 100644\n--- a/t/lib-terminal.sh\n+++ b/t/lib-terminal.sh\n@@ -1,6 +1,20 @@\n # Helpers for terminal output tests.\n \n-test_expect_success PERL 'set up terminal for tests' '\n+# Catch tests which should depend on TTY but forgot to. There's no need\n+# to aditionally check that the TTY prereq is set here.  If the test declared\n+# it and we are running the test, then it must have been set.\n+test_terminal () {\n+\tif ! test_declared_prereq TTY\n+\tthen\n+\t\techo >&4 \"test_terminal: need to declare TTY prerequisite\"\n+\t\treturn 127\n+\tfi\n+\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n+}\n+\n+test_lazy_prereq TTY '\n+\ttest_have_prereq PERL &&\n+\n \t# Reading from the pty master seems to get stuck _sometimes_\n \t# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.\n \t#\n@@ -15,21 +29,8 @@ test_expect_success PERL 'set up terminal for tests' '\n \t# After 2000 iterations or so it hangs.\n \t# https://rt.cpan.org/Ticket/Display.html?id=65692\n \t#\n-\tif test \"$(uname -s)\" = Darwin\n-\tthen\n-\t\t:\n-\telif\n-\t\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \\\n-\t\t\tsh -c \"test -t 1 && test -t 2\"\n-\tthen\n-\t\ttest_set_prereq TTY &&\n-\t\ttest_terminal () {\n-\t\t\tif ! test_declared_prereq TTY\n-\t\t\tthen\n-\t\t\t\techo >&4 \"test_terminal: need to declare TTY prerequisite\"\n-\t\t\t\treturn 127\n-\t\t\tfi\n-\t\t\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n-\t\t}\n-\tfi\n+\ttest \"$(uname -s)\" != Darwin &&\n+\n+\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \\\n+\t\tsh -c \"test -t 1 && test -t 2\"\n '\n-- \n1.9.0.417.gc6bea4f\n"},{"id":"236756","messageId":"xmqqpplofnba.fsf@gitster.dls.corp.google.com","threadId":"36160","inReplyTo":"20140314215723.GB10299@sigill.intra.peff.net","subject":"Re: [PATCH] t/lib-terminal: make TTY a lazy prerequisite","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-14T22:05:45Z","receivedAt":"2014-03-14T22:05:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Mar 14, 2014 at 02:47:14PM -0700, Junio C Hamano wrote:\n>\n>> > Something like the patch below (looks like we should be using $PERL_PATH\n>> > instead of \"perl\", too).\n>\n> Actually, we don't need to do this, as of 94221d2 (t: use perl instead\n> of \"$PERL_PATH\" where applicable, 2013-10-28). If only the author of\n> that commit were here to correct me...\n\nYuck. I forgot all about that, too.\n\nI wonder if that commit (actually the one before it) invites subtle\nbugs by tempting us to say\n\n\tsane_unset VAR &&\n\tVAR=VAL perl -e 0 &&\n        test \"${VAR+isset}\" != \"isset\"\n\n> -- >8 --\n> Subject: t/lib-terminal: make TTY a lazy prerequisite\n>\n> When lib-terminal.sh is sourced by a test script, we\n> immediately set up the TTY prerequisite. We do so inside a\n> test_expect_success, because that nicely isolates any\n> generated output.\n>\n> However, this early test can interfere with a script that\n> later wants to skip all tests (e.g., t5541 then goes on to\n> set up the httpd server, and wants to skip_all if that\n> fails). TAP output doesn't let us skip everything after we\n> have already run at least one test.\n>\n> We could fix this by reordering the inclusion of\n> lib-terminal.sh in t5541 to go after the httpd setup.  That\n> solves this case, but we might eventually hit a case with\n> circular dependencies, where either lib-*.sh include might\n> want to skip_all after the other has run a test.  So\n> instead, let's just remove the ordering constraint entirely\n> by doing the setup inside a test_lazy_prereq construct,\n> rather than in a regular test.  We never cared about the\n> test outcome anyway (it was written to always succeed).\n>\n> Note that in addition to setting up the prerequisite, the\n> current test also defines test_terminal. Since we can't\n> affect the environment from a lazy_prereq, we have to hoist\n> that out. We previously depended on it _not_ being defined\n> when the TTY prereq isn't set as a way to ensure that tests\n> properly declare their dependency on TTY. However, we still\n> cover the case (see the in-code comment for details).\n>\n> Reported-by: Jens Lehmann <Jens.Lehmann@web.de>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n\nThanks.\n\n>  t/lib-terminal.sh | 37 +++++++++++++++++++------------------\n>  1 file changed, 19 insertions(+), 18 deletions(-)\n>\n> diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\n> index 9a2dca5..5184549 100644\n> --- a/t/lib-terminal.sh\n> +++ b/t/lib-terminal.sh\n> @@ -1,6 +1,20 @@\n>  # Helpers for terminal output tests.\n>  \n> -test_expect_success PERL 'set up terminal for tests' '\n> +# Catch tests which should depend on TTY but forgot to. There's no need\n> +# to aditionally check that the TTY prereq is set here.  If the test declared\n> +# it and we are running the test, then it must have been set.\n> +test_terminal () {\n> +\tif ! test_declared_prereq TTY\n> +\tthen\n> +\t\techo >&4 \"test_terminal: need to declare TTY prerequisite\"\n> +\t\treturn 127\n> +\tfi\n> +\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n> +}\n> +\n> +test_lazy_prereq TTY '\n> +\ttest_have_prereq PERL &&\n> +\n>  \t# Reading from the pty master seems to get stuck _sometimes_\n>  \t# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.\n>  \t#\n> @@ -15,21 +29,8 @@ test_expect_success PERL 'set up terminal for tests' '\n>  \t# After 2000 iterations or so it hangs.\n>  \t# https://rt.cpan.org/Ticket/Display.html?id=65692\n>  \t#\n> -\tif test \"$(uname -s)\" = Darwin\n> -\tthen\n> -\t\t:\n> -\telif\n> -\t\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \\\n> -\t\t\tsh -c \"test -t 1 && test -t 2\"\n> -\tthen\n> -\t\ttest_set_prereq TTY &&\n> -\t\ttest_terminal () {\n> -\t\t\tif ! test_declared_prereq TTY\n> -\t\t\tthen\n> -\t\t\t\techo >&4 \"test_terminal: need to declare TTY prerequisite\"\n> -\t\t\t\treturn 127\n> -\t\t\tfi\n> -\t\t\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n> -\t\t}\n> -\tfi\n> +\ttest \"$(uname -s)\" != Darwin &&\n> +\n> +\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \\\n> +\t\tsh -c \"test -t 1 && test -t 2\"\n>  '\n"},{"id":"236758","messageId":"53237EED.3060508@web.de","threadId":"36160","inReplyTo":"20140314215723.GB10299@sigill.intra.peff.net","subject":"Re: [PATCH] t/lib-terminal: make TTY a lazy prerequisite","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2014-03-14T22:13:01Z","receivedAt":"2014-03-14T22:13:01Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 14.03.2014 22:57, schrieb Jeff King:\n> On Fri, Mar 14, 2014 at 02:47:14PM -0700, Junio C Hamano wrote:\n> \n>>> Something like the patch below (looks like we should be using $PERL_PATH\n>>> instead of \"perl\", too).\n> \n> Actually, we don't need to do this, as of 94221d2 (t: use perl instead\n> of \"$PERL_PATH\" where applicable, 2013-10-28). If only the author of\n> that commit were here to correct me...\n> \n>> ;-)  Also a SP between test_terminal and (), perhaps.\n> \n> Fixed below. Here it is with a commit message.\n\nThanks, this fixes the problem for me :-)\n\n> -- >8 --\n> Subject: t/lib-terminal: make TTY a lazy prerequisite\n> \n> When lib-terminal.sh is sourced by a test script, we\n> immediately set up the TTY prerequisite. We do so inside a\n> test_expect_success, because that nicely isolates any\n> generated output.\n> \n> However, this early test can interfere with a script that\n> later wants to skip all tests (e.g., t5541 then goes on to\n> set up the httpd server, and wants to skip_all if that\n> fails). TAP output doesn't let us skip everything after we\n> have already run at least one test.\n> \n> We could fix this by reordering the inclusion of\n> lib-terminal.sh in t5541 to go after the httpd setup.  That\n> solves this case, but we might eventually hit a case with\n> circular dependencies, where either lib-*.sh include might\n> want to skip_all after the other has run a test.  So\n> instead, let's just remove the ordering constraint entirely\n> by doing the setup inside a test_lazy_prereq construct,\n> rather than in a regular test.  We never cared about the\n> test outcome anyway (it was written to always succeed).\n> \n> Note that in addition to setting up the prerequisite, the\n> current test also defines test_terminal. Since we can't\n> affect the environment from a lazy_prereq, we have to hoist\n> that out. We previously depended on it _not_ being defined\n> when the TTY prereq isn't set as a way to ensure that tests\n> properly declare their dependency on TTY. However, we still\n> cover the case (see the in-code comment for details).\n> \n> Reported-by: Jens Lehmann <Jens.Lehmann@web.de>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  t/lib-terminal.sh | 37 +++++++++++++++++++------------------\n>  1 file changed, 19 insertions(+), 18 deletions(-)\n> \n> diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh\n> index 9a2dca5..5184549 100644\n> --- a/t/lib-terminal.sh\n> +++ b/t/lib-terminal.sh\n> @@ -1,6 +1,20 @@\n>  # Helpers for terminal output tests.\n>  \n> -test_expect_success PERL 'set up terminal for tests' '\n> +# Catch tests which should depend on TTY but forgot to. There's no need\n> +# to aditionally check that the TTY prereq is set here.  If the test declared\n> +# it and we are running the test, then it must have been set.\n> +test_terminal () {\n> +\tif ! test_declared_prereq TTY\n> +\tthen\n> +\t\techo >&4 \"test_terminal: need to declare TTY prerequisite\"\n> +\t\treturn 127\n> +\tfi\n> +\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n> +}\n> +\n> +test_lazy_prereq TTY '\n> +\ttest_have_prereq PERL &&\n> +\n>  \t# Reading from the pty master seems to get stuck _sometimes_\n>  \t# on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9.\n>  \t#\n> @@ -15,21 +29,8 @@ test_expect_success PERL 'set up terminal for tests' '\n>  \t# After 2000 iterations or so it hangs.\n>  \t# https://rt.cpan.org/Ticket/Display.html?id=65692\n>  \t#\n> -\tif test \"$(uname -s)\" = Darwin\n> -\tthen\n> -\t\t:\n> -\telif\n> -\t\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \\\n> -\t\t\tsh -c \"test -t 1 && test -t 2\"\n> -\tthen\n> -\t\ttest_set_prereq TTY &&\n> -\t\ttest_terminal () {\n> -\t\t\tif ! test_declared_prereq TTY\n> -\t\t\tthen\n> -\t\t\t\techo >&4 \"test_terminal: need to declare TTY prerequisite\"\n> -\t\t\t\treturn 127\n> -\t\t\tfi\n> -\t\t\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \"$@\"\n> -\t\t}\n> -\tfi\n> +\ttest \"$(uname -s)\" != Darwin &&\n> +\n> +\tperl \"$TEST_DIRECTORY\"/test-terminal.perl \\\n> +\t\tsh -c \"test -t 1 && test -t 2\"\n>  '\n> \n"},{"id":"236769","messageId":"20140315015529.GB9979@sigill.intra.peff.net","threadId":"36160","inReplyTo":"xmqqpplofnba.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] t/lib-terminal: make TTY a lazy prerequisite","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-15T01:55:29Z","receivedAt":"2014-03-15T01:55:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 14, 2014 at 03:05:45PM -0700, Junio C Hamano wrote:\n\n> > Actually, we don't need to do this, as of 94221d2 (t: use perl instead\n> > of \"$PERL_PATH\" where applicable, 2013-10-28). If only the author of\n> > that commit were here to correct me...\n> \n> Yuck. I forgot all about that, too.\n> \n> I wonder if that commit (actually the one before it) invites subtle\n> bugs by tempting us to say\n> \n> \tsane_unset VAR &&\n> \tVAR=VAL perl -e 0 &&\n>         test \"${VAR+isset}\" != \"isset\"\n\nI dunno. A more subtle case is:\n\n  write_script foo <<-\\EOF\n  perl ...\n  EOF\n\nwhich uses the real \"perl\" and not the function. So it's not as airtight\nas I would like, but I think it may be a net win, as the common case can\njust use \"perl\".\n\nHmph. It seems like I raised both of those concerns initially:\n\n  http://article.gmane.org/gmane.comp.version-control.git/236879\n\nWe can revisit it if you want. I think the only options besides leaving\nit or reverting it would be to put \"perl\" into bin-wrappers as a wrapper\nscript. That's fine for the tests, but I suspect it might annoy people\nwho use bin-wrappers to run git straight out of the build directory\nwithout installing.\n\n> > -- >8 --\n> > Subject: t/lib-terminal: make TTY a lazy prerequisite\n> [...]\n> \n> Thanks.\n\nBy the way, I checked for other cases that could use the same treatment\nby grepping for test_expect_* in t/lib-*.sh. Most of them are inside\nfunctions, so presumably the scripts call them at the appropriate time.\n\nThe exceptions are:\n\n  1. lib-read-tree-m-3way.sh; this one has a whole battery of tests\n     that sourced into t1000 and t4002. It could be split into functions\n     and modernized, but it's probably not worth the effort. It's not\n     causing ordering problems, and it's not likely to get used\n     elsewhere.\n\n  2. lib-pager.sh; this one is weird, as it is really about setting the\n     \"$less\" variable to git's default pager. And then the prereq is\n     really just checking that said pager is syntactically simple, I\n     think, so we can override it by writing to a file with the same\n     name. At least that's my impression; frankly I found it a bit\n     confusing to read.\n\n     Converting it to a lazy prereq wouldn't work because we care about\n     its side effect of setting the \"less\" variable.  There are no\n     ordering issues with it currently, so I'm inclined to leave it.\n\n-Peff\n"}]}