From 4710b0971fd6a56c30f0cdb460685af8cfbfce38 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Thu, 6 Aug 2015 15:37:34 +0100 Subject: [PATCH] Issue #2542486 by neclimdul, Wim Leers, Mile23: Fix risky phpunit tests --- .../simpletest/tests/src/Unit/WebTestBaseTest.php | 1 + .../Tests/Component/DrupalComponentTest.php | 15 ++++++--------- .../PhpStorage/FileStorageReadOnlyTest.php | 1 + .../Component/PhpStorage/FileStorageTest.php | 1 + .../PhpStorage/MTimeProtectedFileStorageBase.php | 1 + .../Component/PhpStorage/PhpStorageTestBase.php | 1 + .../Drupal/Tests/ComposerIntegrationTest.php | 5 ++--- .../Tests/Core/Render/RendererBubblingTest.php | 8 +++----- .../Core/Render/RendererPlaceholdersTest.php | 4 +++- .../Drupal/Tests/Core/Render/RendererTestBase.php | 11 +++++++++-- 10 files changed, 28 insertions(+), 20 deletions(-) diff --git a/core/modules/simpletest/tests/src/Unit/WebTestBaseTest.php b/core/modules/simpletest/tests/src/Unit/WebTestBaseTest.php index f86b85e3c08..d81b06cee9e 100644 --- a/core/modules/simpletest/tests/src/Unit/WebTestBaseTest.php +++ b/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']); } /** diff --git a/core/tests/Drupal/Tests/Component/DrupalComponentTest.php b/core/tests/Drupal/Tests/Component/DrupalComponentTest.php index 7cd66f2fa43..f574106e8c6 100644 --- a/core/tests/Drupal/Tests/Component/DrupalComponentTest.php +++ b/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)) { - foreach ($matches[0] as $line) { - if ((strpos($line, '@see ') === FALSE)) { - $this->fail( - "Illegal reference to 'Drupal\\Core' namespace in $class_path" - ); - } - } - } + preg_match_all('/^.*Drupal\\\Core.*$/m', $contents, $matches); + $matches = array_filter($matches[0], function($line) { + // Filter references to @see as they don't really matter. + return strpos($line, '@see') === FALSE; + }); + $this->assertEmpty($matches, "Checking for illegal reference to 'Drupal\\Core' namespace in $class_path"); } /** diff --git a/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageReadOnlyTest.php b/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageReadOnlyTest.php index 8cf9642276c..e5e10821f65 100644 --- a/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageReadOnlyTest.php +++ b/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]); } /** diff --git a/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php b/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php index e0a7d817f34..456e6f0aca1 100644 --- a/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php +++ b/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]); } } diff --git a/core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageBase.php b/core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageBase.php index 5667e42b021..59944ff6891 100644 --- a/core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageBase.php +++ b/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']); } } diff --git a/core/tests/Drupal/Tests/Component/PhpStorage/PhpStorageTestBase.php b/core/tests/Drupal/Tests/Component/PhpStorage/PhpStorageTestBase.php index 53492d87f4b..9abc86bb493 100644 --- a/core/tests/Drupal/Tests/Component/PhpStorage/PhpStorageTestBase.php +++ b/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]); } } diff --git a/core/tests/Drupal/Tests/ComposerIntegrationTest.php b/core/tests/Drupal/Tests/ComposerIntegrationTest.php index c629fabc785..e24195b0d35 100644 --- a/core/tests/Drupal/Tests/ComposerIntegrationTest.php +++ b/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->fail($this->getErrorMessages()[json_last_error()]); - } + $this->assertNotNull($result, $this->getErrorMessages()[json_last_error()]); } } diff --git a/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php b/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php index d659849f471..b9bb4e5f6ad 100644 --- a/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php +++ b/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 () { - global $current_user_role; + '#access_callback' => function() use (&$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 () { - global $current_user_role; + '#access_callback' => function () use (&$current_user_role) { // Only role C can access this subtree. return $current_user_role === 'C'; }, diff --git a/core/tests/Drupal/Tests/Core/Render/RendererPlaceholdersTest.php b/core/tests/Drupal/Tests/Core/Render/RendererPlaceholdersTest.php index 5d43afcd386..46530005168 100644 --- a/core/tests/Drupal/Tests/Core/Render/RendererPlaceholdersTest.php +++ b/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('

This is a rendered placeholder!

', (string) $result); } /** diff --git a/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php b/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php index c536c20c311..4f9a333f8b7 100644 --- a/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php +++ b/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) { - global $current_user_role; + ->willReturnCallback(function($context_tokens) use (&$current_user_role) { $keys = []; foreach ($context_tokens as $context_id) { switch ($context_id) {