Closed Bug 871628 Opened 11 years ago Closed 11 years ago

Add '--show-errors' and '--show-all' options to the automation scrips to provide improved logging/debugging

Categories

(Mozilla QA Graveyard :: Mozmill Automation, enhancement)

Product:
Mozilla QA Graveyard
Component:
Mozmill Automation
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: ffledgling, Assigned: me)

Details

(Whiteboard: [mentor=whimboo][lang=py][good first bug])

Attachments

(1 file, 2 obsolete files)

mypatch.diff
1.75 KB, patch
whimboo
: review-
Details | Diff | Splinter Review 
The mozmill tool has support for --debug, --show-errors options that help debugging and provide improved logging.The need for similar features for the automated test scripts under mozmill-automation is felt.
Whiteboard: [mentor=whimboo][lang=py][good first bug]
--debug is something different and most likely not needed here. What we want instead is --show-all.

If someone wants to work on it please clone: http://hg.mozilla.org/qa/mozmill-automation/
Summary: Add --debug , --show-errors options to the automation scrips to provide improved logging/debugging → Add '--show-errors' and '--show-all' options to the automation scrips to provide improved logging/debugging
I'd like to take on this bug.

I've cloned the repository, and I'll have a poke through the code. I'll let you know if I have any questions.
Great. Thanks Vlad!
Assignee: nobody → me
Status: NEW → ASSIGNED
Two questions. Firstly, I'm having trouble getting any of the mozmill-automation scripts to run, and the website linked in the README file didn't help much. Is this because I'm trying to run them on Mozmill 2.0? Or am I missing something else, in which case is there any other file/website with proper setup instructions for mozmill-automation?

Secondly, Anhad, you mentioned that mozmill supports --debug and --show-errors. I found the --debug option fine, but I grepped the whole mozmill source code and didn't find any --show-errors option, or anything similar-sounding. What did I miss?
Hi Vlad,

I maybe wrong about this and Henrik can correct me if I am but, I think the --show-errors option is found in mozmill 1.5.* and not in mozmill 2.0, also the mozmill-automation scripts at http://hg.mozilla.org/qa/mozmill-automation/ use mozmill 1.5.* .

Mozmill 2.0 is still under development has not yet been released so the automation scripts for it are not yet officially released, you can find them at https://github.com/whimboo/mozmill-automation
and the bug to work on those is Bug 732134.
More info at regarding the same can be found at at https://wiki.mozilla.org/QA/Automation_Services/Projects/Mozmill_Automation/Mozmill2Scripts

I'm using a python virtual environment with pip to use the mozmill-automation 1.5.* scripts.

I think all the virtualenv needs is mozmill and mercurial installed as dependencies, so you can do:

$ virtualenv mozmill-automation
$ cd $_              (cd mozmill-automation, if $_ doesn't work)
$ . bin/activate
$ pip install mozmill==1.5.21 (Or a later 1.5.* if it's available)
$ pip install mercurial
$ hg clone http://hg.mozilla.org/qa/mozmill-automation/

I can't recall the page or file I used to set this up, but I think this should work.
This bug is for the hg version of the automation scripts as given above. We might want to setup something similar for Mozmill 2.0 which will make use of console-level instead. But that should be filed as an issue on my github repository then.

To allow us an easier transition to the new automation scripts I would suggest we already name the command line option --console-level and internally set --show-errors or --show-all, given the value specified:

--console-level ERRORS -> --show-errors
--console-level DEBUG  -> --show-all
Thanks, Anhad, that was very helpful. I've got the virtualenv set up now. But I think I'm missing something obvious about how the tests are supposed to be run. Running for example:

> ./testrun_endurance.py /usr/bin/firefox

returns the error:

*** No builds have been specified. Use --help to see all options.

Having a look at --help, trying out a few commands, or even poking around in the source code didn't really englighten me. What am I missing?
(In reply to Vlad Dolezal from comment #7)
> > ./testrun_endurance.py /usr/bin/firefox
> 
> returns the error:
> 
> *** No builds have been specified. Use --help to see all options.
> 
> Having a look at --help, trying out a few commands, or even poking around in
> the source code didn't really englighten me. What am I missing?

It looks like that you are trying to run Firefox installed by your Linux distribution. This will fail because no application.ini file is available in the same path. Please download Firefox from our FTP server and try it again.
Alright, I've got the automation scripts passing the --show-errors or --show-all arguments to mozmill.

(By the way, thanks for the help above, guys. I got it all running right straight afterwards, I just didn't want to post back here without a progress update.)

One little hitch - the --show-all argument actually doesn't work with the mozmill code as it is. After poking around a little bit, I discovered the piece of code in mozmill responsible:

> if self.options.test and self.options.showall:
>     log_options['level'] = logging.DEBUG

When using the automation script, self.options.test doesn't seem to be set to anything, so the check fails, and the full logging doesn't get turned on. Simplifying the above code to just

> if self.options.showall:

makes it work as intended, and log everything to the console.

Is there a reason for why the condition is written the way it is? Or can we drop the first half of that condition as part of this bug fix, to make it work right?
Attached patch Proposed bugfix (obsolete) — — Splinter Review 
Here's my proposed bugfix. As mentioned above, it depends upon a small change in the mozmill code.
Attachment #771229 - Flags: review?(hskupin)
(In reply to Vlad Dolezal from comment #9)
> > if self.options.test and self.options.showall:
> >     log_options['level'] = logging.DEBUG
> 
> When using the automation script, self.options.test doesn't seem to be set
> to anything, so the check fails, and the full logging doesn't get turned on.
> Simplifying the above code to just
> 
> > if self.options.showall:
> 
> makes it work as intended, and log everything to the console.
> 
> Is there a reason for why the condition is written the way it is? Or can we
> drop the first half of that condition as part of this bug fix, to make it
> work right?

No, I have no idea why that has been implemented that way. It may be in as long as mozmill exists. :/ That could be the reason why the formerly --show-all setting didn't work in our automation scripts. Good catch Vlad!

But I don't think we want to fix this for Mozmill 1.5. So I would suggest that we do the workaround as described by yourself, in setting self.options.test to whatever we want. That will work fine.
Comment on attachment 771229 [details] [diff] [review]
Proposed bugfix

Review of attachment 771229 [details] [diff] [review]:
-----------------------------------------------------------------

In general I like that approach. There are some small things to fix. See my comments inline. Further please try to work on the github version of the repository. We have moved the development over there a couple of weeks ago. The repository you can find here: https://github.com/mozilla/mozmill-automation/. Make sure you work on the hotfix-1.5 branch. Thanks.

::: libs/testrun.py
@@ +152,5 @@
>                                     help="Profile path")
> +
> +        mozmill_options.add_option("--console-level",
> +                                   action="callback",
> +                                   callback=self._add_mozmill_arg,

I wouldn't set it to a callback and directly call the _add_mozmill_arg. Instead lets do it in __init__ after we got the options and arguments from the parser. Reason is that I don't want to overload the above method with special cases. It should be a direct pass-through.

@@ +153,5 @@
> +
> +        mozmill_options.add_option("--console-level",
> +                                   action="callback",
> +                                   callback=self._add_mozmill_arg,
> +                                   choices=['ERRORS','DEBUG'],

We also would need `INFO` which is the default and should not select --show-errors and --show-all. It's just for sanity. Also please use a blank after the comma.

@@ +154,5 @@
> +        mozmill_options.add_option("--console-level",
> +                                   action="callback",
> +                                   callback=self._add_mozmill_arg,
> +                                   choices=['ERRORS','DEBUG'],
> +                                   default=None,

We should default to `INFO`.

@@ +156,5 @@
> +                                   callback=self._add_mozmill_arg,
> +                                   choices=['ERRORS','DEBUG'],
> +                                   default=None,
> +                                   type="choice",
> +                                   metavar="(ERRORS|DEBUG)",

Metavar describes the variable. So please set it to `CONSOLE_LEVEL`.

@@ +157,5 @@
> +                                   choices=['ERRORS','DEBUG'],
> +                                   default=None,
> +                                   type="choice",
> +                                   metavar="(ERRORS|DEBUG)",
> +                                   help="Print extra logging output to console")

Please add the `[default : %default]` part to the help contents. Also I would update the wording to what we have in mozmill 2.0 for that entry.
Attachment #771229 - Flags: review?(hskupin) → review-
Attached patch mypatch.diff (obsolete) — — Splinter Review 
Here are all the changes you asked for, Henrik. Sorry for taking so long to get back to this.

Let me know if you need me to make any other changes.
Attachment #771229 - Attachment is obsolete: true
Attachment #788843 - Flags: review?(hskupin)
Comment on attachment 788843 [details] [diff] [review]
mypatch.diff

Review of attachment 788843 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the update Vlad. I tried to apply the patch for testing but it failed. It looks like you are not working on the latest revision of hotfix-1.5 of the git repository. There is no libs folder anymore the patch references:

> diff --git a/libs/testrun.py b/libs/testrun.py

Please pull again from the official repository, and fix the other nit I mentioned inline.

::: libs/testrun.py
@@ +165,5 @@
> +        # Set mozmill logging level
> +        if self.options.console_level == 'ERRORS':
> +            self.mozmill_args.append('--show-errors')
> +        elif self.options.console_level == 'DEBUG':
> +            self.mozmill_args.append('--show-all')

There is a missing line which most likely needs to be removed now:

> self._mozmill.options.showall = True
Attachment #788843 - Flags: review?(hskupin) → review-
Hi Vlad, would you have the time to update the patch? We kinda would like to have this included in one of our next mozmill-env releases. Thanks!
Flags: needinfo?(me)
Hi Henrik,

sorry, had a lot of stuff come up and this slipped my mind. If I get it to you by the end of the weekend, will that be okay?

Cheers,
Vlad
Flags: needinfo?(me)
Hi Vlad, that should be perfect. We are not going to release 2.0 this week, given that we still have to fix some other stuff for it. Thanks!
Attached patch mypatch.diff — — Splinter Review 
Hi Henrik, here's the updated version. As mentioned before, it depends on making a small change in Mozmill 1.5's __init__.py, changing line 747 from 

>        if self.options.test and self.options.showall:

to 

>       if self.options.showall:

We could try making a workaround, for which we'd need to set mozmill's self.options.test to an empty iterable that evaluates as True. I didn't have the time to try tackling that, though.
Attachment #788843 - Attachment is obsolete: true
Attachment #801406 - Flags: review?(hskupin)
(In reply to Vlad Dolezal from comment #18)
> >        if self.options.test and self.options.showall:
> 
> to 
> 
> >       if self.options.showall:
> 
> We could try making a workaround, for which we'd need to set mozmill's
> self.options.test to an empty iterable that evaluates as True. I didn't have
> the time to try tackling that, though.

Why would we need this? In case of our automation scripts we do not support manifests in combination with Mozmill 1.5. The tests to run, which we hand over to Mozmill, are always given in self.options.test. Have you run it with -m and a manifest?
Flags: needinfo?(me)
(In reply to Henrik Skupin (:whimboo) from comment #19)
> (In reply to Vlad Dolezal from comment #18)
> > >        if self.options.test and self.options.showall:
> > 
> > to 
> > 
> > >       if self.options.showall:
> > 
> > We could try making a workaround, for which we'd need to set mozmill's
> > self.options.test to an empty iterable that evaluates as True. I didn't have
> > the time to try tackling that, though.
> 
> Why would we need this? In case of our automation scripts we do not support
> manifests in combination with Mozmill 1.5. The tests to run, which we hand
> over to Mozmill, are always given in self.options.test. Have you run it with
> -m and a manifest?

Hmm, I was just running the plain automation script, no manifest options. Over here, trying to run a test script (I was using 'testrun_l10n' for testing) with --console-level=DEBUG only returns INFO-level output, unless I made the mentioned change to the mozmill-1.5 file, in which case it really returned DEBUG-level output.
Flags: needinfo?(me)
Comment on attachment 801406 [details] [diff] [review]
mypatch.diff

Review of attachment 801406 [details] [diff] [review]:
-----------------------------------------------------------------

Vlad, I cannot apply this patch. Might be that you are not on tip of the mercurial repository. Can you please check?

Also it's not that strange as I first thought why self.options.test is not existent. The problem here is that we call Mozmill via the API, so in that case we really have to fake an empty array if that helps. But it's a good catch, because that's the reason why we never got detailed output. Thanks!!

Once you got r+ on an upcoming patch, would you mind to update it so we can also apply it onto the hotfix-1.5 branch for our new repository on github? You can find it here: https://github.com/mozilla/mozmill-automation/ If you like git more we could directly head over to that repository and file a new issue over there. Thing is that we want to push mozmill-automation packages from now on, and the latest code is up there.
Attachment #801406 - Flags: review?(hskupin) → review-
I wrote this latest patch on top of the hotfix-1.5 branch on github. Did you want it on top of the mercurial repo instead?
Oh! That would explain it why it failed applying. Would you mind to create a pull request instead on the github repository? In a comment of the pull we can reference this bug for further information, but given that we want to have tracked any changes as pull requests, we cannot handle it here. Thanks!
Alright, I've added a pull request on GitHub. It's my first time making a pull request, so let me know if I did something silly.
Just for information, the PR created for Mozmill 1.5 is:
https://github.com/mozilla/mozmill-automation/pull/66
The pull has actually been fixed about a month ago. So closing this bug.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: