Follow-up to #2083547: PSR-4: Putting it all together

Currently, a unit test 'Drupal\foo\Tests\FooUnitTest' provided by 'foo' module would live in a file like this:
core/modules/foo/tests/Drupal/foo/Tests/FooUnitTest

With PSR-4, this could be shortened to either
core/modules/foo/tests/Tests/FooUnitTest or
core/modules/foo/tests/FooUnitTest
(a question is whether we would ever want a class in this directory that is not in the "Tests" sub-namespace)

This will make the directory structure simpler, but it does require a decision and a few technical changes. So we do it in a follow-up.

Comments

donquixote’s picture

donquixote commented
Issue tags: +PHPUnit, +autoloader, +PSR-4

Tagging.

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

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

(a question is whether we would ever want a class in this directory that is not in the "Tests" sub-namespace)

We have some precedent for something like this:

/modules
  /someModule
    /lib
    /tests
      /Tests
      /modules

But really if you need a module to do a unit test, U R DOING IT RONG.

Does the script end up losing any stuff if you kill the /Tests subdirectory?

donquixote’s picture

donquixote commented

But really if you need a module to do a unit test, U R DOING IT RONG.

Same as in the other issue, I am not trying to change the world here, just tune it to a different frequency.

We have some precedent for something like this:

I think rather like this (now with PSR-0)

/modules
  /someModule
    /lib
    /tests
      /Drupal
        /someModule
          /Tests
            /FooTest.php
      /modules
        /someTestModule
          /lib

The "modules" directory contains further modules, and these have their own "lib" directory.

The thing I would be worried about is sth like "MockFoo" in

/modules
  /someModule
    /lib
    /tests
      /Drupal
        /someModule
          /MockFoo.php
          /Tests
            /FooTest.php
      /modules
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

My example was how I thought a PSR-4 directory might end up looking. I have to admit I'm not entirely clear on how these directories would get folded in for PSR-4, since I can't figure out why MockFoo is a problem.

Same as in the other issue, I am not trying to change the world here, just tune it to a different frequency.

Well, that's the thing... Some approaches are wrong and some aren't. :-)

But what this has convinced me is that I need to make an issue for requiring vfsStream in composer.json. https://github.com/mikey179/vfsStream/wiki/Example

That's because having a test module with its own SimpleTest tests inside a PHPUnit test (FooTest.php above) should really only be about testing discovery of SimpleTests. For that we should mock the file system.

donquixote’s picture

donquixote commented

My example was how I thought a PSR-4 directory might end up looking. I have to admit I'm not entirely clear on how these directories would get folded in for PSR-4, since I can't figure out why MockFoo is a problem.

So, the thing is:

Old situation (PSR-0):
Drupal\system\FooNamespace\FooAnything -> core/modules/system/lib/Drupal/system/FooNamespace/FooAnything.php
Drupal\system\Tests\FooSimpletest -> core/modules/system/lib/Drupal/system/Tests/FooWebTest.php
Drupal\system\Tests\FooUnitTest -> core/modules/system/tests/Drupal/system/Tests/FooUnitTest.php
Drupal\system\MockFoo -> core/modules/system/tests/Drupal/system/MockFoo.php

New situation (PSR-4, option 1):
Drupal\system\FooNamespace\FooAnything -> core/modules/system/lib/FooNamespace/FooAnything.php
Drupal\system\Tests\FooSimpletest -> core/modules/system/lib/Tests/FooWebTest.php
Drupal\system\Tests\FooUnitTest -> core/modules/system/tests/lib/FooTest.php
Drupal\system\MockFoo -> ???

New situation (PSR-4, option 2):
Drupal\system\FooNamespace\FooAnything -> core/modules/system/lib/FooNamespace/FooAnything.php
Drupal\system\Tests\FooSimpletest -> core/modules/system/lib/Tests/FooWebTest.php
Drupal\system\Tests\FooUnitTest -> core/modules/system/tests/lib/Tests/FooTest.php
Drupal\system\MockFoo -> core/modules/system/tests/lib/MockFoo.php

If there is no such a thing as MockFoo, then we don't have to worry about the "???".

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

MockFoo would live in a standard module namespace. It would be discovered as a module by the autoloader and then it would be \Drupal\system\MockFoo.

As I mention, though, this would be more applicable to FooWebTest, since SimpleTest lets you do a functional test on a module, but PHPUnit shouldn't. Therefore, as you say, there's no such thing as MockFoo.

If (that is: when) someone creates an issue saying they can't load their test module into their PHPUnit test, please refer them to me. :-)

Also, this is kind of important:

Drupal\system\Tests\FooUnitTest -> core/modules/system/tests/lib/Tests/FooTest.php

I would much rather have core/modules/system/tests/Tests/FooUnitTest.php. (That is: No /lib under /tests.) /tests should be equivalent to /lib for the purposes of namespace discovery, just for a different use-case. Parallel universes. Then we don't have to spend our time chasing down special cases in autoloading, and it makes a lot more sense in terms of DX for anyone writing tests.

Also, this is the pattern for core/tests. core/tests/bootstrap.php is outside of any namespace, and probably should remain so. Obviously up for debate over on #2025883: Drupal's PHPUnit bootstrap.php does not register module namespaces out of /core

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

And: #2095037: Add vfsStream as a dependency in composer.json.

donquixote’s picture

donquixote commented
Issue summary: View changes
Status: Active » Closed (duplicate)

We finally did this as part of the main issue. So this one is duplicate now.