Enable structured logging command line arguments for marionette + mach

RESOLVED FIXED in mozilla33

Status

Testing
Mozbase
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jgraham, Assigned: chmanchester)

Tracking

(Depends on: 1 bug)

unspecified
mozilla33
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
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).

Comment 1

4 years ago
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.
(Reporter)

Comment 2

4 years ago
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.
(Assignee)

Comment 3

4 years ago
Created attachment 8454019 [details] [diff] [review]
Make mach accept structured logging command line arguments for marionette

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)

Updated

4 years ago
Assignee: nobody → cmanchester
Status: NEW → ASSIGNED
(Assignee)

Comment 4

4 years ago
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)
(Reporter)

Comment 5

4 years ago
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/mach_commands.py
@@ +29,5 @@
>  VARIANT=eng ./build.sh
>  '''
>  
>  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/commandline.py
@@ +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 mach_commands.py as parser=commandline.add_logging_group(argparse.ArgumentsParser())
Attachment #8454019 - Flags: feedback?(james) → feedback+
(Assignee)

Comment 6

4 years ago
Created attachment 8457635 [details] [diff] [review]
Make mach accept structured logging command line arguments for marionette

Ok, this version refrains from adding get_parser to the structured module.
Attachment #8457635 - Flags: review?(james)
(Assignee)

Updated

4 years ago
Attachment #8454019 - Attachment is obsolete: true
(Reporter)

Comment 7

4 years ago
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/mach_commands.py
@@ +67,1 @@
>                                                            options,

Incorrect indentation.
Attachment #8457635 - Flags: review?(james) → review+
https://hg.mozilla.org/mozilla-central/rev/c6886d31a30b
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.