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

(e2e) Add geckodriver for various platforms and corresponding configuration #3087

Closed
wants to merge 3 commits into from

Conversation

geloescht
Copy link
Contributor

Description of changes

This adds binaries from https://github.com/mozilla/geckodriver/releases like the ones we use for Chrome. They are needed for Firefox and Selenium to play nice beginning with FF47.
Some remaining questions, and the reason I am doing this in a PR: (@keystonejs/testing)

  • What about licensing, do we need to add a note somewhere? Is there a note about the license of chromedriver already?
  • Should we try and auto-detect OS and Browser versions in the future, now that it's all not as straightforward as npm run test-e2e anymore?

…ration

* Firefox 47+ is no longer compatible with WebDriver directly and needs geckodriver as an adapter for Selenium to continue working
@geloescht
Copy link
Contributor Author

geloescht commented Jun 28, 2016

Running single tests works with FF47 now, but running multiple tests triggers this bug: mozilla/geckodriver#58
It is hopefully going to be fixed soon in GeckoDriver 0.9. I'll put this PR on hold until then.

@geloescht
Copy link
Contributor Author

We also need to fix e2e with selenium in background, because we are not passing cli_opts from the config to selenium. Unless, hopefully people from nightwatch are going to fix nightwatchjs/nightwatch#470 soon.

@webteckie
Copy link
Contributor

Don't you need to also patch nightwatch-no-process.json with the same changes?

* Note that those are not working yet, because we are not parsing cli_opts from the config
@geloescht
Copy link
Contributor Author

I updated geckodriver to v0.9 but I still can't run our testsuite with it. So keeping this on hold for now.

@JedWatson
Copy link
Member

On a related node, I'd really like to get these binaries out of Keystone's git repo. They're going to blow out the size of the history, forever. Maybe we can encapsulate them in a separate npm package and depend on that? (if they're not already available like that, surely we're not the only ones who have this issue?)

@JedWatson
Copy link
Member

@morenoh149
Copy link
Contributor

@geloescht
Copy link
Contributor Author

Still failing due to Selenium bug, see here: mozilla/geckodriver#58 (comment)

@JedWatson
Copy link
Member

JedWatson commented Aug 22, 2016

@jstockwin and @webteckie is this still relevant or should we close it?

@geloescht for context, those two have been / are massively refactoring our e2e setup

@jstockwin
Copy link
Contributor

@JedWatson and @geloescht - As of #3351, I'd suggest this is no longer relevant. As Jed's above comment says, we don't actually want these drivers in the repo.

@geloescht If you can find a package to do this, that'd be great - otherwise just update the readme with instructions on where to download these drivers from. I'd also like to get a package working for the chromedrivers, but it's nowhere near the top of my list now we're doing a full refactor (#3359).

@mxstbr mxstbr closed this Aug 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants