Skip to content

Commit

Permalink
ENH Improve ManifestFileFinder so it can ignore test based on the tes…
Browse files Browse the repository at this point in the history
…ting library
  • Loading branch information
Maxime Rainville committed Nov 16, 2021
1 parent e993e6c commit caa9ac2
Show file tree
Hide file tree
Showing 11 changed files with 103 additions and 3 deletions.
29 changes: 28 additions & 1 deletion src/Core/Manifest/ManifestFileFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ class ManifestFileFinder extends FileFinder
'include_themes' => false,
'ignore_tests' => true,
'min_depth' => 1,
'ignore_dirs' => ['node_modules']
'ignore_dirs' => ['node_modules'],
'ignore_ci_library' => []
];

public function acceptDir($basename, $pathname, $depth)
Expand Down Expand Up @@ -73,6 +74,14 @@ public function acceptDir($basename, $pathname, $depth)
return false;
}

// Skip if test dir inside vendor module with unexpected CI library
if ($depth > 3 && $basename === self::TESTS_DIR && $ignoreCILib = $this->getOption('ignore_ci_library')) {
$ciLib = $this->findModuleCILib($basename, $pathname, $depth);
if (in_array($ciLib, $ignoreCILib)) {
return false;
}
}

return parent::acceptDir($basename, $pathname, $depth);
}

Expand Down Expand Up @@ -251,4 +260,22 @@ public function isDirectoryIgnored($basename, $pathname, $depth)

return false;
}

private function findModuleCILib(string $basename, string $pathname, int $depth): string
{
if ($depth < 1) {
return Module::CI_PHPUNIT_UNKNOWN;
}

$newBasename = basename($pathname);
$newPathname = dirname($pathname);
$newDepth = $depth - 1;

if ($this->isDirectoryModule($newBasename, $newPathname, $newDepth)) {
$module = new Module($newPathname, $this->upLevels($newPathname, $newDepth));
return $module->getCILibrary();
}

return $this->findModuleCILib($newBasename, $newPathname, $newDepth);
}
}
4 changes: 2 additions & 2 deletions src/Core/Manifest/Module.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use Composer\Semver\Semver;
use Exception;
use InvalidArgumentException;
use RuntimeException;
use Serializable;
use SilverStripe\Core\Path;
use SilverStripe\Dev\Deprecation;
Expand Down Expand Up @@ -301,8 +300,9 @@ public function hasResource($path)
*/
public function getCILibrary(): string
{
// We don't have any composer data at all
if (empty($this->composerData)) {
throw new RuntimeException('No composer data at all');
return self::CI_PHPUNIT_UNKNOWN;
}

// We don't have any dev dependencies
Expand Down
55 changes: 55 additions & 0 deletions tests/php/Core/Manifest/ManifestFileFinderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use SilverStripe\Core\Manifest\ManifestFileFinder;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\Core\Manifest\Module;

/**
* Tests for the {@link ManifestFileFinder} class.
Expand Down Expand Up @@ -55,6 +56,8 @@ public function testBasicOperation()
[
'module/module.txt',
'vendor/myvendor/thismodule/module.txt',
'vendor/myvendor/phpunit5module/code/logic.txt',
'vendor/myvendor/phpunit9module/code/logic.txt',
]
);
}
Expand All @@ -75,6 +78,56 @@ public function testIgnoreTests()
'vendor/myvendor/thismodule/module.txt',
'vendor/myvendor/thismodule/tests/tests.txt',
'vendor/myvendor/thismodule/code/tests/tests2.txt',
'vendor/myvendor/phpunit5module/code/logic.txt',
'vendor/myvendor/phpunit5module/tests/phpunit5tests.txt',
'vendor/myvendor/phpunit9module/code/logic.txt',
'vendor/myvendor/phpunit9module/tests/phpunit9tests.txt',
]
);
}

public function testIgnorePHPUnit5Tests()
{
$finder = new ManifestFileFinder();
$finder->setOption('name_regex', '/\.txt$/');
$finder->setOption('ignore_tests', false);
$finder->setOption('ignore_ci_library', [Module::CI_PHPUNIT_FIVE]);

$this->assertFinderFinds(
$finder,
null,
[
'module/module.txt',
'module/tests/tests.txt',
'module/code/tests/tests2.txt',
'vendor/myvendor/thismodule/module.txt',
'vendor/myvendor/thismodule/tests/tests.txt',
'vendor/myvendor/thismodule/code/tests/tests2.txt',
'vendor/myvendor/phpunit5module/code/logic.txt',
'vendor/myvendor/phpunit9module/code/logic.txt',
'vendor/myvendor/phpunit9module/tests/phpunit9tests.txt',
]
);
}

public function testIgnoreNonePHPUnit9Tests()
{
$finder = new ManifestFileFinder();
$finder->setOption('name_regex', '/\.txt$/');
$finder->setOption('ignore_tests', false);
$finder->setOption('ignore_ci_library', [Module::CI_PHPUNIT_FIVE, Module::CI_PHPUNIT_UNKNOWN]);

$this->assertFinderFinds(
$finder,
null,
[
'module/module.txt',
'module/tests/tests.txt',
'module/code/tests/tests2.txt',
'vendor/myvendor/thismodule/module.txt',
'vendor/myvendor/phpunit5module/code/logic.txt',
'vendor/myvendor/phpunit9module/code/logic.txt',
'vendor/myvendor/phpunit9module/tests/phpunit9tests.txt',
]
);
}
Expand All @@ -92,6 +145,8 @@ public function testIncludeThemes()
'module/module.txt',
'themes/themes.txt',
'vendor/myvendor/thismodule/module.txt',
'vendor/myvendor/phpunit5module/code/logic.txt',
'vendor/myvendor/phpunit9module/code/logic.txt',
]
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<?php
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This file should always be discovered.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "silverstripe-vendor-module",
"require-dev": {
"sminnee/phpunit": "^5.7"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This file should not be discovered when running PHPUnit 9 tests
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<?php
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This file should always be discovered.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "silverstripe-vendor-module",
"require-dev": {
"phpunit/phpunit": "^9"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This file should not be discovered when running PHPUnit 9 tests

0 comments on commit caa9ac2

Please sign in to comment.