Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zoldak/acceptance sauce #678

Merged
merged 25 commits into from
Aug 22, 2013
Merged

Zoldak/acceptance sauce #678

merged 25 commits into from
Aug 22, 2013

Conversation

JonahStanley
Copy link
Contributor

Suggested Reviewers: @wedaly @jzoldak

If everything looks good, let me know and I will try to configure my own jenkins to actually test multi-configuration to ensure it works before merging in. I want to get a preliminary look at it to hopefully avoid the time sink of going through multiple jenkins iterations. If nothing is changed on jenkins or the environment variables are not set, the default behavior remains.

This is everything that is needed to allow our acceptance tests to connect to sauce labs and to run the tests in a matrix of browsers. In order to do this, the following was added:

  1. browser.py will now check the settings to see if it is a sauce connection and will properly set up the desired capabilities. If sauce is not enabled, the tests will proceed as normal. Also, the functionality to update the pass/fail status on sauce is included.
  2. The settings are obtained through environment variables that are sent hopefully through jenkins. It is also important to note that sauce can only be connected to on a small list of ports. In keeping with the idea of having a random port (so acceptance and unit tests do not clash), the acceptance tests will choose a random port from this list.

In order to utilize sauce properly, the following variables must be set by jenkins
SAUCE_ENABLED : true if using sauce
SAUCE_USER_NAME, SAUCE_API_KEY : credentials for accesing sauce
SAUCE_PLATFORM, SAUCE_BROWSER, SAUCE_VERSION: specifications for what sauce is running
SAUCE_DEVICE only applies to one set of sauce configurations and doesn't need to be configured
The build name will correspond to the git commit hash found through JOB_NAME

  1. Some tests are known to not work in Sauce or on specific browsers, those scenarios are annotated with tags so they can be skipped.
  2. test_acceptance.sh was changed so that if sauce or a browser type is specified, it will skip all of the tests that do not work on sauce or that browser type.

Lastly, we should be able to specify what browser we want jenkins to run acceptance tests under. Acceptance.py will now take either the environment variable 'LETTUCE_BROWSER' or use a default of 'chrome'. This is so there can be jobs for multiple browsers that do not need to run through sauce and can be configured easily. test_acceptance.sh also reflects this by also allowing the possibility to skip tests based on this variable

@@ -11,6 +13,7 @@ Feature: Advanced (manual) course policy
Given I am on the Advanced Course Settings page in Studio
Then the settings are alphabetized

@sauce
Copy link
Contributor

Choose a reason for hiding this comment

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

If these are the tests being skipped, the tag should be something that indicates this: maybe, skip_sauce?

@JonahStanley
Copy link
Contributor Author

This has now been tested on a Jenkins server under the following 4 conditions.

  1. Normal acceptance tests can be run as usual with no changes.
  2. Firefox can be selected as the desired browser set by an environment variable LETTUCE_BROWSER
  3. A single configuration can be made for sauce labs (details below)
  4. Mutliple configurations can be made for sauce labs (details below)

In order to set one of these environment variables, the variable must be set before test_acceptance is run. This is done is bash (ex. export LETTUCE_BROWSER='chrome'). It was found that multi-configuration jobs can be (and must in our case) set to run sequentially. The caveat being that if the first fails, the rest will as well.

Due to the fact that not all browsers are on all platforms and version numbering is vastly different, a single axis is used for all 4 sauce variables (platform, browser, version, device)

It was also found that there is a weird interaction between jenkins, bash and python string encodings, the easiest way to keep all four variables together was to have them be a hypen ('-') seperated list. Jenkins uses the variables in the configurations to help form links to information for the configuration so this convention was chosen. If the given configuration is not formatted correctly or has an invalid browser type or platform, a default of 'Linux-chrome--' is used. There are no checks on version numbering or device type.

Also, when a multi-configuration project is run, the main console output will only say pass or fail. By clicking on the configuration, a more detailed console is shown.

Lastly, a jar file containing the sauce-labs tunnel must be running in the background at all times. This can be accomplished with the following command from sauce

while :; do killall java; sleep 30; java -jar Sauce-Connect.jar <ACCESS_KEY>; done

It is important to note that the tunnel can be very delicate. There have been a few times in which sauce throws java thread errors which can introduce some noise into the test. The basic acceptance tests (those not running in sauce) should be the more definitive measure on if a feature is broken all around

@@ -2,6 +2,8 @@ Feature: Advanced (manual) course policy
In order to specify course policy settings for which no custom user interface exists
I want to be able to manually enter JSON key /value pairs

#Sauce labs does not play nicely with CodeMirror
Copy link
Contributor

Choose a reason for hiding this comment

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

space after # pls.

@JonahStanley
Copy link
Contributor Author

All browsers can run LMS acceptance tests now.

# If we were unable to get a valid session within the limit of attempts,
# then we cannot run the tests.
if not success:
raise IOError("Could not acquire valid {driver} browser session.".format(driver='remote'))
Copy link
Contributor

Choose a reason for hiding this comment

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

driver=browser_driver

JonahStanley added 16 commits August 21, 2013 16:05
Conflicts:
	common/djangoapps/terrain/browser.py
Added comment about the feature
Conflicts:
	common/djangoapps/terrain/browser.py
Also, now lettuce browser can be specified by jenkins
…ipped for each browser

This logic also covers if a lettuce_browser is specified

Removing browser matrix from readme
Changed to have a default for device since it isn't always needed

Tags are more clear

Fixed stylistic issues
LMS was arbitrarily chosen for now.

Fixed up pylint and pep8 errors

Fixed up pylint and pep8 errors

Changed naming to be better

Changed Sauce Info to obtaining a JSON string
Changed the port listing

Changing to better readability
Version numbers have very different ranges for different browsers so not having a dictionary of those.

Fixed a whitespace issue

Fixed pylint/pep8 violations

Don't need django_url

Spacing issues

Changed how commenting works

Forgot one

Used wrong name

Changed around importing

Remove django_url

Fixed function orderingn

Made logic nicer for getting a new browser

Modifying tests to run in opera

Needed to increase time to account for slow sauce loading

Now safari LMS works

Forgot an assert statement

Skipping a few tests for opera
Configurations must be defined before hand

Reduced pylint violations

We only support 4 browsers
CMS tests should now run on IE

CMS tests will now run on Safari

Rebased wrong

Forgot to update the test_acceptance.sh

Changed way of string splitting
JonahStanley pushed a commit that referenced this pull request Aug 22, 2013
@JonahStanley JonahStanley merged commit a7851a9 into master Aug 22, 2013
@JonahStanley JonahStanley deleted the zoldak/acceptance-sauce branch August 22, 2013 14:32
chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
Temporary fix to set the speed of video player to 1.0
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
e-kolpakov referenced this pull request in open-craft/edx-platform Jun 22, 2016
fix user group filter to return distinct users data
dfrojas pushed a commit to eduNEXT/edx-platform that referenced this pull request Aug 31, 2017
* po/django-admin:
  Add docstrings for code quality check
johanseto added a commit to nelc/edx-platform that referenced this pull request Jan 22, 2024
feat: update removing priority from nelp koa
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.

4 participants