732134 - Add support to Mozmill 2.0 for automation scripts

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Automation, defect)

Comment 1

[David Guo [:davidg]]

Attachment: Part 1 - Remove dependency on the mozmill wrapper

Comment 2

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

Great work David! It's good to see that we really can get rid of the wrapper class. I will check the patch later today. Would you mind creating a temporary repository on github for the automation scripts repository? It would help us to test the current work without affecting our hg repository.

I also have CC'ed Mark Banner from the Thunderbird team so that he is aware of upcoming changes to our scripts.

Comment 3

[David Guo [:davidg]]

Good idea.
git repo up: https://github.com/dglol/mozmill-automation/

Comment 4

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

Great. It would have been good if you would have imported the original version and created a pull request for the above changes on a separate branch. Please do it for follow-up patches.

Comment 5

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

Clint, do you mind doing a review here?

Comment 6

[David Guo [:davidg]]

Attachment: Part 1 - Remove dependency on the mozmill wrapper v2

Comment 7

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

Comment on attachment 612729 [details] [diff] [review]
Part 1 - Remove dependency on the mozmill wrapper v2

I wouldn't bother Clint for such a review. I will do it once Mozmill 1.5.10 has been released.

Comment 8

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

Comment on attachment 612729 [details] [diff] [review]
Part 1 - Remove dependency on the mozmill wrapper v2

>[mq]: bug-732134-pt1-v2

Please export the patch correctly.

>-import mozmill_wrapper
>+import mozmill

\o/

>@@ -130,16 +130,18 @@ class TestRun(object):
>+        # Consume the system arguments
>+        del sys.argv[1:]

Nice. I wonder why we haven't done this before.

>-            report.JUnitReport(self._mozmill.mozmill.get_report(), filename)
>+            custom_report = self.update_report(self._mozmill.mozmill.get_report())
>+            report.JUnitReport(custom_report, filename)

No, we don't have to put our custom report data in here. It's not necessary for the JUnit report.

>@@ -295,47 +298,45 @@ class TestRun(object):
>         if self.options.port:
>-            self._mozmill.jsbridge_port = self.options.port
>+            self._mozmill.mozmill.jsbridge_port = self.options.port
>         if self.timeout:
>-            self._mozmill.jsbridge_timeout = self.timeout
>+            self._mozmill.mozmill.jsbridge_timeout = self.timeout
[..]
>+            self._mozmill.mozmill.persisted["screenshotPath"] = path

Is it really '._mozmill.mozmill'? I don't think that's right. There are a couple of those instances.

>+    def send_report(self, report_url):
>+        """ Send the report to a CouchDB instance """
>+
>+        report = self.update_report(self._mozmill.mozmill.get_report())
>+        return self._mozmill.mozmill.send_report(report, report_url)
>+

So that means that there is no way to call an update_report on the Mozmill class? If that is still an issue for Mozmill 2 we should check what we can enhance.

Where is the removal of the mozmill_wrapper.py module? I can't find it in this patch.

Comment 9

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

David, will you have time to update the patch in the next couple of days? It would be great so we can get rid of the wrapper class. If not please tell us, so that someone else can pick it up. Thanks.

Comment 10

[David Guo [:davidg]]

