Re: [PATCH 2/3] Test environment of git-remote-mw

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/06/2012 13:49, Matthieu Moy wrote:
Guillaume Sasdy<guillaume.sasdy@xxxxxxxxxxxxxxx>  writes:

  # CONFIGURATION VARIABLES
-# 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
I already mentionned in in your v1, but these fixups do not belong to
PATCH 2/3. You do not want reviewers to see your mistakes in PATCH 1/2
and see the fix in PATCH 2/3.
Got it. With the problem experienced yesterday, we had still this issue this morning. It is now fixed :).
+wiki_getpage () {
+	../test-gitmw.pl get_page -p "$1" "$2"
+}
Any reason why test-gitmw.pl and wiki_getpage have this slightly
different API? The perl version has a "-p" flag, and the shell command
has only positionnal arguments.
The "-p" flag exists to specify if we have to use the admin login on wiki to do the command. For instance, here we fetch a page from the wiki with Admin privilege.
Others arguments remains the same.
I'd rather have a more uniform way to wrap calls to test-gitmw.pl in
shell, like

wiki_<something>  () {
	../test-gitmw.pl<something>  "$@"
}
Do you suggest we include the "-p" flag in <something> ?
I agree for the use of "$@".

Then, you probably want to move the API documentation (i.e. comments you
put before the shell functions) in, or next to the Perl script, and
avoid repeating it in the shell.

Ok.
+# git_content<dir_1>  <dir_2>
+#
+# Compare the contents of directories<dir_1>  and<dir_2>  with diff
+# and exits with error 1 if they do not match. The program will
+# not look into .git in the process.
+git_content () {
Didn't I already say that the naming was strange? A function that
compares something shouldn't be called just "content".
Sorry, we misunderstood what you mean... Update is coming. What about git_diff_directories ? Because this shell function execute a special instance of diff, matching two directories and ignoring blank character.
+	result=$(diff -r -b --exclude=".git" "$1" "$2")
+
+	if echo $result | grep -q ">" ; then
Why grep, when the exit status of diff tells you about the differences
already?

Good idea for the exit status.
+# git_exist<rep_name>  <file_name>
+#
+# Check the existence of<file_name>  into the git repository
+#<rep_name>  and exits with error 1 if it is absent.
+git_exist () {
+	result=$(find "$1" -type f -name "$2")
+
+	if ! echo $result | grep -q "$2" ; then
Missing quotes around $result.

Why do you need grep again? You just want to check whether "$result" is
empty (test -z).
this function is now erased from our code... Actually, it was really useless...
+		echo "test failed: file $2 does not exist in $1"
+		exit 1
die

?

+wiki_page_exist () {
+	wiki_getpage "$1" .
+
+	if test -f "$1".mw ; then
+		echo "test failed: file $1 not found on wiki"
+		exit 1
likewise.
We were supposed to do it with the previous version.
But we are facing some difficulties using die function outside the test harness. We wanted to include the file "test-lib.sh" in our own script "test-gitmw-lib.sh" to use die. It doesn't work, because the harness expect the file "test-gitmw-lib.sh" to be a test script. Wich is not the case.

Do you have any Idea about how to fix this problem ?
+fail()
+{
Style: fail () {

Plus, didn't we say "die" was already there for this?
Agree, see above.
+        # Replace spaces by underscore in the page name
+	$pagename=~s/\ /_/;
Indent with space (didn't I already mention that?).

+my $login = $ARGV[0];
+
+if ($login eq "-p")
Feels weird. If you're not sure it's a login, why call the variable
$login?
Change on the way. renaming this variable "is_login". Is this ok for you ?
Any reason not to use Perl's option parsing?
Not at all, we juste didn't thought about it.

--
CATHEBRAS Simon

2A-ENSIMAG

Filière Ingéniérie des Systèmes d'Information
Membre Bug-Buster

--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]