Differential D19161 Diff 45919 src/unit/parser/ArcanistGoTestResultParser.php

Changeset View

Standalone View

src/unit/parser/ArcanistGoTestResultParser.php

Show First 20 Lines • Show All 62 Lines • ▼ Show 20 Lines	foreach ($test_results as $i => $line) {
$test_case_results[] = $result;		$test_case_results[] = $result;

continue;		continue;
}		}

if (strncmp($line, 'ok', 2) === 0) {		if (strncmp($line, 'ok', 2) === 0) {
$meta = array();		$meta = array();
preg_match(		preg_match(
'/^ok[\s\t]+(?P<test_name>\w.)[\s\t]+(?P<time>.)s.*/',		'/^ok[\s]+(?P<test_name>\w.)[\s]+(?:(?P<time>.)s\|$cached$)/',
		nathanleclaireAuthorUnsubmitted Done Inline Actions So close... Could make it its own variable if desired. nathanleclaire: So close... Could make it its own variable if desired.
		epriestleyUnsubmitted Done Inline Actions You should adhere to the 80 column rule. In this case, the easiest approach might be to remove the `.` at the end, which is unnecessary: the pattern does only needs to match somewhere, it does not need to match the entire input. You could also replace the two instances of `[\s\t]` with `\s`, since `\t` is in the `\s` ("whitespace") class. I think the `\w.` part of the pattern is also a little sketchy and will cause us to match the entire string first (inside the regex engine), then repeatedly backtrack to try to match the second delimiter. If possible (that is, if unit test names may never contain whitespace characters), it would be better to match `\S+` ("not whitespace") instead of `\w.` ('one word character followed by any number of any other character"), which saves one more character. The major goal being to simplify and improve the regexp, of course; the effect on the line width is just a fortunate side effect. (Is it possible for the 'name' to contain "(cached)" or values in the format "0.123s"?) In the more general case where you can't just shrink the regexp, breaking it up across multiple lines is usually the cleanest approach and can improve readability. epriestley:* You should adhere to the 80 column rule. In this case, the easiest approach might be to remove…
$line,		$line,
$meta);		$meta);

$test_case_name = str_replace('/', '::', $meta['test_name']);		$test_case_name = str_replace('/', '::', $meta['test_name']);

// Our test case passed		// Our test case passed
// check to make sure we were in verbose (-v) mode		// check to make sure we were in verbose (-v) mode
if (empty($test_case_results)) {		if (empty($test_case_results)) {
// We weren't in verbose mode		// We weren't in verbose mode
// create one successful result for the whole test case		// create one successful result for the whole test case
$test_name = 'Go::TestCase::'.$test_case_name;		$test_name = 'Go::TestCase::'.$test_case_name;

$result = new ArcanistUnitTestResult();		$result = new ArcanistUnitTestResult();
$result->setName($test_name);		$result->setName($test_name);
$result->setResult(ArcanistUnitTestResult::RESULT_PASS);		$result->setResult(ArcanistUnitTestResult::RESULT_PASS);

		if (array_key_exists('time', $meta)) {
$result->setDuration((float)$meta['time']);		$result->setDuration((float)$meta['time']);
		}

$results[] = $result;		$results[] = $result;
} else {		} else {
$test_case_results = $this->fixNames(		$test_case_results = $this->fixNames(
$test_case_results,		$test_case_results,
$test_case_name);		$test_case_name);
$results = array_merge($results, $test_case_results);		$results = array_merge($results, $test_case_results);
$test_case_results = array();		$test_case_results = array();
Show All 37 Lines