1056329 - Mochitest logging command line arguments only work for mochitest-plain

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Ahmed Kachkach [:akachkach]]

We forgot to add command line args support for other commands (mochitest-browser, mochitest-a11y, mochitest-devtools, etc.).

This should be a straightforward change, there's just a little bug with mochitest-browser dumping options to JSON (which is problematic since we now have an stdout object in options).

Comment 1

[Ahmed Kachkach [:akachkach]]

Attachment: 0001-Bug-1056329-Activate-structured-logging-command-line.patch

Maybe a bit hack-ish for the mochitest-chrome part, but I can't find a better way to do this.

Comment 2

[Andrew Halberstadt [:ahal]]

Comment on attachment 8476314 [details] [diff] [review]
0001-Bug-1056329-Activate-structured-logging-command-line.patch

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

::: testing/mochitest/mach_commands.py
@@ +659,5 @@
>          return self.run_mochitest(test_paths, 'a11y', **kwargs)
>  
>      @Command('webapprt-test-chrome', category='testing',
>          conditions=[conditions.is_firefox],
>          description='Run a webapprt chrome mochitest (Web App Runtime with the browser chrome).')

Did you mean to miss this one?

::: testing/mochitest/runtests.py
@@ +2012,5 @@
>  
>      if "MOZ_HIDE_RESULTS_TABLE" in os.environ and os.environ["MOZ_HIDE_RESULTS_TABLE"] == "1":
>        options.hideResultsTable = True
>  
> +    d = dict((k, v) for k, v in options.__dict__.iteritems() if not k.startswith('log'))

Is this necessary? If it isn't fixing anything, I think it might be safer to leave it as is. Also you could use a dictionary comprehension here (though that would make the file python 2.6 incompatible if it isn't already).

Comment 3

[Ahmed Kachkach [:akachkach]]

(In reply to Andrew Halberstadt [:ahal] from comment #2)
> Comment on attachment 8476314 [details] [diff] [review]
> 0001-Bug-1056329-Activate-structured-logging-command-line.patch
> 
> Review of attachment 8476314 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/mochitest/mach_commands.py
> @@ +659,5 @@
> >          return self.run_mochitest(test_paths, 'a11y', **kwargs)
> >  
> >      @Command('webapprt-test-chrome', category='testing',
> >          conditions=[conditions.is_firefox],
> >          description='Run a webapprt chrome mochitest (Web App Runtime with the browser chrome).')
> 
> Did you mean to miss this one?
> 

Ouch, thanks for pointing this out!

> ::: testing/mochitest/runtests.py
> @@ +2012,5 @@
> >  
> >      if "MOZ_HIDE_RESULTS_TABLE" in os.environ and os.environ["MOZ_HIDE_RESULTS_TABLE"] == "1":
> >        options.hideResultsTable = True
> >  
> > +    d = dict((k, v) for k, v in options.__dict__.iteritems() if not k.startswith('log'))
> 
> Is this necessary? If it isn't fixing anything, I think it might be safer to
> leave it as is. Also you could use a dictionary comprehension here (though
> that would make the file python 2.6 incompatible if it isn't already).

I added this because otherwise chrome tests crash: as I mentioned in the description of the bug: it will try to serialize file objects to JSON, and fail.
I also wanted to do a dict comprehension but since the rest of the code seems to be Python 2.6 compatible I didn't want to break it (arePandasRetiredYet?! :))

Will fix that missing webapprt command and submit an updated patch for checkin

Comment 4

[Ahmed Kachkach [:akachkach]]

Attachment: 0001-Bug-1056329-Activate-structured-logging-command-line.patch

carry r+ forward;

On my local setup, I can't run webapprt-chrome ("assert pathAbs.startswith(self.testRootAbs)" fails) and webapprt-test-content (some random crash), but even without my patch applied I get the same error.

ping ahal, could you test the modified commands on your setup to make sure it works as expected?

Comment 5

[Andrew Halberstadt [:ahal]]

webapprt-test-chrome has test failures for me, but the structured logging options seem to work no problem. webapprt-test-content also works perfectly well. Thanks!

Comment 6

[Ahmed Kachkach [:akachkach]]

Thanks for testing that!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b666f357e0c6

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b666f357e0c6