Allow client options to be passed into Jasmine #519

jsilvestri · 2017-05-05T15:39:17Z

This addresses #518. Example usage in teaspoon_env.rb:

client = Selenium::WebDriver::Remote::Http::Default.new
client.timeout = 180 # 3 mins instead of the default 60 seconds
config.driver_options = {client_driver: :firefox, client_driver_opts: {http_client: client}}

jsilvestri · 2017-05-05T15:53:37Z

I am not quite sure I understand what is failing in the tests...

CyborgMaster · 2017-09-19T14:46:29Z

I would love to get this merged. We also would like to pass client options. In this case, ours is to use chrome headless to run the tests using something like this:

  options = ::Selenium::WebDriver::Chrome::Options.new
  options.add_argument('--headless')
  options.add_argument('--disable-gpu')
  config.driver_options = { client_driver: :chrome, client_driver_ops: { options: options } }

CyborgMaster · 2017-09-19T17:47:08Z

Ok, in order to get things working for our use case, passing in the Chrome::Options as indicated above to activate headless mode, I had to modify things a little bit.

The line in question in this PR is

driver = ::Selenium::WebDriver.for(driver_options[:client_driver], driver_options[:client_driver_opts] || {})

Calling driver_options in this manner sets up a HashWithIndifferentAccess, but that causes problems inside Selenium, producing the following crash:

> [#] ArgumentError: wrong number of arguments (given 1, expected 0)
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/remote/bridge.rb:43:in `handshake'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/chrome/driver.rb:58:in `initialize'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/common/driver.rb:46:in `new'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/common/driver.rb:46:in `for'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver.rb:86:in `for'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/bundler/gems/teaspoon-60c2f9a1a466/lib/teaspoon/driver/selenium.rb:26:in `run_specs'

This is trying to pass the options into this function:

        def self.handshake(**opts)

but because the HashWithIndifferentAccess uses string keys, this fails. It only works with hashes with symbol keys.

Changing the line to:

driver = ::Selenium::WebDriver.for(driver_options[:client_driver], @options[:client_driver_opts] || {})

bypasses the HashWithIndifferentAccess creation and allows the tests to run, however, running them a second time using guard-teaspoon crashes with the following:

> [#] Errno::ECONNREFUSED: Failed to open TCP connection to 127.0.0.1:9515 (Connection refused - connect(2) for "127.0.0.1" port 9515)
> [#] /Users/jeremy/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/net/http.rb:882:in `rescue in block in connect'
> [#] /Users/jeremy/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/net/http.rb:879:in `block in connect'
> [#] /Users/jeremy/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/timeout.rb:74:in `timeout'
> [#] /Users/jeremy/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/net/http.rb:878:in `connect'
> [#] /Users/jeremy/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/net/http.rb:863:in `do_start'
> [#] /Users/jeremy/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/net/http.rb:852:in `start'
> [#] /Users/jeremy/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/net/http.rb:1398:in `request'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/remote/http/default.rb:124:in `response_for'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/remote/http/default.rb:78:in `request'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/remote/http/common.rb:61:in `call'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/remote/bridge.rb:170:in `execute'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/remote/bridge.rb:103:in `create_session'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/remote/bridge.rb:54:in `handshake'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/chrome/driver.rb:58:in `initialize'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/common/driver.rb:46:in `new'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver/common/driver.rb:46:in `for'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/gems/selenium-webdriver-3.5.2/lib/selenium/webdriver.rb:86:in `for'
> [#] /Users/jeremy/.rvm/gems/ruby-2.3.1@allynova/bundler/gems/teaspoon-60c2f9a1a466/lib/teaspoon/driver/selenium.rb:26:in `run_specs'

This can be solved by duping the options each time they are used. My guess is that Selenium modifies the object on use somehow and expects a fresh Chrome::Options object each time.

This gives us the final line (which is working for me) here:

driver = ::Selenium::WebDriver.for(driver_options[:client_driver], @options[:client_driver_opts]&.dup || {})

@jsilvestri, do you want to modify your PR to match this, would you like me to open a PR to your repo to merge this change into yours, or should I just open my own PR?

tilsammans · 2017-11-12T10:19:36Z

@CyborgMaster I would just open a new PR that contains all the needed changes.

Then we can get some movement on this issue! If you need help with anything, don't be shy.

jejacks0n · 2017-11-13T15:39:28Z

I'm a little lost -- can someone explain like I'm 5? I'm slammed at work, so if you just give me a clean PR with an explanation and reasonable certainty that it takes everything needed into account I'll merge it.

tilsammans · 2017-11-14T08:22:56Z

#530 opened which contains all the fixes.

tilsammans · 2017-12-03T18:58:45Z

@jsilvestri @jejacks0n pull requests are all failing now because of CodeClimate changes. Can you or anyone look into this? I'd like to get #530 merged soon as I have fixes waiting for them downstream.

zzak · 2017-12-03T23:49:18Z

@tilsammans I'm trying to fix the Code Climate situation here in #534

zzak · 2017-12-25T04:30:50Z

Hello ya'll, #534 fixes the build so once that's merged we can hopefully push this forward. I'd like to see people be able to use teaspoon with headless chrome :)

Happy Holidays!

jejacks0n · 2017-12-27T18:53:52Z

There's several PRs that are linking to Other PRs.. I've merged the one that fixed the build.. what's the status of other ones?

tilsammans · 2017-12-28T10:11:33Z

In principle #530 contains the fixed code, but tests are now failing because the method signature changed.

I don't have time in the coming weeks to install qmake locally. It's such a pain to do.

zzak · 2017-12-28T11:55:03Z

@tilsammans I can't tell the difference, maybe you can rebase?

tilsammans · 2017-12-28T12:15:19Z

@zzak this one—#519—isn't mine. #530 I rebased already.

zzak · 2017-12-28T12:31:20Z

@tilsammans Yep, I've seen both but I'm clearly not in a place to tell which patch is best -- my suggestion is to rebase so the tests pass and / or explain how they defer. It's very likely someone with more context (i.e. the maintainer) can tell the difference but all your PR proclaims is that it "continues" this PR. How about elaborating on why your patch should be merged instead of this? What does it improve or how does it differ? Adding as much context as possible will help you get your patches merged in an ad-hoc open-source project such as this.

tilsammans · 2017-12-28T14:22:24Z

@zzak allright: my pull request #530 contains the changes proposed by @CyborgMaster which duplicates the options. He never submitted a pull request so I did that.

The tests are currently failing because the signature of the method changed. (both here and in my PR). Since I don't have qmake nor do I have 6 hours to spare to install it. that's where we all stand.

References: - jejacks0n/teaspoon#519 - jejacks0n/teaspoon#530 - codevise/pageflow#952

mathias · 2018-12-17T15:38:22Z

I've been trying to get this working by using the branch, but every time, I get
Selenium::WebDriver::Error::WebDriverError: unable to connect to chromedriver 127.0.0.1:9515
locally.

This works on Travis. Chromedriver is working for the Capybara Rails tests. Any ideas why I get that?

Edit: I tried the branch for #530 too. Not sure what's up.

jejacks0n · 2018-12-17T16:27:12Z

you might need to use the chromedriver-helper gem. https://github.com/flavorjones/chromedriver-helper

Just add it to your gemfile and it should work -- I'm not fully sure about this either, but I've seen that on other projects too.

mathias · 2018-12-17T16:33:26Z

@jejacks0n I've got that (Capybara on Rails suite is using it) and tried updating it, but haven't gotten this working locally. Oddly, it works on Travis using the exact same config.

trcarden · 2019-04-07T15:23:25Z

Since I arrived here trying to get chrome headless working and concluded from this PR originally that chrome headless was not being worked on I thought I would share this update:

I noticed that another PR was merged to support selenium options on master via: #537. If you install from master it looks like you can specify the to use chrome headless via

  config.driver_options = {
    client_driver: :chrome,
    selenium_options: {
      options: Selenium::WebDriver::Chrome::Options.new(args: ['headless', 'disable-gpu'])
    }
  }

I am going to try and help @jejacks0n a little here:

I have updated the wiki with a small note here: https://github.com/jejacks0n/teaspoon/wiki/Using-Selenium-WebDriver
I think the following can be closed:
- Allow client options to be passed into Jasmine #519 - (this PR) unless @jsilvestri objects
- Selenium Net::ReadTimeout #518 - the issue for this PR
- Jasmine client options #530 - (another PR trying to do the same)
A friendly shoutout around Use headless chrome with teaspoon solidusio/solidus#2534 as the monkey patch might not be required anymore (or if all the waiting is might be something the mainline might benefit from)

It sure would be great if we could get a new gem cut too.

mathieujobin · 2021-03-11T22:56:44Z

looks like #537 properly fix this issue, please reopen if you disagree

jsilvestri added 2 commits May 5, 2017 11:38

Allow client options to be passed into Jasmine

c89ee40

handle nil case

60c2f9a

tilsammans mentioned this pull request Nov 12, 2017

Drop PhantomJS; use Chrome headless? codevise/pageflow#887

Closed

tilsammans mentioned this pull request Nov 14, 2017

Jasmine client options #530

Closed

andoneve mentioned this pull request Jan 26, 2018

Use headless chrome with teaspoon solidusio/solidus#2534

Merged

bmulholland mentioned this pull request Mar 5, 2018

allow passing selenium options #537

Merged

ericgriffis added a commit to NYULibraries/eshelf that referenced this pull request Jun 27, 2018

Configure teaspoon on chrome headless; not working

81ccd1c

References: - jejacks0n/teaspoon#519 - jejacks0n/teaspoon#530 - codevise/pageflow#952

trcarden mentioned this pull request Apr 7, 2019

Version Update #494

Closed

jejacks0n force-pushed the master branch 3 times, most recently from 1d95ed2 to d6352cb Compare April 29, 2019 23:34

mathieujobin closed this Mar 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow client options to be passed into Jasmine #519

Allow client options to be passed into Jasmine #519

jsilvestri commented May 5, 2017 •

edited

jsilvestri commented May 5, 2017

CyborgMaster commented Sep 19, 2017

CyborgMaster commented Sep 19, 2017

tilsammans commented Nov 12, 2017

jejacks0n commented Nov 13, 2017

tilsammans commented Nov 14, 2017

tilsammans commented Dec 3, 2017

zzak commented Dec 3, 2017

zzak commented Dec 25, 2017

jejacks0n commented Dec 27, 2017

tilsammans commented Dec 28, 2017

zzak commented Dec 28, 2017

tilsammans commented Dec 28, 2017

zzak commented Dec 28, 2017

tilsammans commented Dec 28, 2017

mathias commented Dec 17, 2018 •

edited

jejacks0n commented Dec 17, 2018

mathias commented Dec 17, 2018

trcarden commented Apr 7, 2019 •

edited

mathieujobin commented Mar 11, 2021

Allow client options to be passed into Jasmine #519

Allow client options to be passed into Jasmine #519

Conversation

jsilvestri commented May 5, 2017 • edited

jsilvestri commented May 5, 2017

CyborgMaster commented Sep 19, 2017

CyborgMaster commented Sep 19, 2017

tilsammans commented Nov 12, 2017

jejacks0n commented Nov 13, 2017

tilsammans commented Nov 14, 2017

tilsammans commented Dec 3, 2017

zzak commented Dec 3, 2017

zzak commented Dec 25, 2017

jejacks0n commented Dec 27, 2017

tilsammans commented Dec 28, 2017

zzak commented Dec 28, 2017

tilsammans commented Dec 28, 2017

zzak commented Dec 28, 2017

tilsammans commented Dec 28, 2017

mathias commented Dec 17, 2018 • edited

jejacks0n commented Dec 17, 2018

mathias commented Dec 17, 2018

trcarden commented Apr 7, 2019 • edited

mathieujobin commented Mar 11, 2021

jsilvestri commented May 5, 2017 •

edited

mathias commented Dec 17, 2018 •

edited

trcarden commented Apr 7, 2019 •

edited