Do not search directories that are excluded from the source

[nicolas-grekas]

The bug

SourceMapper::map() searches every directory of <source><include>, and then searches every directory of <source><exclude> to remove the files it has just collected from them. Both searches enter directories that cannot contribute a file to the map. RecursiveDirectoryIterator throws when it cannot open a directory, so a single unreadable or disappearing directory below an excluded path aborts the whole test run.

Steps to reproduce

project
├── build
│   └── unreadable      (mode 000)
├── phpunit.xml
├── src
│   └── Example.php
└── tests
    └── ExampleTest.php

phpunit.xml:

<source>
    <include>
        <directory>.</directory>
    </include>
    <exclude>
        <directory>build</directory>
    </exclude>
</source>

ExampleTest carries #[RunTestsInSeparateProcesses], so the source map is written for the child process.

What happens

PHPUnit 12.5.33 by Sebastian Bergmann and contributors.

An error occurred inside PHPUnit.

Message:  RecursiveDirectoryIterator::__construct(…/build/unreadable): Failed to open directory: Permission denied
Location: …/php-file-iterator/src/ExcludeIterator.php:62

#5 …/src/TextUI/Configuration/SourceMapper.php(83)
#6 …/src/TextUI/Configuration/SourceMapper.php(40)
#7 …/src/Framework/TestRunner/SeparateProcessTestRunner.php(190)

What I expected

build is excluded from the source, so what it contains should not affect the run.

Where this comes from

In a monorepo, several composer update runs work in parallel below the directory that one test run walks. Composer creates and removes a temporary extraction directory, and PHPUnit walks it in the moment between the two. The directory is gone when opendir() is called, the test run of that package dies with the message above, and the job is red for a reason that has nothing to do with the tests.

The change

• The included directories are now searched with the excluded directories handed to the file iterator, so an excluded tree is not entered. This is done for an excluded directory only when its filters remove everything the include filters collect there, and only when no sibling directory has the excluded path as a prefix, because ExcludeIterator matches on the path prefix.
• The excluded directories are searched only when the map holds a file below them, which is never the case for a directory that was already skipped.

TestSuiteMapper already hands the excluded directories to the file iterator for <testsuites>, so this makes <source> behave the same way.

Tests

• testDoesNotSearchExcludedDirectory fails on 12.5 with the exception above and passes with this change.
• testDoesNotExcludeDirectoryWhoseNameBeginsWithTheNameOfAnExcludedDirectory passes with and without this change. It protects the existing behaviour: without the prefix guard, excluding tests would also drop tests-integration from the map.
• testIgnoresExcludedDirectoryThatDoesNotExist covers an excluded directory that does not exist on disk, which is where both realpath() results are false.
• The fixtures are created below sys_get_temp_dir(). The first test skips on Windows and when the unreadable directory cannot be created.

Checks

• ./phpunit --testsuite unit: OK, 4056 tests, 12127 assertions.
• ./phpunit --testsuite end-to-end: 813 tests, one failure, defaulttestsuite-using-testsuite-without-name.phpt, which fails the same way on 12.5 without this change in my checkout because tests/_files/tests does not exist there.
• ./tools/php-cs-fixer fix: 0 of 2226 files fixed.
• ./tools/phpstan analyse: no errors.

Note

The prefix guard would not be needed if ExcludeIterator::accept() compared on the directory boundary, that is $path === $exclude || str_starts_with($path, $exclude . DIRECTORY_SEPARATOR). I can send that to php-file-iterator separately if you prefer that route.

Use of an LLM-based coding assistant

I used Claude Code for this change: it reproduced the failure, wrote the patch and the tests, and ran the checks listed above. I reviewed the result, and I am able to explain and defend it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.04%. Comparing base (45544b2) to head (c2da8f5).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff            @@
##               12.5    #6915   +/-   ##
=========================================
  Coverage     96.03%   96.04%           
- Complexity     7576     7599   +23     
=========================================
  Files           798      798           
  Lines         23233    23273   +40     
=========================================
+ Hits          22312    22352   +40     
  Misses          921      921           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[sebastianbergmann]

Thank you, Nicolas, for raising this issue. The failure is real, I reproduced it, and it is worth fixing. I do not think the fix belongs in PHPUnit, though.

Why not here

