Update Selenium to 4.25.0 #200

Poundex · 2024-10-26T11:45:37Z

This change updates all Selenium dependencies to the current latest (4.25.0)

As this version now includes the Selenium Manager which will automatically provision browsers and their drivers, the custom CI image with these baked in is now unnecessary and has been replaced by stock images for all stages. This means that builds will use the latest stable versions of browsers in every build without a custom image having to be updated.

This change also moves all CI stages to the Docker executor. Not only is the machine executor not necessary for dind, but CircleCI has built in support for the dind service meaning it doesn't have to be started by the build (so those 20 second sleeps are gone as well).

Additionally, the Gradle JRE and the build JDK are now decoupled.
The build will run 21, except for a couple of steps where Selenium images are used which come with 17. These images are necessary for the local browser tests as although Selenium Manager will provision the browsers, they both have GTK dependencies that are not present in the stock JDK images. These images come with browsers and drivers baked in, and they ship with default config that disable the Selenium Manager. This seems to be fine, however, as these images are built and pushed automatically so they will always contain the latest stable versions of browsers/drivers.
The compile will continue to run with 11, which will be provisioned automatically by Gradle's built-in toolchain stuff.

jonnybot0 · 2024-10-28T16:07:06Z

doc/manual-snippets/src/test/groovy/configuration/DriverConfigSpec.groovy

