Problem/Motivation

Make a contrib module in ./module, give it PHPUnit tests, type ./vendor/bin/phpunit, and you're happy.

Put that same contrib module in a subdirectory, such as ./module/folder/my_module/, and its tests vanish from PHPUnit's consciousness.

This is purely an issue with the configuration of PHPUnit.

Note also that there is a problem with bootstrap.php not autoloading any contrib namespaces, but that issue lives here: #2025883: Drupal's PHPUnit bootstrap.php does not register module namespaces out of /core

Proposed resolution

Modify ./core/phpunit.xml.dist to allow PHPUnit to find the tests.

The main problem with doing this is that SimpleTest also uses ./core/phpunit.xml.dist for configuration, and a proper configuration of PHPUnit means that SimpleTest dies a horrible death.

Remaining tasks

Change SimpleTest so that it doesn't hijack ./core/phpunit.xml.dist, or at least can deal with changes to it. That's outside the scope of this issue.

Add a phpunit and/or testing issue category.

User interface changes

n/a

API changes

n/a

#2025883: Drupal's PHPUnit bootstrap.php does not register module namespaces out of /core

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23’s picture

FileSize
760 bytes

And a patch which will fail.

Mile23’s picture

Status: Active » Needs review
Mile23’s picture

Hmm. Maybe not. :-)

becw’s picture

I encountered this issue too; #2025883: Drupal's PHPUnit bootstrap.php does not register module namespaces out of /core is related but I think the bugs are separate, since patch #14 from that issue doesn't seem to affect this bug.

If tests aren't found in paths like modules/contrib/foo and modules/custom/bar, then they won't be found in submodules, either.

The patch from #1 in this issue doesn't work for me; I get errors like:

Fatal error: Class 'Drush_CommandTestCase' not found in /Applications/MAMP/htdocs/drupal8/modules/devel/develDrushTest.php on line 9

Call Stack:
0.0003 637448 1. {main}() /Applications/MAMP/htdocs/drupal8/core/vendor/phpunit/phpunit/composer/bin/phpunit:0
0.0061 1142384 2. PHPUnit_TextUI_Command::main() /Applications/MAMP/htdocs/drupal8/core/vendor/phpunit/phpunit/composer/bin/phpunit:63
0.0061 1143112 3. PHPUnit_TextUI_Command->run() /Applications/MAMP/htdocs/drupal8/core/vendor/phpunit/phpunit/PHPUnit/TextUI/Command.php:129
0.0061 1143112 4. PHPUnit_TextUI_Command->handleArguments() /Applications/MAMP/htdocs/drupal8/core/vendor/phpunit/phpunit/PHPUnit/TextUI/Command.php:138
0.0148 2019792 5. PHPUnit_Util_Configuration->getTestSuiteConfiguration() /Applications/MAMP/htdocs/drupal8/core/vendor/phpunit/phpunit/PHPUnit/TextUI/Command.php:657
0.0148 2020824 6. PHPUnit_Util_Configuration->getTestSuite() /Applications/MAMP/htdocs/drupal8/core/vendor/phpunit/phpunit/PHPUnit/Util/Configuration.php:789
...

I'm attaching a patch that works for me, but it's very limited; each asterisk in the testsuite directory paths in phpunit.xml.dist only expands to a single directory. This file is interpreted by PHPUnit, so we may not be able to build in as much flexibility at this level.

Status: Needs review » Needs work

The last submitted patch, 2056607-4-drupal-phpunit_xml.patch, failed testing.

Mile23’s picture

I'm attaching a patch that works for me, but it's very limited; each asterisk in the testsuite directory paths in phpunit.xml.dist only expands to a single directory.

That's not actually the case. Each directory path tells PHPUnit to start searching in that directory. If you add the trailing /*, then it will only expand out to the first level of subdirectories. That's why I removed those in my patch.

This file is interpreted by PHPUnit, so we may not be able to build in as much flexibility at this level.

Well, actually, PHPUnit is very good at finding tests where you tell it to find them. The problem is that our phpunit.xml.dist is misconfigured, and only tells PHPUnit to look at the first level of subdirectories, when it should say to keep going to all the subdirectories.

It looks like you're using Drush to run these tests. How are you running the tests to get this error?

I'm trying to get them to work under plain-vanilla PHPUnit, which you can call this way:

cd core
./vendor/bin/phpunit

I'm not sure what special cases Drush might need to run PHPUnit tests.

agentrickard’s picture

The approach in #4 works for me using ./vendor/bin/phpunit. The patch in #1 fails as expected when running that command.

Not sure why the one patch is green and the other red, though.

becw’s picture

I'm not using drush to run the PHPUnit tests--that's just where the error happened to fall in the /modules directory. I run tests in exactly the way you describe (./vendor/bin/phpunit).

Mile23’s picture

I think that once #2025883: Drupal's PHPUnit bootstrap.php does not register module namespaces out of /core happens, #1 will be happy from the command line, just running phpunit. The interaction with SimpleTest and Drush seems like a different problem in those areas.

msonnabaum’s picture

The drush issue is from devel, because it includes a phpunit test for drush in it's root. When you remove the "/*/tests" from the phpunit.xml, it picks up any phpunit test in a module.

I originally added that so that we wouldnt be running vendor tests as well, since it's conceivable that you might use composer and you would want to exclude your dependencies' tests.

I was hoping we could filter by test class, but phpunit doesn't appear to have such an option. I think the best we can do is #1 and then ask that drush tests go in modulename/drush/tests or something, that we can exclude by path.

Mile23’s picture

Yah, the existence of the test for drush within devel means I can't run any tests for devel without drush installed alongside. Drush should really have its own testing subdirectory and config for PHPUnit, since it's not really Drupal. (This is where I idly mention Symfony's console...)

So if we assume it's OK to break the drush test here, to be solved within drush and/or devel then this is all we need.

Also killed the config and views excludes because PHPUnit does a fine job of not running tests that aren't PHPUnit tests. :-)

Mile23’s picture

Status: Needs work » Needs review
agentrickard’s picture

Status: Needs review » Reviewed & tested by the community

Drush / devel can fix itself after we commit this.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2056607-phpunit-config-11.patch, failed testing.

msonnabaum’s picture

Moshe agreed to move drush tests to ./drush/tests, so we should probably test with that path being excluded and then let it be fixed in devel.

Mile23’s picture

Status: Needs work » Needs review
FileSize
974 bytes

#11, excluding ./drush/tests

Status: Needs review » Needs work

The last submitted patch, 2056607-phpunit-config-16.patch, failed testing.

Mile23’s picture

Mile23’s picture

Is there an issue for the ./drush/tests move?

Mile23’s picture

Excluding /vendor for quicker tests.

Excluding /vendor from coverage reports.

Mile23’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: -PHPUnit, -qa

The last submitted patch, 2056607-phpunit-config-20.patch, failed testing.

Mile23’s picture

Status: Needs work » Needs review
Issue tags: +PHPUnit, +qa

#20: 2056607-phpunit-config-20.patch queued for re-testing.

Mile23’s picture

Would be great to have an RTBC so we can test modules that live inside other folders. :-)

dlu’s picture

Status: Needs review » Reviewed & tested by the community

Quick before we need a reroll :-)

Gaelan’s picture

Looks great to me.

webchick’s picture

Component: simpletest.module » phpunit
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

One question though:

+++ b/core/phpunit.xml.dist
@@ -3,14 +3,22 @@
+      <directory>./tests</directory>
+      <directory>./modules/*/tests</directory>
+      <directory>../modules</directory>
+      <directory>../sites/*/modules</directory>

Do you also need /sites/*/modules/*/tests to be parallel with /modules/*/tests?

Mile23’s picture

../sites/*/modules/*/tests would bring back the original problem, just under /sites. PHPUnit is pretty good at finding tests very quickly, so a wider net is better in this case.

The ./modules/*/tests line assumes that core modules will never be in subfolders, which could be wrong.

And yay phpunit component. :-)

webchick’s picture

Got it, thanks. :) And yeah, figured that was easier as a means of finding these. Thanks for all the great work on making it better!

amateescu’s picture

moshe weitzman’s picture

I put devel's tests right in a ./drush subdir so the exclude should be ./drush. I assume that would also exclude drush/tests in case anyone wants to put tests there.

Xano’s picture

I'm having trouble adding a PHPUnit test to a contributed module and as most people who helped me were not aware of this issue, I'd like to explain my case here to see if the patch was actually faulty, or if something else is wrong.

I added a PHPUnit test under the name of \Drupal\payment_form\Tests\Plugin\payment\type\PaymentFormUnitTest at ./modules/payment/payment_form/lib/Drupal/payment_form/Tests/Plugin/payment/type/PaymentFormUnitTest.php. PHPUnit can't find the file, even though it should be covered by the <directory>../modules</directory> directive in phpunit.xml.dist. Only when I add <directory>../modules/payment/payment_form/lib/Drupal/payment_form/Tests/Plugin/payment/type</directory> to it, it finds the file. It then successfully includes it, but PHPUnit_Framework_TestSuite::addTestFile() cannot find any newly loaded classes. However, when simply changing the test class from a UnitTestCase to UnitTestBase, Simpletest correctly finds and runs it.

Part of the reason why I spent so much time on this problem already, is that the test structure is highly inconsistent and I haven't been able to find centralized documentation on this:

  • Core Simpletest tests go in ./core/modules/*/lib/Drupal/*/Tests
  • Core PHPUnit tests go in ./core/modules/*/tests/Drupal
  • Contrib PHPUnit tests go in ./modules/*?
Xano’s picture

Never mind my previous post. It turns out PHPUnit does not follow symlinks, which is what I used to insert the contributed modules into my D8 testing setup.

dlu’s picture

Category: bug » feature
Priority: Major » Normal
Status: Fixed » Needs work

Seems like it would be good to document this behavior. Or would it make sense to "fix" PHPunit? The behavior seems like a bug to me…

Mile23’s picture

There's an issue about it for phpunit-file-iterator: https://github.com/sebastianbergmann/php-file-iterator/pull/21

Ideally, it will just require a composer update. It just looks as though the 1.3.4 release is a phantom.

Xano’s picture

Category: feature » bug
Priority: Normal » Major
Status: Needs work » Fixed

Yes, that's my pull request. I'm waiting for 1.3.4 to be available and then I'll open an issue to update it in core.

Let's set the status back to what this was so my problem doesn't hijack the issue.

Xano’s picture

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

Anonymous’s picture

Issue summary: View changes

Added a next step.

Mile23’s picture

This issue has a follow-up which is EXACTLY THE SAME PROBLEM because no one cares about contrib. #2499239: Use test suite classes to discover different test types under phpunit, allow contrib harmony with run-tests

webchick’s picture

s/no one cares about contrib/we don't have an integration test to ensure this behaviour stays working/

FTFY.