Merge lp:~coreygoldberg/selenium-simple-test/native-to-synthesized into lp:selenium-simple-test

Proposed by Corey Goldberg
Status: Needs review
Proposed branch: lp:~coreygoldberg/selenium-simple-test/native-to-synthesized
Merge into: lp:selenium-simple-test
Diff against target: 16 lines (+6/-0)
1 file modified
src/sst/browsers.py (+6/-0)
To merge this branch: bzr merge lp:~coreygoldberg/selenium-simple-test/native-to-synthesized
Reviewer Review Type Date Requested Status
Canonical ISD QA Team Pending
Review via email: mp+191985@code.launchpad.net

Commit message

switched from native events to synthesized events

Description of the change

turned off native events (this enables JS synthesized events).

see attached bug for details on native vs. synthesized events.

To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :

> also moved sleep in browser launcher, so we don't always get a spurious "can't connect" message every time.

-1 on that, we can succeed without the explicit sleep (that's also what the selenium original implementation is doing), utils.is_connectable will already introduce a delay when it tries to connect, no need for an additional one.

The "can't connect" you're seeing is a real one, it's not spurious. It may be that on jenkins the load is such that the first attempt never succeed but that's not true when working locally so we shouldn't slow down the happy path.

I don't have an opinion about switching the event model so I let others reply on that. As long as the new model has been tested under all envs we care about, whatever works !

Revision history for this message
Vincent Ladeuil (vila) wrote :

Re-reading with a fresh mind, the diff for the events is really:

25 + # disabling native events (use synthesized events instead).
26 + # synthesize events do user emulation via Javascript instead
27 + # of natively. Native keyboard events on Linux break too often
28 + # in Firefox version upgrades.
29 + # see: https://code.google.com/p/selenium/wiki/NativeEventsOnLinux
30 + profile.native_events_enabled = False

The rest is just cosmetic but makes the review harder :-(

It would be better if you split such cleanups out of the real MP IMHO.

Moreover, splitted lines are a pain in tracebacks since python always report
the last line a single ')' in your case. Are you sure it's worth the trouble ?

438. By Corey Goldberg

revert changes except event model

Revision history for this message
Corey Goldberg (coreygoldberg) wrote :

I reverted all changes except switching the event model to synthesized.

will do do any cosmetic or other fixes in new branch.

Revision history for this message
Javier Collado (javier.collado) wrote :

Looking at the bug description, it's quite clear that it's difficult to provide
support for native events, while synthesized events should be much better
supported since they just rely on javascript.

I've made a small investigation on this and it seems that the point of using
native events is that, because of security reasons, some actions might not be
allowed through javascript in forms, which I guess can potentially break a test
case that is working already.

Hence, I'd say that to avoid introducing any regression, it would be safer to
avoid using synthesized events from now on as a default. Alternatively, you
could expose that as a configuration for sst or provide a different test case
class that has some defaults that are considered unstable, so that test cases
have some time to migrate to the new defaults and check if the work for them.

Revision history for this message
Corey Goldberg (coreygoldberg) wrote :

currently, native events are problematic.
I'm using selenium 2.37, Firefox 24, on Saucy... and tests are flaky... occasionally failing due to native events (you can also see this on the selenium CI builds on Precise, which failed at build #96 and #97 : https://jenkins.ubuntone.info/job/u1-qa-sst-trunk-firefox/ ).

I've tracked it down to native events not always working in selenium 2.37, but only when running under Xvfb.

for example,
$ ./sst-test sst.selftests.keys
... will run fine on my machine. but:
$ ./sst-test sst.selftests.keys -x
fails every time. The driver can not retrieve the text in the textfield after sending the native keys events.

running with synthesized fixes things for me. The entire suite of SST tests (unit and functional/acceptance) run good for me locally. I currently do no trust selenium 2.37 with native events, since I can reproduce it failing under Xvfb and have no idea why.

I think the risk and flakiness of staying on native events is greater than replacing with synthesized events.

Revision history for this message
Javier Collado (javier.collado) wrote :

What I meant is that it seems risky to change defaults and force all test cases to use synthesized events because it might fix problems for some test cases and break other ones.

I guess that the decision to make here is what is going to take less work:
- update test cases with problems one by one to use synthesized events
- change the default and fix the test cases that break, if any, because of the change

If you're pretty sure that the way to go is change the default, then go for it, just take into account what are the risks.

Also note that it might happen that someone else is using SST and this change breaks his test cases. A good practice to prevent that from happening would be to have an intermediate release that prints a warning message to let the user know that the defaults are going to change in the future.

Unmerged revisions

438. By Corey Goldberg

revert changes except event model

437. By Corey Goldberg

disable native events and move sleep

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/sst/browsers.py'
2--- src/sst/browsers.py 2013-08-14 13:26:05 +0000
3+++ src/sst/browsers.py 2013-10-22 14:59:44 +0000
4@@ -194,6 +194,12 @@
5
6 def setup_for_test(self, test):
7 profile = webdriver.FirefoxProfile()
8+ # disabling native events (use synthesized events instead).
9+ # synthesize events do user emulation via Javascript instead
10+ # of natively. Native keyboard events on Linux break too often
11+ # in Firefox version upgrades.
12+ # see: https://code.google.com/p/selenium/wiki/NativeEventsOnLinux
13+ profile.native_events_enabled = False
14 profile.set_preference('intl.accept_languages', 'en')
15 if test.assume_trusted_cert_issuer:
16 profile.set_preference('webdriver_assume_untrusted_issuer', False)

Subscribers

People subscribed via source and target branches