From 69d888b5d12f9cb2b4ff4b4398bb3c5632a25480 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 19 Sep 2012 12:25:58 +1200 Subject: [PATCH] FIXED: Issue with test reporting not correctly presenting errors that prevent test execution. In the case of errors arising during setUp or setUpOnce a unit test will fail to run any individual tests. However, this situation was incorrectly being reported as a test pass (as no tests were run, thus no tests had errors). E.g. the output of a test run that raised an error during setUp would be "0 tests run: 0 passes, 0 failures, and 0 incomplete" with a green background. To rectify this the following fixes were made: - Non-cleanly ended tests and test suites are now automatically ended at the end of the test run, as well as at the beginning of subsequent test/suites. This should make catching of errors a lot more robust. - Errors raised during setup are now no longer lost to the mist of time. The test suite itself will record any error status which was generated outside the scope of any individual tests. - An additional "errors" count is added to the output at the end of test running. For example, in the case where setup failed and no tests could be run the error would be written to the browser (along with stacktrace) with a message similar to "0 tests run: 0 passes, 0 failures, and 0 incomplete with 1 errors". The intent of this is to separate the concepts of failed/succeeded/incomplete tests from any errors which may have arisen. I.e. no tests "failed" due to the error, but the test run itself is highlighted as an error (red background on the output). This problem has been a severe cause of issue when testing code that interacts with the database, as any database error during setup would refuse to be shown. --- dev/SapphireTestReporter.php | 158 ++++++++++++++++++++++++++--------- 1 file changed, 118 insertions(+), 40 deletions(-) diff --git a/dev/SapphireTestReporter.php b/dev/SapphireTestReporter.php index d5c8e766e..e57e4c8a0 100644 --- a/dev/SapphireTestReporter.php +++ b/dev/SapphireTestReporter.php @@ -81,7 +81,7 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { * @return void */ public function __construct() { - @include_once 'Benchmark/Timer.php'; + @include_once 'Benchmark/Timer.php'; if(class_exists('Benchmark_Timer')) { $this->timer = new Benchmark_Timer(); $this->hasTimer = true; @@ -116,12 +116,14 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { */ public function startTestSuite( PHPUnit_Framework_TestSuite $suite) { if(strlen($suite->getName())) { + $this->endCurrentTestSuite(); $this->currentSuite = array( 'suite' => $suite, // the test suite 'tests' => array(), // the tests in the suite - 'errors' => 0, // number of tests with errors + 'errors' => 0, // number of tests with errors (including setup errors) 'failures' => 0, // number of tests which failed - 'incomplete' => 0); // number of tests that were not completed correctly + 'incomplete' => 0, // number of tests that were not completed correctly + 'error' => null); // Any error encountered during setup of the test suite } } @@ -137,18 +139,45 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { $this->startTestTime = microtime(true); if($test instanceof PHPUnit_Framework_TestCase) { + $this->endCurrentTest(); $this->currentTest = array( 'name' => preg_replace('(\(.*\))', '', $test->toString()), // the name of the test (without the suite name) 'timeElapsed' => 0, // execution time of the test 'status' => TEST_SUCCESS, // status of the test execution 'message' => '', // user message of test result 'exception' => NULL, // original caught exception thrown by test upon failure/error + 'trace' => NULL, // Stacktrace used for exception handling 'uid' => md5(microtime()) // a unique ID for this test (used for identification purposes in results) ); if($this->hasTimer) $this->timer->start(); } } + /** + * Logs the specified status to the current test, or if no test is currently + * run, to the test suite. + * @param integer $status Status code + * @param string $message Message to log + * @param string $exception Exception body related to this message + * @param array $trace Stacktrace + */ + protected function addStatus($status, $message, $exception, $trace) { + // Build status body to be saved + $status = array( + 'status' => $status, + 'message' => $message, + 'exception' => $exception, + 'trace' => $trace + ); + + // Log either to current test or suite record + if($this->currentTest) { + $this->currentTest = array_merge($this->currentTest, $status); + } else { + $this->currentSuite['error'] = $status; + } + } + /** * Adds the failure detail to the current test and increases the failure * count for the current suite @@ -160,10 +189,7 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { */ public function addFailure(PHPUnit_Framework_Test $test, PHPUnit_Framework_AssertionFailedError $e, $time) { $this->currentSuite['failures']++; - $this->currentTest['status'] = TEST_FAILURE; - $this->currentTest['message'] = $e->toString(); - $this->currentTest['exception'] = $this->getTestException($test, $e); - $this->currentTest['trace'] = $e->getTrace(); + $this->addStatus(TEST_FAILURE, $e->toString(), $this->getTestException($test, $e), $e->getTrace()); } /** @@ -177,10 +203,7 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { */ public function addError(PHPUnit_Framework_Test $test, Exception $e, $time) { $this->currentSuite['errors']++; - $this->currentTest['status'] = TEST_ERROR; - $this->currentTest['message'] = $e->getMessage(); - $this->currentTest['exception'] = $this->getTestException($test, $e); - $this->currentTest['trace'] = $e->getTrace(); + $this->addStatus(TEST_ERROR, $e->getMessage(), $this->getTestException($test, $e), $e->getTrace()); } /** @@ -194,10 +217,7 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { */ public function addIncompleteTest(PHPUnit_Framework_Test $test, Exception $e, $time) { $this->currentSuite['incomplete']++; - $this->currentTest['status'] = TEST_INCOMPLETE; - $this->currentTest['message'] = $e->toString(); - $this->currentTest['exception'] = $this->getTestException($test, $e); - $this->currentTest['trace'] = $e->getTrace(); + $this->addStatus(TEST_INCOMPLETE, $e->toString(), $this->getTestException($test, $e), $e->getTrace()); } /** @@ -210,6 +230,26 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { // not implemented } + + /** + * Cleanly end the current test + */ + protected function endCurrentTest() { + if(!$this->currentTest) return; + + // Time the current test + $testDuration = microtime(true) - $this->startTestTime; + $this->testSpeeds[$this->currentSuite['suite']->getName() . '.' . $this->currentTest['name']] = $testDuration; + if($this->hasTimer) { + $this->timer->stop(); + $this->currentTest['timeElapsed'] = $this->timer->timeElapsed(); + } + + // Save and reset current state + array_push($this->currentSuite['tests'], $this->currentTest); + $this->currentTest = null; + } + /** * Upon completion of a test, records the execution time (if available) and adds the test to * the tests performed in the current suite. @@ -219,20 +259,27 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { * @return void */ public function endTest( PHPUnit_Framework_Test $test, $time) { - $testDuration = microtime(true) - $this->startTestTime; - $this->testSpeeds[$this->currentSuite['suite']->getName() . '.' . $this->currentTest['name']] = $testDuration; - - if($this->hasTimer) { - $this->timer->stop(); - $this->currentTest['timeElapsed'] = $this->timer->timeElapsed(); - } - array_push($this->currentSuite['tests'], $this->currentTest); + $this->endCurrentTest(); if(method_exists($test, 'getActualOutput')) { $output = $test->getActualOutput(); if($output) echo "\nOutput:\n$output"; } } + /** + * Cleanly end the current test suite + */ + protected function endCurrentTestSuite() { + if(!$this->currentSuite) return; + + // Ensure any current test is ended along with the current suite + $this->endCurrentTest(); + + // Save and reset current state + array_push($this->suiteResults['suites'], $this->currentSuite); + $this->currentSuite = null; + } + /** * Upon completion of a test suite adds the suite to the suties performed * @@ -242,7 +289,7 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { */ public function endTestSuite( PHPUnit_Framework_TestSuite $suite) { if(strlen($suite->getName())) { - array_push($this->suiteResults['suites'], $this->currentSuite); + $this->endCurrentTestSuite(); } } @@ -271,6 +318,19 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { } } + /** + * Writes a status message to the output stream in a user readable HTML format + * @param string $name Name of the object that generated the error + * @param string $message Message of the error + * @param array $trace Stacktrace + */ + protected function writeResultError($name, $message, $trace) { + echo "

⊗ ". $this->testNameToPhrase($name) ."

"; + echo "
".htmlentities($message, ENT_COMPAT, 'UTF-8')."
"; + echo SS_Backtrace::get_rendered_backtrace($trace); + echo "
"; + } + /** * Display error bar if it exists */ @@ -279,30 +339,49 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { $failCount = 0; $testCount = 0; $incompleteCount = 0; - $errorCount = 0; + $errorCount = 0; // Includes both suite and test level errors + + // Ensure that the current suite is cleanly ended. + // A suite may not end correctly if there was an error during setUp + $this->endCurrentTestSuite(); foreach($this->suiteResults['suites'] as $suite) { + + // Report suite error. In the case of fatal non-success messages + // These should be reported as errors. Failure/Success relate only + // to individual tests directly + if($suite['error']) { + $errorCount++; + $this->writeResultError( + $suite['suite']->getName(), + $suite['error']['message'], + $suite['error']['trace'] + ); + } + + // Run through all tests in this suite foreach($suite['tests'] as $test) { $testCount++; - if($test['status'] == 2) { - $incompleteCount++; - } elseif($test['status'] == 1) { - $passCount++; - } else { - $failCount++; + switch($test['status']) { + case TEST_ERROR: $errorCount++; break; + case TEST_INCOMPLETE: $incompleteCount++; break; + case TEST_SUCCESS: $passCount++; break; + case TEST_FAILURE: $failCount++; break; } - - if ($test['status'] != 1) { - echo "

⊗ ". $this->testNameToPhrase($test['name']) ."

"; - echo "
".htmlentities($test['message'], ENT_COMPAT, 'UTF-8')."
"; - echo SS_Backtrace::get_rendered_backtrace($test['trace']); - echo "
"; + + // Report test error + if ($test['status'] != TEST_SUCCESS) { + $this->writeResultError( + $test['name'], + $test['message'], + $test['trace'] + ); } } } - $result = ($failCount > 0) ? 'fail' : 'pass'; + $result = ($failCount || $errorCount) ? 'fail' : 'pass'; echo "
"; - echo "

$testCount tests run: $passCount passes, $failCount failures, and $incompleteCount incomplete

"; + echo "

$testCount tests run: $passCount passes, $failCount failures, and $incompleteCount incomplete with $errorCount errors

"; echo "
"; } @@ -310,6 +389,5 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { protected function testNameToPhrase($name) { return ucfirst(preg_replace("/([a-z])([A-Z])/", "$1 $2", $name)); } - }