Skip to content

Jonahstanley/firefox acceptance tests - #594

Merged
JonahStanley merged 27 commits into
masterfrom
jonahstanley/firefox-acceptance-tests
Aug 8, 2013
Merged

Jonahstanley/firefox acceptance tests#594
JonahStanley merged 27 commits into
masterfrom
jonahstanley/firefox-acceptance-tests

Conversation

@JonahStanley

Copy link
Copy Markdown
Contributor

Suggested reviewers: @wedaly @jzoldak

Making a pull request so that this can be merged in and I can work on fixing the remaining tests separately and tests in firefox can begin (and start catching more bugs). In order to run the acceptance tests in firefox, set LETTUCE_BROWSER = 'firefox' in acceptance.py or the browser_driver to 'firefox' in browser.py.

With these changes, only the following will not work in firefox and are marked:

  • Checklist links could not be activated. This is skipped in firefox for now
  • There is a redirect issue in firefox that only occurs when running the whole suite. This is not skipped yet due to the fact that it works individually and technically spans multiple steps. This affects signup and login features on the lms side

The following things were added in order to get the acceptance tests to run on firefox:

  • Links had to be clicked explicitly. This involved changing the css selector and can be applied to chrome
  • Notification bars could not be clicked through the webdriver so instead they are clicked through javascript on firefox
  • A function called is_firefox() was added that will return if the browser_driver name is 'firefox' allowing for some specific solutions / skipping based on browser
  • Firefox will not trigger certain events through the webdriver, thus the function trigger_event was added. This will trigger a jquery event on the given selector. By default, it triggers the 'change' event on index 0. This was helpful in getting certain input fields to update properly. This function should be avoided when possible.
  • Code Mirror will now work on firefox, this was done by forcibly blurring the codemirror field
  • Certain things could not be clicked or touched by firefox because they were underneath other elements. On firefox, they are made visible through javascript
  • Some small changes were made to the code so that less needed to be different between firefox and chrome.

These tests were verified to still work with the latest version of chrome and chrome driver. The version of firefox that was tested was 22.0 and 23.0. The tests were run on both ubuntu and mac OS with the same results

One other major issue is that firefox will not fire certain events if firefox is not the active window. As far as I know, this will only affect the LaTex scenario in problem-editor.feature, the testing automatic quoting in advanced-settings.feature and some of the video alpha tests. Splinter will start the firefox browser as the active window. However, this means that it needs to remain the active window making testing a pain due to not being able to run in the background. Also, this will have implications on running multiple acceptance tests in parallel on firefox as only one of them can be in the foreground.

When this is ready to be merged, I would like to run this branch once in jenkins to verifiy that I did not introduce anything as this is a larger pull request.

AS OF LATE:
The only errors in the test suite for firefox are in the following features:
cms/checklists.feature - due to firefox not being able to make links active afftecting those two scenarios
lms/login.feature - due to firefox having a weird redirect issue only when the whole suite is run
lms/signup.feature - due to firefox having a weird redirect issue only when the whole suite is run

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a risk of a StaleElement exception here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup... I changed both to use css_value instead. However, I am keeping this line so I can get the list of how many elements (but not access them there)

@wedaly

wedaly commented Aug 7, 2013

Copy link
Copy Markdown
Contributor

Good work getting these running in Firefox!

Main points:

  1. We shouldn't be explicitly skipping tests for Firefox -- if it doesn't pass, it should fail.

  2. Put spaces after all comment "#" characters

  3. We should never have to change test expectations between browsers. If there is a difference in behavior, then we should decide which behavior is correct and have the tests fail if that behavior isn't satisfied.

In extreme cases (where the differences are very small and not likely to get fixed), then we should weaken the test expectations so they verify the same (correct, but less specific) behavior on all browsers.

@JonahStanley

Copy link
Copy Markdown
Contributor Author

I have addressed all of the comments as well as updated the PR with the expected firefox failures. As far as relaxing the test expectations, I think this only makes sense for the max_attempts ones in problem-editor.py as that is the only true implementation difference in firefox and chrome. The rest are weird firefox issues

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space after "#"

@wedaly

wedaly commented Aug 8, 2013

Copy link
Copy Markdown
Contributor

👍

@jzoldak

jzoldak commented Aug 8, 2013

Copy link
Copy Markdown
Contributor

👍

JonahStanley pushed a commit that referenced this pull request Aug 8, 2013
@JonahStanley
JonahStanley merged commit 02d8da1 into master Aug 8, 2013
@JonahStanley
JonahStanley deleted the jonahstanley/firefox-acceptance-tests branch August 8, 2013 20:49
chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
…istinguish_instructors

Feature/ibrahim/discussion distinguish instructors
itsjeyd referenced this pull request in open-craft/openedx-platform Dec 30, 2015
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Mar 30, 2016
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Mar 30, 2016
caesar2164 pushed a commit to caesar2164/edx-platform that referenced this pull request Apr 27, 2017
…update

Update xblock SHAs for max_score progress page
dgamanenko referenced this pull request in raccoongang/edx-platform Jun 14, 2018
Sujeet1379 pushed a commit to chandrudev/edx-platform that referenced this pull request Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants