Last Comment Bug 704583 - Add testing mode to FocusManager to address running webdriver tests concurrently
: Add testing mode to FocusManager to address running webdriver tests concurrently
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla12
Assigned To: Olli Pettay [:smaug]
:
Mentors:
Depends on:
Blocks: 718468 774892
  Show dependency treegraph
 
Reported: 2011-11-22 12:30 PST by Marlena Compton [:marlenac]
Modified: 2015-04-21 08:21 PDT (History)
23 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
-
verified
verified
11+
verified


Attachments
add testMode property (8.96 KB, patch)
2011-12-19 05:13 PST, Neil Deakin
no flags Details | Diff | Review
testmode, enabled using a pref (8.81 KB, patch)
2012-01-03 11:38 PST, Olli Pettay [:smaug]
enndeakin: review+
bugs: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta-
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Review

Description Marlena Compton [:marlenac] 2011-11-22 12:30:10 PST
In order to run automated webdriver tests concurrently in Firefox, we will need Firefox to allow multiple windows to have focus at the same time.  

How the tests are run:
On the command line, we specify a number of tests to run concurrently.  Selenium-webdriver opens multiple windows using the same binary but different Firefox processes and profiles to run 5 tests at a time (as an example.)  

Current behavior:
1.  Running tests concurrently:
Webdriver simulates a mouse moving over a webpage using native events.  Because of this, if the window running a test does not have focus, it will delay the native events which causes the test to fail.  If we run 5 tests at a time, this means that roughly 4/5 of the tests will fail.  Here is a screencast which shows this behavior:  http://screencast.com/t/bcNWBRDS
2.  Window behavior within a single test:
Some tests need to work with multiple windows.  We're currently working on tests for paypal and browser id which both open separate windows.  Currently, we have a test for writing a review which opens a new window.  

There are a couple of bugs with some discussion on this topic, but we're opening a new bug to handle comments regarding the new functionality which will be introduced to Firefox.  

