Add path wildcard feature to Enumerator #300
Conversation
| class EnumeratorTest extends \PHPUnit_Framework_TestCase | ||
| { | ||
|
|
||
| static $fixtureBasePath; |
There was a problem hiding this comment.
Visibility must be declared on property $fixtureBasePath
|
|
||
| static $fixtureBasePath; | ||
|
|
||
| static $testPaths = [ |
There was a problem hiding this comment.
Visibility must be declared on property $testPaths
| public function testExclude($expectedPaths, $includePaths, $excludePaths) | ||
| { | ||
| $basePath = static::$fixtureBasePath; | ||
| $addBasePath = function($path) use ($basePath) { |
There was a problem hiding this comment.
Expected 1 space after FUNCTION keyword; 0 found
|
Hi! Thank you for your job! I'll review it soon |
|
@lisachenko can you merge this into the 1.x version line as well? So this needs to be merged into both versions - if possible |
| foreach ($includePaths as $includePath) { | ||
| if (strpos($realPath, $includePath) === 0) { | ||
| foreach ($includePaths as $includePattern) { | ||
| if (preg_match('%' . $includePattern . '%', $filePath)) { |
There was a problem hiding this comment.
I have a worry, that preg_match() could be slow for checking a lot of files during class loading of big framework. Could you please check, if fnmatch() could be faster than preg_match().
There was a problem hiding this comment.
Problem is that fnmatch does not work with multiple subfolders. so /base/*/test only includes /base/sub/test but not /base/sub/sub/test
There was a problem hiding this comment.
Please, see: https://3v4l.org/iPR39#output Your case is second line
There was a problem hiding this comment.
Well... ok. I'll rewrite it then :)
| * | ||
| * @return string | ||
| */ | ||
| protected function parseWildcardPath($path) |
There was a problem hiding this comment.
This function can be replaced with built-in fnmatch(), see fnmatch() function
| throw new \Exception('TEST_DIRECTORY not set, check your phpunit.xml'); | ||
| } | ||
|
|
||
| static::$fixtureBasePath = realpath(TEST_DIRECTORY) . DIRECTORY_SEPARATOR . 'fixtures'; |
There was a problem hiding this comment.
Oh, it's an armed gun ) Because for several cases realpath() can return false which will equal to the empty string. Maybe we can use VFS filesystem for safe tests?
| */ | ||
| public static function tearDownAfterClass() | ||
| { | ||
| exec('rm -rf ' . static::$fixtureBasePath); |
There was a problem hiding this comment.
Very dangerous command, I think VFS will be better to use, because who knows if static::$fixtureBasePath will equal to the '/'?
|
@icanhazstring changed the base to 1.x, please don't forget to rebase your local branch as well |
| * | ||
| * @return string | ||
| */ | ||
| protected function getFileFullPath(\SplFileInfo $file) { |
There was a problem hiding this comment.
Opening brace should be on a new line
| // VFS does not support getRealPath() | ||
| $mock->expects($this->any()) | ||
| ->method('getFileFullPath') | ||
| ->will($this->returnCallback(function(\SplFileInfo $file) { |
There was a problem hiding this comment.
Expected 1 space after FUNCTION keyword; 0 found
|
@lisachenko reworked the tests using vfs and using |
|
Squashed and merged! Thank you very much! 👍 |
* origin/1.x: Add path wildcard feature to Enumerator (#300) Conflicts: composer.json composer.lock
* origin/1.x: Fixed broken logic of matching with base path, related to #300 Enable bootstraping autoload for composer
Add wildcard possibility for
includePathsandexcludePaths