@@ -33,6 +37,18 @@ class DriverConfigSpec extends Specification implements InlineConfigurationLoade

    def setupSpec() {
        CachingDriverFactory.clearCacheAndQuitDriver()
+
+        FirefoxDriver.metaClass.constructor = { ->


This metaclass hackery seems like a bit of a code smell to me.

First question: why do we need it to be headless for every run of this test? A lot of consumers are going to run this in headed mode, so why can't we? Is it due to the docker image changes?

Second question: is there a more natural way to change this, such as through a configuration file?

Yes, the fake display that used to be provided by the XVFB is not available in these stock images, so all browsers need to be headless.

I'm not aware that you could push this back to config, as it is the driver property these tests are setting and so would overwrite anything in config:

driver = { new FirefoxDriver() }

There doesn't seem to be any concept of a "default options", but that would make sense as they are driver specific.

Serious question: will this test fail inside the container without making it headless? All we're really testing is the driver setup. We're not driving to a page or anything, right? All we do is parse the script and spit out a geb.Configuration. We then test that this configuration has the needed properties.

I run the selenium firefox docker image locally, mapped it to my working directory, and ran the gradle test command inside without these lines. The test passed okay... so, do we really need this?

docker run -d --name=ffgeb --shm-size=2g -v /path/to/geb:/path/to/geb --env "GRADLE_USER_HOME=.gradle-home" selenium/standalone-firefox docker exec -it ffgeb bash # Inside container ./gradlew doc:manual-snippets:test

I got BUILD SUCCESSFUL in 2m 6s from that, without these lines.

The call to new FirefoxDriver() will immediately try and start an instance of the browser, even if you don't interact with it.

I had a very different experience from you when running these tests inside the container. Anything that tried to start a local browser failed immediately, as a session could not be started as the browser could not start graphically outside a graphical environment.
log.txt

It was pretty easy to recreate the browser crash inside the container:

ubuntu@368ba22a5097:/proj$ firefox Error: cannot open display: :99.0 ubuntu@368ba22a5097:/proj$

I retried it inside the same container with the command you ran inside the same docker container, but it still passed.

./gradlew --console plain --no-build-cache --rerun-tasks -Pci :doc:manual-snippets:test

I did neglect to mention in my previous comment that I had exported some Gradle options:

export GRADLE_OPTS="-Xmx1024m -XX:MaxMetaspaceSize=256m"

buildSuccessLog.txt

I'm not sure what the difference would be. I ran a git clean -fdx before doing things in the docker container, since I knew that there might be some detritus left over from running the build on my mac in the working directory which could conflict.

Could that be the problem? What if we tried out my suggested change on CI? Here's the patch:
plain_webdricer.patch

I'm trying this off a branch in CI: https://app.circleci.com/pipelines/github/geb/geb?branch=topic%2Fselenium-update-review

Okay, so I can reproduce the same error in CI. I'm going to give myself 30 more minutes of steady thinking to see if I can come up with a better idea. If not, you'll get my ✅ . :)

So, I found something that may work:

I noticed that the Selenium containers have xvfb startup scripts: https://github.com/SeleniumHQ/docker-selenium/blob/trunk/NodeBase/start-xvfb.sh

I added an environment variable to one of the containers used to run this test that should cause this script to run:
0489b9a

That doesn't work, and manually invoking the start-xvfb.sh script seems to hang forever.
https://app.circleci.com/pipelines/github/geb/geb/511/workflows/7da676b2-1d44-437e-98a6-a74952981216/jobs/6538

My gut feel is that we would want Geb to run tests against an environment with windows. It looks like these containers are meant to be able to provide that by default.

It looks like XVFB should work in CircleCI, but it may require some tweaking. See
https://support.circleci.com/hc/en-us/articles/360042959233-Using-xvfb-to-run-applications
https://discuss.circleci.com/t/xvfb-not-working/19752

CircleCI's docs don't do much more than say, "RTFM on XVFB, noob!" 😄

I'm going to approve this, but I'd appreciate it if you can take a stab at starting XVFB. I'm sure I'm just missing something obvious. If the old container could do it, there's no reason the new container can't, right? It's obviously still installed. I don't mind using headless in places, but I also think it makes sense to use xvfb, since many consumers will.

Looking more deeply into the error message you're getting, I found this GitHub thread:
SeleniumHQ/docker-selenium#1873

That fellow's fix was to use SE_OPTS to pass the certificate details. See SeleniumHQ/docker-selenium#1873 (comment)

I wonder if we're using HTTPS by default in the new images and simply need to pass certificate info for WebDriver to talk to it?

jonnybot0 · 2024-10-28T16:26:28Z

internal/test-support/src/main/groovy/GebConfig.groovy

@@ -122,10 +121,7 @@ if (dockerizedDriver) {
 if (System.getProperty("geb.local.driver") == "chrome") {
    driver = {
        def chromeOptions = new ChromeOptions()
-                .setLogLevel(ChromeDriverLogLevel.ALL)


So, the deprecation notice on ChromeDriverLogLevel says to use ChromiumDriverLogLevel.

I'm not 100% sure of the nuances here, but it seems like we could get the same effect using one of:

RemoteWebDriver#setLogLevel

Using the Builder for the ChromeDriver.

If there's some deeper context for why you removed this without replacement (for example, because it isn't needed anymore due to new defaults), let me know.

I added this in the Gradle update change while I was debugging something and forgot to remove it.

Ah, okay. So this is new code we're just cleaning up. Sounds good.

jonnybot0 · 2024-10-28T16:30:43Z

module/geb-core/src/test/groovy/geb/driver/CloudDriverFactorySpec.groovy

-                    }
+                        it.parse(fromJson(NewSessionCommand)).map { command ->
+                            json(
+                                    value: [


I assume this change in the JSON shape is due to the Selenium upgrade. I'll want to see this replaced with Java's simple web server or something similar eventually so we can remove Ratpack, but that'll be an improvement for future PRs.

Yeah I haven't noticed any examples where anything other than HTML or other static resources are being server so something simpler would probably work fine

…r XVFB

Poundex force-pushed the topic/update-selenium branch from 96eab1f to 430c9b9 Compare October 26, 2024 11:46

Poundex requested a review from jonnybot0 October 26, 2024 11:52

jonnybot0 mentioned this pull request Oct 28, 2024

add gradle 8.7 compatibility via org.ysb33r.gradle:grolifant-herd:4.0.0 erdi/webdriver-binaries-gradle-plugin#44

Open

jonnybot0 reviewed Oct 28, 2024

View reviewed changes

Adam Pounder added 2 commits October 28, 2024 19:08

Update Selenium to 4.25.0

2e687bb

Replace VM executors with Docker

bd4c683

Poundex force-pushed the topic/update-selenium branch from 6d89498 to e11826d Compare October 28, 2024 19:09

Adam Pounder added 2 commits October 31, 2024 17:48

Run headless browsers in build stages without graphical environment o…

c3be807

…r XVFB

Use stock images

5bc4794

Poundex force-pushed the topic/update-selenium branch from 67012cc to 5bc4794 Compare October 31, 2024 17:48

jonnybot0 self-assigned this Oct 31, 2024

jonnybot0 merged commit 39013f3 into apache:master Oct 31, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Selenium to 4.25.0 #200

Update Selenium to 4.25.0 #200

Poundex commented Oct 26, 2024

jonnybot0 Oct 28, 2024

Poundex Oct 28, 2024

jonnybot0 Oct 28, 2024

Poundex Oct 28, 2024

jonnybot0 Oct 28, 2024 •

edited

Loading

jonnybot0 Oct 28, 2024

jonnybot0 Oct 28, 2024

jonnybot0 Oct 28, 2024

jonnybot0 Oct 28, 2024

Poundex Oct 28, 2024

jonnybot0 Oct 28, 2024

jonnybot0 Oct 28, 2024

Poundex Oct 28, 2024

Update Selenium to 4.25.0 #200

Update Selenium to 4.25.0 #200

Conversation

Poundex commented Oct 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonnybot0 Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonnybot0 Oct 28, 2024 •

edited

Loading