Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.

From: Tarmigan <tarmigan+git@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, peff@peff.net, jaysoffian@gmail.com,
	drizzd@aon.at, spearce@spearce.org
Subject: Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.
Date: Fri, 20 Nov 2009 11:03:13 -0800	[thread overview]
Message-ID: <905315640911201103w6d1da86duf41a53537672be8e@mail.gmail.com> (raw)
In-Reply-To: <7vd43d7hdo.fsf@alter.siamese.dyndns.org>

On Fri, Nov 20, 2009 at 12:03 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Tarmigan <tarmigan+git@gmail.com> writes:
>
>> With smart http, git over http is likely to become much more common.
>> To increase testing of smart http, enable the http tests by default.
>
> Sorry, but no test that listens to network ports should be enabled by
> default; otherwise it will break automated, unattended tests people have
> already set up randomly, depending on when the port happens to be
> available for use by the tests.

Is this the only concern or are there security or other issues as well?

If that is the only concern, we could have the tests automatically
fall back to listening on a different port.  Even if we didn't, if
httpd cannot startup because it can't bind to the port, the http tests
say
* skipping test, web server setup failed
and exit with test_done before any of the tests actually fail.

Here's a patch (cut-n-paste so it will probably be munged) for
discussion of the port-fallback idea.  If httpd cannot bind to 5541,
it tries 15541 etc.  You can test this by running "nc -l 5541 &"
before the test.  If this approach might be acceptable, I can send a
properly formatted patch.

Comments?

Thanks,
Tarmigan

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 797a2d6..a8eb6fa 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -77,7 +77,7 @@ prepare_httpd() {

        if test -n "$LIB_HTTPD_SSL"
        then
-               HTTPD_URL=https://127.0.0.1:$LIB_HTTPD_PORT
+               HTTPD_URL=https://127.0.0.1

                RANDFILE_PATH="$HTTPD_ROOT_PATH"/.rnd openssl req \
                        -config "$TEST_PATH/ssl.cnf" \
@@ -88,7 +88,7 @@ prepare_httpd() {
                export GIT_SSL_NO_VERIFY
                HTTPD_PARA="$HTTPD_PARA -DSSL"
        else
-               HTTPD_URL=http://127.0.0.1:$LIB_HTTPD_PORT
+               HTTPD_URL=http://127.0.0.1
        fi

        if test -n "$LIB_HTTPD_DAV" -o -n "$LIB_HTTPD_SVN"
@@ -109,16 +109,29 @@ start_httpd() {

        trap 'code=$?; stop_httpd; (exit $code); die' EXIT

-       "$LIB_HTTPD_PATH" -d "$HTTPD_ROOT_PATH" \
-               -f "$TEST_PATH/apache.conf" $HTTPD_PARA \
-               -c "Listen 127.0.0.1:$LIB_HTTPD_PORT" -k start \
-               >&3 2>&4
-       if test $? -ne 0
-       then
-               say "skipping test, web server setup failed"
-               trap 'die' EXIT
-               test_done
-       fi
+       while true
+       do
+               "$LIB_HTTPD_PATH" -d "$HTTPD_ROOT_PATH" \
+                       -f "$TEST_PATH/apache.conf" $HTTPD_PARA \
+                       -c "Listen 127.0.0.1:$LIB_HTTPD_PORT" -k start \
+                       >&3 2>&4
+               if test $? -ne 0
+               then
+                       if test $LIB_HTTPD_PORT -gt 40000
+                       then
+                               say "skipping test, web server setup failed"
+                               trap 'die' EXIT
+                               test_done
+                       fi
+                       LIB_HTTPD_PORT=$(($LIB_HTTPD_PORT + 10000))
+                       say "trying port $LIB_HTTPD_PORT"
+                       continue
+               else
+                       break
+               fi
+       done
+
+       HTTPD_URL="$HTTPD_URL:$LIB_HTTPD_PORT"
 }

 stop_httpd() {
