Enable W3C-compliant webdriver testing

Created on 13 February 2024, 10 months ago
Updated 18 September 2024, 3 months ago

Problem/Motivation

This is a follow-on from 📌 Use lullabot/mink-selenium2-driver and lullabot/php-webdriver for functional browser testing Fixed . This issue enables W3C-compliant webdriver testing, thereby allowing Drupal's core tests, contrib module tests and custom tests to be run against modern browsers using selenium.

Proposed resolution

  • Fix Drupal code that uses non-W3C commands to use work on both W3C and non-W3C webdrivers.
  • Test against both W3C and non-W3C

Remaining tasks


  • All nightwatch and functional JS tests to using selenium/standalone-chrome:latest - this is inline with our browser support policy and if it becomes an issue we will pin it to a verion.

  • This has to be a follow-up there are issues we need to resolve.

  • Using selenium/standalone-chrome:latest is inline with our browser support policy and we no longer have to maintain an image that gets out of date as quickly as we update it.

  • Moved to W3C / selenium

Followups:

User interface changes

None

API changes

\Drupal\FunctionalJavascriptTests\WebDriverCurlService is deprecated. This was added in #2942900-50: Convert JavascriptTestBase Tests to use DrupalSelenium2Driver . @longwave is pretty sure this has been fixed by https://issues.chromium.org/issues/42321790

Data model changes

None

Release notes snippet

Drupal core now tests using selenium/standalone-chrome:latest via a W3C compliant webdriver. To enable W3C mode in WebDriverTestBase tests, the MINK_DRIVER_ARGS_WEBDRIVER environment variable must pass "w3c": true in the options. To enable W3C mode in Nightwatch tests, the DRUPAL_TEST_WEBDRIVER_W3C environment variable must to be set to true.

📌 Task
Status

Needs work

Version

10.3

Component
PHPUnit 

Last updated about 17 hours ago

Created by

