Problem/Motivation

Proposed resolution

  • Change our code to use lullabot/php-webdriver & lullabot/mink-selenium2-driver

This prepares the project to move to W3C compliant testing in #3421202: Enable W3C-compliant webdriver testing. The lullabot driver has supports both W3C and non-W3C modes. In this issue we continue to use a the non-W3C mode and will explore moving to W3C mode testing the aforementioned issue.

Remaining tasks

User interface changes

None

API changes

This issue deprecates support for the chromeOptions key in MINK_DRIVER_ARGS_WEBDRIVER. It is replaced by goog:chromeOptions.

Data model changes

None

Release notes snippet

The chromeOptions key in MINK_DRIVER_ARGS_WEBDRIVER should be renamed goog:chromeOptions.

Dependency evaluations

Mink selenium2 driver
Repository https://github.com/lullabot/mink-selenium2-driver
Release cycle New releases expected if Webdriver standard changes
Security policies None. This is a dev tool and will not be present on production.
Security issue reporting N/a
Contact(s) https://www.drupal.org/u/justafish & https://www.drupal.org/u/alexpott

PHP webdriver
Repository https://github.com/lullabot/php-webdriver
Release cycle New releases expected if Webdriver standard changes
Security policies None. This is a dev tool and will not be present on production.
Security issue reporting N/a
Contact(s) https://www.drupal.org/u/justafish & https://www.drupal.org/u/alexpott
CommentFileSizeAuthor
#5 3240792-5.patch13.83 KBalexpott
#3 3240792-3.patch9.66 KBalexpott

Issue fork drupal-3240792

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

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue tags: +PHP 8.1
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 3240792-3.patch9.66 KB

This passes a JS test locally... let's see

Status: Needs review » Needs work

The last submitted patch, 3: 3240792-3.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 3240792-5.patch13.83 KB

Status: Needs review » Needs work

The last submitted patch, 5: 3240792-5.patch, failed testing. View results

alexpott opened merge request !1316

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#3388162: Investigate realistic resource requests for core jobs, +#2937380: Maintain popular CI presets to improve developer experience
justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented

I've been working on a drop-in replacement for instaclick/php-webdriver https://github.com/Lullabot/drupal-webdriver/pull/1

justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented
Issue summary: View changes
justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented

PR to minkphp/MinkSelenium2Driver for W3C compatibility fixes: https://github.com/minkphp/MinkSelenium2Driver/pull/372

andypost’s picture

andypost
he/him
Primary language Russian
commented

there's Symfony's approach https://github.com/symfony/panther

justafish opened merge request !5803

andypost’s picture

andypost
he/him
Primary language Russian
commented

alexpott opened merge request !6538

justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented
Title: Use php-webdriver/webdriver & oleg-andreyev/mink-phpwebdriver instead of instaclick/php-webdriver & behat/mink-selenium2-driver » Use lullabot/php-webdriver & lullabot/mink-selenium2-driver instead of instaclick/php-webdriver & behat/mink-selenium2-driver
Issue summary: View changes
justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented
Issue summary: View changes
justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented
Title: Use lullabot/php-webdriver & lullabot/mink-selenium2-driver instead of instaclick/php-webdriver & behat/mink-selenium2-driver » Use W3C Webdriver standard for functional browser testing
Issue summary: View changes
justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented
Related issues: +#3407834: Update behat/mink and friends, +#3371963: Update Nightwatch to 3.x, +#2946472: Attempt to autostart chromedriver for selenium tests
justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented
Status: Needs work » Needs review
justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented

Ready for review - I did try and pull in the library changes separately (starting from https://git.drupalcode.org/project/drupal/-/commit/557a63899eacf28a117e7... in this MR), over in https://www.drupal.org/project/drupal/issues/3407834 but I think it's going to be a lot of extra unnecessary work fixing the tests and then re-fixing them as soon as we come back here, that given how small the changeset for this is I don't think it's worth splitting up

alexpott closed merge request !6538

alexpott closed merge request !1316

alexpott changed the visibility of the branch 3240792-use-php-webdriverwebdriver- to hidden.

alexpott changed the visibility of the branch 11.x to hidden.

alexpott changed the visibility of the branch 9.3.x to hidden.

alexpott opened merge request !6578

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Crediting @justafish for all the work to get this issue progressing. Nice one.

smustgrave’s picture

smustgrave commented

@alexpott could you rerun that javascript test please

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Discussed with @justafish - we agreed to split this issue into 2.

This issue will move us to using a W3C driver for JS testing and then a follow-up issue will look at swapping out our chromedriver image for a selenium standalone image.

Will create an MR with the minimum change for us to use the W3C driver from lullabot.

alexpott opened merge request !6583

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Created the issue to use selenium/standalone-chrome - see #3421202: Enable W3C-compliant webdriver testing ... going to close the other MRs here and move the bulk of that work over then.

alexpott closed merge request !6578

alexpott closed merge request !5803

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

The one thing I'm wondering here is should we use a composer replace here to indicate that the lullabot dependencies replace the other ones... not sure.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

The lullabot deps already ship with replace sections so #39 is moot. Nice.

smustgrave’s picture

smustgrave commented

Before marking can change record/release notes be added

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Needs work

Some nits, and yeah it needs a change record and release note.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review

I've resolved @longwave's feedback but I have some concerns that we have failures on a more recent chromedriver than the one we're currently using. Moving to https://hub.docker.com/r/justafish/chromedriver/tags to prove this.

mstrelan’s picture

mstrelan commented
Status: Needs review » Needs work

Assume we need to set w3c: true and possibly other things in core/tests/Drupal/Nightwatch/nightwatch.conf.js.

justafish opened merge request !6616

alexpott closed merge request !6583

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Use W3C Webdriver standard for functional browser testing » Use lullabot/mink-selenium2-driver and lullabot/php-webdriver for functional browser testing
Issue summary: View changes
Status: Needs work » Needs review

Updated issue summary and title inline with current approach on the MR.

Note we will need to fix https://github.com/Lullabot/php-webdriver/pull/9 before we can get a green run.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs release note, -Needs change record

Read both change record and release notes and seem straight forward, so removing those tags

Trying my best to review MR

Applied locally and still seem to be able to run tests in phpstorm. Deprecation link work.

Think this is good. Should it be tagged 10.3 highlight?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work

Via @alexpott in Slack

Been thinking about the deprecation - wanna move it to somewhere better.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Curious is there a way to measure effect of changing driver's protocol?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@andypost we've not changed the protocol in this issue. W3C is set to false by default here. This is about getting the dependency change done. Chromedriver doesn't care whether you use W3C commands whatever W3C is set to. So in order to prove we're using W3C we have to do #3421202: Enable W3C-compliant webdriver testing or something similar.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review

@smustgrave this is not a highlight yet. We need to fix the underlying Drupal tests to be W3C compatible.

Moved deprecation to WebDriverTestBase - this will allow us to differentiate between core and custom/contrib tests in the future which might make things easier.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Added comment, probably the TODO needs link

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Addressed @andypost's review

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Suppose it ready

spokje’s picture

spokje
Primary language Dutch
commented

New dependencies require an evaluation: https://www.drupal.org/about/core/policies/core-dependency-policies/depe...

Does this also need to happen when swapping one dependency for another?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work

Yup we should also do a quick dependency evaluation and add it to https://www.drupal.org/about/core/policies/core-dependency-policies-and-... (seems the existing packages are not there, but that shouldn't stop us adding these ones)

@justafish I assume you/Lullabot are OK to continue maintaining these packages as needed and also handle any security reports?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community

Added dependency evaluations. As dev dependencies that are only used for testing I'm not sure that having a security policy is that important. Also in mitigation @justafish is granting access to the repository for me and @longwave - so two Drupal security team members have access.

justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented

Yes we're ok to continue maintaining and handle security reports - I've also added longwave and alexpott as maintainers to the repository

  • catch committed ec1aa07a on 10.3.x 
    Issue #3240792 by justafish, alexpott, andypost, smustgrave, longwave:...

  • catch committed 5927d8fe on 11.x 
    Issue #3240792 by justafish, alexpott, andypost, smustgrave, longwave:...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Dictionary was the only thing I found...

Getting off the custom container and also mink which is barely/unmaintained and also towards the w3c standard is all great stuff, even if there's a fair bit more to do.

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

catch’s picture

catch
he/him
Primary language English
commented
Status: Fixed » Needs work

If I try to run tests locally, I get a 500 from the chrome container:

1) Drupal\Tests\node\FunctionalJavascript\NodePreviewLinkTest::testPreviewLinks
WebDriver\Exception\CurlExec: Curl error thrown for http GET to http://selenium-chrome:4444/wd/hub/session/9f13b12bce7dcb3f66af76e6dcc717cf/source

