Created attachment 301800 [details] Current output diff of test-output.xml files on a finnish build The current tool does not output decently readable files, and sometimes the error messages can be misleading. For example, if the tool discovers that the Build Under Test did not produce ANY bookmarks output, it reports that "bookmark lists are not the same length" which doesn't necessarily raise the same red flags as "build xxx does not have any default bookmarks". This bug is to address this and clean it up. In short the plan is to: 1. Provide a better diff output when comparing the test-output.xml files 2. Provide more meaningful messages when encountering empty files (bookmarks, output.xml's etc) 3. Provide the same type of diff output when comparing the bookmarks files so that a person can easily analyze the results file without having to bend their mind around two different output schemes. 4. The diff output must of course handle UTF-8 encodings properly I will attach an example of a current output log.
Created attachment 303217 [details] [diff] [review] Full update to minotaur patch Attaching wip patch that covers this bug, 414056, 413742, and 396131 The current output is our standard unified diff format that we use for patches. If a bookmarks file is missing it tells you that IN BIG LETTERs. :-) Feedback welcome. I'll split this into easily reviewable chunks on each bug once I finish testing it.
Assignee: nobody → ctalbert
Status: NEW → ASSIGNED
Created attachment 303640 [details] [diff] [review] Patch to change the way bookmarks are reported This patch changes the way the bookmark data is reported. It makes several other user improvements such as better messaging when a bookmark file is missing and handling 0 args into the user runnable partner.py and run-minotaur.py scripts.
Created attachment 303641 [details] [diff] [review] Full minotaur diff of all changes, just for reference
Attachment #303217 - Attachment is obsolete: true
(In reply to comment #2) > Created an attachment (id=303640) [details] This also addresses bug 396131
I'm seeing some overlap between this patch and that which I just reviewed for bug 413742 (in run-minotaur.py). Can I get a fresh diff that only has the changes for this bug? Unless I've just got myself all confused...
Yes it does contain some similar changes. I tried to mitigate as best I could, but it wasn't that possible. Let me get you another patch.
Created attachment 304613 [details] [diff] [review] Updated Patch since bug 413742 is fixed The new patch you requested
Comment on attachment 304613 [details] [diff] [review] Updated Patch since bug 413742 is fixed I'm not a big fan of this being a hard coded value instead of a command line switch: +DIFFBKMK_DBG = 0 + +def debug(s): + if DIFFBKMK_DBG: + print s + Also, this help message doesn't really indicate what went wrong or what the correct format would look like. + # Print Help if no args passed + if not options.branch or not options.version or not options.url or \ + not options.l10nFile or not options.partner or not options.minDir: + print "Required Items Not Specified. Must specify partner name, minotaur dir,", + print "locale file, branch, url, and version" + parser.print_help() + sys.exit(1)
Attachment #304613 - Flags: review?(anodelman) → review-
(In reply to comment #8) > (From update of attachment 304613 [details] [diff] [review]) > I'm not a big fan of this being a hard coded value instead of a command line > switch: > > +DIFFBKMK_DBG = 0 > + > +def debug(s): > + if DIFFBKMK_DBG: > + print s > + > Good point I'll enable this to be turned on by a command line input > Also, this help message doesn't really indicate what went wrong or what the > correct format would look like. > > + # Print Help if no args passed > + if not options.branch or not options.version or not options.url or \ > + not options.l10nFile or not options.partner or not options.minDir: > + print "Required Items Not Specified. Must specify partner name, minotaur > dir,", > + print "locale file, branch, url, and version" > + parser.print_help() > + sys.exit(1) > Actually, it does. parser.print_help uses the help and metaVar and the parameter name values in the add_option calls to print out a very nice help view. Here is an example of what is printed from the command python partner.py Required Items Not Specified. Must specify partner name, minotaur dir, locale file, branch, url, and version Usage: partner.py [options] Options: -h, --help show this help message and exit -p PARTNER_NAME, --Partner=PARTNER_NAME Partner Name -b BRANCH, --Branch=BRANCH Gecko Branch: 1.8.0|1.8.1|1.9 -v FIREFOX_VERSION, --Version =FIREFOX_VERSION version of firefox to be tested -u URL, --UrlToBuild=URL URL to top level build location, above the OS directories -f VER_FILES, --VerificationFileLocation=VER_FILES location of verification files, leave blank to create verification files -m MINOTAUR_DIR, --MinotaurDirectory=MINOTAUR_DIR Directory of the Minotuar code -e EXT_NAME, --ExtensionName=EXT_NAME Name of the partner extension. Only needed if Partner has EULA -c CREDENTIALS, --Credentials=CREDENTIALS Credentials to download the build in this form: <user>:<password> -a AUS_PARAM, --AusParameter=AUS_PARAM The AUS parameter for the AUS URI (-cck param) -l L10N_FILE, --L10NFile=L10N_FILE A text file containing the language codes for this build, separated by LF The only issue I see with this help output is that the L10N file file is a bit messed up. I'll correct that in the next version of the patch.
Created attachment 306565 [details] [diff] [review] Patch addressing comments New patch that allows user to specify -d <something> on the command line to turn on debug logging. Changed checkBookmarks so that it can inherit the debug setting since most of the useful debug output comes from that class. Also cleaned up the help display in partner.py
Comment on attachment 306565 [details] [diff] [review] Patch addressing comments There is still a hardcoded debug value in partner.py (isDebug = True). I think that it might be worth it to have another look at how the debug value is set and passed to the various modules, but that can be split off to another bug.
Attachment #306565 - Flags: review?(anodelman) → review+
(In reply to comment #11) > (From update of attachment 306565 [details] [diff] [review]) > There is still a hardcoded debug value in partner.py (isDebug = True). I think > that it might be worth it to have another look at how the debug value is set > and passed to the various modules, but that can be split off to another bug. > Opened Bug 420965 for the hard coded debug value issue, and to look into using the standard python logging module. Checked in patch on trunk --> Fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Mass moving Minotaur bugs to Testing : Minotaur. Filter on MinotaurMassMove to ignore.
Component: Testing → Minotaur
Product: Core → Testing
QA Contact: testing → minotaur
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.