Hi,
few comments below:
1) watchers/iso/watcher.py;
* line 77: harnesscall =
['autoqa','post-iso-build',os.path.join(opts.cachedir,iso_file),'--test','mediakit_sanity']
I don't like the "--test mediakit_sanity" parameter. This is
controlled by the test's control.autoqa, and should not be force-runned by the
watcher.
2) tests/mediakit_sanity/mediakit_sanity.py;
* Overall, I'd like to see some comments, at least about the "hitmarker"
variable. Maybe comments describing the process of getting the results from the output
too.
* line 56: starts with tab instead of 8 spaces.
3) tests/mediakit_sanity/sanity.py
* Docstrings for the methods would be great ;-)
* line 190: It would be much better to create the temporary directory using
tempfile.mkdtemp() <
http://docs.python.org/library/tempfile.html#tempfile.mkdtemp>.
Also deleting the temporary directory afterwards, might be a good idea.
* def list_file_names(path) method (lines 65 - 68): what is the purpose of this
method? Plain os.listdir() does exactly the same.
* def check_iso_size(iso_path) method (lines 77 - 103):
* I'd vote for moving the "magic constants" to named constants. It
might seem like a pointless trouble, but the code is easier to understand with named
constants instead of plain numbers.
* Why exactly is the size compared to 2 (i.e. why is size "1" wrong, but
size "2" ok?). Also, this can be done using "if 2 < size < 700:"
construct ;-)
* def iso_to_repo(file_path, mount_dir) (lines 149 - 159): It would be better to
return a tuple containing the two strings (i.e. "return (repoid, mount_dir)").
You can then easily acces both "halves" without the need to split the string all
the time.
Other than that, it seems quite OK to me.
Regards
joza
----- Original Message -----
From: "Hongqing Yang" <hoyang(a)redhat.com>
To: "AutoQA development" <autoqa-devel(a)lists.fedorahosted.org>
Sent: Friday, September 9, 2011 9:31:55 AM
Subject: iso watcher, post-iso-build event, mediakit_sanity test patch for review
Hi, all,
Included are package for iso watcher, which use lftp to mirror
dl.fp.org/pub/alt/stage and kick off mediakit_sanity test,
package for post-iso-build event, and package for mediakit_sanity test
which test the iso's size, md5, sha256sum, repoclosure
and file conflicts.
During the first run of /iso/watcher.py, it will mirror all the isos
at
dl.fp.org/pub/alt/stage and kick off mekiakit_sanity test,
after that, it will only mirror the new isos, the tree compose and
deltaiso are not included in the mirror.
This is my maiden work, thanks for your review.
Hongqing
_______________________________________________
autoqa-devel mailing list
autoqa-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/autoqa-devel