The requested URL returned error: 500 Internal Server Error

The chrome container log looks like this:


16:28:24.192 WARN [SpanWrappedHttpHandler.execute] - Unable to execute request: GET /session/6b6c1251e90c2ec54fe59cf70a5dc61e/source
Build info: version: '4.1.4', revision: '535d840ee2'
System info: host: 'c7da9f0362e9', ip: '192.168.16.6', os.name: 'Linux', os.arch: 'amd64', os.version: '6.5.0-21-generic', java.version: '11.0.14'
Driver info: driver.version: unknown
org.openqa.selenium.UnsupportedCommandException: GET /session/6b6c1251e90c2ec54fe59cf70a5dc61e/source
Build info: version: '4.1.4', revision: '535d840ee2'
System info: host: 'c7da9f0362e9', ip: '192.168.16.6', os.name: 'Linux', os.arch: 'amd64', os.version: '6.5.0-21-generic', java.version: '11.0.14'
Driver info: driver.version: unknown
	at org.openqa.selenium.remote.codec.AbstractHttpCommandCodec.decode(AbstractHttpCommandCodec.java:293)
	at org.openqa.selenium.remote.codec.AbstractHttpCommandCodec.decode(AbstractHttpCommandCodec.java:127)
	at org.openqa.selenium.grid.web.ProtocolConverter.execute(ProtocolConverter.java:123)
	at org.openqa.selenium.grid.node.ProtocolConvertingSession.execute(ProtocolConvertingSession.java:75)
	at org.openqa.selenium.grid.node.l

This is with ddev and https://github.com/ddev/ddev-selenium-standalone-chrome

There is a small change upstream to update the ddev config, but that's not the problem, changing the ddev config locally gets rid of the deprecation message but not the fatal error.

I did a clean drupal core clone in a new directory, ddev config then ddev get ddev/ddev-selenium-standalone-chrome and got the same error. Then I checkout out the commit just before this one, composer installed, and everything runs fine again (with the updated standalone config).

I can't track it down, but the fact that with an identical ddev setup I can run tests without this change, makes me think it's probably not just me.

andypost’s picture

andypost
he/him
Primary language Russian
commented

@catch it looks like material for DDEV or #3421202: Enable W3C-compliant webdriver testing

I'm using image: zenika/alpine-chrome:with-chromedriver image and the test is running fine here

connection options are MINK_DRIVER_ARGS_WEBDRIVER: '["chrome", {"browserName":"chrome","goog:chromeOptions":{"args":["--disable-gpu","--headless"]}}, "http://172.30.1.3:4444"]'