The crash does not come from SourceMapper. It comes from Factory::getFileIterator() in phpunit/php-file-iterator, which builds its RecursiveIteratorIterator without RecursiveIteratorIterator::CATCH_GET_CHILD. When ExcludeIterator::getChildren() constructs a RecursiveDirectoryIterator for a directory that cannot be opened, the UnexpectedValueException escapes the whole traversal. Nothing about that is specific to <source>, and nothing about it is specific to directories that are excluded.

That is also the practical problem with this pull request: it removes the crash from the fixture it ships, and leaves it in place for the case you describe. I ran your branch against three trees:

scenario	this branch
unreadable directory below an excluded build (your reproducer)	ok
unreadable directory below an included src that is not excluded	still crashes
unreadable directory below an excluded vendor, with a vendor-bin sibling	still crashes

The second one is the general case, and it is the one your description is actually about: a Composer temporary directory that appears and disappears during the walk can be anywhere below an included directory, not only below an excluded one. The third is the bamarni/composer-bin-plugin layout, which is common in exactly the monorepos you have in mind — there, hasSiblingWithSameNamePrefix() switches the pruning off again and the crash comes back.

Problems in the change itself

Beyond not covering the case it is meant to cover, the change introduces regressions in the source map:

1. Wildcard and stream-wrapper excludes stop excluding anything. hasFilesIn() calls realpath() on the raw configured path, but FilterDirectory::path() is never glob-expanded — Factory::resolveWildcards() is what resolves * and **. For <exclude><directory>src/*/Generated</directory></exclude>, realpath() returns false, hasFilesIn() returns false, and the exclude scan is skipped entirely. With src/a/Generated/G.php and src/a/Keep.php, 12.5 maps [Keep.php] and this branch maps [Generated/G.php, Keep.php]. The same applies to :// paths, which toAbsolutePath() deliberately preserves.

2. Sibling files sharing a name prefix are dropped from the map. hasSiblingWithSameNamePrefix() only considers sibling directories, but ExcludeIterator::accept() matches on a bare str_starts_with(), which also matches files. With src/Generated/ excluded and src/GeneratedFactory.php present, src/Generated is pruned and the iterator then rejects src/GeneratedFactory.php as well: 12.5 maps [GeneratedFactory.php, Keep.php], this branch maps [Keep.php].

Both are silent, and this map drives identifyIssueTrigger classification as well as the code coverage filter of child processes.

3. scandir() can raise warnings inside a test. map() runs during test execution via SeparateProcessTestRunner::sourceMapFile(), where the error handler is registered. If the parent of an excluded directory is traversable but not readable, realpath() still succeeds, the guard on line 166 does not catch it, and scandir() emits two E_WARNINGs that get attributed to a test — the class of failure this pull request sets out to remove.

4. Symlink semantics change unannounced. The old exclude scan followed symlinks and removed the real path of the target; hasFilesIn() only tests whether a mapped real path lies textually below the excluded directory. With src/Excluded/Link.php -> src/real/Target.php, 12.5 maps [Keep.php] and this branch maps [Keep.php, real/Target.php]. That result is arguably better, but it is a semantic change with no test and no ChangeLog entry.

More generally, I would rather not have SourceMapper grow a hundred lines of filesystem heuristics — sibling scans, prefix guards, filter-subsumption reasoning — in order to work around a limitation of the file iterator it calls. That reasoning is fragile, as points 1 and 2 show, and it would have to be repeated in every other place that walks a directory.

Where it does belong

Please have a look at sebastianbergmann/php-file-iterator#162. It passes RecursiveIteratorIterator::CATCH_GET_CHILD and guards the construction of the root RecursiveDirectoryIterator, so a directory that cannot be opened is skipped wherever it is encountered. That is the same contract the component already applies to a configured path that is not a directory.

With that in vendor/, all three scenarios above pass with SourceMapper unchanged, and both the unreadable-directory case and the Composer race you describe are covered — for <source>, for test discovery, and for extension loading alike.

The boundary fix for ExcludeIterator::accept() that you offer at the end of your description is a genuine bug of its own (excluding tests from a test suite also drops tests-integration, since TestSuiteMapper is the one caller that passes $exclude). I would very much welcome that as a separate pull request against php-file-iterator — it just is not what causes this crash.

Not searching excluded directories remains a reasonable optimisation, but it is a separate concern from this bug, and I would like to treat it as one.

[nicolas-grekas]

Thanks I'll have a look in this direction!