diff --git a/src/lint/linter/xhpast/ArcanistXHPASTLinter.php b/src/lint/linter/xhpast/ArcanistXHPASTLinter.php index f78cd3ab..9e129541 100644 --- a/src/lint/linter/xhpast/ArcanistXHPASTLinter.php +++ b/src/lint/linter/xhpast/ArcanistXHPASTLinter.php @@ -17,7 +17,7 @@ */ /** - * Uses XHPAST to apply lint rules to PHP or PHP+XHP. + * Uses XHPAST to apply lint rules to PHP. * * @group linter */ diff --git a/src/unit/engine/base/ArcanistBaseUnitTestEngine.php b/src/unit/engine/base/ArcanistBaseUnitTestEngine.php index 0668f61d..dc7f492b 100644 --- a/src/unit/engine/base/ArcanistBaseUnitTestEngine.php +++ b/src/unit/engine/base/ArcanistBaseUnitTestEngine.php @@ -1,7 +1,7 @@ enableAsyncTests; } + final public function setEnableCoverage($enable_coverage) { + $this->enableCoverage = $enable_coverage; + return $this; + } + + final public function getEnableCoverage() { + return $this->enableCoverage; + } + abstract public function run(); /** diff --git a/src/unit/engine/phutil/PhutilUnitTestEngine.php b/src/unit/engine/phutil/PhutilUnitTestEngine.php index 7f8f5199..7cef6fae 100644 --- a/src/unit/engine/phutil/PhutilUnitTestEngine.php +++ b/src/unit/engine/phutil/PhutilUnitTestEngine.php @@ -118,17 +118,34 @@ final class PhutilUnitTestEngine extends ArcanistBaseUnitTestEngine { "No tests to run. You may need to rebuild the phutil library map."); } + $enable_coverage = $this->getEnableCoverage(); + if ($enable_coverage !== false) { + if (!function_exists('xdebug_start_code_coverage')) { + if ($enable_coverage === true) { + throw new ArcanistUsageException( + "You specified --coverage but xdebug is not available, so ". + "coverage can not be enabled for PhutilUnitTestEngine."); + } + } else { + $enable_coverage = true; + } + } + $results = array(); foreach ($run_tests as $test_class) { PhutilSymbolLoader::loadClass($test_class); $test_case = newv($test_class, array()); + $test_case->setEnableCoverage($enable_coverage); + $test_case->setProjectRoot($this->getWorkingCopy()->getProjectRoot()); + $test_case->setPaths($this->getPaths()); $results[] = $test_case->run(); } + + if ($results) { $results = call_user_func_array('array_merge', $results); } - return $results; } diff --git a/src/unit/engine/phutil/__init__.php b/src/unit/engine/phutil/__init__.php index c7db7cc4..58bd4ab3 100644 --- a/src/unit/engine/phutil/__init__.php +++ b/src/unit/engine/phutil/__init__.php @@ -6,6 +6,7 @@ +phutil_require_module('arcanist', 'exception/usage'); phutil_require_module('arcanist', 'exception/usage/noeffect'); phutil_require_module('arcanist', 'unit/engine/base'); diff --git a/src/unit/engine/phutil/testcase/ArcanistPhutilTestCase.php b/src/unit/engine/phutil/testcase/ArcanistPhutilTestCase.php index a7e6e1f7..e306c03f 100644 --- a/src/unit/engine/phutil/testcase/ArcanistPhutilTestCase.php +++ b/src/unit/engine/phutil/testcase/ArcanistPhutilTestCase.php @@ -30,6 +30,10 @@ abstract class ArcanistPhutilTestCase { private $runningTest; private $testStartTime; private $results = array(); + private $enableCoverage; + private $coverage = array(); + private $projectRoot; + private $paths; /* -( Making Test Assertions )--------------------------------------------- */ @@ -179,7 +183,10 @@ abstract class ArcanistPhutilTestCase { * @task internal */ final private function failTest($reason) { + $coverage = $this->endCoverage(); + $result = new ArcanistUnitTestResult(); + $result->setCoverage($coverage); $result->setName($this->runningTest); $result->setResult(ArcanistUnitTestResult::RESULT_FAIL); $result->setDuration(microtime(true) - $this->testStartTime); @@ -197,7 +204,10 @@ abstract class ArcanistPhutilTestCase { * @task internal */ final private function passTest($reason) { + $coverage = $this->endCoverage(); + $result = new ArcanistUnitTestResult(); + $result->setCoverage($coverage); $result->setName($this->runningTest); $result->setResult(ArcanistUnitTestResult::RESULT_PASS); $result->setDuration(microtime(true) - $this->testStartTime); @@ -233,6 +243,7 @@ abstract class ArcanistPhutilTestCase { try { $this->willRunOneTest($name); + $this->beginCoverage(); $test_exception = null; try { call_user_func_array( @@ -263,4 +274,76 @@ abstract class ArcanistPhutilTestCase { return $this->results; } + final public function setEnableCoverage($enable_coverage) { + $this->enableCoverage = $enable_coverage; + return $this; + } + + final private function beginCoverage() { + if (!$this->enableCoverage) { + return; + } + + $this->assertCoverageAvailable(); + xdebug_start_code_coverage(XDEBUG_CC_UNUSED | XDEBUG_CC_DEAD_CODE); + } + + final private function endCoverage() { + if (!$this->enableCoverage) { + return; + } + + $result = xdebug_get_code_coverage(); + xdebug_stop_code_coverage($cleanup = false); + + $coverage = array(); + + foreach ($result as $file => $report) { + if (strncmp($file, $this->projectRoot, strlen($this->projectRoot))) { + continue; + } + + $max = max(array_keys($report)); + $str = ''; + for ($ii = 1; $ii <= $max; $ii++) { + $c = idx($report, $ii); + if ($c === -1) { + $str .= 'U'; // Un-covered. + } else if ($c === -2) { + // TODO: This indicates "unreachable", but it flags the closing braces + // of functions which end in "return", which is super ridiculous. Just + // ignore it for now. + $str .= 'N'; // Not executable. + } else if ($c === 1) { + $str .= 'C'; // Covered. + } else { + $str .= 'N'; // Not executable. + } + } + $coverage[substr($file, strlen($this->projectRoot) + 1)] = $str; + } + + // Only keep coverage information for files modified by the change. + $coverage = array_select_keys($coverage, $this->paths); + + return $coverage; + } + + final private function assertCoverageAvailable() { + if (!function_exists('xdebug_start_code_coverage')) { + throw new Exception( + "You've enabled code coverage but XDebug is not installed."); + } + } + + final public function setProjectRoot($project_root) { + $this->projectRoot = $project_root; + return $this; + } + + final public function setPaths(array $paths) { + $this->paths = $paths; + return $this; + } + } diff --git a/src/unit/result/ArcanistUnitTestResult.php b/src/unit/result/ArcanistUnitTestResult.php index 688dc894..2ba78665 100644 --- a/src/unit/result/ArcanistUnitTestResult.php +++ b/src/unit/result/ArcanistUnitTestResult.php @@ -35,6 +35,7 @@ final class ArcanistUnitTestResult { private $result; private $duration; private $userData; + private $coverage; public function setName($name) { $this->name = $name; @@ -72,4 +73,32 @@ final class ArcanistUnitTestResult { return $this->userData; } + public function setCoverage($coverage) { + $this->coverage = $coverage; + return $this; + } + + public function getCoverage() { + return $this->coverage; + } + + /** + * Merge several coverage reports into a comprehensive coverage report. + * + * @param list List of coverage report strings. + * @return string Cumulative coverage report. + */ + public static function mergeCoverage(array $coverage) { + $base = reset($coverage); + foreach ($coverage as $more_coverage) { + $len = min(strlen($base), strlen($more_coverage)); + for ($ii = 0; $ii < $len; $ii++) { + if ($more_coverage[$ii] == 'C') { + $base[$ii] = 'C'; + } + } + } + return $base; + } + } diff --git a/src/workflow/diff/ArcanistDiffWorkflow.php b/src/workflow/diff/ArcanistDiffWorkflow.php index 49d28bcf..74943705 100644 --- a/src/workflow/diff/ArcanistDiffWorkflow.php +++ b/src/workflow/diff/ArcanistDiffWorkflow.php @@ -30,7 +30,7 @@ final class ArcanistDiffWorkflow extends ArcanistBaseWorkflow { private $hasWarnedExternals = false; private $unresolvedLint; - private $unresolvedTests; + private $testResults; private $diffID; private $unitWorkflow; @@ -1067,7 +1067,7 @@ EOTEXT break; } - $this->unresolvedTests = $this->unitWorkflow->getUnresolvedTests(); + $this->testResults = $this->unitWorkflow->getTestResults(); return $unit_result; } catch (ArcanistNoEngineException $ex) { @@ -1571,16 +1571,17 @@ EOTEXT * @task diffprop */ private function updateUnitDiffProperty() { - if (!$this->unresolvedTests) { + if (!$this->testResults) { return; } $data = array(); - foreach ($this->unresolvedTests as $test) { + foreach ($this->testResults as $test) { $data[] = array( 'name' => $test->getName(), 'result' => $test->getResult(), 'userdata' => $test->getUserData(), + 'coverage' => $test->getCoverage(), ); } diff --git a/src/workflow/unit/ArcanistUnitWorkflow.php b/src/workflow/unit/ArcanistUnitWorkflow.php index 4c4c1259..3a2cda6a 100644 --- a/src/workflow/unit/ArcanistUnitWorkflow.php +++ b/src/workflow/unit/ArcanistUnitWorkflow.php @@ -30,6 +30,7 @@ final class ArcanistUnitWorkflow extends ArcanistBaseWorkflow { const RESULT_POSTPONED = 4; private $unresolvedTests; + private $testResults; private $engine; public function getCommandHelp() { @@ -61,6 +62,19 @@ EOTEXT 'help' => "Override configured unit engine for this project." ), + 'coverage' => array( + 'help' => 'Always enable coverage information.', + 'conflicts' => array( + 'no-coverage' => null, + ), + ), + 'no-coverage' => array( + 'help' => 'Always disable coverage information.', + ), + 'detailed-coverage' => array( + 'help' => "Show a detailed coverage report on the CLI. Implies ". + "--coverage.", + ), '*' => 'paths', ); } @@ -108,6 +122,15 @@ EOTEXT $this->engine->setPaths($paths); $this->engine->setArguments($this->getPassthruArgumentsAsMap('unit')); + $enable_coverage = null; // Means "default". + if ($this->getArgument('coverage') || + $this->getArgument('detailed-coverage')) { + $enable_coverage = true; + } else if ($this->getArgument('no-coverage')) { + $enable_coverage = false; + } + $this->engine->setEnableCoverage($enable_coverage); + // Enable possible async tests only for 'arc diff' not 'arc unit' if ($this->getParentWorkflow()) { $this->engine->setEnableAsyncTests(true); @@ -116,6 +139,7 @@ EOTEXT } $results = $this->engine->run(); + $this->testResults = $results; $status_codes = array( ArcanistUnitTestResult::RESULT_PASS => phutil_console_format( @@ -133,6 +157,7 @@ EOTEXT ); $unresolved = array(); + $coverage = array(); $postponed_count = 0; foreach ($results as $result) { $result_code = $result->getResult(); @@ -154,6 +179,11 @@ EOTEXT $unresolved[] = $result; } } + if ($result->getCoverage()) { + foreach ($result->getCoverage() as $file => $report) { + $coverage[$file][] = $report; + } + } } if ($postponed_count) { echo sprintf("%s %d %s\n", @@ -162,6 +192,40 @@ EOTEXT ($postponed_count > 1)?'tests':'test'); } + if ($coverage) { + $file_coverage = array(); + $file_reports = array(); + foreach ($coverage as $file => $reports) { + $report = ArcanistUnitTestResult::mergeCoverage($reports); + $cov = substr_count($report, 'C'); + $uncov = substr_count($report, 'U'); + if ($cov + $uncov) { + $coverage = $cov / ($cov + $uncov); + } else { + $coverage = 0; + } + $file_coverage[$file] = $coverage; + $file_reports[$file] = $report; + } + echo "\n"; + echo phutil_console_format('__COVERAGE REPORT__'); + echo "\n"; + + asort($file_coverage); + foreach ($file_coverage as $file => $coverage) { + echo phutil_console_format( + " **%s%%** %s\n", + sprintf('% 3d', (int)(100 * $coverage)), + $file); + + if ($this->getArgument('detailed-coverage')) { + echo $this->renderDetailedCoverageReport( + $file, + $file_reports[$file]); + } + } + } + $this->unresolvedTests = $unresolved; $overall_result = self::RESULT_OKAY; @@ -186,6 +250,10 @@ EOTEXT return $this->unresolvedTests; } + public function getTestResults() { + return $this->testResults; + } + public function setDifferentialDiffID($id) { if ($this->engine) { $this->engine->setDifferentialDiffID($id); @@ -229,4 +297,46 @@ EOTEXT return ' <1ms'; } + + private function renderDetailedCoverageReport($file, $report) { + $data = $this->getRepositoryAPI()->getCurrentFileData($file); + $data = explode("\n", $data); + + $out = ''; + + $n = 0; + foreach ($data as $line) { + $out .= sprintf('% 5d ', $n + 1); + $line = str_pad($line, 80, ' '); + if (empty($report[$n])) { + $c = 'N'; + } else { + $c = $report[$n]; + } + switch ($c) { + case 'C': + $out .= phutil_console_format( + ' %s ', + $line); + break; + case 'U': + $out .= phutil_console_format( + ' %s ', + $line); + break; + case 'X': + $out .= phutil_console_format( + ' %s ', + $line); + break; + default: + $out .= ' '.$line.' '; + break; + } + $out .= "\n"; + $n++; + } + + return $out; + } }