docker exec \
-u 1000:1000 \
-e SIMPLETEST_BASE_URL=http://172.30.1.2 \
-e BROWSERTEST_OUTPUT_DIRECTORY='db' \
core \
php -dzend.assertions=1 \
vendor/bin/phpunit -c core/phpunit.xml.dist --colors=always --debug \
core/modules/node/tests/src/FunctionalJavascript/NodePreviewLinkTest.php
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\node\FunctionalJavascript\NodePreviewLinkTest
Test 'Drupal\Tests\node\FunctionalJavascript\NodePreviewLinkTest::testPreviewLinks' started
Test 'Drupal\Tests\node\FunctionalJavascript\NodePreviewLinkTest::testPreviewLinks' ended


Time: 00:14.990, Memory: 4.00 MB

OK (1 test, 10 assertions)

HTML output was generated
http://172.30.1.2/sites/simpletest/browser_output/Drupal_Tests_node_FunctionalJavascript_NodePreviewLinkTest-1-13247836.html
http://172.30.1.2/sites/simpletest/browser_output/Drupal_Tests_node_FunctionalJavascript_NodePreviewLinkTest-2-13247836.html
http://172.30.1.2/sites/simpletest/browser_output/Drupal_Tests_node_FunctionalJavascript_NodePreviewLinkTest-3-13247836.html
http://172.30.1.2/sites/simpletest/browser_output/Drupal_Tests_node_FunctionalJavascript_NodePreviewLinkTest-4-13247836.html
http://172.30.1.2/sites/simpletest/browser_output/Drupal_Tests_node_FunctionalJavascript_NodePreviewLinkTest-5-13247836.html
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@catch can you post your MINK_DRIVER_ARGS_WEBDRIVER too?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I haven't changed my ddev setup since before this was committed - still using drupalci/chromedriver:production - and everything still works for me, I just get the deprecation:

$ ddev exec vendor/bin/phpunit -c core core/modules/node/tests/src/FunctionalJavascript/NodePreviewLinkTest.php
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\node\FunctionalJavascript\NodePreviewLinkTest
.                                                                   1 / 1 (100%)

Time: 00:11.365, Memory: 4.00 MB

OK (1 test, 10 assertions)

HTML output was generated
https://drupal8.ddev.site/sites/simpletest/browser_output/Drupal_Tests_node_FunctionalJavascript_NodePreviewLinkTest-1-75597820.html
https://drupal8.ddev.site/sites/simpletest/browser_output/Drupal_Tests_node_FunctionalJavascript_NodePreviewLinkTest-2-75597820.html
https://drupal8.ddev.site/sites/simpletest/browser_output/Drupal_Tests_node_FunctionalJavascript_NodePreviewLinkTest-3-75597820.html
https://drupal8.ddev.site/sites/simpletest/browser_output/Drupal_Tests_node_FunctionalJavascript_NodePreviewLinkTest-4-75597820.html
https://drupal8.ddev.site/sites/simpletest/browser_output/Drupal_Tests_node_FunctionalJavascript_NodePreviewLinkTest-5-75597820.html

Remaining self deprecation notices (1)

  1x: The "chromeOptions" array key is deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. Use "goog:chromeOptions instead. See https://www.drupal.org/node/3422624
    1x in NodePreviewLinkTest::testPreviewLinks from Drupal\Tests\node\FunctionalJavascript

Failed to execute command vendor/bin/phpunit -c core core/modules/node/tests/src/FunctionalJavascript/NodePreviewLinkTest.php: exit status 1
catch’s picture

catch
he/him
Primary language English
commented

I rebuilt my ddev install to try to rule out any stray old config out, so it's the ones that come with the ddev-selenium-standalone-chrome add-on, just with chromeOptions changed to goog:chromeOptions.

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

I'll try dropping that and doing the drupalci/chromedriver:production + 'copy and paste from @mglaman's two year old blog post' approach to see if that gets things running to narrow things down a bit more.

catch’s picture

catch
he/him
Primary language English
commented

Going back to drupalci/chromedriver:production worked for me, so can probably wait for https://github.com/ddev/ddev-selenium-standalone-chrome/pull/35 to get merged and/or to flush more issues out in #3421202: Enable W3C-compliant webdriver testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Fixed

In light of #68 - marking fixed again.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.