607450 - [report] Make stack for test failures a parseable JSON object

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Right now all the information inside the stack of an exception is a plain text. It would be great to have it as a JSON object which we can better use.

That would be a first step. Further improvements have been shown by Mark today. Mark, do you have a link to the source where you construct your custom stack?

Comment 1

[Mark Banner (:standard8)]

Here's what we do:

http://hg.mozilla.org/comm-central/annotate/12096a34ac2a/mail/test/mozmill/runtest.py#l321

Note it is currently broken when run against hotfix-1.5.1 which would be a significant restriction to Thunderbird picking up that code which we need for a different bug. It's possible we can fix up runtest.py but I've not looked at that in detail yet.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Jeff, when we would like to modify the exception stack, where in the current hotfix code we would have to place this code? I would like to make it a full object instead of a big string blob.

Comment 3

[cmtalbert]

I think this is largely a dupe of bug 594404.  But since the "make parse friendly exceptions" is lacking on detail and since the results are already in pseudo-json, I am going to keep them in JSON and make them more parse friendly that way.  Again, since there are a number of 1.5.2 items on the list, this is unfortunately lower in the priority.  I want it for 2.0, but I can't fit it into 1.5.2, unless someone else wants to take on the patch.

Comment 5

[Heather Arthur [:harth]]

So we have a few options here:

1) Just turn the stack into an array, splitting on newlines
2) Also format the lines, ala Mark's patch. This will make it even less machine-parsable however
3) Split into an array of JSON objects with linenumber, func, filename as properties. Possibly making it less readable, but more parsable.

So my question is, Henrik, would you have any use for inspecting things like the line number and filenames of the calls on the stack? If not I vote we keep things at #1

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I would absolutely prefer a JSON object here. At least for the data I retrieve from the database. It it's easy doable to parse each string for the child elements I would be ok in using a string on the client side. I could do the parsing and split in the dashboard itself. Otherwise lets use an object directly. Wouldn't it even be better formatted on the command line?

Comment 7

[Heather Arthur [:harth]]

Attachment: make stack an array of JSON objects

Apply this patch to see what it looks like with the JSON objects. It's readable with --format=pprint, otherwise it's less readable than before, but valid JSON.

I think we might as well do this parsing on the js side, that way the consumers can do whatever they want with the various parts like the line number.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

* I don't see the json object printed in pretty format when specifying --format=pprint 

* Can we use function instead of func?

* Some file names look strange: "resource://mozmill/modules/frame.js -> file:///data/testing/mozmill/default/firefox/testAwesomeBar/testAccessLocationBar.js". Why do we list two files?

* We should be smarter and probably have to remove all stack frames above the test file. Those are not really helpful and only add unnecessary bloat.

Comment 9

[Heather Arthur [:harth]]

(In reply to comment #8)
> * I don't see the json object printed in pretty format when specifying
> --format=pprint

now?

> * Can we use function instead of func?
>

sure
 
> * Some file names look strange: "resource://mozmill/modules/frame.js ->
> file:///data/testing/mozmill/default/firefox/testAwesomeBar/testAccessLocationBar.js".
> Why do we list two files?

The first line, this comes directly from the standard js error object. Not sure why it does this, probably due to the subscript loader.

> * We should be smarter and probably have to remove all stack frames above the
> test file. Those are not really helpful and only add unnecessary bloat.

True, but what about Mozmill framework bugs?

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #9)
> (In reply to comment #8)
> > * I don't see the json object printed in pretty format when specifying
> > --format=pprint
> 
> now?

Now what? What has been changed?

> > * Some file names look strange: "resource://mozmill/modules/frame.js ->
> > file:///data/testing/mozmill/default/firefox/testAwesomeBar/testAccessLocationBar.js".
> > Why do we list two files?
> 
> The first line, this comes directly from the standard js error object. Not sure
> why it does this, probably due to the subscript loader.

Could be the reason for. We could have a regex, which strips the part before '-> '.

> > * We should be smarter and probably have to remove all stack frames above the
> > test file. Those are not really helpful and only add unnecessary bloat.
> 
> True, but what about Mozmill framework bugs?

Or we make it dependent on the --debug flag. Only if this flag is set we report framework bugs?

Comment 11

[Heather Arthur [:harth]]

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > * I don't see the json object printed in pretty format when specifying
> > > --format=pprint
> > 
> > now?
> 
> Now what? What has been changed?

Sometimes you have problems with setuptools, is all. What OS are you on, and what console are you using?

> > > * We should be smarter and probably have to remove all stack frames above the
> > > test file. Those are not really helpful and only add unnecessary bloat.
> > 
> > True, but what about Mozmill framework bugs?
> 
> Or we make it dependent on the --debug flag. Only if this flag is set we report
> framework bugs?

This sounds like a good idea.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As usual my testing happens as always on OS X 10.6.

Comment 13

[Heather Arthur [:harth]]

Attachment: break stack into array of JSON objects, and filter for chrome calls

This patch will turn the stack into an array of JSON objects like this:

    "stack": [
      {
        "function": "([object Object])", 
        "lineno": "94", 
        "filename": "file:///Users/harth/repos/mozmill-tests/firefox/testCookies/testDisableCookies.js"
      }, 
      {
        "function": "handleWindow(\"type\",\"Browser:Preferences\",[object Proxy])", 
        "lineno": "403", 
        "filename": "file:///Users/harth/repos/mozmill-tests/shared-modules/utils.js"
      }, 
      {
        "function": "openPreferencesDialog([object Proxy],[object Proxy])", 
        "lineno": "373", 
        "filename": "file:///Users/harth/repos/mozmill-tests/shared-modules/prefs.js"
      }, 
      {
        "function": "()", 
        "lineno": "69", 
        "filename": "file:///Users/harth/repos/mozmill-tests/firefox/testCookies/testDisableCookies.js"
      }
    ]


And will filter out calls that come from the harness code, unless --debug is specified.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #13)
>         "function": "([object Object])", 
[..]
>         "function": "()", 

What happened to those function names? Why are those empty?

Comment 15

[Heather Arthur [:harth]]

(In reply to comment #14)
> (In reply to comment #13)
> >         "function": "([object Object])", 
> [..]
> >         "function": "()", 
> 
> What happened to those function names? Why are those empty?

I think those happen if the code is an anonymous function. We can't help it, that's just what the call looks like in the stack that comes from an Error()

Comment 16

[Jeff Hammel]

Comment on attachment 518112 [details] [diff] [review]
break stack into array of JSON objects, and filter for chrome calls

You probably want to move prog = re.compile("(.*)@(.*?)(?: -> (file\:\/\/\/\S*))?\:(\d*)$") to a class-level variable since its static, but other than that looks good!

Comment 17

[Heather Arthur [:harth]]

master:
https://github.com/mozautomation/mozmill/commit/c20ff7540d619e4c9a825a5d12e60ea50d0f838e