From 739881a3f4021269ba60e0817d7ee2fd31fcdbd1 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 2 Jan 2014 12:02:22 -0800 Subject: [PATCH] Print a more useful error message if any PHPUnit tests crash Summary: Before, if PHPUnit crashed (syntax error, undefined constant, etc), you would get a relatively unhelpful error message: firehed@Eric-Sterns-Mac-Pro ~/dev/php-lcd: arc unit Exception Clover coverage XML report file is empty, it probably means that phpunit failed to run tests. Try running arc unit with --trace option and then run generated phpunit command yourself, you might get the answer. (Run with --trace for a full exception trace.) This now checks the json and code coverage reports and tries to pull a useful error message: firehed@Eric-Sterns-Mac-Pro ~/dev/php-lcd: arc unit Exception The test '/Users/firehed/dev/php-lcd/tests/PlateButtonsTest.php' crashed with the following output: Fatal error: Undefined class constant 'EFT' in /Users/firehed/dev/php-lcd/tests/PlateButtonsTest.php on line 25 Call Stack: 0.0002 233104 1. {main}() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/composer/bin/phpunit:0 0.0039 564872 2. PHPUnit_TextUI_Command::main() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/composer/bin/phpunit:63 0.0039 565496 3. PHPUnit_TextUI_Command->run() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/TextUI/Command.php:129 0.0247 2280168 4. PHPUnit_TextUI_TestRunner->doRun() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/TextUI/Command.php:176 0.0293 2730760 5. PHPUnit_Framework_TestSuite->run() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/TextUI/TestRunner.php:349 0.1211 3996832 6. PHPUnit_Framework_TestSuite->runTest() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestSuite.php:745 0.1211 3996832 7. PHPUnit_Framework_TestCase->run() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestSuite.php:775 0.1211 3996832 8. PHPUnit_Framework_TestResult->run() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestCase.php:783 0.1233 3999752 9. PHPUnit_Framework_TestCase->runBare() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestResult.php:648 0.1236 4016432 10. PHPUnit_Framework_TestCase->runTest() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestCase.php:838 0.1237 4017240 11. ReflectionMethod->invokeArgs() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestCase.php:983 0.1237 4017520 12. Firehed\PlateButtonsTest->testSingleButtonOnButtonDown() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestCase.php:983 (Run with --trace for a full exception trace.) Or if nothing was written to `stderr`, you may get something like this: firehed@Eric-Sterns-Mac-Pro ~/dev/php-lcd: arc unit --no-coverage Exception Test Firehed\PlateButtonsTest::testSingleButtonOnButtonDown did not finish (Run with --trace for a full exception trace.) Test Plan: `arc unit` for arcanist itself `arc unit` before and after the change on a project where: * all tests pass * some tests are failing * a test causes a fatal error (undefined constant, bad method call, etc) * a test file is invalid (syntax error) No regressions that I could find, and all crashes now display a more useful error. Reviewers: #blessed_reviewers, epriestley Reviewed By: epriestley CC: aurelijus, epriestley, aran Differential Revision: https://secure.phabricator.com/D7848 --- src/unit/ArcanistUnitTestResult.php | 1 - .../engine/ArcanistBaseTestResultParser.php | 6 ++++ src/unit/engine/PhpunitResultParser.php | 32 ++++++++++++++----- src/unit/engine/PhpunitTestEngine.php | 13 +++++--- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/unit/ArcanistUnitTestResult.php b/src/unit/ArcanistUnitTestResult.php index 6372efcc..a004ee68 100644 --- a/src/unit/ArcanistUnitTestResult.php +++ b/src/unit/ArcanistUnitTestResult.php @@ -14,7 +14,6 @@ final class ArcanistUnitTestResult { const RESULT_UNSOUND = 'unsound'; const RESULT_POSTPONED = 'postponed'; - private $namespace; private $name; private $link; private $result; diff --git a/src/unit/engine/ArcanistBaseTestResultParser.php b/src/unit/engine/ArcanistBaseTestResultParser.php index 97eea371..d10fe123 100644 --- a/src/unit/engine/ArcanistBaseTestResultParser.php +++ b/src/unit/engine/ArcanistBaseTestResultParser.php @@ -8,6 +8,7 @@ abstract class ArcanistBaseTestResultParser { protected $enableCoverage; protected $projectRoot; protected $coverageFile; + protected $stderr; public function setEnableCoverage($enable_coverage) { $this->enableCoverage = $enable_coverage; @@ -33,6 +34,11 @@ abstract class ArcanistBaseTestResultParser { return $this; } + public function setStderr($stderr) { + $this->stderr = $stderr; + return $this; + } + /** * Parse test results from provided input and return an array * of ArcanistUnitTestResult diff --git a/src/unit/engine/PhpunitResultParser.php b/src/unit/engine/PhpunitResultParser.php index 146ba6d2..152d337d 100644 --- a/src/unit/engine/PhpunitResultParser.php +++ b/src/unit/engine/PhpunitResultParser.php @@ -23,6 +23,14 @@ final class PhpunitResultParser extends ArcanistBaseTestResultParser { */ public function parseTestResults($path, $test_results) { + if (!$test_results) { + $result = id(new ArcanistUnitTestResult()) + ->setName($path) + ->setUserData($this->stderr) + ->setResult(ArcanistUnitTestResult::RESULT_BROKEN); + return array($result); + } + $report = $this->getJsonReport($test_results); // coverage is for all testcases in the executed $path @@ -33,8 +41,14 @@ final class PhpunitResultParser extends ArcanistBaseTestResultParser { $results = array(); foreach ($report as $event) { - if ('test' != $event->event) { - continue; + switch ($event->event) { + case 'test': + break; + case 'testStart': + $lastTestFinished = false; + // fall through + default: + continue 2; // switch + loop } $status = ArcanistUnitTestResult::RESULT_PASS; @@ -72,8 +86,15 @@ final class PhpunitResultParser extends ArcanistBaseTestResultParser { $result->setUserData($user_data); $results[] = $result; + $lastTestFinished = true; } + if (!$lastTestFinished) { + $results[] = id(new ArcanistUnitTestResult()) + ->setName($event->test) // use last event + ->setUserData($this->stderr) + ->setResult(ArcanistUnitTestResult::RESULT_BROKEN); + } return $results; } @@ -85,12 +106,7 @@ final class PhpunitResultParser extends ArcanistBaseTestResultParser { private function readCoverage() { $test_results = Filesystem::readFile($this->coverageFile); if (empty($test_results)) { - throw new Exception('Clover coverage XML report file is empty, ' - . 'it probably means that phpunit failed to run tests. ' - . 'Try running arc unit with --trace option and then run ' - . 'generated phpunit command yourself, you might get the ' - . 'answer.' - ); + return array(); } $coverage_dom = new DOMDocument(); diff --git a/src/unit/engine/PhpunitTestEngine.php b/src/unit/engine/PhpunitTestEngine.php index 4a29af66..95ea8adc 100644 --- a/src/unit/engine/PhpunitTestEngine.php +++ b/src/unit/engine/PhpunitTestEngine.php @@ -71,8 +71,10 @@ final class PhpunitTestEngine extends ArcanistBaseUnitTestEngine { $config = $this->configFile ? csprintf('-c %s', $this->configFile) : null; - $futures[$test_path] = new ExecFuture('%C %C --log-json %s %C %s', - $this->phpunitBinary, $config, $json_tmp, $clover, $test_path); + $stderr = "-d display_errors=stderr"; + + $futures[$test_path] = new ExecFuture('%C %C %C --log-json %s %C %s', + $this->phpunitBinary, $config, $stderr, $json_tmp, $clover, $test_path); $tmpfiles[$test_path] = array( 'json' => $json_tmp, 'clover' => $clover_tmp, @@ -89,7 +91,8 @@ final class PhpunitTestEngine extends ArcanistBaseUnitTestEngine { $results[] = $this->parseTestResults( $test, $tmpfiles[$test]['json'], - $tmpfiles[$test]['clover']); + $tmpfiles[$test]['clover'], + $stderr); } return array_mergev($results); @@ -101,16 +104,18 @@ final class PhpunitTestEngine extends ArcanistBaseUnitTestEngine { * @param string $path Path to test * @param string $json_tmp Path to phpunit json report * @param string $clover_tmp Path to phpunit clover report + * @param string $stderr Data written to stderr * * @return array */ - private function parseTestResults($path, $json_tmp, $clover_tmp) { + private function parseTestResults($path, $json_tmp, $clover_tmp, $stderr) { $test_results = Filesystem::readFile($json_tmp); return id(new PhpunitResultParser()) ->setEnableCoverage($this->getEnableCoverage()) ->setProjectRoot($this->projectRoot) ->setCoverageFile($clover_tmp) ->setAffectedTests($this->affectedTests) + ->setStderr($stderr) ->parseTestResults($path, $test_results); }