Problem/Motivation

Following up from #2927012: _drupal_log_error() returns a 0 exit code on errors, I've been investigating how to make \Drupal\simpletest\TestDiscovery not load PHP fixtures and other PHP files which may not be PHP classes.

The suggestion was made to simply exclude any directory named 'fixtures'. On closer reading of the PSR-4 spec, it seems that all it requires is that a given class name maps to a file path - not that a file path in a PSR-4 directory maps back to a valid class. Yet, this is exactly what we do in \Drupal\simpletest\TestDiscovery::scanDirectory.

I took a quick look at adding test classes to the autoloader in \Drupal\Core\Composer\Composer, which would let us remove TestDiscovery entirely. Unfortunately, TestDiscovery has a dependency on the module handler, so it can call hook_simpletest_alter(), and it's impossible to call that from the Composer script. Since that would be a compatibility break, it seems like this would be a 9.x task (which I'll file a followup for).

Proposed resolution

For 8.x, we simply ignore all directories named 'fixtures'. In 9.x, use a composer script to discover test classes.

Remaining tasks

Document this somewhere obvious so test writers understand that a 'fixtures' directory will never load classes, and what the best directories are for fixtures for a variety of use cases.

CommentFileSizeAuthor
#10 2942530.10-test-discovery-fixtures.patch4.02 KBdeviantintegral
#2 2942530.2-test-discovery-fixtures.patch4.02 KBdeviantintegral
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

deviantintegral created an issue. See original summary.

deviantintegral’s picture

deviantintegral CreditAttribution: deviantintegral at Lullabot commented
FileSize
2942530.2-test-discovery-fixtures.patch4.02 KB

This patch:

  1. Removes a todo to limit by file name - core has several hundred non-test classes in test namespaces, and that's a common pattern in contrib as well.
  2. Ignores anything in a lower-case 'fixtures' directory.
  3. Completes test coverage around scanDirectory().
deviantintegral’s picture

deviantintegral CreditAttribution: deviantintegral at Lullabot commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 2942530.2-test-discovery-fixtures.patch, failed testing. View results

deviantintegral’s picture

deviantintegral CreditAttribution: deviantintegral at Lullabot commented
Status: Needs work » Needs review 
1) Drupal\Tests\action\Kernel\Migrate\d7\MigrateActionsTest::testActions
Drupal\Core\Database\DatabaseNotFoundException: SQLSTATE[HY000] [1049] Unknown database 'jenkins_drupal_patches_47744'

Retesting as there's strange database errors and this and the same test passes fine locally.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Related issues: +#2863055: Move TestDiscovery out of simpletest module, minimize dependencies, +#2460521: Deprecate hook_simpletest_alter(), +#2296635: Evaluate performance impact of limiting test discovery to *Test.php filename suffix

We're trying to remove TestDiscovery's dependencies #2863055: Move TestDiscovery out of simpletest module, minimize dependencies and #2460521: Deprecate hook_simpletest_alter(). One solution to requiring the module handler is that there be two TestDiscovery classes: One that does the discovering, and one that's wraps it to be the service for simpletest module.

Generally +1 on this idea. Requiring *Test.php allows for loading fixture tests that maybe we shouldn't.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

Pretty sure this is solved by #2973127: Exclude *TestBase.php, *Trait.php and *Interface.php files from test discovery reflection

deviantintegral’s picture

deviantintegral CreditAttribution: deviantintegral at Lullabot commented
FileSize
2942530.10-test-discovery-fixtures.patch4.02 KB

The above issue doesn't cover non-class php files, and the test case in this issue fails on 8.7.x:

$ vendor/bin/phpunit -c core core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php                                                                                                        
PHPUnit 6.5.14 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\simpletest\Unit\TestDiscoveryTest
..............................F..                                 33 / 33 (100%)

Time: 1.44 seconds, Memory: 10.00MB

There was 1 failure:

1) Drupal\Tests\simpletest\Unit\TestDiscoveryTest::testScanDirectoryIgnoresFixtures
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     'Drupal\test_module\Tests\tests\src\Functional\FunctionalExampleTest' => 'vfs://drupal/modules/test_mod...st.php'
     'Drupal\test_module\Tests\tests\src\Functional\FunctionalExampleTest2' => 'vfs://drupal/modules/test_mod...t2.php'
     'Drupal\test_module\Tests\tests\src\Kernel\KernelExampleTest3' => 'vfs://drupal/modules/test_mod...t3.php'
+    'Drupal\test_module\Tests\tests\src\Functional\fixtures\empty' => 'vfs://drupal/modules/test_mod...ty.php'

/Users/andrew/vagrant/d8/www/docroot/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php:541

FAILURES!
Tests: 33, Assertions: 41, Failures: 1.

Here's a reroll against 8.7.x.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Title: Fix TestDiscovery loading fixtures » Fix TestDiscovery scanning fixtures
Version: 8.8.x-dev » 8.7.x-dev
Status: Needs review » Needs work

On closer reading of the PSR-4 spec, it seems that all it requires is that a given class name maps to a file path - not that a file path in a PSR-4 directory maps back to a valid class. Yet, this is exactly what we do in \Drupal\simpletest\TestDiscovery::scanDirectory.

TestDiscovery comes from The Time Before PSR-4... It builds a hypothetical class map and then discovers files that would be in it. It's a thing we should change, but can't (apparently).

In 9.x, use a composer script to discover test classes.

Actually, use phpunit's bootstrap.php, which we already do. :-) bootstrap.php calls TestDiscovery::scanDirectory() in order to provide the filtering we're working on here. We don't want autoloading of test files. We want them to be discovered as tests with as little class loading as possible, auto or otherwise.

Also, the dependency on the module handler should go away with #2863055: Move TestDiscovery out of simpletest module, minimize dependencies which would necessitate a re-roll here.

+++ b/core/modules/simpletest/src/TestDiscovery.php
@@ -287,6 +284,10 @@ public static function scanDirectory($namespace_prefix, $path) {
+      // Fixtures may contain PHP scripts that should not be autoloaded.
+      if ($current->isDir() && $current->getBasename() == 'fixtures') {

This is in scanDirectory(), so the file might not be subsequently loaded. Should read 'Fixtures may contain PHP scripts that should not be discovered.'

Moving to 8.7.x since this is a testing improvement.

Also citing #2927012-21: _drupal_log_error() returns a 0 exit code on errors because that's where @alexpott says it's a good idea. :-)

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.9 was released on November 6 and is the final full bugfix release for the Drupal 8.7.x series. Drupal 8.7.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.8.0 on December 4, 2019. (Drupal 8.8.0-beta1 is available for testing.)

Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Component: simpletest.module » phpunit

Triaging issues in simpletest.module as part of the Bug Smash Initiative to determine if they should be in the Simpletest Project or core.

This looks like it a Phpunit issue, changing component.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

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

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should 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.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should 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.

Version: 9.5.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. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.