Closed Bug 704583 Opened 8 years ago Closed 8 years ago
Add testing mode to Focus
Manager to address running webdriver tests concurrently
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
(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.)
(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.
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.
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.
Marlena, David, have you tried Neil's patch? Is it enough for your use cases?
Thanks for getting to this. Is this in Aurora and I just pass in --testmode?
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.
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.
Thanks, I'll download and try it out.
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.
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?
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?
@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!
Resetting QA contact because it should not contain a single person.
Status: NEW → ASSIGNED
QA Contact: mozmarlena → general
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.
Assignee: bugs → nobody
Component: General → DOM
OS: Mac OS X → All
Product: Firefox → Core
QA Contact: general → general
Hardware: x86 → All
(In reply to David Burns :automatedtester from comment #15) > Awesome work! Thank Neil :)
Thanks @Neil! @smaug, @neil can we get this landed please since this can have huge impact to testing! Thanks again!
Comment on attachment 585487 [details] [diff] [review] testmode, enabled using a pref Neil, could you review my changes.
Attachment #585487 - Flags: review?(enndeakin)
Comment on attachment 585487 [details] [diff] [review] testmode, enabled using a pref r+ for the code Neil wrote.
Attachment #585487 - Flags: review+
Comment on attachment 585487 [details] [diff] [review] testmode, enabled using a pref If that's all you need, then ok.
Attachment #585487 - Flags: review?(enndeakin) → review+
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.
Attachment #582798 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/742d5f9f284d If other changes are needed for test mode, please file new bugs.
Assignee: nobody → enndeakin
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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.
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.
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
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
(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 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):
(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 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.
(In reply to Olli Pettay [:smaug] from comment #32) > also tests. also tested.
Comment on attachment 585487 [details] [diff] [review] testmode, enabled using a pref [Triage Comment] Approved for Aurora. Beta is denied per comment comment#31.
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
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
Smaug or anyone else, can we please make sure to land the patch on the ESR branch, or are we already too late?
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.
Attachment #585487 - Flags: approval-mozilla-esr10+
Henrik, can you please verify this works for you now with Firefox 10.0.3ESR?
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
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.
You need to log in before you can comment on or make changes to this bug.