795282 - Further improve time until first test gets started by not blocking on get_appinfo()

Status: RESOLVED WONTFIX

(Testing Graveyard :: Mozmill, defect)

Description

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

On bug 753752 we did a good start in reducing the startup time for the first test, but we are still waiting about 2s until the application information has been transferred. That means the python process is blocked on it before the first test can be started.

We should remove this blocking state by letting mozmill transfer the application_information asynchronously via a new event.

It's not blocking us right now but would be great to have for Mozmill 2.1.

Comment 1

[Usurelu Catalin]

Hi, can I try to fix this bug?

Comment 2

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

Hi Usurelu. We would appreciate if you could work on this bug. Please let me know if it would be still on your plan. If you need more information please ask or join us on IRC in the #automation or #ateam channel. I will be away the next two weeks but others might also help out as needed. CC'ing some folks.

Comment 3

[jbonatakis]

how can i help on this? I am new here and i need directives. Let me know if i can do something.

Comment 4

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

Since we haven't heard back from Usurelu, you can take up this bug if you want. Please check my comment 2 how to get in contact with us. Until then check the URL and get familiar with Mozmill itself. It will help in getting this thing fixed.

https://github.com/mozilla/mozmill
https://developer.mozilla.org/en/Mozmill
https://wiki.mozilla.org/Auto-tools/Projects/Mozmill/RepoSetup

You can ask at any time if something is unclear to you.

Comment 5

[Juan C. Mojica Roa (:jcmojicar)]

Hi, i would like to work on this. I have already followed the steps described above (forking the mozmill repo, installing git, cloning to a local repo, creating a virtualenv and running the setup_development.py to install some python modules).

At this point i need some directions to carry on.

Hope i can help you guys with this.

Comment 6

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

Hi jcmojicar. It's good to see that you have interests in working on this bug. Let me assign it to you now.

Regarding assistance I will be always around for answering your questions. Best would be if you join us on IRC in the #automation channel. It would make it easier to discuss details for this issue. But as a starter please have a look at the existent event handlers (listeners) in the Python code and how those events are getting triggered in the Mozmill extension.

Comment 7

[Juan C. Mojica Roa (:jcmojicar)]

hi Henrik, this what i tried yesterday:

In the mozmill __init__.py:

1- changed the get_appinfo() method by removing the return sentence and the app_info.update... 

2- Change the new listener created for receiving the message to include the app_info.update...

In mozmill.js:

1- in getApplicationDetails(), removed the return sentence and added a fireEvent as frame.events.fireEvent(... -> (based on another fireEvent found in mozmill.js).

In frame.js:

1- Register the listener by adding an entry in broker.addObject (in the same format as the existing ones).
2- Created a function events.<event name> that calls the fireEvent function (as done in the other listeners in frame.js.

This is not working the event is never received back in Python and, dont know how to add some debugging to the js to know whats happening in there.

Comment 8

[Juan C. Mojica Roa (:jcmojicar)]

In mozmill __init__.py, mozmill.js is imported first than frame.js ; in mozmill.js i added:

var frame = {};     Cu.import('resource://mozmill/modules/frame.js', frame);

after that the getAppInformation Event is received in python and the app details are stored in app_info.

I dont see a big improvement over the original code (sometimes 1 second faster to start the first test, and most of the times its the same time than the original code), but now the calling of get_appinfo is not blocking the rest of the code.

If in the original code i move the get_appinfo call after the tests are run, the start of the first test is between 1 or 2 seconds faster, so i dont know if something else is blocking, i was trying to insert a delay in the mozmill.js -> getApplicationDetails function to check if the asynch event can be received in python later (to check that is really not blocking) but could not find how to do it.

Comment 9

[Juan C. Mojica Roa (:jcmojicar)]

Attachment: mozmill __init__.py, with a non-blocking call to get_appinfo()

Comment 10

[Juan C. Mojica Roa (:jcmojicar)]

Attachment: mozmill.js, with the generation of an event in the getApplicationDetails() function.

I added a timeout with nsITimer, and checked that the app info is received asynch in Python.

Comment 11

[Juan C. Mojica Roa (:jcmojicar)]

Attachment: frame.js with the modifications for the getAppInformation event generation

Comment 12

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

(In reply to jcmojicar from comment #7)
> hi Henrik, this what i tried yesterday:

All that sounds reasonable. But I would like to have a look at a complete patch so I could also test it. Therefore please try to work with git to create the patch for all the modifications you did. This is the required method compared to attaching all modified files. I will remove my review flags for now.

> This is not working the event is never received back in Python and, dont
> know how to add some debugging to the js to know whats happening in there.

If you want to debug the JS side you can add `dump('foobar')` calls, which will be printed to stdout.

Comment 13

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

Oh, and please have a look at the following link in how to create the patch. If you have questions regarding git please ask or find me on IRC in the #automation channel.

Comment 14

[Juan C. Mojica Roa (:jcmojicar)]

Attachment: 0001-Modified-__init__.py-frame.js-and-mozmill.js-to-have.patch

Patch for a non-blocking call to get_appinfo().

Comment 15

[Juan C. Mojica Roa (:jcmojicar)]

Just added the patch, i followed the instructions on https://wiki.mozilla.org/Auto-tools/Projects/Mozmill/RepoSetup.

Sorry for the previous uploads, did not know about the patch generation yet. If you find anything that should not be there please let me know.

Comment 16

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

This bug will not be taken for version 2.1. We might want to re-evaluate for 2.2.

Comment 17

[Juan C. Mojica Roa (:jcmojicar)]

hi henrik, is there anything else i can do for this bug?.

Comment 18

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

Sorry that i wasn't able to review your patch yet. But we have massive problems with Mozmill and restart tests and have to get them fixed first. This is mostly taking up my time. I will try to get to a review ASAP.

Comment 19

[Joel Maher ( :jmaher ) (UTC -8)]

Whimboo, can you review this patch?

Comment 20

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

This slipped through. Sorry. I will get to it in a couple of days, when we start the main work for Mozmill 2.1. I really would like to see this included.

Comment 21

[Joel Maher ( :jmaher ) (UTC -8)]

Henrik,  this is a really old review- also a mozmill specific thing; quite possibly this doesn't have a lot of value given the changes required for e10s.

Comment 22

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

Comment on attachment 8355930 [details] [diff] [review]
0001-Modified-__init__.py-frame.js-and-mozmill.js-to-have.patch

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

I feel bad that this review got delayed that massively. I even had a tab still open for that review but actually never submitted my comments. :( 

Given the last comment from Joel and that we are really working hard in getting Marionette into a state that we can convert our Mozmill tests, I would propose that we may not want to fix this bug for Mozmill. As you can see in my inline comments it can cause some weird behavior.

jcmojicar, if you are still interested in another bug please let us know or look at the open automation bugs on bugsahoy: http://www.joshmatthews.net/bugsahoy/?automation=1&unowned=1&simple=1

::: mozmill/mozmill/__init__.py
@@ +450,4 @@
>  
>          try:
>              mozmill = jsbridge.JSObject(self.bridge, js_module_mozmill)
> +            mozmill.getApplicationDetails()

Hm, now that we return the data asynchronously we need a way to ensure that we really got this information returned. If a test would finish kinda quickly and Firefox closes, we might not have any information at all. Maybe we can have a kind of wait for method in stop runner?

https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/__init__.py#L609

::: mozmill/mozmill/extension/resource/driver/mozmill.js
@@ +30,4 @@
>  var os = {};          Cu.import('resource://mozmill/stdlib/os.js', os);
>  var utils = {};       Cu.import('resource://mozmill/stdlib/utils.js', utils);
>  var windows = {};     Cu.import('resource://mozmill/modules/windows.js', windows);
> +var frame = {};     Cu.import('resource://mozmill/modules/frame.js', frame);

nit: please bring this in alphabetical order

::: mozmill/mozmill/extension/resource/modules/frame.js
@@ +312,5 @@
>    events.fireEvent('skip', reason);
>  }
>  
> +events.getAppInformation = function (appDetails) {
> +  events.fireEvent('getAppInformation', appDetails);

The event type should be 'appInfo' or such, so without the actual action.