(In reply to Henrik Skupin (:whimboo) from comment #9)
> David, will you have time to update the patch in the next couple of days? It
> would be great so we can get rid of the wrapper class. If not please tell
> us, so that someone else can pick it up. Thanks.


Hi Henrik, I should have enough time to update the patch later this week.

Comment 11

[David Guo [:davidg]]

> No, we don't have to put our custom report data in here. It's not necessary for the
> JUnit report.

Thanks for the clarification.

> Is it really '._mozmill.mozmill'? I don't think that's right. There are a couple of
> those instances.

Yes, this should be right. The _mozmill is the instance of CLI, which itself has an instance of Mozmill.

> So that means that there is no way to call an update_report on the Mozmill class?
> If that is still an issue for Mozmill 2 we should check what we can enhance.

Correct. Mozmill 2 actually makes it more difficult right now with the addition of a separate reporting class.

Comment 12

[David Guo [:davidg]]

Attachment: Part 1 - Remove dependency on the mozmill wrapper v3

Comment 13

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

(In reply to David Guo [:davidg] from comment #11)
> > Is it really '._mozmill.mozmill'? I don't think that's right. There are a couple of
> > those instances.
> 
> Yes, this should be right. The _mozmill is the instance of CLI, which itself
> has an instance of Mozmill.

Ok, should be fine for this version of the automation scripts. For Mozmill 2 it will hopefully be cleaner.

> > So that means that there is no way to call an update_report on the Mozmill class?
> > If that is still an issue for Mozmill 2 we should check what we can enhance.
> 
> Correct. Mozmill 2 actually makes it more difficult right now with the
> addition of a separate reporting class.

Can you please file this as a new bug given that you know what the problems are you were facing? Please add the [mozmill-2.0?] whiteboard entry so that it is tracked.

Comment 14

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

Comment on attachment 623290 [details] [diff] [review]
Part 1 - Remove dependency on the mozmill wrapper v3

Looks good. But something is wrong with the creation of the junit report. I get a 'list index out of range' failure when running any type of test. Can you please correct that? Further please check the other command line options too. With that fixed it will be ready to land I assume.

Comment 15

[David Guo [:davidg]]

(In reply to Henrik Skupin (:whimboo) from comment #14)
> Comment on attachment 623290 [details] [diff] [review]
> Part 1 - Remove dependency on the mozmill wrapper v3
> 
> Looks good. But something is wrong with the creation of the junit report. I
> get a 'list index out of range' failure when running any type of test. Can
> you please correct that? Further please check the other command line options
> too. With that fixed it will be ready to land I assume.

It seems that there was a reason why I put the update_report in _generate_custom_report! With the mozmill_wrapper, the get_report method was overrided to update the report before returning it:

http://hg.mozilla.org/qa/mozmill-automation/file/894d36f87616/libs/mozmill_wrapper.py#l52

I will update this for the next patch.

Comment 16

[David Guo [:davidg]]

Attachment: Part 1 - Remove dependency on the mozmill wrapper v4

Comment 17

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

Comment on attachment 623872 [details] [diff] [review]
Part 1 - Remove dependency on the mozmill wrapper v4

Works fine now. Sorry that I haven't spotted that specific part.

Comment 18

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

Landed as:
http://hg.mozilla.org/qa/mozmill-automation/rev/05966ff9a7f5

From now on this bug will be a tracking bug for the Mozmill 2.0 work. Please file new issues as new blocking bugs.

Comment 19

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

I had to backout the patch because it caused a regression. No more results of non-restart tests get send. The same would also apply to other test-runs where we run multiple test-runs in a row.

http://hg.mozilla.org/qa/mozmill-automation/rev/0f930421dcb0

Dave is on it and hopefully has it fixed soon.

Comment 20

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Part 1 - Remove dependency on the mozmill wrapper v5

Fixed regression that caused report to be sent per binary rather than per testrun.

Comment 21

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I've tested this with Nightly and each testrun. Here are the reports:
http://mozmill-crowd.blargon7.com/#/addons/report/f87375a634b1a5ba746e5f763a091970
http://mozmill-crowd.blargon7.com/#/endurance/report/f87375a634b1a5ba746e5f763a0928b2
http://mozmill-crowd.blargon7.com/#/functional/report/f87375a634b1a5ba746e5f763a093541
http://mozmill-crowd.blargon7.com/#/functional/report/f87375a634b1a5ba746e5f763a093e46
http://mozmill-crowd.blargon7.com/#/l10n/report/f87375a634b1a5ba746e5f763a09c4c6
http://mozmill-crowd.blargon7.com/#/remote/report/f87375a634b1a5ba746e5f763a09e4da
http://mozmill-crowd.blargon7.com/#/update/report/f87375a634b1a5ba746e5f763a09f444
http://mozmill-crowd.blargon7.com/#/update/report/f87375a634b1a5ba746e5f763a09f44a

Comment 22

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

Re-landed as:
http://hg.mozilla.org/qa/mozmill-automation/rev/6a1577ca9a31

I hope it will stick this time.

Comment 23

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Henrik Skupin (:whimboo) from comment #8)
> >@@ -130,16 +130,18 @@ class TestRun(object):
> >+        # Consume the system arguments
> >+        del sys.argv[1:]
> 
> Nice. I wonder why we haven't done this before.

Possibly because it causes bug 756515?

Comment 24

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

For a WIP patch of a possible update testrun see the attachment on bug 659488. It's kinda old and we will probably not do it this way, but I think it could help just for reference.

Comment 25

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I have an initial working version of a functional testrun for Mozmill 2.0 here: https://github.com/davehunt/mozmill-automation/tree/functional-testrun

Comment 26

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

Basically this is done. Mozmill 2.0 is in production for about a month now and works fine. Also issues are tracked at https://github.com/mozilla/mozmill-automation/ now. Closing as fixed.