🇬🇧United Kingdom alexpott 🇪🇺🌍

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @alexpott
  • Merge request !6584Resolve #3421202 use selenium standalone image → (Open) created by alexpott
  • Status changed to Needs review 10 months ago
  • 🇬🇧United Kingdom justafish London, UK
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    Crediting @justafish as a tonne of the effort here is theirs.

  • 🇺🇸United States smustgrave

    Can you rerun the javascript tests? Want to make sure they aren't random but even I request access I can't rerun.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    @smustgrave it's definitely not random.

  • Status changed to Needs work 10 months ago
  • 🇺🇸United States smustgrave

    Glad I asked!

  • 🇬🇧United Kingdom justafish London, UK
  • 🇬🇧United Kingdom catch

    I've started using/recommending this locally too fwiw - a lot of people (more than 2-3 I could name) have trouble with the drupalci one which they don't run into with the selenium one.

  • 🇬🇧United Kingdom justafish London, UK

    I've split the part about upgrading the tests to https://www.drupal.org/project/drupal/issues/3432099 📌 Update our tests to be W3C compatible Active as we can test this is working using geckodriver

  • 🇬🇧United Kingdom catch

    After 📌 Use lullabot/mink-selenium2-driver and lullabot/php-webdriver for functional browser testing Fixed I wasn't able run test locally with selenium/standalone-chrome, which I'd otherwise been using successfully for months, gave up trying to debug it and went back to the drupalci image (which has been working OK). I haven't seen anyone else run into issues since that commit, but presumably people will start to switch over if we switch gitlab, so just making a note here in case other people do indeed run into issues down the line.

  • 🇬🇧United Kingdom AndyF

    Just to chime in re #12 I was seeing exactly the same thing (see https://github.com/ddev/ddev-selenium-standalone-chrome/issues/41). It appears that \WebDriver\WebDriver::session() is checking for the w3c flag one level higher up than where it's being set. I'm not sure if the issue should be fixed in lullabot/php-webdriver or by changing the capabilities we're passing. Locally I've just changed the capabilities, but I've seen plenty of examples on Stack Exchange etc. of the w3c flag being within goog:chromeOptions and I can't find a canonical reference on the matter.

  • 🇳🇱Netherlands tinto Amsterdam

    I was seeing the same problem as #12, running test on my local machine with Drupal 11.x-dev.

    I fixed it by implementing the workaround posted in #13 - I simply replaced line 15 in config.selenium-standalone-chrome.yaml to this:

    - MINK_DRIVER_ARGS_WEBDRIVER=[\"chrome\", {\"browserName\":\"chrome\",\"w3c\":false,\"goog:chromeOptions\":{\"args\":[\"--disable-gpu\",\"--headless\", \"--no-sandbox\", \"--disable-dev-shm-usage\"]}}, \"http://selenium-chrome:4444/wd/hub\"]
    

    Thank you @AndyF!

  • Pipeline finished with Failed
    5 months ago
    Total: 668s
    #220983
  • First commit to issue fork.
  • Pipeline finished with Failed
    5 months ago
    Total: 497s
    #226359
  • Pipeline finished with Failed
    5 months ago
    Total: 419s
    #226612
  • Pipeline finished with Success
    5 months ago
    Total: 588s
    #226633
  • Pipeline finished with Canceled
    5 months ago
    Total: 68s
    #226826
  • Pipeline finished with Canceled
    5 months ago
    Total: 409s
    #226827
  • Pipeline finished with Canceled
    5 months ago
    Total: 175s
    #226838
  • Pipeline finished with Canceled
    5 months ago
    Total: 435s
    #226835
  • Pipeline finished with Success
    5 months ago
    Total: 468s
    #226844
  • Status changed to Needs review 5 months ago
  • 🇬🇧United Kingdom longwave UK

    The change record needs to be written but otherwise the tests are green so marking NR, hopefully someone else familiar with Chrome testing can take a look.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    My plan was to run a limited subset tests using non-W3C mode. while we still support it - i.e. until Drupal 12 and then in Drupal 12 we should only support W3C testing.

    I think we should also add a manual job to run tests using Firefox.

  • Pipeline finished with Failed
    5 months ago
    Total: 945s
    #226950
  • 🇫🇷France andypost

    I think we should also add a manual job to run tests using Firefox.

    nice idea, there's selenium/standalone-firefox image

  • Pipeline finished with Canceled
    5 months ago
    Total: 172s
    #226973
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍
  • Pipeline finished with Canceled
    5 months ago
    #226976
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    alexpott changed the visibility of the branch 3421202-use-seleniumstandalone-chrome-instead to hidden.

  • Pipeline finished with Success
    5 months ago
    Total: 591s
    #226981
  • 🇫🇷France andypost

    Locally in docker I got few tests running fine using Firefox image with following arguments
    MINK_DRIVER_ARGS_WEBDRIVER: '["firefox", {"browserName":"fixrefox","moz:firefoxOptions":{"args":["--headless"]}}, "http://browser:4444"]'

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    Fixed up issue summary and title to be inline with current MR. Also added more to remaining tasks for things that we need to decide.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍
  • 🇫🇷France andypost

    Somehow FIrefox tests are x2 faster then selenium jobs

  • Pipeline finished with Success
    5 months ago
    Total: 801s
    #227038
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    @andypost that's not the case... they were skipping every test :D we should fail instead of skip when a WebDriverTestBase test fails to connect to a browser. Once I fixed the webdriver json string we can see that the tests are about the same speed - see https://git.drupalcode.org/project/drupal/-/pipelines/227038

    Also Firefox has a different approach when elements are not in the viewport and you do a click - see https://stackoverflow.com/questions/44777053/selenium-movetargetoutofbou... - I think we should address this in our webdriver in the same way that webdriver-classic-driver does - see https://github.com/minkphp/webdriver-classic-driver/blob/73ad0b6ce21cf69...

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    Lolz previous me pointed out that PHPUnit would skip if something required by the test is not available - see #3187577-11: FunctionalJavascript tests should fail when ChromeDriver is not running - I guess that comment is still correct. But it would be great if somehow the reason things are skipped was actually listed in the test output.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    To be fair you can see the skipped tests in the report on gitlab - see https://git.drupalcode.org/project/drupal/-/pipelines/227006/test_report

  • 🇫🇷France andypost

    Thanks! it explains difference (all skipped for ff)

    btw performance metrics also could be collected from FF, at least it has tracing out of box and allows to enable dev-tools (even via remote somehow) and get better details... for follow-up

  • 🇫🇷France andypost

    Moreover it makes sense to explore optimizations related to --headless mode in browsers

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    alexpott changed the visibility of the branch 3421202-on-top-of-3240792 to hidden.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    Answered questions in the issue summary and update MR to reflect the answers.

  • Pipeline finished with Failed
    5 months ago
    Total: 505s
    #228928
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    Created the follow-ups.

    I think this is ready for review.

  • Pipeline finished with Success
    5 months ago
    Total: 1195s
    #228937
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    One thing for reviewers to not is the move from 2 runners to 4 runners for the JS testing job. This is because testing on selenium is slower. There is not much we can do about this as far as I can see and the benefits of actually testing against current browsers and not maintaining our own chromedriver image are large.

  • 🇬🇧United Kingdom catch

    An interesting thing with the four vs. two javascript runners is that overall time still seems to be constrained by the slowest individual tests (either Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest or Drupal\Tests\ckeditor5\FunctionalJavascript\SourceEditingTest seem to be the longest running) and overall run duration is also similar to what it is now. I would kind of have expected the bottleneck to shift to the number of http requests away from installing Drupal, although maybe those tests also do loads of http requests too.

    I wonder if we're hitting concurrency/cpu limits. However anything like that we can explore in a follow-up - four runners seems fine and there's no overall regression in the pipeline feedback.

  • 🇬🇧United Kingdom longwave UK

    If we're just moving Nightwatch entirely to w3c mode then we can drop the separate selenium environment and just turn on the w3c switch in the default environment.

  • Status changed to Needs work 5 months ago
  • 🇬🇧United Kingdom longwave UK

    Added some nits/comments to the MR.

  • Status changed to Needs review 5 months ago
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    I've run core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php --filter testMultipleReplacements on both old chromedriver and selenium and the test fails with 2000 and the fix from 🐛 big_pipe sometimes fails to load blocks Active reverted so reducing to 2000 seems fine and we don't need a comment. That was useful to explain why we have reduced it but will be meaningless in the future.

    I've answered the rest of @longwave's feedback on the MR - but back to needs review. I think we need to add a nightwatch section to the MR.

  • Status changed to Needs work 5 months ago
  • 🇬🇧United Kingdom longwave UK

    Let's update the CR for Nightwatch and also add a followup to investigate Selenium switches, otherwise this is ready to go.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    Added testing performance follow-up as requested.

  • Status changed to Needs review 5 months ago
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    I've tweaked the way W3C mode is set in nightwatch to negate the need for an extra environment. I realised that the selenium environment was missing the merge of chrome args etc... and changing how this is set up seemed best.

    I've also update the Cr to reflect the test configuration changes someone would need to make to use W3C compliant browser testing.

  • Pipeline finished with Failed
    5 months ago
    Total: 571s
    #231897
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍
  • Pipeline finished with Success
    5 months ago
    Total: 1378s
    #231909
  • Pipeline finished with Success
    5 months ago
    Total: 1007s
    #231952
  • Status changed to RTBC 5 months ago
  • 🇬🇧United Kingdom longwave UK

    Alright, let's ship this - tests are green, and the test and configuration changes are the minimum required to enable W3C mode. This then unlocks some future improvements including hopefully being able to upgrade Nightwatch to a more recent version.

    • catch committed 9eb4cb9d on 11.x
      Issue #3421202 by alexpott, longwave, andypost, justafish: Enable W3C-...
  • Status changed to Fixed 5 months ago
  • 🇬🇧United Kingdom catch

    Committed/pushed to 11.x, thanks!

    If this does indeed make functional js tests a lot slower, it may break the performance tests dashboard. I've opened 📌 Consolidate Umami performance tests Fixed which should cover that.

    Alongside that, I also opened 📌 Consolidate ckeditor5's FunctionalJavascript tests Needs review for core's other longest running functional js tests.

    I think we should open a follow-up to revisit concurrency once those two issues are in, since they may be enough to lower it again, I'll open it postponed now.

    I am assuming that we don't want to try to backport this to 10.4.x, but if we do, please re-open with a backport MR.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    I think we do want to get this in 10.4.x and maybe even get the test base class changes into 10.3.x and 11.x so that projects can move to W3C driver testing early. I'll open when I get round to making the MR for 10.4.x

  • Status changed to Downport 5 months ago
  • 🇬🇧United Kingdom catch

    Performance test job is broken since this went in 🐛 Performance test gitlab job is broken Active .

  • Merge request !8995Backport work to 10.4.x → (Open) created by alexpott
  • Status changed to Needs review 5 months ago
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    Created two MRs - one for 11.0.x and one for 10.4.x (and 10.3.x) so we can backport this so that projects can start the process of testing on modern browsers...

  • Pipeline finished with Success
    5 months ago
    Total: 465s
    #239504
  • Pipeline finished with Failed
    5 months ago
    #239499
  • Pipeline finished with Success
    5 months ago
    Total: 462s
    #239506
  • Pipeline finished with Success
    5 months ago
    #239532
  • 🇫🇷France andypost

    The only question is deprecation message, let's update it to "deprecated in 10.0.4 and removed in 12.0.0"

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    @andypost - ah the best thing to do here is to remove the deprecation... makes core/tests/Drupal/Tests/Core/Test/PhpUnitCliTest.php pass on 10.x without modification which is nice.

  • Pipeline finished with Success
    5 months ago
    Total: 438s
    #244333
  • Status changed to Needs work 5 months ago
  • 🇬🇧United Kingdom longwave UK

    Added some comments about being slightly more conservative in the 11.0/10.4 backports. Should this also go back to 10.3 to keep in sync?

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    It'd be great if this went into 10.3 too.

  • Status changed to Needs review 5 months ago
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    Great review point @longwave - I've addressed them on both 10.4.x and 11.0.x branches. The 10.4.x also works on 10.3.x

  • Status changed to Needs work 5 months ago
  • 🇬🇧United Kingdom longwave UK

    Various tests are broken.

  • Status changed to Needs review 5 months ago
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    Superfluous dump() doh!

  • Pipeline finished with Success
    5 months ago
    Total: 5623s
    #245771
  • Pipeline finished with Failed
    5 months ago
    Total: 5696s
    #245772
  • Status changed to Needs work 5 months ago
  • 🇬🇧United Kingdom longwave UK

    📌 Fix selenium performance/stampede issues in gitlab config and BrowserTestBase Needs review landed so that change needs to be incorporated here now.

    Unit tests also failed on 10.4.x in WebAssertTest.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    What's the status of this? #47 landed, but then this issue was reopened, without a revert. The two change records are still .

    Can contrib follow core's example/lead? Should it?

  • 🇬🇧United Kingdom catch

    @Wim This was re-opened for backport, not due to regressions.

    However 📌 Fix selenium performance/stampede issues in gitlab config and BrowserTestBase Needs review landed in 11.x, and needs to be incorporated into the backport here to avoid serious performance/instability issues on pipelines. The gitlab YAML files have diverged quite a lot between branches so it is a real backport not just a cherry-pick unfortunately.

    Contrib w3c testing is happening in 📌 Update templates so 11.0 is the default/current branch RTBC .

  • 🇩🇪Germany chr.fritsch 🇩🇪🇪🇺🌍

    chr.fritsch made their first commit to this issue’s fork.

Production build 0.71.5 2024