Improve tests on SocketHandler

src/Monolog/Handler/SocketHandler.php

@@ -29,6 +29,8 @@ class SocketHandler extends AbstractProcessingHandler
     private $resource;
     private $timeout = 0;
     private $persistent = false;
+    private $errno;
+    private $errstr;
 
     /**
      * @param string $connectionString
@@ -133,21 +135,6 @@ class SocketHandler extends AbstractProcessingHandler
         return $this->timeout;
     }
 
-    /**
-     * Allow injecting a resource opened somewhere else. Used in tests.
-     *
-     * @throws \InvalidArgumentException
-     * @param resource $resource 
-     */
-    public function setResource($resource)
-    {
-        if (is_resource($resource)) {
-            $this->resource = $resource;
-        } else {
-            throw new \InvalidArgumentException("Expected a resource");
-        }
-    }
-
     private function connectIfNotConnected()
     {
         if ($this->isConnected()) {
@@ -175,27 +162,27 @@ class SocketHandler extends AbstractProcessingHandler
         $this->setSocketTimeout();
     }
 
-    protected function createSocketResource()
+    private function createSocketResource()
     {
-        if ($this->persistent) {
-            @$resource = pfsockopen($this->connectionString, -1, $errno, $errstr, $this->connectionTimeout);
+        if ($this->isPersistent()) {
+            $resource = $this->pfsockopen();
         } else {
-            @$resource = fsockopen($this->connectionString, -1, $errno, $errstr, $this->connectionTimeout);
+            $resource = $this->fsockopen();
         }
         if (!$resource) {
-            throw new ConnectionException("Failed connecting to $this->connectionString ($errno: $errstr)");
+            throw new ConnectionException("Failed connecting to $this->connectionString ($this->errno: $this->errstr)");
         }
-        $this->resource = $resource;
+        return $this->resource = $resource;
     }
 
     private function setSocketTimeout()
     {
-        if (!stream_set_timeout($this->resource, $this->timeout)) {
+        if (!$this->stream_set_timeout()) {
             throw new ConnectionException("Failed setting timeout with stream_set_timeout()");
         }
     }
 
-    protected function writeToSocket($data)
+    private function writeToSocket($data)
     {
         $length = strlen($data);
         $sent = 0;
@@ -215,6 +202,30 @@ class SocketHandler extends AbstractProcessingHandler
         }
     }
 
+    /**
+     * Allow mock
+     */
+    protected function pfsockopen()
+    {
+        return @pfsockopen($this->connectionString, -1, $this->errno, $this->errstr, $this->connectionTimeout);
+    }
+
+    /**
+     * Allow mock
+     */
+    protected function fsockopen()
+    {
+        return @fsockopen($this->connectionString, -1, $this->errno, $this->errstr, $this->connectionTimeout);
+    }
+
+    /**
+     * Allow mock
+     */
+    protected function stream_set_timeout()
+    {
+        return stream_set_timeout($this->resource, $this->timeout);
+    }
+
     /**
      * Allow mock
      */

tests/Monolog/Handler/SocketHandlerTest.php

@@ -94,12 +94,37 @@ class SocketHandlerTest extends TestCase
     /**
      * @expectedException Monolog\Handler\SocketHandler\Exception\ConnectionException
      */
-    public function testConnectionTimeoutWithMock()
+    public function testExceptionIsThrownOnFsockopenError()
     {
-        $this->setMockHandler(array('createSocketResource'));
+        $this->setMockHandler(array('fsockopen'));
         $this->handler->expects($this->once())
-                ->method('createSocketResource')
-                ->will($this->throwException(new ConnectionException()));
+                ->method('fsockopen')
+                ->will($this->returnValue(false));
+        $this->writeRecord('Hello world');
+    }
+
+    /**
+     * @expectedException Monolog\Handler\SocketHandler\Exception\ConnectionException
+     */
+    public function testExceptionIsThrownOnPfsockopenError()
+    {
+        $this->setMockHandler(array('pfsockopen'));
+        $this->handler->expects($this->once())
+                ->method('pfsockopen')
+                ->will($this->returnValue(false));
+        $this->handler->setPersistent(true);
+        $this->writeRecord('Hello world');
+    }
+
+    /**
+     * @expectedException Monolog\Handler\SocketHandler\Exception\ConnectionException
+     */
+    public function testExceptionIsThrownIfCannotSetTimeout()
+    {
+        $this->setMockHandler(array('stream_set_timeout'));
+        $this->handler->expects($this->once())
+                ->method('stream_set_timeout')
+                ->will($this->returnValue(false));
         $this->writeRecord('Hello world');
     }
 
@@ -119,7 +144,6 @@ class SocketHandlerTest extends TestCase
                 ->method('fwrite')
                 ->will($this->returnValueMap($map));
 
-        $this->injectMemoryResource();
         $this->writeRecord('Hello world');
     }
 
@@ -143,7 +167,6 @@ class SocketHandlerTest extends TestCase
                 ->will($this->returnValue(array('timed_out' => true)));
 
 
-        $this->injectMemoryResource();
         $this->writeRecord('Hello world');
     }
 
@@ -152,34 +175,27 @@ class SocketHandlerTest extends TestCase
      */
     public function testWriteFailsOnIncompleteWrite()
     {
-        $this->setMockHandler(array('fwrite', 'isConnected'));
+        $this->setMockHandler(array('fwrite', 'stream_get_meta_data'));
 
-        $map = array(
-            array('Hello world', 6),
-            array('world', 5),
-        );
+        $res = $this->res;
+        $callback = function($string) use ($res) {
+                    fclose($res);
+                    return strlen('Hello');
+                };
 
         $this->handler->expects($this->exactly(1))
                 ->method('fwrite')
-                ->will($this->returnValueMap($map));
-        $this->handler->expects($this->at(0))
-                ->method('isConnected')
-                ->will($this->returnValue(true));
-        $this->handler->expects($this->at(1))
-                ->method('isConnected')
-                ->will($this->returnValue(true));
-        $this->handler->expects($this->at(2))
-                ->method('isConnected')
-                ->will($this->returnValue(false));
+                ->will($this->returnCallback($callback));
+        $this->handler->expects($this->exactly(1))
+                ->method('stream_get_meta_data')
+                ->will($this->returnValue(array('timed_out' => false)));
 
-        $this->injectMemoryResource();
         $this->writeRecord('Hello world');
     }
 
     public function testWriteWithMemoryFile()
     {
-        $this->createHandler('localhost:54321');
-        $this->injectMemoryResource();
+        $this->setMockHandler();
         $this->writeRecord('test1');
         $this->writeRecord('test2');
         $this->writeRecord('test3');
@@ -200,14 +216,12 @@ class SocketHandlerTest extends TestCase
                 ->method('fwrite')
                 ->will($this->returnValueMap($map));
 
-        $this->injectMemoryResource();
         $this->writeRecord('Hello world');
     }
 
     public function testClose()
     {
-        $this->createHandler('localhost:54321');
-        $this->injectMemoryResource();
+        $this->setMockHandler();
         $this->writeRecord('Hello world');
         $this->assertTrue(is_resource($this->res));
         $this->handler->close();
@@ -216,24 +230,14 @@ class SocketHandlerTest extends TestCase
 
     public function testCloseDoesNotClosePersistentSocket()
     {
-        $this->createHandler('localhost:54321');
+        $this->setMockHandler();
         $this->handler->setPersistent(true);
-        $this->injectMemoryResource();
         $this->writeRecord('Hello world');
         $this->assertTrue(is_resource($this->res));
         $this->handler->close();
         $this->assertTrue(is_resource($this->res));
     }
 
-    /**
-     * @expectedException \InvalidArgumentException
-     */
-    public function testInjectBadResourceThrowsException()
-    {
-        $this->createHandler('');
-        $this->handler->setResource('');
-    }
-
     private function createHandler($connectionString)
     {
         $this->handler = new SocketHandler($connectionString);
@@ -245,18 +249,38 @@ class SocketHandlerTest extends TestCase
         $this->handler->handle($this->getRecord(Logger::WARNING, $string));
     }
 
-    private function injectMemoryResource()
+    private function setMockHandler(array $methods = array())
     {
         $this->res = fopen('php://memory', 'a');
-        $this->handler->setResource($this->res);
-    }
 
-    private function setMockHandler(array $methods)
-    {
+        $defaultMethods = array('fsockopen', 'pfsockopen', 'stream_set_timeout');
+        $newMethods = array_diff($methods, $defaultMethods);
+
+        $finalMethods = array_merge($defaultMethods, $newMethods);
+
         $this->handler = $this->getMock(
-                '\Monolog\Handler\SocketHandler', $methods, array('localhost:1234')
+                '\Monolog\Handler\SocketHandler', $finalMethods, array('localhost:1234')
         );
+
+        if (!in_array('fsockopen', $methods)) {
+            $this->handler->expects($this->any())
+                    ->method('fsockopen')
+                    ->will($this->returnValue($this->res));
+        }
+
+        if (!in_array('pfsockopen', $methods)) {
+            $this->handler->expects($this->any())
+                    ->method('pfsockopen')
+                    ->will($this->returnValue($this->res));
+        }
+
+        if (!in_array('stream_set_timeout', $methods)) {
+            $this->handler->expects($this->any())
+                    ->method('stream_set_timeout')
+                    ->will($this->returnValue(true));
+        }
+
         $this->handler->setFormatter($this->getIdentityFormatter());
     }
-
+    
 }