Previous bugs:
https://bugzilla.mozilla.org/show_bug.cgi?id=566671
https://bugzilla.mozilla.org/show_bug.cgi?id=702605
Comment 1 Olli Pettay [:smaug] 2011-11-22 12:37:38 PST
(In reply to Marlena Compton [:marlenac] from comment #0)
> In order to run automated webdriver tests concurrently in Firefox, we will
> need Firefox to allow multiple windows to have focus at the same time.
To be clear, this means separate Firefox processes, right?
Since currently *everything* in Firefox assumes that that the focus in only in one  place at
a time - I mean inside one Firefox process.


> 1.  Running tests concurrently:
> Webdriver simulates a mouse moving over a webpage using native events. 
> Because of this, if the window running a test does not have focus, it will
> delay the native events which causes the test to fail.  If we run 5 tests at
> a time, this means that roughly 4/5 of the tests will fail.  Here is a
> screencast which shows this behavior:  http://screencast.com/t/bcNWBRDS
(I don't quite understand why you need to run more than one test at a time, but I guess that is
not relevant here.)
Comment 2 David Burns :automatedtester 2011-11-22 12:46:08 PST
(In reply to Olli Pettay [:smaug] from comment #1)
> (In reply to Marlena Compton [:marlenac] from comment #0)
> > In order to run automated webdriver tests concurrently in Firefox, we will
> > need Firefox to allow multiple windows to have focus at the same time.
> To be clear, this means separate Firefox processes, right?
> Since currently *everything* in Firefox assumes that that the focus in only
> in one  place at
> a time - I mean inside one Firefox process.
> 

Every browser that Selenium fires up is in its own process and has its own profile. Each is siloed off because the remote server we deploy keeps each test separate by having a session.

Let me know if you have any other questions.
Comment 3 Alex Keybl [:akeybl] 2011-12-18 18:41:47 PST
Not tracking for Firefox since this wouldn't block ship.

Sending Olli's way since it sounds like he's going to take a crack at this.
Comment 4 Neil Deakin 2011-12-19 05:13:33 PST
Created attachment 582798 [details] [diff] [review]
add testMode property

This is what I had partially implemented. It's just a flag on focus manager to indicate test mode. It should work for the most basic usage, but it won't handle other windows opening.
Comment 5 Olli Pettay [:smaug] 2012-01-02 07:26:33 PST
Marlena, David, have you tried Neil's patch? Is it enough for your use cases?
Comment 6 Marlena Compton [:marlenac] 2012-01-03 10:47:38 PST
Thanks for getting to this.  Is this in Aurora and I just pass in --testmode?
Comment 7 Olli Pettay [:smaug] 2012-01-03 10:52:24 PST
You need to make a build yourself (or use tryserver) and enable testMode in focusmanager service.
I guess I could push the patch to tryserver, and add a pref to enable testMode.
Comment 8 Olli Pettay [:smaug] 2012-01-03 11:37:40 PST
Search for be5171a98e4b in http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds
in a few hours.
To enable testmode, set focusmanager.testmode to true.
Comment 9 Olli Pettay [:smaug] 2012-01-03 11:38:12 PST
Created attachment 585487 [details] [diff] [review]
testmode, enabled using a pref
Comment 11 Marlena Compton [:marlenac] 2012-01-03 13:20:42 PST
Thanks, I'll download and try it out.
Comment 12 Marlena Compton [:marlenac] 2012-01-05 13:52:51 PST
I've now got the nightly build, a build of selenium to run against it and selenium grid to use both.

Just for reference:
There are 2 tests in this file that fail because of the problem: 
https://github.com/mozilla/Addon-Tests/blob/master/tests/test_layout.py
test_the_applications_listed_in_other_applications
test_other_applications_thunderbird


Currently, I'm still trying to figure out where to set the focusmanager preference to true in selenium grid's firefox profile.  
https://github.com/mozilla/moz-grid-config/tree/master/firefoxprofiles/certificateExceptions

Tried altering prefs.js but that's clearly not the right place.
Comment 13 Marlena Compton [:marlenac] 2012-01-05 14:58:18 PST
Dave Hunt and I had a chat about the previous comment.  We're working out the best way to have our test runner plugin pytest-mozwebqa alter the the preference for each test.  

Olli: Is it possible to get a nightly try-build with the pref set to true by default for the purpose of testing the patch?
Comment 14 Olli Pettay [:smaug] 2012-01-05 15:11:09 PST
Would it help if there was support for some environment variable to enable testmode?

export MOZ_ENABLE_FOCUS_TEST_MODE=1
./firefox runtests...

Though, have you looked at how mochitest sets some preferences to test profile?
Comment 15 David Burns :automatedtester 2012-01-09 13:57:30 PST
@smaug

I have tested this and it looks good for my first set of tests. I would like to run another set that loads a native component into the addon and uses that. Thats where I believe most people were seeing the issue.

Awesome work!
Comment 16 Henrik Skupin (:whimboo) 2012-01-09 14:59:16 PST
Resetting QA contact because it should not contain a single person.
Comment 17 Henrik Skupin (:whimboo) 2012-01-09 15:12:35 PST
This build is even working perfectly fine for Mozmill tests when those are getting run in parallel. Having such a fix available would drastically reduce the time QA needs to run release tests for functional and update tests and sign off on releases.
Comment 18 Olli Pettay [:smaug] 2012-01-10 02:24:34 PST
(In reply to David Burns :automatedtester from comment #15)

> Awesome work!
Thank Neil :)
Comment 19 David Burns :automatedtester 2012-01-10 12:34:03 PST
Thanks @Neil!

@smaug, @neil can we get this landed please since this can have huge impact to testing!

Thanks again!
Comment 20 Olli Pettay [:smaug] 2012-01-10 12:41:28 PST
Comment on attachment 585487 [details] [diff] [review]
testmode, enabled using a pref

Neil, could you review my changes.
Comment 21 Olli Pettay [:smaug] 2012-01-10 12:42:00 PST
Comment on attachment 585487 [details] [diff] [review]
testmode, enabled using a pref

r+ for the code Neil wrote.
Comment 22 Neil Deakin 2012-01-10 12:44:58 PST
Comment on attachment 585487 [details] [diff] [review]
testmode, enabled using a pref

If that's all you need, then ok.
Comment 23 Dão Gottwald [:dao] 2012-01-12 02:34:32 PST
Which patches need to land? Both? Is the first one obsolete? What commit message(s) should be used, who should be the author? Please make such things clear if you want to use checkin-needed.
Comment 24 Olli Pettay [:smaug] 2012-01-13 05:30:40 PST
https://hg.mozilla.org/mozilla-central/rev/742d5f9f284d

If other changes are needed for test mode, please file new bugs.
Comment 25 Henrik Skupin (:whimboo) 2012-01-16 12:42:18 PST
Tested the latest Nightly build with Mozmill and the focusmanager testing preference set to true. It works perfectly. Waiting for feedback from David and Dave regarding Selenium. Once it has been verified we should try to get this backported to older branches.
Comment 26 Henrik Skupin (:whimboo) 2012-01-19 06:42:49 PST
We were trying to verify this patch on the latest Nightly with Selenium too, but due to bug 719371 we aren't able to without having to update Selenium first. Given that the patch has been marked as verified with Selenium for the tryserver build, and also works as expected with Mozmill in the Nightly build, we would like to go ahead and ask for approval for the beta and release branch.

Drivers, this patch is very important for QA to be able to run our functional tests with Mozmill and the existing Selenium tests concurrently in different processes of Firefox at the same time. This will give us an immense performance boost and will reduce the time we need for signing off from testing releases of Firefox and web properties. The code introduced by the patch is pref'ed of per default and will only be invoked in testing mode.

QA really wants to have this feature added to Firefox 10 because that's our upcoming ESR and we do have to support it for the next 9 month. If we don't land it for Firefox 10 it would hold us off from improving our performance across our supported branches.

We would really appreciate any effort in making it available in Firefox 10 and 11. Thanks.
Comment 27 Olli Pettay [:smaug] 2012-01-19 07:40:47 PST
Tryserver builds for aurora and beta (I didn't compile the patch, just pushed using aurora and beta)
https://tbpl.mozilla.org/?tree=Try&rev=350355ed6003
https://tbpl.mozilla.org/?tree=Try&rev=dd7cec72b916
Comment 28 David Burns :automatedtester 2012-01-20 04:07:15 PST
Selenium tests are running much better than before. Events are being fired in the correct order when using the Synthetic Events in Selenium. 

There are some times where it doesnt work cleanly with jQuery widgets. There is some flakiness there but that could be related to bug 702605 which I need to work on

I am happy with this patch as it stands atm
Comment 29 Henrik Skupin (:whimboo) 2012-01-20 06:59:08 PST
(In reply to Olli Pettay [:smaug] from comment #27)
> https://tbpl.mozilla.org/?tree=Try&rev=350355ed6003
> https://tbpl.mozilla.org/?tree=Try&rev=dd7cec72b916

Both tryserver builds are also working perfectly with Mozmill. I will request approval for the patch on the older branches.
Comment 30 Henrik Skupin (:whimboo) 2012-01-20 07:00:08 PST
Comment on attachment 585487 [details] [diff] [review]
testmode, enabled using a pref

Olli, could you please fill in the details since you are the patch author? Thanks.

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
Comment 31 Alex Keybl [:akeybl] 2012-01-22 18:37:08 PST
(In reply to Henrik Skupin (:whimboo) from comment #26)
> QA really wants to have this feature added to Firefox 10 because that's our
> upcoming ESR and we do have to support it for the next 9 month. If we don't
> land it for Firefox 10 it would hold us off from improving our performance
> across our supported branches.
> 
> We would really appreciate any effort in making it available in Firefox 10
> and 11. Thanks.

This is not something we'd consider taking in FF10 at this point. When putting together the ESR proposal, we agreed that we would take no extra risk in order to better support FF10's ESR. We'll have to take the hit on performance for FF10. Sorry for the inconvenience.

For the same reason as before, there is no reason to track for 10/11. I'm also unsetting the Aurora flag since this is not yet ready for nomination (no risk assessment, etc.).
Comment 32 Olli Pettay [:smaug] 2012-01-23 01:57:39 PST
Comment on attachment 585487 [details] [diff] [review]
testmode, enabled using a pref

[Approval Request Comment]
Regression caused by (bug #): NA
User impact if declined: NA
Testing completed (on m-c, etc.): Tested on m-c, Aurora and Beta tryserver builds
also tests.
Risk to taking this patch (and alternatives if risky):
No risk. The patch add a normally pref'ed off code path for testing, which
is apparently very useful for certain testing.
Comment 33 Olli Pettay [:smaug] 2012-01-23 01:58:22 PST
(In reply to Olli Pettay [:smaug] from comment #32)
> also tests.
also tested.
Comment 34 Alex Keybl [:akeybl] 2012-01-23 08:45:35 PST
Comment on attachment 585487 [details] [diff] [review]
testmode, enabled using a pref

[Triage Comment]
Approved for Aurora. Beta is denied per comment comment#31.
Comment 36 Henrik Skupin (:whimboo) 2012-01-26 05:08:37 PST
Verified fixed with:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0a1) Gecko/20120118 Firefox/12.0a1

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:11.0a2) Gecko/20120125 Firefox/11.0a2 ID:20120125042008
Comment 37 Alex Keybl [:akeybl] 2012-02-16 15:34:40 PST
Please land on mozilla-esr10 as soon as possible. For more information on the landing process, please see https://wiki.mozilla.org/Release_Management/ESR_Landing_Process
Comment 38 Henrik Skupin (:whimboo) 2012-02-27 04:03:57 PST
Smaug or anyone else, can we please make sure to land the patch on the ESR branch, or are we already too late?
Comment 39 Lukas Blakk [:lsblakk] use ?needinfo 2012-02-28 12:32:35 PST
Comment on attachment 585487 [details] [diff] [review]
testmode, enabled using a pref

This needs to land today preferably, but tomorrow if necessary so we can meet the deadline for go to build.  Please let me know if there are any concerns.
Comment 40 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2012-02-29 19:18:42 PST
https://hg.mozilla.org/releases/mozilla-esr10/rev/9611190d4db4
Comment 41 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-05 10:33:41 PST
Henrik, can you please verify this works for you now with Firefox 10.0.3ESR?
Comment 42 Henrik Skupin (:whimboo) 2012-03-05 12:34:41 PST
Looks good with the 10.0.3ESR candidate build.
Comment 43 Maxime LM 2015-04-21 01:58:29 PDT
Hello, 

I'm facing this issue with selenium webdriver and parallel/concurrently tests on firefox : https://code.google.com/p/selenium/issues/detail?id=157

One of the suggestion of solution is to use the focusmanager.testmode at true, but without success in our cases.

Should we reopen this issue or create a new one?

OS : Windows 7 Pro N 64bits
selenium-java : 2.44 / 2.45
selenium-firefox-driver : 2.44 / 2.45
Firefox version : 33.0.1
Comment 44 Dave Hunt (:davehunt) 2015-04-21 02:38:36 PDT
Firefox automation is now based on Marionette (https://developer.mozilla.org/en-US/docs/Mozilla/QA/Marionette), and is also the future backend for Selenium/WebDriver when using Firefox. This issue was verified almost three years ago, so if it's regressed you should open a new issue. You should first try using the latest version of Firefox supported by your Selenium version.

Note You need to log in before you can comment on or make changes to this bug.