Skip to content

[Issue] Improved composer's autoloader exclusion list a bit. #40109

@m2-assistant

Description

@m2-assistant

This issue is automatically created based on existing pull request: #40107: Improved composer's autoloader exclusion list a bit.


Description (*)

This started out with noticing how dumping an optimized autoloader on Magento 2.4.8(-p1) outputted some errors from test classes in the setup/src/ directory which were using unexpected namespaces (unexpected in scope of a project, not in scope of the tests themselves).

# Output from vanilla Magento 2.4.7-p6:
$ composer dump-autoload -o
Generating optimized autoload files
Generated optimized autoload files containing 27863 classes

# Output from vanilla Magento 2.4.8-p1:
$ composer dump-autoload -o
Generating optimized autoload files
Class Some\TestNamespace\TestClass located in ./setup/src/Magento/Setup/Test/Unit/Module/Di/Code/Reader/_files/TestClass.php does not comply with psr-4 autoloading standard (rule: Magento\Setup\ => ./setup/src/Magento/Setup). Skipping.
Class Magento\SomeModule\Helper\TestHelper located in ./setup/src/Magento/Setup/Test/Unit/Module/Di/_files/app/code/Magento/SomeModule/Helper/TestHelper.php does not comply with psr-4 autoloading standard (rule: Magento\Setup\ => ./setup/src/Magento/Setup). Skipping.
Class Magento\SomeModule\ElementFactory located in ./setup/src/Magento/Setup/Test/Unit/Module/Di/_files/app/code/Magento/SomeModule/ElementFactory.php does not comply with psr-4 autoloading standard (rule: Magento\Setup\ => ./setup/src/Magento/Setup). Skipping.
Class Magento\SomeModule\Element located in ./setup/src/Magento/Setup/Test/Unit/Module/Di/_files/app/code/Magento/SomeModule/Element.php does not comply with psr-4 autoloading standard (rule: Magento\Setup\ => ./setup/src/Magento/Setup). Skipping.
Class Magento\SomeModule\NestedElement located in ./setup/src/Magento/Setup/Test/Unit/Module/Di/_files/app/code/Magento/SomeModule/NestedElement.php does not comply with psr-4 autoloading standard (rule: Magento\Setup\ => ./setup/src/Magento/Setup). Skipping.
Class Magento\SomeModule\Model\DoubleColon located in ./setup/src/Magento/Setup/Test/Unit/Module/Di/_files/app/code/Magento/SomeModule/Model/DoubleColon.php does not comply with psr-4 autoloading standard (rule: Magento\Setup\ => ./setup/src/Magento/Setup). Skipping.
Class Magento\SomeModule\Model\StubWithAnonymousClass located in ./setup/src/Magento/Setup/Test/Unit/Module/Di/_files/app/code/Magento/SomeModule/Model/StubWithAnonymousClass.php does not comply with psr-4 autoloading standard (rule: Magento\Setup\ => ./setup/src/Magento/Setup). Skipping.
Class Magento\SomeModule\Model\TestModel located in ./setup/src/Magento/Setup/Test/Unit/Module/Di/_files/app/code/Magento/SomeModule/Model/TestModel.php does not comply with psr-4 autoloading standard (rule: Magento\Setup\ => ./setup/src/Magento/Setup). Skipping.
Class Magento\SomeModule\Api\Data\SomeInterface located in ./setup/src/Magento/Setup/Test/Unit/Module/Di/_files/app/code/Magento/SomeModule/Api/Data/SomeInterface.php does not comply with psr-4 autoloading standard (rule: Magento\Setup\ => ./setup/src/Magento/Setup). Skipping.
Generated optimized autoload files containing 34391 classes

Notice that the number of optimized autoload files increased with about 6.500 files and we get 9 warnings about incorrect namespaces being used (which is fine in itself, it's just not expected to see those in project-scope as warnings).
Especially this becomes bad if you try to detect these kind of wrong usages of class names or namespaces in custom code in CI/CD systems where you can use the --strict-psr option in the dump-autoload command, to give you an exit code of 1, which should stop your CI/CD. So this is not good to have this regression in Magento 2.4.8 compared to 2.4.7


I dug a bit deeper and found some more interesting things and the cause of this change.

So, Magento 2.4.8 came with the following commit: 52b936d, the interesting part of this is:

         "exclude-from-classmap": [
             "**/dev/**",
             "**/update/**",
-            "**/Test/**"
+            "*/*/Test/**/*Test"
         ]

After a bunch of digging around, I figured out why this got changed, see this discussion on the phpunit repository: sebastianbergmann/phpunit#5570

The exclude-from-classmap is typically being used to add some path patterns to be excluded from the generated (optimized) autoloader, like Test classes which aren't in some dev dependency package. This is so that the autoloader is faster and not bloated with classes that make no sense to be autoloaded in a production system.
With Magento, most test code comes included with the modules themselves, so using exclude-from-classmap to strip them out of the autoloader makes sense.
However, since PHPUnit v10, the old pattern also stripped out classes that phpunit required to be autoloaded (yes, also on non-production systems). So the old pattern had to be updated to keep those around while still trying to reduce the autoloader size in Magento.
However, I believe the choice of the new pattern was made poorly as it doesn't match many paths (even none at all in my testing). The pattern I'm introducing in this PR seems to match a lot more files and still keeps phpunit functioning like it should.


I'm proposing instead the pattern **/Test/Unit/** in this PR as it solves all problems. It significantly reduces the number of autoloaded classes. It keeps phpunit working like it should. And it removes those warnings when dumping an optimised autoloader.

Related Pull Requests

N/A

Fixed Issues (if relevant)

N/A

Manual testing scenarios (*)

Give the following steps a try with these 3 patterns:

  • "**/Test/**" (from Magento 2.4.7 and earlier)
  • "*/*/Test/**/*Test" (from Magento 2.4.8)
  • "**/Test/Unit/**" (proposal in this PR)
  1. Run composer dump-autoload -o
  2. Expect no warnings
  3. Look at the number in Generated optimized autoload files containing xxx classes, it should be smaller then before
  4. Do a count of amount of classes in a Test directory the classmap autoloader of composer: grep '/Test/' vendor/composer/autoload_classmap.php | wc -l, the number should be smaller then before
  5. Run a small phpunit check, to see if it works without autoload problems: cd dev/tests/unit && ../../../vendor/bin/phpunit -c phpunit.xml.dist ../../../app/code/Magento/ReleaseNotification/; cd ../../..

Questions or comments

Does it make sense to exclude more test classes? I don't think it's worth it, only a couple of hundreds of classes will get removed.

Examples:

  • **/Test/Integration/** => good for reduction of 285 extra classes
  • **/Test/Mftf/** => good for reduction of 19 extra classes
  • **/Test/Api/** => good for reduction of 93 extra classes
  • **/Test/Fixture/** => good for reduction of 99 extra classes
  • ...

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)

Metadata

Metadata

Assignees

No one assigned

    Labels

    Issue: ready for confirmationPriority: P3May be fixed according to the position in the backlog.Reported on 2.4.xIndicates original Magento version for the Issue report.Triage: Dev.ExperienceIssue related to Developer Experience and needs help with Triage to Confirm or Reject it

    Type

    No type

    Projects

    Status

    Ready for Development

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions