Context
Our code base includes many types of content including:
- Source code.
- Documentation (in the form of markdown).
- Misc structural files.
When we run arc unit --everything, the coverage results include files that will never be covered by tests.
Repro steps
- Have an .arcunit config with at least one test engine.
- Run arc unit --everything
Expected output
A list filtered down to paths that can actually be covered by the unit test engines.
COVERAGE REPORT
0% src/private/SomePrivateSource.m
62% src/SourceFile1.m
100% src/SourceFile2.mActual output (specific names filtered)
COVERAGE REPORT
0% src/SomeSource.h
0% src/SomeSource.h
0% examples/apps/Catalog/Catalog/SomeSource.m
0% examples/apps/Catalog/Catalog/SomeFolder/SomeSource.h
0% examples/apps/Catalog/Catalog/SomeFolder/SomeSource.m
0% examples/apps/Catalog/Catalog/SomeSource.m
0% src/private/SomePrivateSource.m
0% src/private/SomePrivateSource.h
0% third_party/clang-format-linter
0% tests/unit/SomeTest.m
0% tests/apps/Catalog/Info.plist
0% tests/apps/Catalog/Catalog.xcodeproj/project.pbxproj
0% .travis.yml
0% .jazzy.yaml
0% AUTHORS
0% CONTRIBUTING.md
0% LICENSE
0% .gitmodules
0% .gitignore
0% .arcconfig
0% .arclint
0% .arcunit
0% .clang-format
0% SomePod.podspec
0% Podfile
0% Podfile.lock
0% examples/apps/Catalog/Catalog/Info.plist
0% README.md
62% src/SourceFile1.m
100% src/SourceFile2.mMy explorations of a fix
My first hack-fix involved filtering the $paths used to output the coverage table by the same include/exclude regex defined in .arcunit. This gave me a result that was close to but not exactly the desired behavior.
My .arcunit looks like so:
{
"engines": {
"xcode": {
"type": "xcode-test-engine",
"include": "(src/.+\\.(m|h|mm|swift)$)",
"exclude": "(/Pods/)"
}
}
}The output after my patch:
COVERAGE REPORT
0% src/SomeSource1.h
0% src/private/SomePrivateSource.h
0% src/private/SomePrivateSource.m
0% src/SomeSource2.h
62% src/SomeSource1.m
100% src/SomeSource2.mThe patch:
diff --git a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php index 6cc659a..9fe502a 100644 --- a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php +++ b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php @@ -58,6 +58,8 @@ final class ArcanistConfigurationDrivenUnitTestEngine $built_test_engines = array(); $all_paths = $this->getPaths(); + + $affectedCoveragePaths = array(); foreach ($config['engines'] as $name => $spec) { $type = idx($spec, 'type'); @@ -106,10 +108,14 @@ final class ArcanistConfigurationDrivenUnitTestEngine $exclude); $test_engine->setPaths($paths); + + $affectedCoveragePaths = array_merge($affectedCoveragePaths, $paths); } $built_test_engines[] = $test_engine; } + + $this->setAffectedCoveragePaths($affectedCoveragePaths); return $built_test_engines; } diff --git a/src/unit/engine/ArcanistUnitTestEngine.php b/src/unit/engine/ArcanistUnitTestEngine.php index b54bafe..674d993 100644 --- a/src/unit/engine/ArcanistUnitTestEngine.php +++ b/src/unit/engine/ArcanistUnitTestEngine.php @@ -7,6 +7,7 @@ abstract class ArcanistUnitTestEngine extends Phobject { private $workingCopy; private $paths = array(); + private $affectedCoveragePaths = array(); private $enableCoverage; private $runAllTests; private $configurationManager; @@ -78,6 +79,15 @@ abstract class ArcanistUnitTestEngine extends Phobject { return $this->enableCoverage; } + final public function setAffectedCoveragePaths(array $paths) { + $this->affectedCoveragePaths = $paths; + return $this; + } + + final public function getAffectedCoveragePaths() { + return $this->affectedCoveragePaths; + } + final public function setRenderer(ArcanistUnitRenderer $renderer = null) { $this->renderer = $renderer; return $this; diff --git a/src/workflow/ArcanistUnitWorkflow.php b/src/workflow/ArcanistUnitWorkflow.php index a1aadd3..27a9b37 100644 --- a/src/workflow/ArcanistUnitWorkflow.php +++ b/src/workflow/ArcanistUnitWorkflow.php @@ -148,9 +148,8 @@ EOTEXT $this->engine = $this->newUnitTestEngine($this->getArgument('engine')); if ($everything) { $this->engine->setRunAllTests(true); - } else { - $this->engine->setPaths($paths); } + $this->engine->setPaths($paths); $renderer = new ArcanistUnitConsoleRenderer(); $this->engine->setRenderer($renderer); @@ -197,7 +196,7 @@ EOTEXT if ($coverage) { $file_coverage = array_fill_keys( - $paths, + $this->engine->getAffectedCoveragePaths(), 0); $file_reports = array(); foreach ($coverage as $file => $reports) {
The feature request
My faulty assumption was that the regex that starts a unit test is equal to the regex of coverable files. In practice I think they might be different regexes.
My follow-up idea is to add a new filter to .arcunit. This filter would allow you to provide a regex of coverable files. This filter would have to be read in some manner by ArcanistConfigurationDrivenUnitTestEngine and made available to ArcanistUnitWorkflow. This seems like a much bigger change (adding a new field to .arcunit for everyone), thus me posting the issue before pursuing any further changes.
An example of what this new field might look like:
{
"engines": {
"xcode": {
"type": "xcode-test-engine",
"include": "(src/.+\\.(m|h|mm|swift)$)",
"exclude": "(/Pods/)",
"coverable": "(src/.+\\.(m|mm|swift)$)"
}
}
}And the desired output:
COVERAGE REPORT
0% src/private/SomePrivateSource.m
62% src/SourceFile1.m
100% src/SourceFile2.m