instaclick / php-webdriver

W3C and Selenium 2 webdriver "thin client" for php 5.3+ and namespaces.
Other
435 stars 62 forks source link

Support for injecting CURL options into WebDriver #51

Closed aik099 closed 10 years ago

aik099 commented 10 years ago

Right now WebDriver is using Curl to send commands to Selenium server. This works just fine, when we're able to make a connection.

However when server isn't responding for 1 minute then connection just drops (default socket connection timeout in PHP). E.g. https://github.com/instaclick/php-webdriver/blob/master/lib/WebDriver/WebDriver.php#L72-L77

I propose that we allow user setting extra curl options (will go directly to service.curl used and to curl_setopt calls).

Related to Behat/MinkSelenium2Driver#48

robocoder commented 10 years ago

This can be accomplished by setter injection thru the ServiceFactory. Just inject your own curl service (or subclass the existing one).

aik099 commented 10 years ago

Sounds complicated for a simple like socket connection timeout setting.

Maybe WebDriver::setConnectionTimeout or WebDriver::setSocketTimeout would be more elegant solution.

robocoder commented 10 years ago

The original ticket referred to curl options in general. I believe the setter injection approach is sufficient for the edge use cases.

robocoder commented 10 years ago

In the specific case of timeouts, I think consumers of the library have to consciously make that implementation choice themselves to avoid UI interaction problems when there's a mismatch with $session->timeouts().

gggeek commented 9 years ago

@robocoder could you post a gist to show how it is possible to inject anything?

I might be tired, but I have reversed the whole call chain backwards (using behat+mink), and I failed to find at any point where a service or class could be swapped out for another:

mink creates its sessions using a list of drivers which is statically defined mink::getSession()->getDriver() Selenium2Driver : calls new WebDriver() calls Servicefactory

aik099 commented 9 years ago

That's the problem. There is no such point currently. The service container is creating curl class internally without way to inject anything: https://github.com/instaclick/php-webdriver/blob/master/lib/WebDriver/AbstractWebDriver.php#L123

The factory is obtained via static method call.

markpol commented 8 years ago

Customizing the curl options for the webdriver service class is actually pretty straight forward. Here is an example:

https://gist.github.com/markpol/d92918d21612f56a6548

aik099 commented 8 years ago

It's possible yes, but surely isn't straightforward. To change 1 curl option you need to:

  1. create sub-class of used curl service
  2. hardcode new option in that sub-class constructor
  3. replace the class in used service locator

If driver would be using dependency injection, then supplying CurlService right in it's constructor would be much easier.

aik099 commented 8 years ago

I've created #73 with proposition to use dependency injection instead of service factory.