Problem/Motivation
When running phpunit tests for a module, if that module does not have its own composer.json setup, Drupal will register autoloading for the test namespace.
The details are in drupal_phpunit_get_extension_namespaces()
and drupal_phpunit_populate_class_loader()
.
The following namespaces directories are registered:
- "Drupal\\Tests\\$extension\\Unit\\" -> /src/tests/Unit/.
- "Drupal\\Tests\\$extension\\Kernel\\" -> /src/tests/Kernel/.
- "Drupal\\Tests\\$extension\\Functional\\" -> /src/tests/Functional/.
- "Drupal\\Tests\\$extension\\Build\\" -> /src/tests/Build/.
- "Drupal\\Tests\\$extension\\FunctionalJavascript\\" -> /src/tests/FunctionalJavascript/.
- "Drupal\\Tests\\$extension\\Traits\\" -> /src/tests/Traits/.
However, not having the top-level namespace registered for class loading means that we cannot easily use helper classes.
See https://drupal.stackexchange.com/questions/309143/autoload-helper-classe...
Steps to reproduce
Create a module with tests.
Have one of the tests use a trait in /tests/src/Traits/.
Have another test use a helper class in /tests/src/Helper/.
Run the tests from a Drupal installation, where the module is in a subdirectory.
E.g. "./vendor/bin/phpunit web/modules/custom/mymodule/tests/src/Kernel/MyKernelTest.php".
Result:
- The test using the trait works, because the trait is correctly autoloaded.
- The test using the helper class will break, because the helper class cannot be autoloaded.
Proposed resolution
Register all of \Drupal\\Tests\\$extension
at /src/tests/
so any sub-namespace can be found, not just the predefined list above.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|
Issue fork drupal-3258817
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3258817-register-all-tests changes, plain diff MR !6870
Comments
Comment #2
pwolanin CreditAttribution: pwolanin as a volunteer and at SciShield commented+1 just ran into this myself
Comment #3
cilefen CreditAttribution: cilefen as a volunteer commentedComment #4
mglaman+1 this would simplify phpstan-drupal's autoloader for Drupal: https://github.com/mglaman/phpstan-drupal/blob/main/src/Drupal/DrupalAut...
Comment #5
Mile23I vaguely recall that the reason we did that was because it was unclear how the tests would be organized as we got closer to 8.0.0 release, and we wanted to avoid simpletest-based tests.
It's still the case that a contrib module might use the (now-contrib) simpletest module for some testing, so the guardrails might still be needed, unless, of course, an actual subsystem maintainer says otherwise.
Comment #6
longwaveAccording to TestDiscovery there should be no conflict with simpletest namespaces or paths:
ie. Simpletest tests live in
$module/src/Tests
but PHPUnit tests live in$module/tests/src
We should also remove the remaining Simpletest support from core anyway, please see #3254726: Remove SimpleTest support from run-tests.sh, TestDiscovery and TestRunnerKernel for that.
Comment #7
longwaveLet's see what happens if we do module tests first.
If this passes we can try the core test locations identified in #3 as well.
Comment #10
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #11
Nikhil_110 CreditAttribution: Nikhil_110 at Material commentedAttached patch against Drupal 10.1.x
Patch #7 is not applied for Drupal 10.1.x so Inter-diff file is not added.
Comment #12
Nikhil_110 CreditAttribution: Nikhil_110 at Material commentedComment #13
smustgrave CreditAttribution: smustgrave at Mobomo commentedNeeds work per #7/ #3
Comment #15
andypostComment #16
craigra CreditAttribution: craigra as a volunteer commentedI've tested the patch in #11 on Drupal 10.2 and confirmed that this fixes the issue with test classes being unable to find a helper class under tests/src/Helper within a module test directory, as described in the steps to reproduce. Similarly I've verified this for theme and profile test directories.
What additional work is needed for core test locations (#7), and could this be split to another issue so the existing fix for modules, themes and profiles can be RTBC?
Comment #18
longwaveI don't actually think we can do much about the core namespaces, because they sit directly under
\Drupal
and we don't want to go looking incore/tests/Drupal
for non-test Drupal code.Converted #11 to an MR, inlined a variable for readability, and updated the IS with the proposed solution.
Comment #19
longwaveHiding patches.
Comment #20
smustgrave CreditAttribution: smustgrave at Mobomo commentedSeems to have a test failure.
Comment #21
longwaveJust needed rebase.
Comment #22
smustgrave CreditAttribution: smustgrave at Mobomo commentedSweet. Change didn’t break anything
Comment #25
catchCommitted/pushed to 11.x and cherry-picked to 10.3.x, thanks!