Merge pull request sebastianbergmann#1352 from sun/result-isolation

Remove instantiated (child process) objects from TestFailure

src/Framework/Error.php

@@ -54,7 +54,7 @@
  * @link       http://www.phpunit.de/
  * @since      Class available since Release 2.2.0
  */
-class PHPUnit_Framework_Error extends Exception
+class PHPUnit_Framework_Error extends PHPUnit_Framework_Exception
 {
     /**
      * Constructor.

src/Framework/Exception.php

@@ -44,6 +44,25 @@
  */
 
 /**
+ * Base class for all PHPUnit Framework exceptions.
+ *
+ * Ensures that exceptions thrown during a test run do not leave stray
+ * references behind.
+ *
+ * Every Exception contains a stack trace. Each stack frame contains the 'args'
+ * of the called function. The function arguments can contain references to
+ * instantiated objects. The references prevent the objects from being
+ * destructed (until test results are eventually printed), so memory cannot be
+ * freed up.
+ *
+ * With enabled process isolation, test results are serialized in the child
+ * process and unserialized in the parent process. The stack trace of Exceptions
+ * may contain objects that cannot be serialized or unserialized (e.g., PDO
+ * connections). Unserializing user-space objects from the child process into
+ * the parent would break the intended encapsulation of process isolation.
+ *
+ * @see http://fabien.potencier.org/article/9/php-serialization-stack-traces-and-exceptions
+ *
  * @package    PHPUnit
  * @subpackage Framework
  * @author     Sebastian Bergmann <[email protected]>
@@ -54,4 +73,47 @@
  */
 class PHPUnit_Framework_Exception extends RuntimeException implements PHPUnit_Exception
 {
+    /**
+     * @var array
+     */
+    protected $serializableTrace;
+
+    public function __construct($message = '', $code = 0, Exception $previous = null)
+    {
+        parent::__construct($message, $code, $previous);
+
+        $this->serializableTrace = $this->getTrace();
+        foreach ($this->serializableTrace as $i => $call) {
+            unset($this->serializableTrace[$i]['args']);
+        }
+    }
+
+    /**
+     * Returns the serializable trace (without 'args').
+     *
+     * @return array
+     */
+    public function getSerializableTrace()
+    {
+        return $this->serializableTrace;
+    }
+
+    /**
+     * @return string
+     */
+    public function __toString()
+    {
+        $string = PHPUnit_Framework_TestFailure::exceptionToString($this);
+
+        if ($trace = PHPUnit_Util_Filter::getFilteredStacktrace($this)) {
+            $string .= "\n" . $trace;
+        }
+
+        return $string;
+    }
+
+    public function __sleep()
+    {
+        return array_keys(get_object_vars($this));
+    }
 }

src/Framework/ExceptionWrapper.php

@@ -0,0 +1,128 @@
+<?php
+/**
+ * PHPUnit
+ *
+ * Copyright (c) 2001-2014, Sebastian Bergmann <[email protected]>.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *   * Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *
+ *   * Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in
+ *     the documentation and/or other materials provided with the
+ *     distribution.
+ *
+ *   * Neither the name of Sebastian Bergmann nor the names of his
+ *     contributors may be used to endorse or promote products derived
+ *     from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * @package    PHPUnit
+ * @subpackage Framework
+ * @author     Sebastian Bergmann <[email protected]>
+ * @copyright  2001-2014 Sebastian Bergmann <[email protected]>
+ * @license    http://www.opensource.org/licenses/BSD-3-Clause  The BSD 3-Clause License
+ * @link       http://www.phpunit.de/
+ * @since      File available since Release 4.1.5
+ */
+
+/**
+ * Wraps Exceptions thrown by code under test.
+ *
+ * Re-instantiates Exceptions thrown by user-space code to retain their original
+ * class names, properties, and stack traces (but without arguments).
+ *
+ * Unlike PHPUnit_Framework_Exception, the complete stack of previous Exceptions
+ * is processed.
+ *
+ * @package    PHPUnit
+ * @subpackage Framework
+ * @author     Daniel F. Kudwien <[email protected]>
+ * @copyright  2001-2014 Sebastian Bergmann <[email protected]>
+ * @license    http://www.opensource.org/licenses/BSD-3-Clause  The BSD 3-Clause License
+ * @link       http://www.phpunit.de/
+ * @since      Class available since Release 4.1.5
+ */
+class PHPUnit_Framework_ExceptionWrapper extends PHPUnit_Framework_Exception
+{
+    /**
+     * @var string
+     */
+    protected $classname;
+
+    /**
+     * @var PHPUnit_Framework_ExceptionWrapper|null
+     */
+    protected $previous;
+
+    public function __construct(Exception $e)
+    {
+        // PDOException::getCode() is a string.
+        // @see http://php.net/manual/en/class.pdoexception.php#95812
+        parent::__construct($e->getMessage(), (int) $e->getCode());
+
+        $this->classname = get_class($e);
+        $this->file = $e->getFile();
+        $this->line = $e->getLine();
+
+        $this->serializableTrace = $e->getTrace();
+        foreach ($this->serializableTrace as $i => $call) {
+            unset($this->serializableTrace[$i]['args']);
+        }
+
+        if ($e->getPrevious()) {
+            $this->previous = new self($e->getPrevious());
+        }
+    }
+
+    /**
+     * @return string
+     */
+    public function getClassname()
+    {
+        return $this->classname;
+    }
+
+    /**
+     * @return PHPUnit_Framework_ExceptionWrapper
+     */
+    public function getPreviousWrapped()
+    {
+        return $this->previous;
+    }
+
+    /**
+     * @return string
+     */
+    public function __toString()
+    {
+        $string = PHPUnit_Framework_TestFailure::exceptionToString($this);
+
+        if ($trace = PHPUnit_Util_Filter::getFilteredStacktrace($this)) {
+            $string .= "\n" . $trace;
+        }
+
+        if ($this->previous) {
+            $string .= "\nCaused by\n" . $this->previous;
+        }
+
+        return $string;
+    }
+}

src/Framework/TestFailure.php

@@ -57,9 +57,9 @@
 class PHPUnit_Framework_TestFailure
 {
     /**
-     * @var    PHPUnit_Framework_Test
+     * @var string
      */
-    protected $failedTest;
+    private $testName;
 
     /**
      * @var    Exception
@@ -74,7 +74,11 @@ class PHPUnit_Framework_TestFailure
      */
     public function __construct(PHPUnit_Framework_Test $failedTest, Exception $thrownException)
     {
-        $this->failedTest      = $failedTest;
+        if ($failedTest instanceof PHPUnit_Framework_SelfDescribing) {
+            $this->testName = $failedTest->toString();
+        } else {
+            $this->testName = get_class($failedTest);
+        }
         $this->thrownException = $thrownException;
     }
 
@@ -87,8 +91,7 @@ public function toString()
     {
         return sprintf(
           '%s: %s',
-
-          $this->failedTest->toString(),
+          $this->testName,
           $this->thrownException->getMessage()
         );
     }
@@ -125,6 +128,8 @@ public static function exceptionToString(Exception $e)
             }
         } elseif ($e instanceof PHPUnit_Framework_Error) {
             $buffer = $e->getMessage() . "\n";
+        } elseif ($e instanceof PHPUnit_Framework_ExceptionWrapper) {
+            $buffer = $e->getClassname() . ': ' . $e->getMessage() . "\n";
         } else {
             $buffer = get_class($e) . ': ' . $e->getMessage() . "\n";
         }
@@ -133,13 +138,13 @@ public static function exceptionToString(Exception $e)
     }
 
     /**
-     * Gets the failed test.
+     * Returns the name of the failing test (including data set, if any).
      *
-     * @return PHPUnit_Framework_Test
+     * @return string
      */
-    public function failedTest()
+    public function getTestName()
     {
-        return $this->failedTest;
+        return $this->testName;
     }
 
     /**

src/Framework/TestResult.php

@@ -663,7 +663,10 @@ public function run(PHPUnit_Framework_Test $test)
             } elseif ($e instanceof PHPUnit_Framework_SkippedTestError) {
                 $skipped = true;
             }
+        } catch (PHPUnit_Framework_Exception $e) {
+            $error = true;
         } catch (Exception $e) {
+            $e = new PHPUnit_Framework_ExceptionWrapper($e);
             $error = true;
         }
 

src/TextUI/ResultPrinter.php

@@ -251,20 +251,12 @@ protected function printDefect(PHPUnit_Framework_TestFailure $defect, $count)
      */
     protected function printDefectHeader(PHPUnit_Framework_TestFailure $defect, $count)
     {
-        $failedTest = $defect->failedTest();
-
-        if ($failedTest instanceof PHPUnit_Framework_SelfDescribing) {
-            $testName = $failedTest->toString();
-        } else {
-            $testName = get_class($failedTest);
-        }
-
         $this->write(
           sprintf(
             "\n%d) %s\n",
 
             $count,
-            $testName
+            $defect->getTestName()
           )
         );
     }
@@ -274,26 +266,11 @@ protected function printDefectHeader(PHPUnit_Framework_TestFailure $defect, $cou
      */
     protected function printDefectTrace(PHPUnit_Framework_TestFailure $defect)
     {
-        $this->write($defect->getExceptionAsString());
-
-        $trace = PHPUnit_Util_Filter::getFilteredStacktrace(
-          $defect->thrownException()
-        );
-
-        if (!empty($trace)) {
-            $this->write("\n" . $trace);
-        }
-
-        $e = $defect->thrownException()->getPrevious();
-
-        while ($e) {
-          $this->write(
-            "\nCaused by\n" .
-            PHPUnit_Framework_TestFailure::exceptionToString($e). "\n" .
-            PHPUnit_Util_Filter::getFilteredStacktrace($e)
-          );
+        $e = $defect->thrownException();
+        $this->write((string) $e);
 
-          $e = $e->getPrevious();
+        while ($e = $e->getPrevious()) {
+          $this->write("\nCaused by\n" . $e);
         }
     }
 

src/Util/Filter.php

@@ -82,12 +82,15 @@ public static function getFilteredStacktrace(Exception $e, $asString = true)
             $eTrace = $e->getSyntheticTrace();
             $eFile  = $e->getSyntheticFile();
             $eLine  = $e->getSyntheticLine();
+        } elseif ($e instanceof PHPUnit_Framework_Exception) {
+            $eTrace = $e->getSerializableTrace();
+            $eFile  = $e->getFile();
+            $eLine  = $e->getLine();
         } else {
             if ($e->getPrevious()) {
-                $eTrace = $e->getPrevious()->getTrace();
-            } else {
-                $eTrace = $e->getTrace();
+                $e = $e->getPrevious();
             }
+            $eTrace = $e->getTrace();
             $eFile  = $e->getFile();
             $eLine  = $e->getLine();
         }