Problem/Motivation

Follow-up from #3471104: Nightwatch and Functional JavaScript fails since selenium/standalone-chrome:128.

Steps to reproduce

Proposed resolution

  • Bump the standalone-chrome version as high as we can
  • Attempt to use standalone-chrome:latest in updated deps job

Remaining tasks

Postponed on #3515403: Update to selenium/standalone-chrome:133

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3471113

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

catch created an issue. See original summary.

mstrelan made their first commit to this issue’s fork.

mstrelan opened merge request !11587

mstrelan opened merge request !11588

mstrelan opened merge request !11589

mstrelan’s picture

mstrelan commented
Title: Update to selenium/standalone-chrome:128 » Update to selenium/standalone-chrome:134
Issue summary: View changes
Status: Active » Needs work

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.

mstrelan’s picture

mstrelan commented

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.

catch’s picture

catch
he/him
Primary language English
commented

Could we add a ::waitForElement() for those cases or something similar?

mstrelan’s picture

mstrelan commented

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.

mstrelan changed the visibility of the branch 3471113-chrome-update-deps to hidden.

mstrelan changed the visibility of the branch 3471113-chrome-133 to hidden.

mstrelan’s picture

mstrelan commented
Issue summary: View changes
Status: Needs work » Postponed
Related issues: +#2909782: Provide the ability for JS functional tests to track whether the page has been reloaded, +#2936122: Find out why JavscriptTestBase occasionally needs a waitForElement on a normal page load

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.

mstrelan opened merge request !11626

chi’s picture

chi commented

I got lots of test failures with Chrome 134.

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

graber’s picture

graber commented

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.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

mstrelan’s picture

mstrelan commented
Title: Update to selenium/standalone-chrome:134 » Update to selenium/standalone-chrome:134+

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.

mstrelan’s picture

mstrelan commented
Status: Postponed » Needs work

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

herved’s picture

herved commented

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

longwave made their first commit to this issue’s fork.

longwave opened merge request !16675

longwave-bot made their first commit to this issue’s fork.

catch’s picture

catch
he/him
Primary language English
commented
Related issues: +#3616235: Performance test job crashes on main
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

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

graber opened merge request !16706

graber’s picture

graber commented

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).

graber’s picture

graber commented

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

mstrelan opened merge request !17053

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review

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.