From b1a87360228e3bb9cfc29972833a8238d4a149b5 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Wed, 6 Dec 2023 11:09:40 +0000
Subject: [PATCH] Issue #3352851 by catch, Fabianx, mondrake, xjm, alexpott:
Allow assertions on the number of database queries run during tests
---
core/lib/Drupal/Core/Database/Connection.php | 4 +-
.../performance_test.info.yml | 5 +
.../performance_test.services.yml | 9 +
.../src/DatabaseEventEnabler.php | 32 +++
.../src/PerformanceDataCollector.php | 51 +++++
.../NoJavaScriptAnonymousTest.php | 85 --------
.../StandardPerformanceTest.php | 202 ++++++++++++++++++
.../PerformanceTestBase.php | 5 +
core/tests/Drupal/Tests/PerformanceData.php | 100 +++++++++
.../Drupal/Tests/PerformanceTestTrait.php | 53 ++++-
10 files changed, 459 insertions(+), 87 deletions(-)
create mode 100644 core/modules/system/tests/modules/performance_test/performance_test.info.yml
create mode 100644 core/modules/system/tests/modules/performance_test/performance_test.services.yml
create mode 100644 core/modules/system/tests/modules/performance_test/src/DatabaseEventEnabler.php
create mode 100644 core/modules/system/tests/modules/performance_test/src/PerformanceDataCollector.php
delete mode 100644 core/profiles/standard/tests/src/FunctionalJavascript/NoJavaScriptAnonymousTest.php
create mode 100644 core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
diff --git a/core/lib/Drupal/Core/Database/Connection.php b/core/lib/Drupal/Core/Database/Connection.php
index 97b528c25bed..cdaf0d19ddd5 100644
--- a/core/lib/Drupal/Core/Database/Connection.php
+++ b/core/lib/Drupal/Core/Database/Connection.php
@@ -2205,7 +2205,9 @@ public static function removeDatabaseEntriesFromDebugBacktrace(array $backtrace,
* The debug backtrace.
*/
protected function getDebugBacktrace(): array {
- return debug_backtrace();
+ // @todo: allow a backtrace including all arguments as an option.
+ // See https://www.drupal.org/project/drupal/issues/3401906
+ return debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);
}
}
diff --git a/core/modules/system/tests/modules/performance_test/performance_test.info.yml b/core/modules/system/tests/modules/performance_test/performance_test.info.yml
new file mode 100644
index 000000000000..8d909c8a09ef
--- /dev/null
+++ b/core/modules/system/tests/modules/performance_test/performance_test.info.yml
@@ -0,0 +1,5 @@
+name: 'Performance test'
+type: module
+description: 'Supports performance testing with PerformanceTestTrait'
+package: Testing
+version: VERSION
diff --git a/core/modules/system/tests/modules/performance_test/performance_test.services.yml b/core/modules/system/tests/modules/performance_test/performance_test.services.yml
new file mode 100644
index 000000000000..e598315bea9a
--- /dev/null
+++ b/core/modules/system/tests/modules/performance_test/performance_test.services.yml
@@ -0,0 +1,9 @@
+services:
+ Drupal\performance_test\PerformanceDataCollector:
+ tags:
+ - { name: event_subscriber }
+ - { name: needs_destruction, priority: -1000 }
+ Drupal\performance_test\DatabaseEventEnabler:
+ arguments: ['@database']
+ tags:
+ - { name: http_middleware, priority: 1000, responder: true }
diff --git a/core/modules/system/tests/modules/performance_test/src/DatabaseEventEnabler.php b/core/modules/system/tests/modules/performance_test/src/DatabaseEventEnabler.php
new file mode 100644
index 000000000000..ec75884fd972
--- /dev/null
+++ b/core/modules/system/tests/modules/performance_test/src/DatabaseEventEnabler.php
@@ -0,0 +1,32 @@
+<?php
+
+namespace Drupal\performance_test;
+
+use Drupal\Core\Database\Connection;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Drupal\Core\Database\Event\StatementExecutionEndEvent;
+use Drupal\Core\Database\Event\StatementExecutionStartEvent;
+
+class DatabaseEventEnabler implements HttpKernelInterface {
+
+ public function __construct(protected readonly HttpKernelInterface $httpKernel, protected readonly Connection $connection) {}
+
+ /**
+ * {@inheritdoc}
+ */
+ public function handle(Request $request, $type = self::MAIN_REQUEST, $catch = TRUE): Response {
+ if ($type === static::MAIN_REQUEST) {
+ $this->connection->enableEvents([
+ // StatementExecutionStartEvent must be enabled in order for
+ // StatementExecutionEndEvent to be fired, even though we only subscribe
+ // to the latter event.
+ StatementExecutionStartEvent::class,
+ StatementExecutionEndEvent::class,
+ ]);
+ }
+ return $this->httpKernel->handle($request, $type, $catch);
+ }
+
+}
diff --git a/core/modules/system/tests/modules/performance_test/src/PerformanceDataCollector.php b/core/modules/system/tests/modules/performance_test/src/PerformanceDataCollector.php
new file mode 100644
index 000000000000..318ff23528fa
--- /dev/null
+++ b/core/modules/system/tests/modules/performance_test/src/PerformanceDataCollector.php
@@ -0,0 +1,51 @@
+<?php
+
+namespace Drupal\performance_test;
+
+use Drupal\Core\Database\Event\StatementExecutionEndEvent;
+use Drupal\Core\DestructableInterface;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+
+class PerformanceDataCollector implements EventSubscriberInterface, DestructableInterface {
+
+ /**
+ * Database events collected during the request.
+ *
+ * @var Drupal\Core\Database\Event\StatementExecutionEndEvent[]
+ */
+ protected array $databaseEvents = [];
+
+ /**
+ * {@inheritdoc}
+ */
+ public static function getSubscribedEvents(): array {
+ return [
+ StatementExecutionEndEvent::class => 'onStatementExecutionEnd',
+ ];
+ }
+
+ /**
+ * Logs database statements.
+ */
+ public function onStatementExecutionEnd(StatementExecutionEndEvent $event): void {
+ // Use the event object as a value object.
+ $this->databaseEvents[] = $event;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function destruct(): void {
+ // Get the events now before issuing any more database queries so that this
+ // logging does not become part of the recorded data.
+ $database_events = $this->databaseEvents;
+
+ // Deliberately do not use an injected key value service to avoid any
+ // overhead up until this point.
+ $collection = \Drupal::keyValue('performance_test');
+ $existing_data = $collection->get('performance_test_data') ?? ['database_events' => []];
+ $existing_data['database_events'] = array_merge($existing_data['database_events'], $database_events);
+ $collection->set('performance_test_data', $existing_data);
+ }
+
+}
diff --git a/core/profiles/standard/tests/src/FunctionalJavascript/NoJavaScriptAnonymousTest.php b/core/profiles/standard/tests/src/FunctionalJavascript/NoJavaScriptAnonymousTest.php
deleted file mode 100644
index 67495663e48c..000000000000
--- a/core/profiles/standard/tests/src/FunctionalJavascript/NoJavaScriptAnonymousTest.php
+++ /dev/null
@@ -1,85 +0,0 @@
-<?php
-
-namespace Drupal\Tests\standard\FunctionalJavascript;
-
-use Drupal\Tests\PerformanceData;
-use Drupal\FunctionalJavascriptTests\PerformanceTestBase;
-use Drupal\node\NodeInterface;
-
-/**
- * Tests that anonymous users are not served any JavaScript.
- *
- * This is tested with the core modules that are enabled in the 'standard'
- * profile.
- *
- * @group Common
- */
-class NoJavaScriptAnonymousTest extends PerformanceTestBase {
-
- /**
- * {@inheritdoc}
- */
- protected $defaultTheme = 'stark';
-
- /**
- * {@inheritdoc}
- */
- protected $profile = 'standard';
-
- /**
- * {@inheritdoc}
- */
- protected function setUp(): void {
- parent::setUp();
-
- // Grant the anonymous user the permission to look at user profiles.
- user_role_grant_permissions('anonymous', ['access user profiles']);
- }
-
- /**
- * Tests that anonymous users are not served any JavaScript.
- */
- public function testNoJavaScript() {
- // Create a node of content type 'article' that is listed on the frontpage.
- $this->drupalCreateNode([
- 'type' => 'article',
- 'promote' => NodeInterface::PROMOTED,
- ]);
-
- // Test frontpage.
- $performance_data = $this->collectPerformanceData(function () {
- $this->drupalGet('');
- });
- $this->assertNoJavaScript($performance_data);
-
- // Test node page.
- $performance_data = $this->collectPerformanceData(function () {
- $this->drupalGet('node/1');
- });
- $this->assertNoJavaScript($performance_data);
-
- // Test user profile page.
- $user = $this->drupalCreateUser();
- $performance_data = $this->collectPerformanceData(function () use ($user) {
- $this->drupalGet('user/' . $user->id());
- });
- $this->assertNoJavaScript($performance_data);
- }
-
- /**
- * Passes if no JavaScript is found on the page.
- *
- * @param Drupal\Tests\PerformanceData $performance_data
- * A PerformanceData value object.
- *
- * @internal
- */
- protected function assertNoJavaScript(PerformanceData $performance_data): void {
- // Ensure drupalSettings is not set.
- $settings = $this->getDrupalSettings();
- $this->assertEmpty($settings, 'drupalSettings is not set.');
- $this->assertSession()->responseNotMatches('/\.js/');
- $this->assertSame(0, $performance_data->getScriptCount());
- }
-
-}
diff --git a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
new file mode 100644
index 000000000000..334da1444476
--- /dev/null
+++ b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
@@ -0,0 +1,202 @@
+<?php
+
+namespace Drupal\Tests\standard\FunctionalJavascript;
+
+use Drupal\FunctionalJavascriptTests\PerformanceTestBase;
+use Drupal\Tests\PerformanceData;
+use Drupal\node\NodeInterface;
+
+/**
+ * Tests the performance of basic functionality in the standard profile.
+ *
+ * Stark is used as the default theme so that this test is not Olivero specific.
+ *
+ * @group Common
+ */
+class StandardPerformanceTest extends PerformanceTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected $defaultTheme = 'stark';
+
+ /**
+ * {@inheritdoc}
+ */
+ protected $profile = 'standard';
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp(): void {
+ parent::setUp();
+
+ // Grant the anonymous user the permission to look at user profiles.
+ user_role_grant_permissions('anonymous', ['access user profiles']);
+ }
+
+ /**
+ * Tests performance for anonymous users.
+ */
+ public function testAnonymous() {
+ // Create two nodes to be shown on the front page.
+ $this->drupalCreateNode([
+ 'type' => 'article',
+ 'promote' => NodeInterface::PROMOTED,
+ ]);
+ // Request a page that we're not otherwise explicitly testing to warm some
+ // caches.
+ $this->drupalGet('search');
+
+ // Test frontpage.
+ $performance_data = $this->collectPerformanceData(function () {
+ $this->drupalGet('');
+ });
+ $this->assertNoJavaScript($performance_data);
+ // This test observes a variable number of cache gets and sets, so to avoid
+ // random test failures, assert greater than equal the highest and lowest
+ // number of observed during test runs.
+ // See https://www.drupal.org/project/drupal/issues/3402610
+ $this->assertGreaterThanOrEqual(58, $performance_data->getQueryCount());
+ $this->assertLessThanOrEqual(66, $performance_data->getQueryCount());
+
+ $this->assertGreaterThanOrEqual(129, $performance_data->getCacheGetCount());
+ $this->assertLessThanOrEqual(132, $performance_data->getCacheGetCount());
+ $this->assertSame(59, $performance_data->getCacheSetCount());
+ $this->assertSame(0, $performance_data->getCacheDeleteCount());
+
+ // Test node page.
+ $performance_data = $this->collectPerformanceData(function () {
+ $this->drupalGet('node/1');
+ });
+ $this->assertNoJavaScript($performance_data);
+ $this->assertSame(38, $performance_data->getQueryCount());
+
+ // This test observes a variable number of cache gets and sets, so to avoid
+ // random test failures, assert greater than equal the highest and lowest
+ // number of queries observed during test runs.
+ // See https://www.drupal.org/project/drupal/issues/3402610
+ $this->assertGreaterThanOrEqual(87, $performance_data->getCacheGetCount());
+ $this->assertLessThanOrEqual(88, $performance_data->getCacheGetCount());
+ $this->assertSame(20, $performance_data->getCacheSetCount());
+ $this->assertSame(0, $performance_data->getCacheDeleteCount());
+
+ // Test user profile page.
+ $user = $this->drupalCreateUser();
+ $performance_data = $this->collectPerformanceData(function () use ($user) {
+ $this->drupalGet('user/' . $user->id());
+ });
+ $this->assertNoJavaScript($performance_data);
+ $this->assertSame(40, $performance_data->getQueryCount());
+
+ // This test observes a variable number of cache gets and sets, so to avoid
+ // random test failures, assert greater than equal the highest and lowest
+ // number of queries observed during test runs.
+ // See https://www.drupal.org/project/drupal/issues/3402610
+ $this->assertGreaterThanOrEqual(74, $performance_data->getCacheGetCount());
+ $this->assertLessThanOrEqual(80, $performance_data->getCacheGetCount());
+ $this->assertSame(19, $performance_data->getCacheSetCount());
+ $this->assertSame(0, $performance_data->getCacheDeleteCount());
+ }
+
+ /**
+ * Tests the performance of logging in.
+ */
+ public function testLogin(): void {
+ // Create a user and log them in to warm all caches. Manually submit the
+ // form so that we repeat the same steps when recording performance data. Do
+ // this twice so that any caches which take two requests to warm are also
+ // covered.
+ $account = $this->drupalCreateUser();
+ foreach (range(0, 1) as $index) {
+ $this->drupalGet('node');
+ $this->drupalGet('user/login');
+ $this->submitLoginForm($account);
+ $this->drupalLogout();
+ }
+
+ $this->drupalGet('node');
+ $this->drupalGet('user/login');
+ $performance_data = $this->collectPerformanceData(function () use ($account) {
+ $this->submitLoginForm($account);
+ });
+
+ // This test observes a variable number of database queries, so to avoid
+ // random test failures, assert greater than equal the highest and lowest
+ // number of queries observed during test runs.
+ // See https://www.drupal.org/project/drupal/issues/3402610
+ $this->assertLessThanOrEqual(40, $performance_data->getQueryCount());
+ $this->assertGreaterThanOrEqual(39, $performance_data->getQueryCount());
+ $this->assertSame(28, $performance_data->getCacheGetCount());
+ $this->assertSame(1, $performance_data->getCacheSetCount());
+ $this->assertSame(1, $performance_data->getCacheDeleteCount());
+ }
+
+ /**
+ * Tests the performance of logging in via the user login block.
+ */
+ public function testLoginBlock(): void {
+ $this->drupalPlaceBlock('user_login_block');
+ // Create a user and log them in to warm all caches. Manually submit the
+ // form so that we repeat the same steps when recording performance data. Do
+ // this twice so that any caches which take two requests to warm are also
+ // covered.
+ $account = $this->drupalCreateUser();
+ $this->drupalLogout();
+
+ foreach (range(0, 1) as $index) {
+ $this->drupalGet('node');
+ $this->assertSession()->responseContains('Password');
+ $this->submitLoginForm($account);
+ $this->drupalLogout();
+ }
+
+ $this->drupalGet('node');
+ $this->assertSession()->responseContains('Password');
+ $performance_data = $this->collectPerformanceData(function () use ($account) {
+ $this->submitLoginForm($account);
+ });
+ $this->assertSame(48, $performance_data->getQueryCount());
+ $this->assertSame(30, $performance_data->getCacheGetCount());
+
+ // This test observes a variable number of cache sets, so to avoid random
+ // test failures, assert greater than equal the highest and lowest number
+ // observed during test runs.
+ // See https://www.drupal.org/project/drupal/issues/3402610
+ $this->assertLessThanOrEqual(4, $performance_data->getCacheSetCount());
+ $this->assertGreaterThanOrEqual(1, $performance_data->getCacheSetCount());
+ $this->assertSame(1, $performance_data->getCacheDeleteCount());
+ }
+
+ /**
+ * Submit the user login form.
+ */
+ protected function submitLoginForm($account) {
+ $this->submitForm([
+ 'name' => $account->getAccountName(),
+ 'pass' => $account->passRaw,
+ ], 'Log in');
+ }
+
+ /**
+ * Passes if no JavaScript is found on the page.
+ *
+ * @param Drupal\Tests\PerformanceData $performance_data
+ * A PerformanceData value object.
+ *
+ * @internal
+ */
+ protected function assertNoJavaScript(PerformanceData $performance_data): void {
+ // Ensure drupalSettings is not set.
+ $settings = $this->getDrupalSettings();
+ $this->assertEmpty($settings, 'drupalSettings is not set.');
+ $this->assertSession()->responseNotMatches('/\.js/');
+ $this->assertSame(0, $performance_data->getScriptCount());
+ }
+
+ /**
+ * Provides an empty implementation to prevent the resetting of caches.
+ */
+ protected function refreshVariables() {}
+
+}
diff --git a/core/tests/Drupal/FunctionalJavascriptTests/PerformanceTestBase.php b/core/tests/Drupal/FunctionalJavascriptTests/PerformanceTestBase.php
index b7b2bed33ef6..fbd7cd32e0bd 100644
--- a/core/tests/Drupal/FunctionalJavascriptTests/PerformanceTestBase.php
+++ b/core/tests/Drupal/FunctionalJavascriptTests/PerformanceTestBase.php
@@ -15,6 +15,11 @@
class PerformanceTestBase extends WebDriverTestBase {
use PerformanceTestTrait;
+ /**
+ * {@inheritdoc}
+ */
+ protected static $modules = ['performance_test'];
+
/**
* {@inheritdoc}
*/
diff --git a/core/tests/Drupal/Tests/PerformanceData.php b/core/tests/Drupal/Tests/PerformanceData.php
index 528fefd81c95..0336c3a398e3 100644
--- a/core/tests/Drupal/Tests/PerformanceData.php
+++ b/core/tests/Drupal/Tests/PerformanceData.php
@@ -19,6 +19,26 @@ class PerformanceData {
*/
protected int $scriptCount = 0;
+ /**
+ * The number of database queries recorded.
+ */
+ protected int $queryCount = 0;
+
+ /**
+ * The number of cache gets recorded.
+ */
+ protected int $cacheGetCount = 0;
+
+ /**
+ * The number of cache sets recorded.
+ */
+ protected int $cacheSetCount = 0;
+
+ /**
+ * The number of cache deletes recorded.
+ */
+ protected int $cacheDeleteCount = 0;
+
/**
* The original return value.
*/
@@ -64,6 +84,86 @@ public function getScriptCount(): int {
return $this->scriptCount;
}
+ /**
+ * Sets the query count.
+ *
+ * @param int $count
+ * The number of database queries recorded.
+ */
+ public function setQueryCount(int $count): void {
+ $this->queryCount = $count;
+ }
+
+ /**
+ * Gets the query count.
+ *
+ * @return int
+ * The number of database queries recorded.
+ */
+ public function getQueryCount(): int {
+ return $this->queryCount;
+ }
+
+ /**
+ * Sets the cache get count.
+ *
+ * @param int $count
+ * The number of cache gets recorded.
+ */
+ public function setCacheGetCount(int $count): void {
+ $this->cacheGetCount = $count;
+ }
+
+ /**
+ * Gets the cache get count.
+ *
+ * @return int
+ * The number of cache gets recorded.
+ */
+ public function getCacheGetCount(): int {
+ return $this->cacheGetCount;
+ }
+
+ /**
+ * Sets the cache set count.
+ *
+ * @param int $count
+ * The number of cache sets recorded.
+ */
+ public function setCacheSetCount(int $count): void {
+ $this->cacheSetCount = $count;
+ }
+
+ /**
+ * Gets the cache set count.
+ *
+ * @return int
+ * The number of cache sets recorded.
+ */
+ public function getCacheSetCount(): int {
+ return $this->cacheSetCount;
+ }
+
+ /**
+ * Sets the cache delete count.
+ *
+ * @param int $count
+ * The number of cache deletes recorded.
+ */
+ public function setCacheDeleteCount(int $count): void {
+ $this->cacheDeleteCount = $count;
+ }
+
+ /**
+ * Gets the cache delete count.
+ *
+ * @return int
+ * The number of cache deletes recorded.
+ */
+ public function getCacheDeleteCount(): int {
+ return $this->cacheDeleteCount;
+ }
+
/**
* Sets the original return value.
*
diff --git a/core/tests/Drupal/Tests/PerformanceTestTrait.php b/core/tests/Drupal/Tests/PerformanceTestTrait.php
index 77bdfb21573a..9f910d338775 100644
--- a/core/tests/Drupal/Tests/PerformanceTestTrait.php
+++ b/core/tests/Drupal/Tests/PerformanceTestTrait.php
@@ -93,12 +93,63 @@ private function doGetMinkDriverArgs(): string {
* A PerformanceData value object.
*/
public function collectPerformanceData(callable $callable, ?string $service_name = NULL): PerformanceData {
+ // Clear all existing performance logs before collecting new data. This is
+ // necessary because responses are returned back to tests prior to image
+ // and asset responses are returning to the browser, and before
+ // post-response tasks are guaranteed to have run. Assume that if there is
+ // no performance data logged by the child request within one second, that
+ // this means everything has finished.
+ $collection = \Drupal::keyValue('performance_test');
+ while ($collection->get('performance_test_data')) {
+ $collection->deleteAll();
+ sleep(1);
+ }
+
$session = $this->getSession();
$session->getDriver()->getWebDriverSession()->log('performance');
+ $collection = \Drupal::keyValue('performance_test');
+ $collection->deleteAll();
$return = $callable();
$performance_data = $this->processChromeDriverPerformanceLogs($service_name);
if (isset($return)) {
- $performance_data->setReturnValue($performance_data);
+ $performance_data->setReturnValue($return);
+ }
+
+ $performance_test_data = $collection->get('performance_test_data');
+ if ($performance_test_data) {
+ // Separate queries into two buckets, one for queries from the cache
+ // backend, and one for everything else (including those for cache tags).
+ $query_count = 0;
+ $cache_get_count = 0;
+ $cache_set_count = 0;
+ $cache_delete_count = 0;
+ foreach ($performance_test_data['database_events'] as $event) {
+ if (isset($event->caller['class']) && is_a(str_replace('\\\\', '\\', $event->caller['class']), '\Drupal\Core\Cache\DatabaseBackend', TRUE)) {
+ $method = strtolower($event->caller['function']);
+ if (str_contains($method, 'get')) {
+ $cache_get_count++;
+ }
+ elseif (str_contains($method, 'set')) {
+ $cache_set_count++;
+ }
+ elseif (str_contains($method, 'delete')) {
+ $cache_delete_count++;
+ }
+ elseif ($event->caller['function'] === 'ensureBinExists') {
+ // Don't record anything for ensureBinExists().
+ }
+ else {
+ throw new \Exception("Tried to record a cache operation but did not recognize {$event->caller['function']}");
+ }
+ }
+ else {
+ $query_count++;
+ }
+ }
+ $performance_data->setQueryCount($query_count);
+ $performance_data->setCacheGetCount($cache_get_count);
+ $performance_data->setCacheSetCount($cache_set_count);
+ $performance_data->setCacheDeleteCount($cache_delete_count);
}
return $performance_data;
--
GitLab