mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-25 16:22:42 +01:00
Have Arcanist record how much of my life was spent running each unit test
Summary: Keeping unit tests speedy keeps them useful since people actually won't mind running them. This diff records the time taken by each test and displays it nice and colorized. Really, I just want to discourage non-unit tests from making their way into ##__tests__##. Some thoughts: - The "acceptableness" times are subjective but if dependencies are properly mocked the times seem to be ok. Integration tests that make network requests to third-party endpoints and pull in megabytes of data will not survive. This is a good thing. - Fast tests get a gold star, encouraging small tests. I am sorry that the star does not sparkle. - There is no way for a programmer to admit that their test is going to be slow in some cases. They will be shamed with red text for the life of their test. - It might be confusing that fast but failing tests get green text and maybe a gold star. Test Plan: Ran some of the unit tests within Arcanist and libphutil. See https://secure.phabricator.com/file/view/PHID-FILE-cdd3c94c219e0fd7470b/ for sample output. Reviewed By: epriestley Reviewers: epriestley CC: aran, epriestley Differential Revision: 588
This commit is contained in:
parent
304a2c73aa
commit
2892c6ef1a
3 changed files with 56 additions and 1 deletions
|
@ -28,6 +28,7 @@
|
||||||
abstract class ArcanistPhutilTestCase {
|
abstract class ArcanistPhutilTestCase {
|
||||||
|
|
||||||
private $runningTest;
|
private $runningTest;
|
||||||
|
private $testStartTime;
|
||||||
private $results = array();
|
private $results = array();
|
||||||
|
|
||||||
|
|
||||||
|
@ -162,6 +163,7 @@ abstract class ArcanistPhutilTestCase {
|
||||||
$result = new ArcanistUnitTestResult();
|
$result = new ArcanistUnitTestResult();
|
||||||
$result->setName($this->runningTest);
|
$result->setName($this->runningTest);
|
||||||
$result->setResult(ArcanistUnitTestResult::RESULT_FAIL);
|
$result->setResult(ArcanistUnitTestResult::RESULT_FAIL);
|
||||||
|
$result->setDuration(microtime(true) - $this->testStartTime);
|
||||||
$result->setUserData($reason);
|
$result->setUserData($reason);
|
||||||
$this->results[] = $result;
|
$this->results[] = $result;
|
||||||
}
|
}
|
||||||
|
@ -179,6 +181,7 @@ abstract class ArcanistPhutilTestCase {
|
||||||
$result = new ArcanistUnitTestResult();
|
$result = new ArcanistUnitTestResult();
|
||||||
$result->setName($this->runningTest);
|
$result->setName($this->runningTest);
|
||||||
$result->setResult(ArcanistUnitTestResult::RESULT_PASS);
|
$result->setResult(ArcanistUnitTestResult::RESULT_PASS);
|
||||||
|
$result->setDuration(microtime(true) - $this->testStartTime);
|
||||||
$result->setUserData($reason);
|
$result->setUserData($reason);
|
||||||
$this->results[] = $result;
|
$this->results[] = $result;
|
||||||
}
|
}
|
||||||
|
@ -206,6 +209,7 @@ abstract class ArcanistPhutilTestCase {
|
||||||
$name = $method->getName();
|
$name = $method->getName();
|
||||||
if (preg_match('/^test/', $name)) {
|
if (preg_match('/^test/', $name)) {
|
||||||
$this->runningTest = $name;
|
$this->runningTest = $name;
|
||||||
|
$this->testStartTime = microtime(true);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
$this->willRunOneTest($name);
|
$this->willRunOneTest($name);
|
||||||
|
|
|
@ -33,6 +33,7 @@ class ArcanistUnitTestResult {
|
||||||
private $namespace;
|
private $namespace;
|
||||||
private $name;
|
private $name;
|
||||||
private $result;
|
private $result;
|
||||||
|
private $duration;
|
||||||
private $userData;
|
private $userData;
|
||||||
|
|
||||||
public function setName($name) {
|
public function setName($name) {
|
||||||
|
@ -53,6 +54,15 @@ class ArcanistUnitTestResult {
|
||||||
return $this->result;
|
return $this->result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function setDuration($duration) {
|
||||||
|
$this->duration = $duration;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getDuration() {
|
||||||
|
return $this->duration;
|
||||||
|
}
|
||||||
|
|
||||||
public function setUserData($user_data) {
|
public function setUserData($user_data) {
|
||||||
$this->userData = $user_data;
|
$this->userData = $user_data;
|
||||||
return $this;
|
return $this;
|
||||||
|
|
|
@ -132,7 +132,11 @@ EOTEXT
|
||||||
$unresolved[] = $result;
|
$unresolved[] = $result;
|
||||||
} else {
|
} else {
|
||||||
if ($this->engine->shouldEchoTestResults()) {
|
if ($this->engine->shouldEchoTestResults()) {
|
||||||
echo ' '.$status_codes[$result_code].' '.$result->getName()."\n";
|
echo ' '.$status_codes[$result_code];
|
||||||
|
if ($result_code == ArcanistUnitTestResult::RESULT_PASS) {
|
||||||
|
echo ' '.self::formatTestDuration($result->getDuration());
|
||||||
|
}
|
||||||
|
echo ' '.$result->getName()."\n";
|
||||||
}
|
}
|
||||||
if ($result_code != ArcanistUnitTestResult::RESULT_PASS) {
|
if ($result_code != ArcanistUnitTestResult::RESULT_PASS) {
|
||||||
if ($this->engine->shouldEchoTestResults()) {
|
if ($this->engine->shouldEchoTestResults()) {
|
||||||
|
@ -179,4 +183,41 @@ EOTEXT
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static function formatTestDuration($seconds) {
|
||||||
|
// Very carefully define inclusive upper bounds on acceptable unit test
|
||||||
|
// durations. Times are in milliseconds and are in increasing order.
|
||||||
|
$acceptableness = array(
|
||||||
|
50 => "<fg:green>%s</fg><fg:yellow>\xE2\x98\x85</fg> ",
|
||||||
|
200 => '<fg:green>%s</fg> ',
|
||||||
|
500 => '<fg:yellow>%s</fg> ',
|
||||||
|
INF => '<fg:red>%s</fg> ',
|
||||||
|
);
|
||||||
|
|
||||||
|
$milliseconds = $seconds * 1000;
|
||||||
|
$duration = self::formatTime($seconds);
|
||||||
|
foreach ($acceptableness as $upper_bound => $formatting) {
|
||||||
|
if ($milliseconds <= $upper_bound) {
|
||||||
|
return phutil_console_format($formatting, $duration);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return phutil_console_format(end($acceptableness), $duration);
|
||||||
|
}
|
||||||
|
|
||||||
|
private static function formatTime($seconds) {
|
||||||
|
if ($seconds >= 60) {
|
||||||
|
$minutes = floor($seconds / 60);
|
||||||
|
return sprintf('%dm%02ds', $minutes, round($seconds % 60));
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($seconds >= 1) {
|
||||||
|
return sprintf('%4.1fs', $seconds);
|
||||||
|
}
|
||||||
|
|
||||||
|
$milliseconds = $seconds * 1000;
|
||||||
|
if ($milliseconds >= 1) {
|
||||||
|
return sprintf('%3dms', round($milliseconds));
|
||||||
|
}
|
||||||
|
|
||||||
|
return ' <1ms';
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue