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

Issue fork drupal-3258817

Command icon 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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

donquixote created an issue. See original summary.

pwolanin’s picture

+1 just ran into this myself

cilefen’s picture

Title: Why don't we register all of /tests/src/ for class loading? » Register all of /tests/src/ for class loading
Category: Support request » Task
mglaman’s picture

+1 this would simplify phpstan-drupal's autoloader for Drupal: https://github.com/mglaman/phpstan-drupal/blob/main/src/Drupal/DrupalAut...

    protected function addCoreTestNamespaces(): void
    {
        // Add core test namespaces.
        $core_tests_dir = $this->drupalRoot . '/core/tests/Drupal';
        $this->namespaces['Drupal\\BuildTests'] = $core_tests_dir . '/BuildTests';
        $this->namespaces['Drupal\\FunctionalJavascriptTests'] = $core_tests_dir . '/FunctionalJavascriptTests';
        $this->namespaces['Drupal\\FunctionalTests'] =  $core_tests_dir . '/FunctionalTests';
        $this->namespaces['Drupal\\KernelTests'] = $core_tests_dir . '/KernelTests';
        $this->namespaces['Drupal\\Tests'] = $core_tests_dir . '/Tests';
        $this->namespaces['Drupal\\TestSite'] = $core_tests_dir . '/TestSite';
        $this->namespaces['Drupal\\TestTools'] = $core_tests_dir . '/TestTools';
        $this->namespaces['Drupal\\Tests\\TestSuites'] = $this->drupalRoot . '/core/tests/TestSuites';
    }
Mile23’s picture

I 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.

longwave’s picture

According to TestDiscovery there should be no conflict with simpletest namespaces or paths:

      // Add Simpletest test namespace.
      $this->testNamespaces["Drupal\\$name\\Tests\\"][] = "$base_path/src/Tests";

      // Add PHPUnit test namespaces.
      $this->testNamespaces["Drupal\\Tests\\$name\\Unit\\"][] = "$base_path/tests/src/Unit";
      $this->testNamespaces["Drupal\\Tests\\$name\\Kernel\\"][] = "$base_path/tests/src/Kernel";
      $this->testNamespaces["Drupal\\Tests\\$name\\Functional\\"][] = "$base_path/tests/src/Functional";
      $this->testNamespaces["Drupal\\Tests\\$name\\Build\\"][] = "$base_path/tests/src/Build";
      $this->testNamespaces["Drupal\\Tests\\$name\\FunctionalJavascript\\"][] = "$base_path/tests/src/FunctionalJavascript";

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.

longwave’s picture

Status: Active » Needs review
FileSize
2.74 KB

Let'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.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
FileSize
143 bytes

The 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.

Nikhil_110’s picture

Attached patch against Drupal 10.1.x

Patch #7 is not applied for Drupal 10.1.x so Inter-diff file is not added.

Checking patch core/lib/Drupal/Core/Test/TestDiscovery.php...
error: while searching for:
$this->testNamespaces["Drupal\\$name\\Tests\\"][] = "$base_path/src/Tests";

// Add PHPUnit test namespaces.
$this->testNamespaces["Drupal\\Tests\\$name\\Unit\\"][] = "$base_path/tests/src/Unit";
$this->testNamespaces["Drupal\\Tests\\$name\\Kernel\\"][] = "$base_path/tests/src/Kernel";
$this->testNamespaces["Drupal\\Tests\\$name\\Functional\\"][] = "$base_path/tests/src/Functional";
$this->testNamespaces["Drupal\\Tests\\$name\\Build\\"][] = "$base_path/tests/src/Build";
$this->testNamespaces["Drupal\\Tests\\$name\\FunctionalJavascript\\"][] = "$base_path/tests/src/FunctionalJavascript";

// Add discovery for traits which are shared between different test
// suites.
$this->testNamespaces["Drupal\\Tests\\$name\\Traits\\"][] = "$base_path/tests/src/Traits";
}

foreach ($this->testNamespaces as $prefix => $paths) {

error: patch failed: core/lib/Drupal/Core/Test/TestDiscovery.php:102
error: core/lib/Drupal/Core/Test/TestDiscovery.php: patch does not apply
Checking patch core/tests/bootstrap.php...
Hunk #1 succeeded at 79 (offset -1 lines).
Hunk #2 succeeded at 87 (offset -1 lines).

Nikhil_110’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

Needs work per #7/ #3

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

andypost’s picture

craigra’s picture

I'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?

longwave’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update

I 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 in core/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.

longwave’s picture

Hiding patches.

smustgrave’s picture

Status: Needs review » Needs work

Seems to have a test failure.

longwave’s picture

Status: Needs work » Needs review

Just needed rebase.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Sweet. Change didn’t break anything

  • catch committed d39f152b on 10.3.x
    Issue #3258817 by longwave, donquixote: Register all of /tests/src/ for...

  • catch committed 7c6547de on 11.x
    Issue #3258817 by longwave, donquixote: Register all of /tests/src/ for...
catch’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.