Update to selenium/standalone-chrome:134+

Tried with Chrome 133 and 134, both seem to have failing tests. Also tried to only upgrade in the latest deps job but I guess I messed up the formatting.

Not sure what's changed, but it seems after any kind of pressButton we need to wait for the next page to load. I don't know the best way to do that, but for proof of concept I've introduced a waitForAddressEquals helper. Hope there is a more generic way.

re #8: I'm hoping for something more generic, it's quite burdensome figuring out what to wait for after ::pressButton. I've introduced a waitForDocumentReady function which seems to work for most cases. Still not sure what changed in selenium or chrome that means this is necessary though. Still working through the fails but pushing my progress so far.

I found #2909782: Provide the ability for JS functional tests to track whether the page has been reloaded and #2936122: Find out why JavscriptTestBase occasionally needs a waitForElement on a normal page load which are related.

Given the amount of the changes here maybe it's better to just go to 133 first and see if any big brains have better solutions for 134. Opened #3515403: Update to selenium/standalone-chrome:133 and postponing this.

I got lots of test failures with Chrome 134.

Ref:
https://issues.chromium.org/issues/405607581
https://github.com/teamcapybara/capybara/issues/2800

This issue was recently heavily affecting Drupal LMS tests where my test plan was one long JS test to improve performance and avoid setting up multiple times and.. there are lots of buttons pressed so it's a very good example.

Here's what we came up with (thanks to @catch for finding all the issues with partial solutions that work combined with a bit of my own logic): https://git.drupalcode.org/project/lms/-/merge_requests/82/diffs#e8d889b...
That combines waiting for a change in page HTML after pressing the button and checking the document.readyState.
Hope that helps, we could implement something similar in core.

Current latest version is 149. The links provided in #14 are the same leads I've got. The chromium issue has been marked as a duplicate of https://issues.chromium.org/issues/402796660.

#3515403: Update to selenium/standalone-chrome:133 is in

Root cause found for form submissions bug, the DeferRendererTasksAfterInput feature.
See https://issues.chromium.org/issues/402796660#comment18

Not sure if I'm doing something wrong but --disable-features=DeferRendererTasksAfterInput doesn't seem to make much, if any, difference.

I know it'd be nice to have a "magic" external solution we could implement by adding a parameter or similar without doing any big work but..
The fact is that we have unresolved race conditions in the testing logic that can always surface with this or that update and IMHO the only reliable solution is fixing them in Drupal test logic. Also, it's not the first time I'm encountering race conditions as mentioned in #15 so it's not something completely new really and was already an issue on previously used selenium/whatever-chrome.

Created a POC MR that adds some global methods that use waiting and fixes the RegistrationWithUserFieldsTest that had 3 random results previously (2 different failures and a pass).

.. and converted those methods to a single executeWithWaiting(callable $action) that does the job in all cases.

Tried a new approach against Chrome 152 in MR !17053. There are two changes here:

1. We wait for two requestAnimationFrames after click and submitForm events
2. We prevent dialogs from auto-closing, so we can make assertions on them

There is a green run, except for one test - CKEditor5AllowedTagsTest. From what I can tell there are some issues with filter tips, and I'm not sure if perhaps the test should have been updated after The long format 'filter tips' are deprecated. I think older Chrome is passing even though the tips might not actually be visible. EDIT: opened #3621903: Filter tips not working due to missing preprocess

Setting to NR for a general review of the approach. It's a bit messy, but it's a lot closer than the 32 and 29 test fails we had in previous attempts.

Taking this now.

Ok, polished @mstrelan's solution and applied a fix for the filter tips regression caused by https://git.drupalcode.org/project/drupal/-/commit/49c2dc16930 and already referenced in #3621903: Filter tips not working due to missing preprocess (Ok, I was too fast and applied my own solution) that wasn't testable with the previously used Chromium. https://git.drupalcode.org/project/drupal/-/merge_requests/16706 Green. I'll check performance impact next but I think with this people will stop rerunning tests on all those random race condition failures so it should be a gain anyway.

Edit (performance testing results): The driver wait time cost is about +7%. Selenium 152 is faster than Selenium 133 so the actual additional time is about +4.5%. Tested with:
CKEditor5AllowedTagsTest
JavascriptStatesTest
RegistrationWithUserFieldsTest

Removed select / checkbox waits for a basically no performance impact. Looks like all tests that handled such elements with AJAX callbacks attached already had some waiting implemented.

I suspect the attempt in #24 in MR 16675 didn't apply the flag properly and MR 16706 is working around the issue.
I'm going to attempt something in 16675, ignore me if it doesn't work...

Edit: it seems like it worked. I also cherry picked graber's fix for CKEditor5AllowedTagsTest
https://git.drupalcode.org/project/drupal/-/merge_requests/16675

Great it works after all, I was wrong in #26. I think we should enable the flag in DrupalSelenium2Driver though instead of adding it in .gitlab-ci.yml, it should be set globally on all test runners as it's required for tests to pass regardless of the runner.

Investigated a bit more and.. MR16675 disables a default feature of Chromium. That flag may be removed in the future, tests will silently fail and we'll have to figure out what happened again. On the other hand there's Chromedriver that may fix it but then an override (as in MR16706) will still work.

Moving to review for an opinion if we should fix it in house or by disabling a defalut Chromium feature before more work is done.

I think disabling the flag is the way to go, the workaround is pretty nasty. We can either pin to latest and accept the flag may be removed and our tests might fail, or we can pin to the latest known working version.

NW and back to #39 then.

I think I can RTBC this despite having worked on earlier iterations - the latest MR is nothing like the one I posted, which was really only meant to be for inspiration.

Thank you. It's fine as we both reviewed each other's changes.
Before this gets merged I just wanted to say that we should still consider your fix from MR !16706 - it should handle all random test failures like https://git.drupalcode.org/issue/drupal-3471113/-/jobs/12861034 that happened once here but also happens on other core jobs on main causing people to rerun tests. Somehow I feel that it is a better solution instead of sticking to that flag..

Leaving the decision to the final reviewer.

*EDIT* I couldn't reproduce that failure locally or with the CI runner and tried many times (https://git.drupalcode.org/issue/drupal-3471113/-/pipelines/1001802/builds), just wasting CPU time. With no failure on the randomly failing branch I cannot compare to the supposedly non-failing one so.. I think it'll be best to just land this (the visible MR) and either handle those random failures 1 by 1 or try a follow-up with something like MR !16706.

RTBC for 11.x backport.