Ensure that PHPUnit tests are in the correct namespace

RTBC+1 for me as is.
However, two (probably stupid) questions :
- Does'nt this means that unit tests from contrib modules will always throw exceptions because they won't be in Drupal\Tests namespace ?
- I'm not sure if we can but can't this be UnitTested ? This is a bit test-ception but could be nice to validate this expected behavior.

Hi,
Unless I got it wrong, tests in Rules have for namespace Drupal\rules\Tests which seems normal. What did I miss ?

Indeed sorry

+++ b/core/tests/Drupal/Tests/UnitTestCase.php
@@ -42,6 +42,11 @@ protected function setUp() {
+
+    // Ensure that tests are in the Drupal\Tests namespace.
+    if (__NAMESPACE__ != 'Drupal\Tests' && strpos(__NAMESPACE__, 'Drupal\Tests\\', 0) !== 0) {
+      throw new \Exception('Unit tests must be in the "Drupal\Tests" namespace.');
+    }

Sure, this works, but I'm curious whether \Drupal\Tests\Standards\DrupalStandardsListener might be a better place? Maybe ask mile23 about it?

So I guess the idea here is that you can have a listener that does a couple of things:

1) As you rush to write more and more unit tests in a haze of coding frenzy, if you forget to namespace your test properly, this reminds you. The test is discovered and run, but this burps and lets you know you're in the wrong.

2) We have a nice fence against changes to the test discovery system finding non-Drupal tests.

Here's a review of what's here. It needs a re-roll so I'm not working too hard at this. :-)

1. +++ b/core/tests/Drupal/DrupalStandardsListenerTest/DrupalStandardsListenerTest.php
   @@ -0,0 +1,35 @@
   +  public function testEndTest() {
   

   We really need a test of checkValidNamespaceForTest() with some sample data so we know it complains about bad namespaces.

2. +++ b/core/tests/bootstrap.php
   @@ -70,6 +70,7 @@ function drupal_phpunit_register_extension_dirs(Composer\Autoload\ClassLoader $l
   +$loader->add('Drupal\\DrupalStandardsListenerTest', __DIR__);
   

   That's not where the standards listener is, and we don't really need to autoload it this way.

Weird. The tag renamed itself to 'undefined'.

This is the re-roll for this patch. The next file has changed:

core/tests/bootstrap.php

Sorry, I forgot the "Needs review" tag.

Rerolled patch https://www.drupal.org/node/2465029#comment-9793051

+++ b/core/tests/bootstrap.php
@@ -97,6 +97,10 @@ function drupal_phpunit_get_extension_namespaces($dirs) {
+// Start with classes in known locations.
+$loader = require __DIR__ . '/../../autoload.php';
+$loader->add('Drupal\\Tests', __DIR__);
+$loader->add('Drupal\\DrupalStandardsListenerTest', __DIR__);

this missed in last reroll

Assigning myself to include the small changes mentioned in #31 by andypost.

Added below code in bootstrap.php and rerolled the patch in https://www.drupal.org/node/2465029#comment-9793051

+++ b/core/tests/bootstrap.php
@@ -97,6 +97,10 @@ function drupal_phpunit_get_extension_namespaces($dirs) {
+// Start with classes in known locations.
+$loader = require __DIR__ . '/../../autoload.php';
+$loader->add('Drupal\\Tests', __DIR__);
+$loader->add('Drupal\\DrupalStandardsListenerTest', __DIR__);

This is a testing improvement, so it can go into 8.2.x, though it might end up being disruptive if we find tests in the wrong namespaces.

The patch applies to either branch.

1. --- /dev/null
   +++ b/core/tests/Drupal/DrupalStandardsListenerTest/DrupalStandardsListenerTest.php
   

   Put this test in core/tests/Drupal/Listeners, right next to the listener.

2. +++ b/core/tests/Drupal/DrupalStandardsListenerTest/DrupalStandardsListenerTest.php
   @@ -0,0 +1,35 @@
   +/**
   + * @file
   + * Contains \Drupal\UnitTestCaseTest\UnitTestCaseTest.
   + */
   

   Don't use @file.

3. +++ b/core/tests/Drupal/DrupalStandardsListenerTest/DrupalStandardsListenerTest.php
   @@ -0,0 +1,35 @@
   +   * @expectedException PHPUnit_Framework_ExpectationFailedException
   

   Convert this to $this->setExpectedException(PHPUnit_Framework_ExpectationFailedException::class);

4. +++ b/core/tests/Drupal/DrupalStandardsListenerTest/DrupalStandardsListenerTest.php
   @@ -0,0 +1,35 @@
   +   * @expectedExceptionMessage Test is not in the Drupal\Tests namespace.
   

   Convert this to use $this->setExpectedExceptionMessage().

5. +++ b/core/tests/bootstrap.php
   @@ -77,6 +77,11 @@ function drupal_phpunit_contrib_extension_directory_roots($root = NULL) {
    function drupal_phpunit_get_extension_namespaces($dirs) {
   +  // Start with classes in known locations.
   +  $loader = require __DIR__ . '/../../autoload.php';
   +  $loader->add('Drupal\\Tests', __DIR__);
   +  $loader->add('Drupal\\DrupalStandardsListenerTest', __DIR__);
   +
   

   drupal_phpunit_get_extension_namespaces() shouldn't do any of this.

6. +++ b/core/tests/bootstrap.php
   @@ -108,6 +113,10 @@ function drupal_phpunit_get_extension_namespaces($dirs) {
    function drupal_phpunit_populate_class_loader() {
   +// Start with classes in known locations.
   +$loader = require __DIR__ . '/../../autoload.php';
   +$loader->add('Drupal\\Tests', __DIR__);
   +$loader->add('Drupal\\DrupalStandardsListenerTest', __DIR__);
    
      /** @var \Composer\Autoload\ClassLoader $loader */
      $loader = require __DIR__ . '/../../autoload.php';
   

   drupal_phpunit_populate_class_loader() shouldn't do any of this, either. The listener is already registered. Basically, don't change bootstrap.php.

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.