Repository navigation
Fix PHPUnit configuration discovery for Drupal QA - #80
Conversation
| $iterator = new RecursiveCallbackFilterIterator( | ||
| new RecursiveDirectoryIterator( | ||
| $scanDirectory, | ||
| RecursiveDirectoryIterator::FOLLOW_SYMLINKS | RecursiveDirectoryIterator::SKIP_DOTS |
There was a problem hiding this comment.
Why do we remove symlinks?
There was a problem hiding this comment.
Good point. Removing FOLLOW_SYMLINKS avoided a recursive link, but it also removed valid linked-extension support. I changed this to retain symlink traversal while tracking canonical directory paths and skipping only paths already visited, including cycles.
| /** | ||
| * Tests that extension discovery does not recurse into directory symlinks. | ||
| */ | ||
| public function testExtensionDiscoveryDoesNotFollowDirectorySymlinks(): void { |
There was a problem hiding this comment.
Why do we remove symlink support?
There was a problem hiding this comment.
Agreed. The revised tests now cover both a valid symlinked extension, which is discovered, and a directory-link cycle, which is safely skipped.
|
|
||
| // PHPUnit configuration schemas are specific to the runner major. | ||
| if ($taskInfo['filename'] === 'phpunit' && Version::majorVersionNumber() === 11) { | ||
| if ($taskInfo['filename'] === 'phpunit' && PhpunitVersionResolver::installedMajorVersion() === 11) { |
There was a problem hiding this comment.
Maybe better to add a mapping array between phpunit versions and the file to include?
This to prevent altering the actual code instead of a "mapping config".
There was a problem hiding this comment.
Agreed. This is now an explicit mapping of supported PHPUnit majors to template suffixes (11 => '-11', 12 => ''), while still resolving the consumer project's installed version from Composer metadata.
| <exclude> | ||
| <directory suffix=".api.php">./</directory> | ||
| <directory suffix="Spy.php">./</directory> | ||
| <directory suffix="Stub.php">./</directory> | ||
| <directory suffix="Test.php">./</directory> | ||
| <directory suffix="TestBase.php">./</directory> | ||
| <directory suffix="TestCase.php">./</directory> | ||
| </exclude> |
There was a problem hiding this comment.
Why do we remove these?
This prevents PHPUnit wanting to have code coverage for test Fixtures and base test classes.
There was a problem hiding this comment.
You're right. The root-wide exclusions were removed to stop PHPUnit traversing the project and vendor tree, but the exclusions themselves are still needed. They are now restored and bounded to ./src and ./modules/**/src, so fixtures, stubs, and test base classes remain outside coverage without reintroducing recursive scanning.
| <exclude> | ||
| <directory suffix=".api.php">./</directory> | ||
| <directory suffix="Spy.php">./</directory> | ||
| <directory suffix="Stub.php">./</directory> | ||
| <directory suffix="Test.php">./</directory> | ||
| <directory suffix="TestBase.php">./</directory> | ||
| <directory suffix="TestCase.php">./</directory> | ||
| </exclude> |
There was a problem hiding this comment.
Why do we remove these?
This prevents PHPUnit wanting to have code coverage for test Fixtures and base test classes.
There was a problem hiding this comment.
Correct. The root-wide exclusions were removed to stop PHPUnit traversing the project and vendor tree, but the exclusions themselves are still needed. They are now restored and bounded to ./src and ./modules/**/src, so fixtures, stubs, and test base classes remain outside coverage without reintroducing recursive scanning.
| <directory suffix=".api.php">./</directory> | ||
| <directory suffix="Spy.php">./</directory> | ||
| <directory suffix="Stub.php">./</directory> | ||
| <directory suffix="Test.php">./</directory> | ||
| <directory suffix="TestBase.php">./</directory> | ||
| <directory suffix="TestCase.php">./</directory> |
There was a problem hiding this comment.
Why do we remove these?
This prevents PHPUnit wanting to have code coverage for test Fixtures and base test classes.
There was a problem hiding this comment.
You're right. The root-wide exclusions were removed to stop PHPUnit traversing the project and vendor tree, but the exclusions themselves are still needed. They are now restored and bounded to the custom module, profile, and theme roots, so fixtures, stubs, and test base classes remain outside coverage without reintroducing recursive scanning.
| <directory suffix=".api.php">./</directory> | ||
| <directory suffix="Spy.php">./</directory> | ||
| <directory suffix="Stub.php">./</directory> | ||
| <directory suffix="Test.php">./</directory> | ||
| <directory suffix="TestBase.php">./</directory> | ||
| <directory suffix="TestCase.php">./</directory> |
There was a problem hiding this comment.
Why do we remove these?
This prevents PHPUnit wanting to have code coverage for test Fixtures and base test classes.
There was a problem hiding this comment.
Agreed, see previous comments
c970205 to
753de54
Compare
2 participants
Summary
Fix PHPUnit configuration generation and source discovery in QA Drupal for
Drupal 11 and Drupal 12 consumers.
Changes
installed
phpunit/phpunitversion rather than the PHPUnit version bundledinside the GrumPHP PHAR.
district09/qa-php:^3.1.1so generated PHPStan configuration canresolve the GrumPHP API stubs used by the shared task event listener.
recursive traversal of Composer dependencies and linked repositories.
Verification
composer validate --strictvendor/bin/phpunit --configuration=phpunit.xml.dist tests/src/Unit/GrumPHPgit diff --check