From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Thu, 31 May 2012 07:17:37 GMT
Subject: Re: [PATCH 2/3] Test environment of git-remote-mw
Message-ID: <vpqzk8oq0oe.fsf@bauges.imag.fr>
In-Reply-To: <1338397492-13360-2-git-send-email-simon.cathebras@ensimag.imag.fr>

Simon Cathebras <simon.cathebras@ensimag.imag.fr> writes:

> index 2deead7..8698625 100755
> --- a/t/test-gitmw-lib.sh
> +++ b/t/test-gitmw-lib.sh
> @@ -1,27 +1,136 @@
> +# Copyright (C) 2012
> +#     Charles Roussel <charles.roussel@ensimag.imag.fr>
> +#     Simon Cathebras <simon.cathebras@ensimag.imag.fr>
> +#     Julien Khayat <julien.khayat@ensimag.imag.fr>
> +#     Guillaume Sasdy <guillaume.sasdy@ensimag.imag.fr>
> +#     Simon Perrat <simon.perrat@ensimag.imag.fr>
> +#     Matthieu Moy <matthieu.moy@imag.fr>
> +# License: GPL v2 or later
> +

Why is this added by [PATCH 2/3]?

> -# You might want to change those ones ...
> +# You might want to change these ones
>  #
>  WIKI_DIR_NAME="wiki"            # Name of the wiki's directory
>  WIKI_DIR_INST="/var/www"        # Directory of the web server
>  TMP="/tmp"                      # Temporary directory for downloads
> -                                # Absolute address needed!
> +                                # Absolute path required!
>  SERVER_ADDR="localhost"         # Web server's address
>  
> -#
>  # CONFIGURATION
> -# You should not change those ones unless you know what you to
> +# You should not change these ones unless you know what you do

These fixups should have been squashed into the previous one. Bad use of
"git rebase -i"?

> +	../test-gitmw.pl "get_page" -p "$1" "$2"

Useless quotes around "get_page".

(but useful ones around $1 and $2 ;-))

> +	result=$(diff -r -B -w --exclude=".git" "$1" "$2")

Doesn't seem to be in POSIX:

http://pubs.opengroup.org/onlinepubs/009695399/utilities/diff.html

Will this run on all platforms?

> +# wiki_page_content <file_name> <page_name> 
> +#
> +# Compares the contents of the file <file_name> and the wiki page
> +# <page_name> and exits with error 1 if they do not match.

If it does a comparison, why isn't it named "check_content", or
"diff_content"?

> +# Usage:
> +#       ./test-gitmw.pl <function> [argument]*

I'd say s/function/command/

> +my $wiki_url="http://localhost/wiki/api.php";
> +my $wiki_admin='WikiAdmin';
> +my $wiki_admin_pass='AdminPass';

It would be nice to allow overriding these from the command-line. But as
you are now writting tests, you may as well apply the YAGNI
principle ;-).

> +        # Replace spaces by underscore in the page name

Indent with space.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/