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
gggeek · Jul 4, 2015 · 8d6ebb6 · 8d6ebb6
1 parent bb2bc96
commit 8d6ebb6
Show file tree

Hide file tree

Showing 10 changed files with 196 additions and 22 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -6,6 +6,16 @@ php:
   - 5.5
   - hhvm
 
+before_install:
+  # This update is mandatory or the 'apt-get install' calls following will fail
+  - sudo apt-get update -qq
+  - sudo apt-get install -y apache2 libapache2-mod-fastcgi
+  # start the xvfb display needed for firefox
+  - export DISPLAY=:99.0
+  - sh -e /etc/init.d/xvfb start
+  - sh ./test/CI/Travis/setup_selenium.sh
+  - sh ./test/CI/Travis/setup_apache.sh
+
 before_script:
   - composer install --dev --no-interaction
 

diff --git a/lib/WebDriver/AbstractWebDriver.php b/lib/WebDriver/AbstractWebDriver.php
@@ -122,15 +122,28 @@ protected function curl($requestMethod, $command, $parameters = null, $extraOpti
 
         list($rawResult, $info) = ServiceFactory::getInstance()->getService('service.curl')->execute($requestMethod, $url, $parameters, $extraOptions);
 
+        // According to https://w3c.github.io/webdriver/webdriver-spec.html all 4xx responses are to be considered an error and return plaintext,
+        // while 5xx responses are json encoded
+        if (substr($info['http_code'], 0, 1) === '4') {
+            throw WebDriverException::factory(WebDriverException::CURL_EXEC, 'Webdriver http error: ' . $info['http_code'] . ', payload :' . substr($rawResult, 0, 1000));
+        }
+
         $result = json_decode($rawResult, true);
-        $value   = null;
 
-        if (is_array($result) && array_key_exists('value', $result)) {
+        if ($result === null && json_last_error() != JSON_ERROR_NONE) {
+            throw WebDriverException::factory(WebDriverException::CURL_EXEC, 'Payload received from webdriver is not valid json: ' . substr($rawResult, 0, 1000));
+        }
+
+        if (!is_array($result) || !array_key_exists('status', $result)) {
+            throw WebDriverException::factory(WebDriverException::CURL_EXEC, 'Payload received from webdriver is valid but unexpected json: ' . substr($rawResult, 0, 1000));
+        }
+
+        $value = null;
+        if (array_key_exists('value', $result)) {
             $value = $result['value'];
         }
 
         $message = null;
-
         if (is_array($value) && array_key_exists('message', $value)) {
             $message = $value['message'];
         }

diff --git a/lib/WebDriver/SauceLabs/SauceRest.php b/lib/WebDriver/SauceLabs/SauceRest.php
@@ -62,6 +62,8 @@ public function __construct($userId, $accessKey)
      *
      * @return mixed
      *
+     * @throws \WebDriver\Service\CurlServiceException
+     *
      * @see http://saucelabs.com/docs/saucerest
      */
     protected function execute($requestMethod, $url, $parameters = null)
@@ -75,6 +77,7 @@ protected function execute($requestMethod, $url, $parameters = null)
             CURLOPT_SSL_VERIFYHOST => false,
 
             CURLOPT_HTTPHEADER => array('Expect:'),
+            CURLOPT_FAILONERROR => true,
         );
 
         $url = 'https://saucelabs.com/rest/v1/' . $url;

diff --git a/lib/WebDriver/Service/CurlService.php b/lib/WebDriver/Service/CurlService.php
@@ -23,8 +23,6 @@
 
 namespace WebDriver\Service;
 
-use WebDriver\Exception as WebDriverException;
-
 /**
  * WebDriver\Service\CurlService class
  *
@@ -90,17 +88,23 @@ public function execute($requestMethod, $url, $parameters = null, $extraOptions
 
         $rawResult = trim(curl_exec($curl));
         $info = curl_getinfo($curl);
+        // enrich the info a bit
+        $info['request_method'] = $requestMethod;
+
+        // NB: this only gets triggered when CURLOPT_FAILONERROR has been set in extraOptions.
+        // In that case, $rawResult is always empty.
+        if (CURLE_GOT_NOTHING !== ($errno = curl_errno($curl)) && $error = curl_error($curl)) {
+
+            curl_close($curl);
 
-        if (CURLE_GOT_NOTHING !== curl_errno($curl) && $error = curl_error($curl)) {
             $message = sprintf(
-                'Curl error thrown for http %s to %s%s',
+                "Curl error thrown for http %s to %s%s\n\n%s",
                 $requestMethod,
                 $url,
-                $parameters && is_array($parameters)
-                ? ' with params: ' . json_encode($parameters) : ''
+                $parameters && is_array($parameters) ? ' with params: ' . json_encode($parameters) : '',
+                $error
             );
-
-            throw WebDriverException::factory(WebDriverException::CURL_EXEC, $message . "\n\n" . $error);
+            throw new CurlServiceException($message, $errno, null, $info);
         }
 
         curl_close($curl);

diff --git a/lib/WebDriver/Service/CurlServiceException.php b/lib/WebDriver/Service/CurlServiceException.php
@@ -0,0 +1,17 @@
+<?php
+
+namespace WebDriver\Service;
+
+
+final class CurlServiceException extends \Exception {
+    protected $curlInfo = array();
+
+    public function __construct($message = "", $code = 0, \Exception $previous = null, $curlInfo = array()) {
+        parent::__construct($message, $code, $previous);
+        $this->curlInfo = $curlInfo;
+    }
+
+    public function getCurlInfo() {
+        return $this->curlInfo;
+    }
+}
diff --git a/lib/WebDriver/Service/CurlServiceInterface.php b/lib/WebDriver/Service/CurlServiceInterface.php
@@ -39,7 +39,7 @@ interface CurlServiceInterface
      *
      * @return array
      *
-     * @throws \WebDriver\Exception if error
+     * @throws \WebDriver\Service\CurlServiceException only if http error and CURLOPT_FAILONERROR has been set in extraOptions
      */
     public function execute($requestMethod, $url, $parameters = null, $extraOptions = array());
 }
diff --git a/test/Assets/index.html b/test/Assets/index.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head lang="en">
+    <meta charset="UTF-8">
+    <title></title>
+</head>
+<body>
+
+</body>
+</html>
diff --git a/test/CI/Travis/setup_apache.sh b/test/CI/Travis/setup_apache.sh
@@ -0,0 +1,10 @@
+#!/bin/sh
+
+# set up Apache
+# @see https://github.com/travis-ci/travis-ci.github.com/blob/master/docs/user/languages/php.md#apache--php
+
+sudo a2enmod rewrite actions fastcgi alias ssl
+
+# configure apache root dir
+sudo sed -i -e "s,/var/www,$(pwd),g" /etc/apache2/sites-available/default
+sudo service apache2 restart
diff --git a/test/CI/Travis/setup_selenium.sh b/test/CI/Travis/setup_selenium.sh
@@ -0,0 +1,7 @@
+#!/bin/sh
+
+# set up Selenium for functional tests
+
+wget http://goo.gl/cvntq5 -O selenium.jar
+
+java -jar selenium.jar &
diff --git a/test/Test/WebDriver/WebDriverTest.php b/test/Test/WebDriver/WebDriverTest.php
@@ -32,13 +32,21 @@ class WebDriverTest extends \PHPUnit_Framework_TestCase
 {
     private $driver;
     private $session;
+    private $testDocumentRootUrl = 'http://localhost';
+    private $testSeleniumRootUrl = 'http://localhost:4444/wd/hub';
 
     /**
      * {@inheritdoc}
      */
     protected function setUp()
     {
-        $this->driver  = new WebDriver();
+        if ($url = getenv('ROOT_URL')) {
+            $this->testDocumentRootUrl = $url;
+        }
+        if ($url = getenv('SELENIUM_URL')) {
+            $this->testSeleniumRootUrl = $url;
+        }
+        $this->driver  = new WebDriver($this->getTestSeleniumRootUrl());
         $this->session = null;
     }
 
@@ -52,27 +60,52 @@ protected function tearDown()
         }
     }
 
