Issue #2542486 by neclimdul, Wim Leers, Mile23: Fix risky phpunit tests

core/modules/simpletest/tests/src/Unit/WebTestBaseTest.php

@@ -218,6 +218,7 @@ class WebTestBaseTest extends UnitTestCase {
     $get_absolute_url_method->setAccessible(TRUE);
 
     $this->assertSame($expected_absolute_path, $get_absolute_url_method->invoke($web_test, $href));
+    unset($GLOBALS['base_url'], $GLOBALS['base_path']);
   }
 
   /**

core/tests/Drupal/Tests/Component/DrupalComponentTest.php

@@ -68,15 +68,12 @@ class DrupalComponentTest extends UnitTestCase {
    */
   protected function assertNoCoreUsage($class_path) {
     $contents = file_get_contents($class_path);
-    if (preg_match_all('/^.*Drupal\\\Core.*$/m', $contents, $matches)) {
+    preg_match_all('/^.*Drupal\\\Core.*$/m', $contents, $matches);
-      foreach ($matches[0] as $line) {
+    $matches = array_filter($matches[0], function($line) {
-        if ((strpos($line, '@see ') === FALSE)) {
+      // Filter references to @see as they don't really matter.
-          $this->fail(
+      return strpos($line, '@see') === FALSE;
-            "Illegal reference to 'Drupal\\Core' namespace in $class_path"
+    });
-          );
+    $this->assertEmpty($matches, "Checking for illegal reference to 'Drupal\\Core' namespace in $class_path");
-        }
-      }
-    }
   }
 
   /**

core/tests/Drupal/Tests/Component/PhpStorage/FileStorageReadOnlyTest.php

@@ -75,6 +75,7 @@ class FileStorageReadOnlyTest extends PhpStorageTestBase {
     // Saving and deleting should always fail.
     $this->assertFalse($php_read->save($name, $code));
     $this->assertFalse($php_read->delete($name));
+    unset($GLOBALS[$random]);
   }
 
   /**

core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php

@@ -86,6 +86,7 @@ class FileStorageTest extends PhpStorageTestBase {
 
     // Should still return TRUE if directory has already been deleted.
     $this->assertTrue($php->deleteAll(), 'Delete all succeeds with nothing to delete');
+    unset($GLOBALS[$random]);
   }
 
 }

core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageBase.php

@@ -124,6 +124,7 @@ abstract class MTimeProtectedFileStorageBase extends PhpStorageTestBase {
       $this->assertSame($php->load($name), $this->expected[$i]);
       $this->assertSame($GLOBALS['hacked'], $this->expected[$i]);
     }
+    unset($GLOBALS['hacked']);
   }
 
 }

core/tests/Drupal/Tests/Component/PhpStorage/PhpStorageTestBase.php

@@ -60,6 +60,7 @@ abstract class PhpStorageTestBase extends UnitTestCase {
     // Ensure delete() can be called on a non-existing file. It should return
     // FALSE, but not trigger errors.
     $this->assertFalse($php->delete($name), 'Delete fails on missing file');
+    unset($GLOBALS[$random]);
   }
 
 }

core/tests/Drupal/Tests/ComposerIntegrationTest.php

@@ -22,6 +22,7 @@ class ComposerIntegrationTest extends UnitTestCase {
    */
   protected function getErrorMessages() {
     $messages = [
+      0 => 'No errors found',
       JSON_ERROR_DEPTH => 'The maximum stack depth has been exceeded',
       JSON_ERROR_STATE_MISMATCH => 'Invalid or malformed JSON',
       JSON_ERROR_CTRL_CHAR => 'Control character error, possibly incorrectly encoded',
@@ -63,9 +64,7 @@ class ComposerIntegrationTest extends UnitTestCase {
       $json = file_get_contents($path . '/composer.json');
 
       $result = json_decode($json);
-      if (is_null($result)) {
+      $this->assertNotNull($result, $this->getErrorMessages()[json_last_error()]);
-        $this->fail($this->getErrorMessages()[json_last_error()]);
-      }
     }
   }
 

core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php

@@ -302,7 +302,7 @@ class RendererBubblingTest extends RendererTestBase {
    * Tests the self-healing of the redirect with conditional cache contexts.
    */
   public function testConditionalCacheContextBubblingSelfHealing() {
-    global $current_user_role;
+    $current_user_role = &$this->currentUserRole;
 
     $this->setUpRequest();
     $this->setupMemoryCache();
@@ -319,8 +319,7 @@ class RendererBubblingTest extends RendererTestBase {
           'tags' => ['b'],
         ],
         'grandchild' => [
-          '#access_callback' => function () {
+          '#access_callback' => function() use (&$current_user_role) {
-            global $current_user_role;
             // Only role A cannot access this subtree.
             return $current_user_role !== 'A';
           },
@@ -331,8 +330,7 @@ class RendererBubblingTest extends RendererTestBase {
             'max-age' => 1800,
           ],
           'grandgrandchild' => [
-            '#access_callback' => function () {
+            '#access_callback' => function () use (&$current_user_role) {
-                global $current_user_role;
                 // Only role C can access this subtree.
                 return $current_user_role === 'C';
               },

core/tests/Drupal/Tests/Core/Render/RendererPlaceholdersTest.php

@@ -599,7 +599,9 @@ class RendererPlaceholdersTest extends RendererTestBase {
       'null' => NULL,
     ]];
 
-    $this->renderer->renderRoot($element);
+    $result = $this->renderer->renderRoot($element);
+    $this->assertInstanceOf('\Drupal\Core\Render\SafeString', $result);
+    $this->assertEquals('<p>This is a rendered placeholder!</p>', (string) $result);
   }
 
   /**

core/tests/Drupal/Tests/Core/Render/RendererTestBase.php

@@ -79,6 +79,13 @@ class RendererTestBase extends UnitTestCase {
    */
   protected $memoryCache;
 
+  /**
+   * The simulated "current" user role, for use in tests with cache contexts.
+   *
+   * @var string
+   */
+  protected $currentUserRole;
+
   /**
    * The mocked renderer configuration.
    *
@@ -113,10 +120,10 @@ class RendererTestBase extends UnitTestCase {
     $this->cacheContextsManager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
       ->disableOriginalConstructor()
       ->getMock();
+    $current_user_role = &$this->currentUserRole;
     $this->cacheContextsManager->expects($this->any())
       ->method('convertTokensToKeys')
-      ->willReturnCallback(function($context_tokens) {
+      ->willReturnCallback(function($context_tokens) use (&$current_user_role) {
-        global $current_user_role;
         $keys = [];
         foreach ($context_tokens as $context_id) {
           switch ($context_id) {