Add PerformanceTestBase for allowing browser performance assertions within FunctionalJavaScriptTests

Started adding the new test type to have a base class to work in without polluting WebDriverTestBase, haven't started on getting the actual data out of chrome yet.

We should add the docs here, and also I think we need to update phpunit.xml.dist to add the new test type alongside the others in there.

Apart from that there's the general issue of actually putting the performance data somewhere accessible.

Both me and @heddn were seeing 2.3-2.7 seconds for largest contentful paint. I cross-checked that against lighthouse on a 10.1.x install, and confirmed it's accurate, opened #3349447: Improve Largest Contentful Paint in Umami front page to improve it. Once we fix that, we can see if the test can pick up the improvement.

A couple of recent updates:

- now sending time to first byte -> first paint and first paint -> largest contentful paint as spans to opentelemetry.

- added assertions for number of styles/scripts/images requested on the page. Not sure if the image assertion is really possible because chromedriver browser window could be non-deterministic potentially.

I've added an additional test method + supporting code. We now collect the number of scripts, stylesheets and images via chrome network requests. Testing the Umami front page with a user logged in that has access to tours, I'm able to get a proper failure if I revert #2695871: Aggregation creates two extra aggregates when it encounters {media: screen} in a library declaration locally - i.e. the test expects four stylesheets but with that issue reverted there are six.

Added image support in there, but (ironically) lazy image loading makes it hard to assert on a specific number of images, because it depends what the browser engine decides needs to be loaded or not, and that appears to be slightly variables - sometimes I get 19, sometimes 20. Not sure what if anything to do about that yet - we can always put that data into OpenTelemetry even if it's not viable for assertions.

Here's a test only patch includes the diff from the MR and a revert of #2695871: Aggregation creates two extra aggregates when it encounters {media: screen} in a library declaration.

Now that it's possible to do useful assertions in phpunit (although I still need a failing patch that fails in the right way), I've split the OpenTelemetry part of this out to #3352459: Add OpenTelemetry Application Performance Monitoring to core performance tests and #3352389: Add open-telemetry/sdk and open-telemetry/exporter-otlp as dev dependencies.

Here's another attempt at a fail patch - code from the MR plus a revert of #2695871: Aggregation creates two extra aggregates when it encounters {media: screen} in a library declaration.

I think we're approaching a committable patch here so marking needs review.

Discussed with @heddn whether we could use a trait to add server-side metrics to functional/kernel and maybe unit tests.

I think this is doable. The chromedriver stuff in here wouldn't be relevant for those tests, but we can also make this a (different) trait instead of adding an entirely new test suite. The only complication is having to override ::setUp() to re-enable css/js aggregation, so I moved that out to the Umami test. Also moved the Umami test to Umami itself.

OK issue with the recent test fails, which I wasn't seeing locally as dramatically, was that chromedriver stores the performance log until it's retrieved, then empties it out - so if you have http requests in-between drupalGet, you'd get the log for all of them, not just the drupalGet - this is easily solved by requesting the performance log prior to making the request, which resets it so you only get what you're expecting next time it's retrieved.

Attaching a fail patch that should fail properly this time - code from the MR + a revert of #2695871: Aggregation creates two extra aggregates when it encounters {media: screen} in a library declaration, you'll be able to see the extra two http requests causing the test to fail. We can apply the same pattern to database queries and assert on those, since that will be consistent across environments.

1) Drupal\Tests\demo_umami\FunctionalJavascript\PerformanceTest::testFrontPageTour
Failed asserting that 4 is identical to 2.

That's what we wanted with the fail patch.

Back to needs review.

Untagging for release manager review since there's been a +1 from @quietone to the general concept elsewhere, and any dependency additions have been split out of here now. However the tests needs framework and front-end framework reviews still I think - it's a light patch but it's got implications once we keep going.

Hiding the fail patch so it doesn't get retested and restoring RTBC.

After doing some extra testing I don't think we can add assertions on exact image count because I think chromedriver is slightly indeterminate there, however we might be able to figure out the maximum by running a test lots of times and seeing what level it fails at then using assertLessThan(). With opentelemetry we can also send the number of image requests as a trace attribute.

That should definitely be in a follow-up (which I'll open next week), question is whether to leave the supporting code in here or also move that to the follow-up.

I looked at converting NoJavaScriptAnonymousTest and there is one problem:

We could disable those modules maybe but opened #3354344: [PP-1] Convert NoJavaScriptAnonymousTest to use PerformanceTestTrait since it won't be a direct conversion of just the test.

Also opened #3354347: Add xhr and BigPipe assertions to PerformanceTestTrait for the xhr and BigPipe assertions.

If @Wim agrees with the two follow-ups I think we're back to needs review here.

hm, that makes me wonder whether we should this not as a trait, but as a new base class, because that'd allow us to not do \Drupal\FunctionalJavascriptTests\WebDriverTestBase::installModulesFromClassProperty(). But based on #20 that was highly intentional: to allow non-functional tests to use this too. But … I see no such uses yet?

I think this and the demo_umami setUp() override are the clincher - we need a base class, although 'just' a base class this time not a new test type as I started with.

We can still put any shared logic with functional tests like database querylogging into a trait. Have made that change in the MR and hopefully addressed the other points too.

With the base class approach that allows #3354344: [PP-1] Convert NoJavaScriptAnonymousTest to use PerformanceTestTrait to pass without having to do its own overrides now.

Do we really need to update core/tests/README.md like #7 and #8 were saying?

Ah no that's when I was considering a full new test type, over and above a base class, we don't need it for just a base class.

Doesn't that mean we should move this out of core/modules/system and into core/profiles/standard for consistency?

I thought about that but it felt out of scope... don't really mind either way.

Opened #3355364: Move NoJavaScriptAnonymousTest to the standard profile for the test move.

Split image handling out to #3361833: Add image request counts to PerformanceTestBase, it works but it's not entirely clear how to assert on it. Makes the diff here a dozen lines smaller. Leaving RTBC because this is pure removals to a follow-up.

This issue is now doing a lot less than when I opened it, so moving the needs x review tags over to #3352459: Add OpenTelemetry Application Performance Monitoring to core performance tests which is the one with more implications. There is other dependent work following this issue that doesn't involve opentelemetry, so that can be assessed on its own merits when we get there.