On Tue, Feb 26, 2008 at 12:28:20AM +0100, Clemens Buchacher wrote:
> http-push tests require a web server with WebDAV support.
>
> This commit introduces a HTTPD test library, which can be configured using
> the following environment variables.
>
> LIB_HTTPD_PATH web server path
> LIB_HTTPD_MODULE_PATH web server modules path
> LIB_HTTPD_PORT listening port
> LIB_HTTPD_DAV enable DAV
> LIB_HTTPD_SVN enable SVN
> LIB_HTTPD_SSL enable SSL
Overall, this looks fine to me, though I haven't taken a deep look at
what the test itself does. I was more interested in the "library".
Just a few nitpicks.
> +HTTPD_ROOT_PATH="$PWD"/httpd
> +HTTPD_DOCUMENT_ROOT_PATH="$PWD"/httpd/www
Why not just use $HTTPD_ROOT_PATH/www and don't use the
$HTTPD_DOCUMENT_ROOT_PATH variable ?
> + mkdir $HTTPD_ROOT_PATH
> + mkdir $HTTPD_DOCUMENT_ROOT_PATH
This could become mkdir -p $HTTPD_ROOT_PATH/www
> + if test -n "$LIB_HTTPD_SSL"
> + if test "$LIB_HTTPD_SSL" != ""
Why use different tests?
> + openssl req \
> + -config $TEST_PATH/ssl.cnf \
> + -new -x509 -nodes \
> + -out $HTTPD_ROOT_PATH/httpd.pem \
> + -keyout $HTTPD_ROOT_PATH/httpd.pem
> + export GIT_SSL_NO_VERIFY=t
> + HTTPD_PARA="$HTTPD_PARA -DSSL"
For future improvement (that could be another patch), it would probably
be nice to generate a CAcert and a certificate signed by it, so we can
also test without GIT_SSL_NO_VERIFY.
By the way, why not just include the .pem ?
> +stop_httpd() {
> + "$LIB_HTTPD_PATH" $HTTPD_PARA -k stop
> +}
As Junio said, it might be good to add a trap.
> +RANDFILE = $ENV::HOME/.rnd
Wouldn't it be better elsewhere ?
Mike
-
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
