Changeset View
Standalone View
src/unit/engine/MochaTestEngine.php
- This file was added.
<?php | |||||
joshuaspence: There should be a blank line between `<?php` and any code, see D13534. | |||||
/** | |||||
* Uses [[https://mochajs.org/ | Mocha ]] to test Javascript code. | |||||
Done Inline ActionsPrefer to write this as [[https://mochajs.org/ | Mocha]]. joshuaspence: Prefer to write this as `[[https://mochajs.org/ | Mocha]]`. | |||||
* Uses [[ https://gotwarlost.github.io/istanbul | Istambul ]] | |||||
Done Inline ActionsPrefer to write this as [[https://gotwarlost.github.io/istanbul/ | Istanbul]] joshuaspence: Prefer to write this as `[[https://gotwarlost.github.io/istanbul/ | Istanbul]]` | |||||
* to cover Javascript code. | |||||
* | |||||
* Assumes that when modifying a file with a path like `SomeDir/SomeFile.js`, | |||||
* that the unit test that verifies the functionality of `SomeFile` is | |||||
* located by default at `SomeDir/__tests__/SomeFile-test.js`. | |||||
*/ | |||||
final class MochaTestEngine extends ArcanistUnitTestEngine { | |||||
Done Inline ActionsThis isn't quite correct. @concrete-extensible only makes sense for non-final, non-abstract classes. joshuaspence: This isn't quite correct. `@concrete-extensible` only makes sense for non-`final`, non… | |||||
public function getEngineConfigurationName() { | |||||
return 'mocha'; | |||||
} | |||||
protected function supportsRunAllTests() { | |||||
return true; | |||||
Done Inline ActionsSee my next comment. joshuaspence: See my next comment. | |||||
} | |||||
public function run() { | |||||
$pattern_tests = $this->getAllTestsPattern(); | |||||
if (!$pattern_tests) { | |||||
throw new ArcanistNoEffectException(pht('No tests to run.')); | |||||
Done Inline ActionsPersonally, I find this comment to be fairly useless and somewhat distracting. In this particular case, I don't think the comment provides any explanation that the code does not. joshuaspence: Personally, I find this comment to be fairly useless and somewhat distracting. In this… | |||||
} | |||||
$working_copy = $this->getWorkingCopy(); | |||||
$project_root = $working_copy->getProjectRoot(); | |||||
$junit_tmp = new TempFile(); | |||||
$cover_tmp = null; | |||||
if ($this->getEnableCoverage()) { | |||||
$cover_path = './coverage/cobertura-coverage.xml'; | |||||
$cover_tmp = Filesystem::resolvePath($cover_path, $project_root); | |||||
} | |||||
$future = $this->buildTestFuture($pattern_tests, $junit_tmp); | |||||
list($err, $stdout, $stderr) = $future->resolve(); | |||||
if (!Filesystem::pathExists($junit_tmp)) { | |||||
Done Inline ActionsIMO, this doesn't provide any value. joshuaspence: IMO, this doesn't provide any value. | |||||
Done Inline ActionsWill get rid of all this useless comments :) tycho.tatitscheff: Will get rid of all this useless comments :) | |||||
throw new CommandException( | |||||
pht('Testing Command failed with error #%s!', $err), | |||||
$future->getCommand(), | |||||
$err, | |||||
$stdout, | |||||
$stderr); | |||||
} | |||||
if ($this->getEnableCoverage()) { | |||||
Done Inline ActionsAs above. joshuaspence: As above. | |||||
if (!Filesystem::pathExists($cover_tmp)) { | |||||
throw new CommandException( | |||||
pht('Coverage Command failed with error #%s!', $err), | |||||
$future->getCommand(), | |||||
$err, | |||||
$stdout, | |||||
Done Inline ActionsAs aabove. joshuaspence: As aabove. | |||||
$stderr); | |||||
} | |||||
} | |||||
return $this->parseTestResults($junit_tmp, $cover_tmp); | |||||
} | |||||
Done Inline ActionsAs above. joshuaspence: As above. | |||||
private function getAllTestsPattern() { | |||||
$pattern = $this->getConfigurationManager() | |||||
Done Inline ActionsWhy not just if ($this->getEnableCoverage()) { ? joshuaspence: Why not just `if ($this->getEnableCoverage()) {` ? | |||||
Done Inline ActionsIndeed ! tycho.tatitscheff: Indeed ! | |||||
->getConfigFromAnySource('unit.mocha.pattern'); | |||||
if (!$pattern) { | |||||
$pattern = 'tests/*.spec.js'; | |||||
} | |||||
return $pattern; | |||||
} | |||||
private function buildTestFuture($pattern_tests, $junit_tmp) { | |||||
$cmd_line = csprintf( | |||||
Done Inline ActionsAs above. joshuaspence: As above. | |||||
'MOCHA_FILE=%s '. | |||||
'mocha test -u exports '. | |||||
'--reporter mocha-junit-reporter %s', | |||||
$junit_tmp, $pattern_tests); | |||||
if ($this->getEnableCoverage()) { | |||||
$cmd_line = csprintf( | |||||
'MOCHA_FILE=%s '. | |||||
'istanbul cover --report cobertura '. | |||||
Done Inline ActionsAs above. joshuaspence: As above. | |||||
'node_modules/mocha/bin/_mocha '. | |||||
'-- -u exports '. | |||||
'--compilers js:babel/register-without-polyfill '. | |||||
// Maybe make previous line optional. | |||||
// It is useful to run test when code use ES6 | |||||
'--reporter mocha-junit-reporter %s', | |||||
$junit_tmp, $pattern_tests); | |||||
} | |||||
return new ExecFuture($cmd_line); | |||||
Lint: Unsafe Usage of Dynamic String Parameter 1 of ExecFuture() should be a scalar string, otherwise it's not safe. Lint: Unsafe Usage of Dynamic String: Parameter 1 of ExecFuture() should be a scalar string, otherwise it's not safe. | |||||
} | |||||
private function parseTestResults($junit_tmp, $cover_tmp) { | |||||
$parser = new ArcanistXUnitTestResultParser(); | |||||
$results = $parser->parseTestResults(Filesystem::readFile($junit_tmp)); | |||||
if ($this->getEnableCoverage()) { | |||||
$coverage = $this->readCoverage($cover_tmp); | |||||
foreach ($results as $result) { | |||||
$result->setCoverage($coverage); | |||||
} | |||||
Done Inline ActionsThis should (later) come from arclint. joshuaspence: This should (later) come from `arclint`. | |||||
Done Inline ActionsAnd get getConfigFromAnySource won't be froward compatible with .arclint and D13534 ? tycho.tatitscheff: And get getConfigFromAnySource won't be froward compatible with `.arclint` and D13534 ?
I will… | |||||
} | |||||
return $results; | |||||
} | |||||
/** | |||||
* This is copied from @{class:NoseTestEngine}. | |||||
* I didn't ever try to understand what is going on. | |||||
*/ | |||||
public function readCoverage($cover_file) { | |||||
$coverage_dom = new DOMDocument(); | |||||
$coverage_dom->loadXML(Filesystem::readFile($cover_file)); | |||||
$reports = array(); | |||||
$classes = $coverage_dom->getElementsByTagName('class'); | |||||
foreach ($classes as $class) { | |||||
$path = $class->getAttribute('filename'); | |||||
$root = $this->getWorkingCopy()->getProjectRoot(); | |||||
if (!Filesystem::isDescendant($path, $root)) { | |||||
continue; | |||||
} | |||||
// get total line count in file | |||||
$lines_list = phutil_split_lines(Filesystem::readFile($path)); | |||||
$line_count = count($lines_list); | |||||
$coverage = ''; | |||||
$start_line = 1; | |||||
$lines = $class->getElementsByTagName('line'); | |||||
for ($ii = 0; $ii < $lines->length; $ii++) { | |||||
Done Inline ActionsI'm not sure about &&... particularly I'm not sure if this will work on Windows. joshuaspence: I'm not sure about `&&`... particularly I'm not sure if this will work on Windows. | |||||
Done Inline ActionsOn production I have an other syntax that works on windows too. I will switch for this. tycho.tatitscheff: On production I have an other syntax that works on windows too. I will switch for this. | |||||
$line = $lines->item($ii); | |||||
$next_line = intval($line->getAttribute('number')); | |||||
Done Inline ActionsThis is equivalent to ExecFuture($cmd_line) joshuaspence: This is equivalent to `ExecFuture($cmd_line)` | |||||
for ($start_line; $start_line < $next_line; $start_line++) { | |||||
$coverage .= 'N'; | |||||
} | |||||
if (intval($line->getAttribute('hits')) == 0) { | |||||
$coverage .= 'U'; | |||||
} else if (intval($line->getAttribute('hits')) > 0) { | |||||
$coverage .= 'C'; | |||||
} | |||||
$start_line++; | |||||
} | |||||
if ($start_line < $line_count) { | |||||
foreach (range($start_line, $line_count) as $line_num) { | |||||
$coverage .= 'N'; | |||||
} | |||||
} | |||||
$reports[$path] = $coverage; | |||||
} | |||||
return $reports; | |||||
} | |||||
} |
There should be a blank line between <?php and any code, see D13534.