Skip to content

Commit 8d6ebb6

Browse files
gggeekgggeek
committed
WIP: Try a better fix for issue 57, with functional testing added
Fix: require phpunit when installing via composer in dev mode Allow configuring testsuite via env var (for root url) Refactor error handling for curl requests Clean up leftover code Remove one more leftover var_dump comment Add one more test for Selenium different response types Better detection of non-running selenium (tested on php 5.5.17 windows) Proper fix for last commit (skip tests when selenium down, not otherwise) Merge branch 'master' of github.com:instaclick/php-webdriver into issue_57_bis Conflicts: lib/WebDriver/SauceLabs/SauceRest.php Fix the travis build which runs selenium fix travis/selenium/firefox: 2nd try Remove one more leftover Revert: declare dependency on phpunit Merge branch 'master' of github.com:instaclick/php-webdriver into issue_57_bis Implement changes according to PR review One more PR fix: even more detailed error message for bad responses from Selenium Fix previous commit: typo Implement changes recommended by @robocoder
1 parent bb2bc96 commit 8d6ebb6

File tree

10 files changed

+196
-22
lines changed

10 files changed

+196
-22
lines changed

‎.travis.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@ php:
66
- 5.5
77
- hhvm
88

9+
before_install:
10+
# This update is mandatory or the 'apt-get install' calls following will fail
11+
- sudo apt-get update -qq
12+
- sudo apt-get install -y apache2 libapache2-mod-fastcgi
13+
# start the xvfb display needed for firefox
14+
- export DISPLAY=:99.0
15+
- sh -e /etc/init.d/xvfb start
16+
- sh ./test/CI/Travis/setup_selenium.sh
17+
- sh ./test/CI/Travis/setup_apache.sh
18+
919
before_script:
1020
- composer install --dev --no-interaction
1121

‎lib/WebDriver/AbstractWebDriver.php

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,15 +122,28 @@ protected function curl($requestMethod, $command, $parameters = null, $extraOpti
122122

123123
list($rawResult, $info) = ServiceFactory::getInstance()->getService('service.curl')->execute($requestMethod, $url, $parameters, $extraOptions);
124124

125+
// According to https://w3c.github.io/webdriver/webdriver-spec.html all 4xx responses are to be considered an error and return plaintext,
126+
// while 5xx responses are json encoded
127+
if (substr($info['http_code'], 0, 1) === '4') {
128+
throw WebDriverException::factory(WebDriverException::CURL_EXEC, 'Webdriver http error: ' . $info['http_code'] . ', payload :' . substr($rawResult, 0, 1000));
129+
}
130+
125131
$result = json_decode($rawResult, true);
126-
$value = null;
127132

128-
if (is_array($result) && array_key_exists('value', $result)) {
133+
if ($result === null && json_last_error() != JSON_ERROR_NONE) {
134+
throw WebDriverException::factory(WebDriverException::CURL_EXEC, 'Payload received from webdriver is not valid json: ' . substr($rawResult, 0, 1000));
135+
}
136+
137+
if (!is_array($result) || !array_key_exists('status', $result)) {
138+
throw WebDriverException::factory(WebDriverException::CURL_EXEC, 'Payload received from webdriver is valid but unexpected json: ' . substr($rawResult, 0, 1000));
139+
}
140+
141+
$value = null;
142+
if (array_key_exists('value', $result)) {
129143
$value = $result['value'];
130144
}
131145

132146
$message = null;
133-
134147
if (is_array($value) && array_key_exists('message', $value)) {
135148
$message = $value['message'];
136149
}

‎lib/WebDriver/SauceLabs/SauceRest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ public function __construct($userId, $accessKey)
6262
*
6363
* @return mixed
6464
*
65+
* @throws \WebDriver\Service\CurlServiceException
66+
*
6567
* @see http://saucelabs.com/docs/saucerest
6668
*/
6769
protected function execute($requestMethod, $url, $parameters = null)
@@ -75,6 +77,7 @@ protected function execute($requestMethod, $url, $parameters = null)
7577
CURLOPT_SSL_VERIFYHOST => false,
7678

7779
CURLOPT_HTTPHEADER => array('Expect:'),
80+
CURLOPT_FAILONERROR => true,
7881
);
7982

8083
$url = 'https://saucelabs.com/rest/v1/' . $url;

‎lib/WebDriver/Service/CurlService.php

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323

2424
namespace WebDriver\Service;
2525

26-
use WebDriver\Exception as WebDriverException;
27-
2826
/**
2927
* WebDriver\Service\CurlService class
3028
*
@@ -90,17 +88,23 @@ public function execute($requestMethod, $url, $parameters = null, $extraOptions
9088

9189
$rawResult = trim(curl_exec($curl));
9290
$info = curl_getinfo($curl);
91+
// enrich the info a bit
92+
$info['request_method'] = $requestMethod;
93+
94+
// NB: this only gets triggered when CURLOPT_FAILONERROR has been set in extraOptions.
95+
// In that case, $rawResult is always empty.
96+
if (CURLE_GOT_NOTHING !== ($errno = curl_errno($curl)) && $error = curl_error($curl)) {
97+
98+
curl_close($curl);
9399

94-
if (CURLE_GOT_NOTHING !== curl_errno($curl) && $error = curl_error($curl)) {
95100
$message = sprintf(
96-
'Curl error thrown for http %s to %s%s',
101+
"Curl error thrown for http %s to %s%s\n\n%s",
97102
$requestMethod,
98103
$url,
99-
$parameters && is_array($parameters)
100-
? ' with params: ' . json_encode($parameters) : ''
104+
$parameters && is_array($parameters) ? ' with params: ' . json_encode($parameters) : '',
105+
$error
101106
);
102-
103-
throw WebDriverException::factory(WebDriverException::CURL_EXEC, $message . "\n\n" . $error);
107+
throw new CurlServiceException($message, $errno, null, $info);
104108
}
105109

106110
curl_close($curl);

‎lib/WebDriver/Service/CurlServiceException.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
namespace WebDriver\Service;
4+
5+
6+
final class CurlServiceException extends \Exception {
7+
protected $curlInfo = array();
8+
9+
public function __construct($message = "", $code = 0, \Exception $previous = null, $curlInfo = array()) {
10+
parent::__construct($message, $code, $previous);
11+
$this->curlInfo = $curlInfo;
12+
}
13+
14+
public function getCurlInfo() {
15+
return $this->curlInfo;
16+
}
17+
}

‎lib/WebDriver/Service/CurlServiceInterface.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ interface CurlServiceInterface
3939
*
4040
* @return array
4141
*
42-
* @throws \WebDriver\Exception if error
42+
* @throws \WebDriver\Service\CurlServiceException only if http error and CURLOPT_FAILONERROR has been set in extraOptions
4343
*/
4444
public function execute($requestMethod, $url, $parameters = null, $extraOptions = array());
4545
}

‎test/Assets/index.html

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head lang="en">
4+
<meta charset="UTF-8">
5+
<title></title>
6+
</head>
7+
<body>
8+
9+
</body>
10+
</html>

‎test/CI/Travis/setup_apache.sh

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#!/bin/sh
2+
3+
# set up Apache
4+
# @see https://github.com/travis-ci/travis-ci.github.com/blob/master/docs/user/languages/php.md#apache--php
5+
6+
sudo a2enmod rewrite actions fastcgi alias ssl
7+
8+
# configure apache root dir
9+
sudo sed -i -e "s,/var/www,$(pwd),g" /etc/apache2/sites-available/default
10+
sudo service apache2 restart

‎test/CI/Travis/setup_selenium.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#!/bin/sh
2+
3+
# set up Selenium for functional tests
4+
5+
wget http://goo.gl/cvntq5 -O selenium.jar
6+
7+
java -jar selenium.jar &

‎test/Test/WebDriver/WebDriverTest.php

Lines changed: 110 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,21 @@ class WebDriverTest extends \PHPUnit_Framework_TestCase
3232
{
3333
private $driver;
3434
private $session;
35+
private $testDocumentRootUrl = 'http://localhost';
36+
private $testSeleniumRootUrl = 'http://localhost:4444/wd/hub';
3537

3638
/**
3739
* {@inheritdoc}
3840
*/
3941
protected function setUp()
4042
{
41-
$this->driver = new WebDriver();
43+
if ($url = getenv('ROOT_URL')) {
44+
$this->testDocumentRootUrl = $url;
45+
}
46+
if ($url = getenv('SELENIUM_URL')) {
47+
$this->testSeleniumRootUrl = $url;
48+
}
49+
$this->driver = new WebDriver($this->getTestSeleniumRootUrl());
4250
$this->session = null;
4351
}
4452

@@ -52,27 +60,52 @@ protected function tearDown()
5260
}
5361
}
5462

