Use PerformanceTestTrait::assertMetrics() in the navigation performance test

Refactored the test to use PerformanceTestTrait::assertMetrics() instead of asserting all the properties individually.

While working on this I noticed that locally the test was failing consistently for me with the following error:

1) Drupal\Tests\navigation\FunctionalJavascript\PerformanceTest::testLogin
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
     0 => 'SELECT "session" FROM "sessions" WHERE "sid" = "SESSION_ID" LIMIT 0, 1',
     1 => 'SELECT * FROM "users_field_data" "u" WHERE "u"."uid" = "2" AND "u"."default_langcode" = 1',
     2 => 'SELECT "roles_target_id" FROM "user__roles" WHERE "entity_id" = "2"',
-    3 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "theme:stark" ) AND "collection" = "config.entity.key_store.block"',
+    3 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "system.maintenance_mode" ) AND "collection" = "state"',
+    4 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "routing.non_admin_routes" ) AND "collection" = "state"',
+    5 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "theme:stark" ) AND "collection" = "config.entity.key_store.block"',
+    6 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "twig_extension_hash_prefix" ) AND "collection" = "state"',
+    7 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "system.private_key" ) AND "collection" = "state"',
+    8 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "asset.css_js_query_string" ) AND "collection" = "state"',
+    9 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "drupal.test_wait_terminate" ) AND "collection" = "state"',
+    10 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "system.cron_last" ) AND "collection" = "state"',
+    11 => 'INSERT INTO "semaphore" ("name", "value", "expire") VALUES ("state:Drupal\Core\Cache\CacheCollector", "LOCK_ID", "EXPIRE")',
+    12 => 'DELETE FROM "semaphore"  WHERE ("name" = "state:Drupal\Core\Cache\CacheCollector") AND ("value" = "LOCK_ID")',
 ]

Looks like the test was expecting State cache to be populated, but it doesn't. Given that tests are working as expected in the CI, I wonder if I'm missing any specific required local setup.

@plopsec do you have apcu enabled locally? We should make all of these tests require apcu to be enabled and skip if it isn't.

It seems enabled according to Status Report:

Should I make something extra to have it available for tests?

hmm actually looking at the queries, apcu shouldn't make a difference there anyway.

Can you try making the sleep(1) a sleep(5) and/or adding a third ::drupalGet() warm up request in there? I think what is probably happening is a timing issue between aggregate requests setting the state cache and the main request setting the cache, so that it's not warm enough by the time we get to the request we collect data for.

Tried increasing the sleep time to 5 or 10. Also tried adding a new sleep & drupalGet() to the mix.

The error is different, but it still fails:

1) Drupal\Tests\navigation\FunctionalJavascript\PerformanceTest::testLogin
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
     0 => 'SELECT "session" FROM "sessions" WHERE "sid" = "SESSION_ID" LIMIT 0, 1',
     1 => 'SELECT * FROM "users_field_data" "u" WHERE "u"."uid" = "2" AND "u"."default_langcode" = 1',
     2 => 'SELECT "roles_target_id" FROM "user__roles" WHERE "entity_id" = "2"',
-    3 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "theme:stark" ) AND "collection" = "config.entity.key_store.block"',
+    3 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "routing.non_admin_routes" ) AND "collection" = "state"',
+    4 => 'SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "theme:stark" ) AND "collection" = "config.entity.key_store.block"',
+    5 => 'INSERT INTO "semaphore" ("name", "value", "expire") VALUES ("state:Drupal\Core\Cache\CacheCollector", "LOCK_ID", "EXPIRE")',
+    6 => 'DELETE FROM "semaphore"  WHERE ("name" = "state:Drupal\Core\Cache\CacheCollector") AND ("value" = "LOCK_ID")',
 ]

/var/www/html/core/modules/navigation/tests/src/FunctionalJavascript/PerformanceTest.php:72

I'm pretty sure this test worked for me locally in the past. So checkout to the point where the test was committed (12db3ff9b4d) and tried to run it again.

It worked as expected, so tried git bisect to find the point where the test started to fail for me.

Executed git bisect between HEAD and 12db3ff9b4d twice and the result for both tests was:

32a13379001ac8c57449020debddd2a2cf7b8117 is the first bad commit
commit 32a13379001ac8c57449020debddd2a2cf7b8117
Author: Alex Pott <alex.a.pott@googlemail.com>
Date:   Tue Jan 7 10:15:58 2025 +0000

    Issue #2422681 by berdir, catch, ressa, nicxvan, alexpott, longwave: Remove the automatic cron run from the installer

Since this is a local error, not sure if we should spend more time here, but his could help somehow if others experience the same error.

Reverted the commit mentioned above (32a13379001ac8c57449020debddd2a2cf7b8117) in HEAD, and test is passing again locally.

Do you think could it be worth to open a follow-up to investigate it?

I think this was mentioned elsewhere last week but can't find an issue, so I opened one 🐛 Performance tests need to run cron Active .

Seems like a good refactor.

Committed/pushed to 11.x, thanks!

catch → closed merge request !10931