Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
code core
../vendor/bin/phpunit tests/Drupal/Tests/TestSuites/TestSuiteBaseTest.php
PHP Fatal error: Class 'Drupal\Tests\TestSuites\TestSuiteBase' not found in core/tests/Drupal/Tests/TestSuites/TestSuiteBaseTest.php on line 101
TestSuiteBase is not part of the autoloader, so PHPUnit does no know where to find it. All Drupal core tests should be executable standalone, so we need to require the file manually.
Discovered while running Drupal core phpunit tests on Travis CI with a different test suite setup https://travis-ci.org/klausi/drupal/jobs/157279819
Comment | File | Size | Author |
---|---|---|---|
#11 | testsuite-2794715-2.patch | 569 bytes | Mile23 |
#5 | 2794715_5.patch | 6.2 KB | Mile23 |
#2 | testsuite-2794715-2.patch | 569 bytes | klausi |
Comments
Comment #2
klausiPatch.
Comment #3
GrimreaperHello,
Thanks for the patch.
I have just tested it against the 8.2.x and it solved the issue. I encountered this issue when trying to configure automated tests in PhpStorm. (#2782221-4: Result summary Area plugin not displayed when there is no result.)
Unfortunately for me, now I have another issue:
And I don't know what is the directory that is tried to be created. I follow https://www.drupal.org/node/2116263 and https://www.drupal.org/node/2288559
As I tested against 8.2.x, should I change the status into RTBC?
Comment #4
dawehnerI'm wondering whether we should add
core/tests/TestSuites
to the autoloader inside the bootstrap.phpComment #5
Mile23This is a bug fix of the testing system that helps prevent regressions of the testing system. So it should be in the 8.1.x branch. The patch applies in 8.2.x and 8.3.x as well.
The idea with leaving TestSuites in another namespace was that it was all about discovery and not about testing per se. But since we have to test the discovery process, we can just move it to where it makes sense to autoload.
This patch moves the test suite classes to
\Drupal\TestSuites\*
The test stays where it is. This solves this issue and also #2781203: Move TestSuite/* into a dedicated namespace.
Comment #6
dawehnerThis looks like a really nice solution to the problem ...
Comment #7
klausiLooks good, but this will break test execution for everybody that has copied a custom phpunit.xml. I guess we don't care too much about that since tests are not part of the API compatibility promise? In #2499239: Use test suite classes to discover different test types under phpunit, allow contrib harmony with run-tests we also changed phpunit.xml without thinking about that, so this is probably ok.
Otherwise RTBC+1, this is certainly the most cleanest solution to the problem.
Comment #8
catchWith the customized phpunit.yml what would happen if we copied the classes back to their old location? Usually we try not to break test runs in patch releases unless it's necessary to fix a (non-testing) bug.
Comment #9
Mile23I moved
TestSuiteBase
here because we're trying to test it and because we also have #2781203: Move TestSuite/* into a dedicated namespaceIt was probably a mistake to put them in weird places to begin with. I'd hate to put a bunch of
require_once
all over this system to allow the test to run.Also:
This won't break test runs for the testbot or for anyone doing core dev unless they've rigged their own phpunit.xml file as mentioned.
If all your CI script does is say
phpunit -c core/
then you're golden. And as @klausi mentioned, we did this previously to add the suite classes to begin with. I'm pretty sure other than this issue and the duplicate, we haven't heard of any problems.We could split this back out into two issues and get @klausi's test running using a require_once to load the class we're resting, and then address the locations of the files in #2781203: Move TestSuite/* into a dedicated namespace. In that case, the patch in #2 is the way to go.
Comment #10
catchOK I prefer patch #2 for this issue in 8.2.x, then a new issue for 8.3.x for the move.
Comment #11
Mile23Here's a re-upload of #2 just to keep it clear. Please give all credit to @klausi. :-)
And a manual test run:
Comment #12
catchThanks. RTBC-ing that patch, but please open the follow-up, I think that'd be OK for a minor release (i.e. 8.3.x).
Comment #13
Mile23The follow-up is #2781203: Move TestSuite/* into a dedicated namespace which I just re-opened. :-)
Comment #14
xjmI think @catch, @effulgentsia, @Cottser, @webchick, and I also agreed this can be rc eligible as a non-disruptive testing improvement. Thanks @Mile23.
Comment #16
catchCommitted/pushed to 8.3.x, thanks!
Leaving RTBC against 8.2.x.
Comment #17
catchEr rc eligible, so cherry picked.
Comment #20
dawehner