Closed Bug 1036869 Opened 6 years ago Closed 6 years ago

Enable structured logging command line arguments for marionette + mach


(Testing :: Mozbase, defect)

Not set


(Not tracked)



(Reporter: jgraham, Assigned: chmanchester)


(Depends on 1 open bug)



(1 file, 1 obsolete file)

At the moment running marionette tests through mach doesn't show any useful debug output. One could in-principle change this by using something like --log-tbpl=-. However the --log-* arguments aren't enabled for marionette when run through mach. This makes debugging rather hard.

(there may be a seperate bug that we need to make the mach output more verbose in some cases, but this is independent of that, since there are use cases for e.g. getting the raw logs out of mach).
I'm guessing the common scenario looks something like this:

1) Run a mach command
2) See that verbose output isn't verbose enough
3) Run mach command again with verbose output

This is inefficient because it requires you to run a command again. Furthermore, if you encountered a transient/intermittent failure, there's no guarantee you'll see a failure a second time.

IMO, we should save the super verbose logs to a file no matter what. If we need something beyond "cat" to view them in a human friendly way, we can provide a mach command for that.

We have the objdir/.mozbuild directory for objdir-specific data. This is MachCommandBase.statedir.
We have ~/.mozbuild for global Mozilla state foo. This would be self._mach_context.state_dir.
I still think it makes sense to enable the command line arguments, but yes, mach defaulting to logging structured output to a file might make sense. There is already a structured logger command for formatting a file containing structured log arguments, so we could certainly add a mach command for post-processing. Nevertheless the ability to get e.g. a HTML report using --log-html=test_results.html seems valuable.

This is all possible within the structured logging framework; we just need mach to set appropriate defaults for commands that use structured logging.
I think the original impetus for this bug could be solved by making the mach formatter include more in the output when it detects a failure (stacktraces are in the "message" field of the logged object, right now machformatter ignores this unconditionally for test_status and test_end events).

At any rate, here's a really awful but effective way to get mach to accept structured commandline arguments I stumbled across while working on bug 1027665 in case anyone is interested in it short-term.
Assignee: nobody → cmanchester
Comment on attachment 8454019 [details] [diff] [review]
Make mach accept structured logging command line arguments for marionette

The issue with this is for scripts that instantiate an option parser in addition to mach's argument parser (as marionette does), which means both parsers need to be aware of the '--log-*' arguments. Although I do think the way to do this is through the parser kwarg to Command, so maybe this is a necessary evil for those scripts instantiating optionparsers directly.
Attachment #8454019 - Flags: feedback?(james)
Comment on attachment 8454019 [details] [diff] [review]
Make mach accept structured logging command line arguments for marionette

Review of attachment 8454019 [details] [diff] [review]:

This basically looks like it will work. It doesn't seem any more crazy than the existing code, so I'm happy to proceed with this approach for now.

::: testing/marionette/
@@ +29,5 @@
>  VARIANT=eng ./
>  '''
>  def run_marionette(tests, b2g_path=None, emulator=None, testtype=None,
> +    address=None, binary=None, topsrcdir=None, **kwargs):

This indentation is pretty weird (I know it was like that before).

::: testing/mozbase/mozlog/mozlog/structured/
@@ +26,5 @@
>          return sys.stdout
>      else:
>          return open(name, "w")
> +def get_parser():

How often do we expect to need this? If we are going to have to use this as a stopgap for all testsuites, it makes sense to have it here. Otherwise we could just make add_logging_group return the parser and inline the code in as parser=commandline.add_logging_group(argparse.ArgumentsParser())
Attachment #8454019 - Flags: feedback?(james) → feedback+
Ok, this version refrains from adding get_parser to the structured module.
Attachment #8457635 - Flags: review?(james)
Attachment #8454019 - Attachment is obsolete: true
Comment on attachment 8457635 [details] [diff] [review]
Make mach accept structured logging command line arguments for marionette

Review of attachment 8457635 [details] [diff] [review]:

r+ with code formatting fixup.

::: testing/marionette/
@@ +67,1 @@
>                                                            options,

Incorrect indentation.
Attachment #8457635 - Flags: review?(james) → review+
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.