ArcanistUnitWorkflow does not pass paths into test engines when asked to run tests for everything


#1

Observed Behavior:
Unit test engines does not have a reliably way to get VCS tracked files because it does not have access to RepositoryAPI and the ArcanistUnitWorkflow does not pass the paths into the engine when it is asked to run all tests.

Expected Behavior:
ArcanistUnitWorkflow should set the paths for engines regardless if it is asked to run all tests.

Phabricator Version:
arcanist master, the code can be seen in the source here. https://secure.phabricator.com/diffusion/ARC/browse/master/src/workflow/ArcanistUnitWorkflow.php$155

Reproduction Steps:
Write a custom unit test engine and see that $this->getPaths() will return empty when you run arc unit --everything

Discussions:
I see that some unit test engine get around that by using FileFinder API, for example: https://secure.phabricator.com/diffusion/ARC/browse/master/src/unit/engine/XUnitTestEngine.php$107
The problem with using FileFinder is that it will list all files without regards to the .gitignore file (or the equivalent file for other VCS).

My proposed fix is pretty simple, change: ArcanistUnitWorkflow.php Line 152

    if ($everything) {
      $this->engine->setRunAllTests(true);
    } else {
      $this->engine->setPaths($paths);
    }

to

    if ($everything) {
      $this->engine->setRunAllTests(true);
    }
    $this->engine->setPaths($paths);

The existing unit test engines will still work because they check on the run all tests flag and newer unit test engine can take advantage of getPaths() containing only VCS-tracked files.

This is my first bug report on phabricator community. Any feedback is appreciated! Thanks!