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

Use TravisCI + Saucelabs to run seltest #720

Merged
merged 4 commits into from
Jun 24, 2015
Merged

Use TravisCI + Saucelabs to run seltest #720

merged 4 commits into from
Jun 24, 2015

Conversation

ihodes
Copy link
Member

@ihodes ihodes commented Jun 12, 2015

This change has two main parts:

  1. ./scripts/update-screenshots.sh starts a test server locally, and then updates the seltest screenshots via a browser on SauceLabs
  2. ./scripts/travis-run-pdiff-tests.sh runs the screenshot tests on Travis via a browser on SauceLabs

Fixes #644.

To do:

  • Run seltest tests on SauceLabs from TravisCL.
  • Run screenshot updates with a local Cycledash test server in a remote browser via SauceLabs
  • Figure out how to handle Sauce Labs credentials locally (right now require SAUCE_USERNAME and SAUCE_ACCESS_KEY to be in ENV, but we don't want this in the repo (other than encrypted, like they are in the travis-test.sh script).
  • Use hammerlab credentials.
  • Start sauce connect in a better way (no sleep allowed—Travis seems to do this well).
  • Update docs including information about SauceLabs.
  • Upload failed screenshots to Imgur.

filed #759

Review on Reviewable

@ihodes ihodes force-pushed the sauce-travis-labs branch 6 times, most recently from bb4ef54 to cfe2efd Compare June 12, 2015 21:19
@ihodes ihodes changed the title Use Saucelabs to run seltest Use TravisCI + Saucelabs to run seltest Jun 12, 2015
@ihodes ihodes force-pushed the sauce-travis-labs branch 20 times, most recently from 4609d0b to 33b60f2 Compare June 24, 2015 18:36
Locally and automatically on Travis
Now we blur the input so we don't sometimes capture a screenshot of the
cursor blinking.

Also generate imgur screenshots on Travis if a test fails. (Add
encrypted IMGUR_CLIENT_ID to .travis.yml to support this, too).
Factor out shared config into JSON
Don't try to start the SC tunnel if it's already open
@danvk
Copy link
Contributor

danvk commented Jun 24, 2015

Just a few comments. Thanks for putting in all the work to figure this out! The vertically truncated screenshots issue is unfortunate, but hopefully we can find a workaround.


Reviewed 29 of 29 files at r1.
Review status: all files reviewed at latest revision, 6 unresolved discussions, all commit checks successful.


.travis.yml, line 11 [r1] (raw file):
say something about where these came from


cycledash/static/scss/style.scss, line 31 [r1] (raw file):
Is this a merge issue or an intended change?


DEVELOP.md, line 157 [r1] (raw file):
consistency (sp)


scripts/travis-run-pdiff-tests.sh, line 3 [r1] (raw file):
on Travison a Travis worker, as written this could be read as "this kicks off a Travis job".


scripts/travis-run-pdiff-tests.sh, line 26 [r1] (raw file):
What does the error look like if either of these environment variables doesn't exist? Might be clearer to add an explicit check for them at the top of the script, like you do in update-screenshots.sh.

Speaking of which, how do these get set?


tests/pdifftests/test.py, line 127 [r1] (raw file):
removedremoves


Comments from the review on Reviewable.io

@ihodes
Copy link
Member Author

ihodes commented Jun 24, 2015

I agree. Chrome team has been punting on this for years, though. One solution may be to use Firefox. We can also try running Chrome in a headless GUI.


Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


cycledash/static/scss/style.scss, line 31 [r1] (raw file):
merge, nixed


scripts/travis-run-pdiff-tests.sh, line 26 [r1] (raw file):
The error is super explicit (says the keys were not set/set to none).

They're set in the .travis.yml file (now commented), so they'll always be there.


Comments from the review on Reviewable.io

@danvk
Copy link
Contributor

danvk commented Jun 24, 2015

LGTM, feel free to merge on green.

After this goes in, can you send a quick message to everyone who works on Cycledash summarizing what we need to do (i.e. register for Sauce Labs, set environment variables) to stay up to date?


Review status: 24 of 29 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.


Comments from the review on Reviewable.io

ihodes added a commit that referenced this pull request Jun 24, 2015
Use TravisCI + Saucelabs to run seltest
@ihodes ihodes merged commit 759632c into master Jun 24, 2015
@ihodes ihodes deleted the sauce-travis-labs branch December 23, 2015 16:53
@hammer hammer unassigned danvk Feb 8, 2016
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.

None yet

2 participants