698769 - mutt should be brought up to the reality that the MozMill class is now used via the API

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Jeff Hammel]

Currently the mutt js results are gathered by screen-scraping stdout: 

https://github.com/mozautomation/mozmill/blob/master/mutt/mutt/mutt.py#L241

Instead, the JSON handler or other output handler should be used with
a mozmill.Results object and the test results accumulated precisely.
With the way things are done, if the output is messed up, we have no
way of knowing when we fail so the tests look like they pass when they
in fact don't.  For instance, we ignore skips which could even be
browser crashes.

Comment 1

[Andrew Halberstadt [:ahal]]

I believe that this is the updated link from the description: https://github.com/mozilla/mozmill/blob/8cbe87ef4f013db9c1807dba55fc35edf0c61dfd/mutt/mutt/mutt.py#L192

We recently landed MozTest which contains a results class. We should use this new results class instead of the mozmill one. This means that a fix for this bug might also involve patches to https://github.com/mozilla/mozbase/tree/master/moztest along the way.

Comment 2

[jdev005]

can i be assigned this bug ?

Comment 3

[Andrew Halberstadt [:ahal]]

Thanks for the last patch, here you go! Once again, let us know if you need help either here or on #ateam.

Comment 4

[Jeff Hammel]

So I'm not sure if this is still a good first bug.  mutt has changed a lot since this was filed. It now runs mozmill JS tests via API vs subshelling: https://github.com/mozilla/mozmill/blob/8cbe87ef4f013db9c1807dba55fc35edf0c61dfd/mutt/mutt/mutt.py#L182

The JSResults class isn't used at all and should be removed: https://github.com/mozilla/mozmill/blob/8cbe87ef4f013db9c1807dba55fc35edf0c61dfd/mutt/mutt/mutt.py#L182/jsresult

Probably quite a few other things in this file need to catch up to reality here.

But the biggest barrier is that the mutt tests seem horribly and utterly broken, which needs to be fixed.  I don't know how or why we continue to get in this state.  Someone needs to look at them and fix them.  I get the tests getting into an infinite loop with 100% CPU usage 

I also get

(mozmill)│mutt -b `which firefox`
Running python tests
Usage: mozmill [options]

Its my opinion that probably all these things should be fixed.

mozmill: error: Options -t/--test and -m/--manifest are mutually exclusive.
.

Comment 5

[Jeff Hammel]

I get this in an infinite loop:

ERROR | ERROR | Traceback (most recent call last):
ERROR | Traceback (most recent call last):
ERROR | ERROR | Traceback (most recent call last):
ERROR | ERROR | ERROR | Traceback (most recent call last):
ERROR | ERROR | ERROR | ERROR | Traceback (most recent call last):
ERROR | ERROR | ERROR | ERROR | ERROR | Traceback (most recent call last):
ERROR | ERROR | ERROR | ERROR | ERROR | ERROR | Traceback (most recent call last):
ERROR | ERROR | ERROR | ERROR | ERROR | ERROR | ERROR | Traceback (most recent call last):
ERROR | ERROR | ERROR | ERROR | ERROR | ERROR | ERROR | ERROR | Traceback (most recent call last):
ERROR | ERROR | ERROR | ERROR | ERROR | ERROR | ERROR | ERROR | ERROR | Traceback (most recent call last):
ERROR | Traceback (most recent call last):
ERROR | Traceback (most recent call last):
ERROR | ERROR | Traceback (most recent call last):
ERROR | Traceback (most recent call last):
ERROR | Traceback (most recent call last):
ERROR | ERROR | Traceback (most recent call last):

Comment 6

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

Jeff, the infinite loop is bug 761564.

Comment 7

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

So the infinite loop has been fixed by my patch on bug 792412. The other patch on bug 761564 would change a bit how Mozmill would have to be used but shouldn't longer block this bug.

jdev005, would you still be interested in continuing on that bug? Jeff, I assume you would be up for mentoring on this bug? :)

Comment 8

[Jeff Hammel]

> Jeff, I assume you would be up for mentoring on this bug? :)

sure

Comment 9

[cmtalbert]

Henrik, I think that this kind of work is going on in other areas, so I am removing the mentorship flags on this bug. If this is indeed a good mentored bug, then we need a good description of what is required and an updated mentor for it.

Comment 10

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

We never call the `aquire()` method which gets the information from the console but directly make use of the results object return by Mozmill after the tests have been finished:

https://github.com/mozilla/mozmill/blob/master/mutt/mutt/mutt.py#L211
https://github.com/mozilla/mozmill/blob/master/mutt/mutt/mutt.py#L189

So it should be safe to completely remove the JSResults class. There is no need for it anymore, now with the logger implemented correctly.

Comment 11

[Sambuddha Basu [:sambuddhabasu1]]

I am interested in solving this bug. Can someone assign it to me?

Comment 12

[Andrei Eftimie]

Hi Sambuddha, I've assigned this bug to you.

Henrik will mentor you for this issue, but he is away for a couple more days.
Until he returns I might be able to help you get started.

As linked in this bug, here is documentation on starting with mozmill:
https://wiki.mozilla.org/Auto-tools/Projects/Mozmill/RepoSetup

Comment 13

[Sambuddha Basu [:sambuddhabasu1]]

Attachment: mutt has been brought up to the reality that the MozMill class is now used via the API

Comment 14

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

Comment on attachment 8467196 [details] [diff] [review]
mutt has been brought up to the reality that the MozMill class is now used via the API

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

The patch looks fine. Thank you Sambuddha! There is one thing you might want to obey in the future. Please export the patch via 'git format-patch' as instructed here: https://wiki.mozilla.org/Auto-tools/Projects/Mozmill/RepoSetup#Ready_for_a_review. I will fix that this time after applying it.

Comment 15

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

Landed on master for 2.1:
https://github.com/mozilla/mozmill/commit/d0df8ed7cb4737aad2482ce930cad6faa4bed05e