search

thephpleague / factory-muffin

Enables the rapid creation of objects for testing
https://factory-muffin.thephpleague.com/
MIT License
532 stars 72 forks source link

Change factory loader to not filter all paths with hidden directories #455

Closed deancsmith closed 4 years ago

deancsmith commented 5 years ago

This PR fixes an issue where FactoryMuffin will ignore loading factory definitions if they have a period anywhere in the absolute path to the factories folder. This change disregards any periods in the actual absolute path to the factories folder for filtering purposes, but still filters out hidden subdirectories within the factories folder when looking for factories.

For example, an absolute path of /home/ci/.jenkins-slave/workspace/some_project/tests/_support/factories passed to FactoryMuffin->loadDirectory() will be filtered out by the RegexIterator using the regex '/^[^\.](?:(?!\/\.).)+?\.php$/i'. This causes no factories to be loaded, despite having valid definitions in the factories folder, and a DefinitionNotFoundException to be ultimately thrown by any tests requiring model definitions.

The loader within this PR will still load factories from within /home/ci/.jenkins-slave/workspace/some_project/tests/_support/factories and /home/ci/.jenkins-slave/workspace/some_project/tests/_support/factories/some/namespace but not /home/ci/.jenkins-slave/workspace/some_project/tests/_support/factories/.hidden/.

I couldn't find a pure regex way to get to the above solution, so I reverted to an earlier regex that filters for .php extensions only and tweaked the loader foreach() loop to contain the additional directory filtering logic. This is a bit ugly but makes the filtering logic clear, especially when compared to a long regex, which will get complicated if it has to look for the n-1 directory, check if it has a period, and also work with both *nix(/) and Windows (\) paths.

Background

We use FactoryMuffin within Codeception. The latter generates an absolute URL to the factories directory which is then passed to FactoryMuffin::loadFactories(). This is fine during local development, as there tends to not be hidden directories in the absolute path. During CI, however, this is a problem as there are often auto-generated directories like .jenkins-slave in the absolute path.

This issue was introduced in this commit as a fix for this. It is likely the root cause for this issue here, although no one has posted their paths there.

Tests

I haven't added any tests because this would likely require adding a mock filesystem but I'm happy to do this.

GrahamCampbell commented 4 years ago

Thanks for this. Sorry this took so long to be merged!