+    /**
+     * Returns the full url to the test site (corresponding to the root dir of the library).
+     * You can set this via env var ROOT_URL
+     * @return string
+     */
+    protected function getTestDocumentRootUrl()
+    {
+        return $this->testDocumentRootUrl;
+    }
+
+    /**
+     * Returns the full url to the Selenium server used for functional tests
+     * @return string
+     *
+     * @todo make this configurable via env var
+     */
+    protected function getTestSeleniumRootUrl()
+    {
+        return $this->testSeleniumRootUrl;
+    }
+
+    protected function isSeleniumDown($exception)
+    {
+        return preg_match('/Failed to connect to .* Connection refused/', $exception->getMessage()) != false
+            || strpos($exception->getMessage(), 'couldn\'t connect to host') !== false;
+    }
+
     /**
      * @group Functional
      */
     public function testSessions()
     {
         try {
-	    $this->assertCount(0, $this->driver->sessions());
+        $this->assertCount(0, $this->driver->sessions());
 
             $this->session = $this->driver->session();
         } catch (\Exception $e) {
-            if (strpos($e->getMessage(),'Failed connect to localhost:4444; Connection refused') !== false
-                || strpos($e->getMessage(), 'couldn\'t connect to host') !== false
-            ) {
+            if ($this->isSeleniumDown($e)) {
                 $this->markTestSkipped('selenium server not running');
             } else {
                 throw $e;
             }
         }
 
-	$this->assertCount(1, $this->driver->sessions());
-        $this->assertEquals('http://localhost:4444/wd/hub', $this->driver->getUrl());
+        $this->assertCount(1, $this->driver->sessions());
+        $this->assertEquals($this->getTestSeleniumRootUrl(), $this->driver->getUrl());
     }
 
     /**
@@ -83,9 +116,7 @@ public function testStatus()
         try {
             $status = $this->driver->status();
         } catch (\Exception $e) {
-            if (strpos($e->getMessage(),'Failed connect to localhost:4444; Connection refused') !== false
-                || strpos($e->getMessage(), 'couldn\'t connect to host') !== false
-            ) {
+            if ($this->isSeleniumDown($e)) {
                 $this->markTestSkipped('selenium server not running');
             } else {
                 throw $e;
@@ -97,4 +128,73 @@ public function testStatus()
         $this->assertTrue(isset($status['os']));
         $this->assertTrue(isset($status['build']));
     }
+
+    /**
+     * Checks that an error connecting to Selenium gives back the expected exception
+     * @group Functional
+     */
+    public function testSeleniumError()
+    {
+        try {
+            $this->driver = new WebDriver($this->getTestSeleniumRootUrl().'/../invalidurl');
+            $status = $this->driver->status();
+
+            $this->fail('Exception not thrown while connecting to invalid Selenium url');
+        } catch (\Exception $e) {
+            if ($this->isSeleniumDown($e)) {
+                $this->markTestSkipped('selenium server not running');
+            } else {
+                $this->assertEquals('WebDriver\Exception\CurlExec', get_class($e));
+            }
+        }
+    }
+
+    /**
+     * Checks that a successful command to Selenium which returns an http error response gives back the expected exception
+     * @group Functional
+     */
+    public function testSeleniumErrorResponse()
+    {
+        try {
+            $status = $this->driver->status();
+        } catch (\Exception $e) {
+            if ($this->isSeleniumDown($e)) {
+                $this->markTestSkipped('selenium server not running');
+            } else {
+                throw $e;
+            }
+        }
+
+        try {
+            $this->session = $this->driver->session();
+            $this->session->open($this->getTestDocumentRootUrl().'/test/Assets/index.html');
+            $element = $this->session->element('id', 'a-quite-unlikely-html-element-id');
+
+            $this->fail('Exception not thrown while looking for missing element in page');
+        } catch (\Exception $e) {
+            $this->assertEquals('WebDriver\Exception\NoSuchElement', get_class($e));
+        }
+    }
+
+    /**
+     * Checks that a successful command to Selenium which returns 'nothing' according to spec does not raise an error
+     * @group Functional
+     */
+    public function testSeleniumNoResponse()
+    {
+        try {
+            $status = $this->driver->status();
+        } catch (\Exception $e) {
+            if ($this->isSeleniumDown($e)) {
+                $this->markTestSkipped('selenium server not running');
+            } else {
+                throw $e;
+            }
+        }
+
+        $this->session = $this->driver->session();
+        $timeouts = $this->session->timeouts();
+        $out = $timeouts->async_script(array('type' => 'implicit', 'ms' => 1000));
+        $this->assertEquals(null, $out);
+    }
 }