Problem/Motivation
We ship with Chromedriver 98 but Chromedriver 107 is out.
The JavaScript Chromedriver is only used by local Nightwatch tests if the environment variable DRUPAL_TEST_CHROMEDRIVER_AUTOSTART is set. This is not documented anywhere.
I am also fairly sure that Chromedriver needs to be close to your local version of Chrome for things to work properly. As we are shipping with Chromedriver 98 and Chrome 107 is currently out, I suspect nobody is using this feature. If someone does want to use this, it should be their responsibility to install a local Chromedriver that matches their version of Chrome.
Steps to reproduce
Proposed resolution
Remove the dependency and the supporting code.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comments
Comment #2
longwaveI just realised that I think the only time this dependency is used is if the environment variable
DRUPAL_TEST_CHROMEDRIVER_AUTOSTARTis set; I assume DrupalCI uses the separate chromedriver container instead.Comment #3
longwaveI can't find where DRUPAL_TEST_CHROMEDRIVER_AUTOSTART is documented. Should we consider dropping this feature and dependency? It is only used in local Nightwatch tests if the environment var is set to true. It seems a bit pointless updating this dependency all the time as it should stay in sync with your local Chrome, afaik it doesn't work properly if it is too out of date. Nobody seems to be complaining about it, so I suspect nobody actually uses this feature.
Comment #4
catchMakes sense to drop it if it's not being used. Since it's test-only, I think we could do it in 9.5/10.0 with a CR still.
Comment #5
andypost++ to remove as this dependency really huge (>100MB for no reason for every run) with binary elements which are not available for every hardware architecture
Comment #6
longwaveUpdated IS with new direction.
Comment #7
longwaveNot even sure we need the "local" Nightwatch test_settings any more?
Comment #8
nod_That's fine with me. I think we might need to point to an doc that explains how to start chromedriver in the docs as well : https://www.drupal.org/docs/automated-testing/phpunit-in-drupal/running-...
Need to remove
DRUPAL_TEST_CHROMEDRIVER_AUTOSTARTfrom.env.exampletoo.Comment #9
pooja saraah commentedComment #10
pooja saraah commentedThanks for your suggestions @nod_
Addressed the comment #8
Attached patch against Drupal 10.0.x
Attached reroll patch
Comment #11
pooja saraah commentedComment #12
pooja saraah commentedComment #13
longwave#10 does not apply.
Comment #14
longwaveAfter playing with this for a bit I have a better suggestion: we can keep the DRUPAL_TEST_CHROMEDRIVER_AUTOSTART variable, but Nightwatch can handle starting Chromedriver by itself; we don't need any custom code to do this. If Chromedriver is not present, then you get a nice error message:
This means we can keep the environment variable but drop the dependency; anyone who was using this setup can continue as before just by installing Chromedriver themselves using the instructions.
Comment #15
andypostNice idea! probably it needs CR to explain how to start/use it now
Comment #16
longwaveAdded CR: https://www.drupal.org/node/3319135
Comment #17
andypostfixed https://www.drupal.org/node/3031731/revisions/view/12800444/12849312 about how to start chromedriver the current version
+1 RTBC but #8 requires approval from @nod_
Comment #18
nod_All good with me, nice find.
Added to the CR an easier way to run chromedriver:
npx chromedriverComment #19
catchCommitted/pushed to 10.1.x and cherry-picked to 10.0.x, thanks!
Comment #21
longwaveComment #22
longwaveDiscussed with @nod_, we think this should also be removed from 9.5.x.
Comment #23
longwaveComment #24
longwaveComment #25
nod_Comment #26
alexpottRemoving this in 9.5.x makes sense to me. If you were using it you'd not be testing on the currently supported versions of chrome.
Comment #27
catchNeeds a re-roll against 9.5.x
Comment #28
nod_Comment #29
longwaveComment #31
catchCommitted/pushed to 9.5.x, thanks!
Comment #33
2dareis2do commentedIf I understand this correctly this docker image is used when testing this:
image: drupalci/chromedriver:productionThat seems to run (Session info: headless chrome=106.0.5249.103)
As npm chromedriver is a dependency when running Drupal's PHPUnit Functional Javascript tests, I think it makes sense that this value is a requirement in package.json and should match that version.
e.g.
"chromedriver": "^106.0.1",I think the confusion arises with running Nightwatch Tests that are run in the browser that require a version that matches the browser version installed.
Comment #34
andypostThere's few issue issues https://www.drupal.org/project/issues/drupalci_environments?text=chromed...
But to update chromedriver docker image it needs policy because it's used in Drupal 9.5 still
Comment #35
2dareis2do commentedPatch attached if required but probably needs a new issue to test it. Patch based on Drupal 10.1.4
Comment #36
2dareis2do commentedAdditional patch for installing patch in web directory, Also adding patch again for 10.1.x as I can see I can test against that.
Comment #37
2dareis2do commentedComment #38
2dareis2do commentedOk test is not running, I guess because the issue is closed?
Comment #39
2dareis2do commentedok added new issue https://www.drupal.org/project/drupal/issues/3392646