63+
/**
64+
* Returns the full url to the test site (corresponding to the root dir of the library).
65+
* You can set this via env var ROOT_URL
66+
* @return string
67+
*/
68+
protected function getTestDocumentRootUrl()
69+
{
70+
return $this->testDocumentRootUrl;
71+
}
72+
73+
/**
74+
* Returns the full url to the Selenium server used for functional tests
75+
* @return string
76+
*
77+
* @todo make this configurable via env var
78+
*/
79+
protected function getTestSeleniumRootUrl()
80+
{
81+
return $this->testSeleniumRootUrl;
82+
}
83+
84+
protected function isSeleniumDown($exception)
85+
{
86+
return preg_match('/Failed to connect to .* Connection refused/', $exception->getMessage()) != false
87+
|| strpos($exception->getMessage(), 'couldn\'t connect to host') !== false;
88+
}
89+
5590
/**
5691
* @group Functional
5792
*/
5893
public function testSessions()
5994
{
6095
try {
61-
$this->assertCount(0, $this->driver->sessions());
96+
$this->assertCount(0, $this->driver->sessions());
6297

6398
$this->session = $this->driver->session();
6499
} catch (\Exception $e) {
65-
if (strpos($e->getMessage(),'Failed connect to localhost:4444; Connection refused') !== false
66-
|| strpos($e->getMessage(), 'couldn\'t connect to host') !== false
67-
) {
100+
if ($this->isSeleniumDown($e)) {
68101
$this->markTestSkipped('selenium server not running');
69102
} else {
70103
throw $e;
71104
}
72105
}
73106

74-
$this->assertCount(1, $this->driver->sessions());
75-
$this->assertEquals('http://localhost:4444/wd/hub', $this->driver->getUrl());
107+
$this->assertCount(1, $this->driver->sessions());
108+
$this->assertEquals($this->getTestSeleniumRootUrl(), $this->driver->getUrl());
76109
}
77110

78111
/**
@@ -83,9 +116,7 @@ public function testStatus()
83116
try {
84117
$status = $this->driver->status();
85118
} catch (\Exception $e) {
86-
if (strpos($e->getMessage(),'Failed connect to localhost:4444; Connection refused') !== false
87-
|| strpos($e->getMessage(), 'couldn\'t connect to host') !== false
88-
) {
119+
if ($this->isSeleniumDown($e)) {
89120
$this->markTestSkipped('selenium server not running');
90121
} else {
91122
throw $e;
@@ -97,4 +128,73 @@ public function testStatus()
97128
$this->assertTrue(isset($status['os']));
98129
$this->assertTrue(isset($status['build']));
99130
}
131+
132+
/**
133+
* Checks that an error connecting to Selenium gives back the expected exception
134+
* @group Functional
135+
*/
136+
public function testSeleniumError()
137+
{
138+
try {
139+
$this->driver = new WebDriver($this->getTestSeleniumRootUrl().'/../invalidurl');
140+
$status = $this->driver->status();
141+
142+
$this->fail('Exception not thrown while connecting to invalid Selenium url');
143+
} catch (\Exception $e) {
144+
if ($this->isSeleniumDown($e)) {
145+
$this->markTestSkipped('selenium server not running');
146+
} else {
147+
$this->assertEquals('WebDriver\Exception\CurlExec', get_class($e));
148+
}
149+
}
150+
}
151+
152+
/**
153+
* Checks that a successful command to Selenium which returns an http error response gives back the expected exception
154+
* @group Functional
155+
*/
156+
public function testSeleniumErrorResponse()
157+
{
158+
try {
159+
$status = $this->driver->status();
160+
} catch (\Exception $e) {
161+
if ($this->isSeleniumDown($e)) {
162+
$this->markTestSkipped('selenium server not running');
163+
} else {
164+
throw $e;
165+
}
166+
}
167+
168+
try {
169+
$this->session = $this->driver->session();
170+
$this->session->open($this->getTestDocumentRootUrl().'/test/Assets/index.html');
171+
$element = $this->session->element('id', 'a-quite-unlikely-html-element-id');
172+
173+
$this->fail('Exception not thrown while looking for missing element in page');
174+
} catch (\Exception $e) {
175+
$this->assertEquals('WebDriver\Exception\NoSuchElement', get_class($e));
176+
}
177+
}
178+
179+
/**
180+
* Checks that a successful command to Selenium which returns 'nothing' according to spec does not raise an error
181+
* @group Functional
182+
*/
183+
public function testSeleniumNoResponse()
184+
{
185+
try {
186+
$status = $this->driver->status();
187+
} catch (\Exception $e) {
188+
if ($this->isSeleniumDown($e)) {
189+
$this->markTestSkipped('selenium server not running');
190+
} else {
191+
throw $e;
192+
}
193+
}
194+
195+
$this->session = $this->driver->session();
196+
$timeouts = $this->session->timeouts();
197+
$out = $timeouts->async_script(array('type' => 'implicit', 'ms' => 1000));
198+
$this->assertEquals(null, $out);
199+
}
100200
}

0 commit comments

Comments
 (0)